Skip to content

Conversation

@k80bowman
Copy link
Contributor

WI

Adds the redaction of the x-addon-sso field for API requests and anything being returned from an endpoint that ends in /sso from the debug logs.

Testing

Passing CI should suffice. I added tests for both situations and you should be able to see the redactions in the test logs.

@k80bowman k80bowman requested a review from a team as a code owner May 16, 2025 14:22
Copy link
Contributor

@eablack eablack left a comment

Choose a reason for hiding this comment

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

Looks good to me! Minor comment on the change to chalk dependency


private get _chalk(): any {
try {
return require('chalk')
Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessarily opposed to this, but this does change the behavior of this package. I think this was done so that if a package uses this, chalk will only be required/used if the dependent package has chalk.

I'm fine with changing this to simplify things, just figured I'd point it out. Probably not many packages utilize http-call at this point other than the CLI and the CLI already depends on chalk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, however multiple versions of chalk are already being imported as the dependency of several dependencies and we had another place that was just requiring chalk without this check. Plus I was having trouble in the testing because it was difficult to figure out which version of chalk was being targeted, since it was being used as a sub-dependency. I thought it was better to just go ahead and import it.

@k80bowman k80bowman merged commit 199a0dc into main May 16, 2025
6 checks passed
@k80bowman k80bowman deleted the k80/redact-tokens branch May 16, 2025 18:25
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