add verbose flag to prevent logging authtoken #44
Conversation
There was a problem hiding this comment.
@LenaHoinkis -- keeping the auth token out of logs is a good idea. I've proposed an alternative strategy that I think will meet your needs and satisfy the typical deployments for M-Lab. Let me know what you think.
Reviewable status: 0 of 1 LGTMs obtained
cmd/github_receiver/main.go, line 42 at r1 (raw file):
var ( authtoken = flag.String("authtoken", "", "Oauth2 token for access to github API.") authtokenFile = flagx.FileBytes{}
Hmm, I was just talking about this issue in another repo. I think it's time to fix generally. How about the following strategy instead?
flagx.FileBytesArray{} does the right thing about managing the original file name for flag logging and making the file contents available.
In general, M-Lab wants to preserve the ArgsFromEnv feature for daemons by default. The *WithLog option was added to disable that behavior for CLI commands.
Using flagx.FileBytesArray here should satisfy the same requirements.
There was a problem hiding this comment.
Reviewable status: 0 of 1 LGTMs obtained
cmd/github_receiver/main.go, line 42 at r1 (raw file):
Previously, stephen-soltesz (Stephen Soltesz) wrote…
Hmm, I was just talking about this issue in another repo. I think it's time to fix generally. How about the following strategy instead?
flagx.FileBytesArray{}does the right thing about managing the original file name for flag logging and making the file contents available.In general, M-Lab wants to preserve the ArgsFromEnv feature for daemons by default. The
*WithLogoption was added to disable that behavior for CLI commands.Using flagx.FileBytesArray here should satisfy the same requirements.
@LenaHoinkis -- I'm working on a better fix for this in m-lab/go#124
There was a problem hiding this comment.
@LenaHoinkis I've merged m-lab/go#124 which adds a new flagx.File type that has the properties we want here.
If you'd like to complete this PR, please use that type for the authtokenFile.
If I don't hear from you by EOD Monday, I'll make the corresponding change, and close this PR.
Reviewable status: 0 of 1 LGTMs obtained
Currently all arguments get logged including the authorization token. Even if a authtokenFile is given it will print the token.
For security reasons I would suggest to only print the argument logs, if the verbose parameter is set.
This change is