Skip to content

Redact credentials from URLs before returning errors#2091

Merged
tonistiigi merged 1 commit intomoby:masterfrom
alexcb:redact-credentials-from-remote-url-errors
Apr 28, 2021
Merged

Redact credentials from URLs before returning errors#2091
tonistiigi merged 1 commit intomoby:masterfrom
alexcb:redact-credentials-from-remote-url-errors

Conversation

@alexcb
Copy link
Copy Markdown
Collaborator

@alexcb alexcb commented Apr 27, 2021

this is to prevent errors such as

failed to fetch remote https://user:password@github.com/user/private-repo-failure.git: exit status 128

from leaking the password; now it will be displayed like:

failed to fetch remote https://user:xxxxx@github.com/user/private-repo-failure.git: exit status 128

Signed-off-by: Alex Couture-Beil alex@earthly.dev

@alexcb alexcb force-pushed the redact-credentials-from-remote-url-errors branch from e1b94fc to 2d02185 Compare April 27, 2021 20:41
@alexcb alexcb marked this pull request as ready for review April 27, 2021 20:58
Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should also remove it from the cache key to avoid plaintext attacks.

@alexcb
Copy link
Copy Markdown
Collaborator Author

alexcb commented Apr 28, 2021

My motivation was to remove the password from any errors that might show up in error logs.

I only see the gs.cacheKey being set to a sha or ref; is there somewhere else where it's set to the remote (url)?

Comment thread util/stringutil/redact.go Outdated
@tonistiigi
Copy link
Copy Markdown
Member

I only see the gs.cacheKey being set to a sha or ref; is there somewhere else where it's set to the remote (URL)?

Ah yes. I don't remember my own code anymore.

@alexcb alexcb force-pushed the redact-credentials-from-remote-url-errors branch from 2d02185 to 322a63c Compare April 28, 2021 17:36
this is to prevent errors such as

    failed to fetch remote https://user:password@github.com/user/private-repo-failure.git: exit status 128

from leaking the password; now it will be displayed like:

    failed to fetch remote https://user:xxxxx@non-existant-host/user/private-repo.git: exit status 128

Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
@alexcb alexcb force-pushed the redact-credentials-from-remote-url-errors branch from 322a63c to 5d2fd7e Compare April 28, 2021 17:39
@tonistiigi tonistiigi merged commit 3e80895 into moby:master Apr 28, 2021
@alexcb alexcb deleted the redact-credentials-from-remote-url-errors branch April 28, 2021 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants