-
Notifications
You must be signed in to change notification settings - Fork 26
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(cli): improve provider resolution outputs and remove sensitive information from debug logs #418
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 2a9b3ae. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
Changes in this PR:
|
packages/cli/src/settings.ts
Outdated
debug('got settings', finalSettings); | ||
// Filter out private key for logging | ||
/* eslint-disable @typescript-eslint/no-unused-vars */ | ||
const { cannonDirectory, privateKey, ...filteredSettings } = finalSettings; |
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.
- Provider ur contains api key
- Afaik many other places log full path and expose username
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.
Good catch thanks for the feedback @noisekit!
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.
also publishIpfsUrl
contains auth in the url
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.
ideally those values not excluded from logs but replaced with smth like ****
…formation from debug logs (#418)
No description provided.