-
-
Notifications
You must be signed in to change notification settings - Fork 677
feat: redact credentials in remote urls #2220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@iwittkau, very open to comments as you were the author of the last PR. |
94ff7fa
to
474eec1
Compare
I think this test should be added an pass: func TestGitNode_redaction(t *testing.T) {
t.Parallel()
node, err := NewGitNode("https://git:token@github.com/foo/bar.git//directory/Taskfile.yml?ref=main", "", false)
assert.NoError(t, err)
assert.Equal(t, "main", node.ref)
assert.Equal(t, "directory/Taskfile.yml", node.path)
assert.Equal(t, "https://git:xxxxx@github.com/foo/bar.git//directory/Taskfile.yml?ref=main", node.Location())
// assert.Equal(t, "https://git:xxxxx@github.com/foo/bar.git", node.URL.String())
entrypoint, err := node.ResolveEntrypoint("common.yml")
assert.NoError(t, err)
assert.Equal(t, "https://git:xxxxx@github.com/foo/bar.git//directory/common.yml?ref=main", entrypoint)
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 🎉
I'll give it a try with my work setup tomorrow.
I think you could keep GitNode.URL unexported to make sure its |
Yeah, we don't really need any of the properties on |
Fixes #2100 and replaces #2045
Ensures that any credentials embedded in the URL are redacted by using the built in
url.Redacted()
function. The cache keys have also been altered slightly, so users may have to redownload some files that they already have cached.