Skip to content
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

Linter Updates: Ignore .env files and redacted keys #245

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

gracegoo-stripe
Copy link
Contributor

@gracegoo-stripe gracegoo-stripe commented Jun 3, 2021

The redaction change is targeted to protect against the specific redacted format that Stripe returns which looks something like "sk_live_aa********************1234".
It does not attempt to detect other potential ways users can modify the keys nor actually validate that the key is real.

This change fixes #220 and #222

Testing

.env file before:
Screen Shot 2021-06-03 at 2 15 51 PM

after:
Screen Shot 2021-06-03 at 2 25 26 PM

redacted keys before:
Screen Shot 2021-06-03 at 3 24 33 PM

after:
Screen Shot 2021-06-03 at 3 11 06 PM

This change only protects against the specific redacted format that Stripe returns which looks something like "sk_live_aa********************1234".
It does not attempt to detect other potential ways users can modify the keys nor actually validate that the key is real.

This change fixes #220 and #222
Comment on lines +82 to +103
prepareLineDiagnostics =
(message: string) =>
(line: string, index: number): Diagnostic[] => {
const diagnostics: Diagnostic[] = [];

let match;
while ((match = stripeKeysRegex.exec(line)) !== null) {
const severity = /sk_live/.test(match[0])
? DiagnosticSeverity.Error
: DiagnosticSeverity.Warning;

// specify line and character range to draw the squiggly line under the API Key in the document
const range = new Range(index, match.index, index, match.index + match[0].length);
// create new diagnostic and add to the list of total diagnostics for this line of code
const diagnostic = new Diagnostic(range, message, severity);

this.telemetry.sendEvent('diagnostics.show', severity);
diagnostics.push(diagnostic);
}

return diagnostics;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about this, please hit "hide whitespace changes"

Copy link
Collaborator

@vcheung-stripe vcheung-stripe left a comment

Choose a reason for hiding this comment

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

Looks great! So to summarize the linter behavior for posterity:

  • don't flag if the key is in a file that's git-ignored
  • don't flag if the key is in a .env file
  • don't flag if the key has less than 3 unredacted chars in the body

This all makes sense to me 👍

@gracegoo-stripe
Copy link
Contributor Author

Looks great! So to summarize the linter behavior for posterity:

  • don't flag if the key is in a file that's git-ignored
  • don't flag if the key is in a .env file
  • don't flag if the key has less than 3 unredacted chars in the body

This all makes sense to me 👍

Exactly!

@gracegoo-stripe gracegoo-stripe merged commit 5680aec into master Jun 3, 2021
@gracegoo-stripe gracegoo-stripe deleted the gracegoo-linting-fixes branch June 3, 2021 22:43
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.

API key linting false positive detects obscured keys from stripe error messages
2 participants