fix(scan): take notice of a gosec G101 annotation already on the line - #108
Open
ralyodio wants to merge 1 commit into
Open
fix(scan): take notice of a gosec G101 annotation already on the line#108ralyodio wants to merge 1 commit into
ralyodio wants to merge 1 commit into
Conversation
A repository that has triaged a finding did so in the one place a reviewer will look, which is the line itself. Re-raising it at full severity asks the operator to decide the same thing a second time in another tool, and the excerpt printed with it read "G101 false positive: HTTP header name, not a credential", which is the clearest possible sign that nothing read it. Two things keep this from becoming a way to hide real findings. The rule has to be named, so a bare //nolint suppresses nothing: it is a statement about something and there is no reason to think it is about credentials. And the finding is downgraded rather than dropped, the way isTestPath already downgrades a fixture, so it stays in the report for anyone auditing the suppressions themselves. Checked against the file in the report: thecodearcher/limen response.go went from two high findings to two low ones, and a real key added to that same file is still reported at its own severity. Closes #106
ThreatCrush Security Scan67 finding(s) HIGH/CRITICAL: 11 | MEDIUM: 55 | LOW: 1
…and 17 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #106.
A repository that has already triaged a finding did so in the one place a reviewer will look, which is the line itself. Re-raising it at full severity asks the operator to make the same decision a second time in another tool. On thecodearcher/limen the excerpt printed with the finding literally read
G101 false positive: HTTP header name, not a credential.The two guards
The rule has to be named. A bare
//nolintsuppresses nothing here — it is a statement about something, and there is no reason to think it is about credentials. Onlynolint:gosecornosecnaming G101 counts, which is a statement about hardcoded credentials specifically.It downgrades rather than drops. The finding lands at
lowthe wayisTestPathalready downgrades a fixture, so it stays in the report for anyone auditing the suppressions themselves, and it stops dominating the first page.Checked against the file from the report
Run against limen's real
response.go, not a copy of it. A live-looking key appended to that same file is still reported at its own severity — that is one of the four tests.pnpm testinpackages/scan: 161 passing.pnpm typecheckclean. Pre-commit hook (CLI + landing page build) passed.