-
Notifications
You must be signed in to change notification settings - Fork 293
fix: redact output of credential tools #500
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
fix: redact output of credential tools #500
Conversation
There might be a different way by just looking at the tool category in the monitor when it's printing. This is effectively what the TUI does, we don't print output for tool category "context", "provider" or "credential". For the CLI logs I would still like provider and context, but it make sense maybe to not print the credential? I'm not 100% why we care though. |
Oh looking at this a big more, I think we probably need to drop the output in the place where we write the event. The event should not have the clear text of a password in it. Regardless if it's printed. But I'm still lost on how the event even has the clear text... Soo many layers. What madness have we created. |
@ibuildthecloud I can figure this out. I will probably have to plumb some gross thing through all the layers to get it to work, but as you said, that's the nature of the madness. |
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
4ca869f
to
bf402f9
Compare
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
for #483
This creates a little wrapper type around the monitor, to be used only for credential calls, so that we do not print the plaintext output to the console.Update: this instead excludes the output of credential tools from the CallFinish Event entirely, so the Monitor never has a chance to see it.