Remote Taskfiles: redact credentials of remote URLs in prompts and logs #2045
Conversation
taskfile/node.go
Outdated
| Location() string | ||
| RedactedLocation() string |
There was a problem hiding this comment.
Just had a quick look on my phone, not a full review.
Is the raw (non-redacted) location actually used outside of the git node implementation? You could make the argument that the credentials should never be exposed via the interface and that Location should just return the redacted version. This would save us having to add an interface method that only one of the implementations needs.
There was a problem hiding this comment.
I agree with you @pd93 !
AFAIK, the Location is used only to reference it in the graph so, imho no objection to return the redacted version.
Also, the http node implementation could / should have the same behavior, as we can use http://USER:PASS@URL pattern
There was a problem hiding this comment.
That would make the whole change a lot simpler! I saw that the node.Location is used to provide the cache key.
I think ignoring the username and password combination for the cache should be fine since different combinations shouldn't yield different Taskfile contents.
Adding the HTTP node redaction should be easy too.
iwittkau
changed the title
| @@ -22,7 +22,7 @@ type Node interface { | |||
| Remote() bool | |||
| ResolveEntrypoint(entrypoint string) (string, error) | |||
| ResolveDir(dir string) (string, error) | |||
| FilenameAndLastDir() (string, string) | |||
| FilenameAndLastDir() (lastDir string, file string) // TODO the return order is implemented opposite to the naming | |||
There was a problem hiding this comment.
It looks like this caused some bugs? #1317 (comment)
|
I've turned this into a draft for now. |
| @@ -122,5 +121,5 @@ func (node *GitNode) ResolveDir(dir string) (string, error) { | |||
| } | |||
|
|
|||
| func (node *GitNode) FilenameAndLastDir() (string, string) { | |||
| return filepath.Base(node.path), filepath.Base(filepath.Dir(node.path)) | |||
| return filepath.Base(filepath.Dir(node.filepath)), filepath.Base(node.filepath) | |||
There was a problem hiding this comment.
taskfile/cache.go uses it like this:
lastDir, filename := node.FilenameAndLastDir()
iwittkau
force-pushed
the
redact-git-urls
branch
from
035469d to
03a5551
Compare
iwittkau
force-pushed
the
redact-git-urls
branch
from
03a5551 to
3fc6d75
Compare
|
Compiling it and running this as a development version actually breaks my own git-based Remote Taskfiles setup 😅 Back to draft, I guess. |
| u.Path = basePath | ||
| gitURL.RawQuery = "" | ||
| gitURL.Path = urlPath | ||
| baseURL := gitURL.Redacted() |
There was a problem hiding this comment.
We actually have to use the unredacted URL here.
This is to improve #1317.
I've implemented redaction support for the
taskfile.Nodeinterface, specifically for thetaskfile.GitNodeimplementation which was logged with the clear-text user access tokens before.