Skip to content

Make detector Result.SecretParts initialization stricter#4948

Merged
mcastorina merged 3 commits into
mainfrom
fix-renamed-secret-parts
May 5, 2026
Merged

Make detector Result.SecretParts initialization stricter#4948
mcastorina merged 3 commits into
mainfrom
fix-renamed-secret-parts

Conversation

@mcastorina

@mcastorina mcastorina commented May 4, 2026

Copy link
Copy Markdown
Contributor

Description:

Update the hack/checksecretparts tool to enforce initializing Result with a SecretParts field and clean up existing detectors to pass this lint check.

The reason for this change is because all Result objects must have SecretParts set.

Checklist:

  • Tests passing (make test-community)?
  • Lint passing (make lint this requires golangci-lint)?

Note

Medium Risk
Touches many detector implementations and changes the static check to fail any detectors.Result{} literal missing SecretParts, which could surface new CI failures or subtly change reported metadata. Core scanning/verification logic is largely unchanged, but the breadth of detector edits raises regression risk.

Overview
Updates hack/checksecretparts to be stricter: it now flags every detectors.Result{} / &detectors.Result{} composite literal that omits a SecretParts key, and no longer treats later SecretParts assignments or other package references as satisfying the rule; messaging/docs/tests were updated accordingly.

Migrates a broad set of detectors to comply by initializing Result.SecretParts at construction time (and in a few cases mutating the map later to add fields like endpoints/usernames), removing prior patterns that only set SecretParts after successful verification.

Reviewed by Cursor Bugbot for commit 3e6aaa4. Bugbot is set up for automated code reviews on this repo. Configure here.

@mcastorina mcastorina requested review from a team May 4, 2026 18:42

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3e6aaa4. Configure here.


for username := range usernames {
s1.RawV2 = []byte(fmt.Sprintf("%s:%s", username, token))
s1.SecretParts["username"] = username

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shared map mutation corrupts previously appended results

Medium Severity

The SecretParts map is initialized once per token at line 70, then mutated inside the inner username loop at line 75 via s1.SecretParts["username"] = username. Because maps in Go are reference types, when s1 is appended to results at line 88, the copy shares the same underlying map. Subsequent iterations overwrite "username" for all previously appended results, causing them to all reflect the last username processed rather than their own.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3e6aaa4. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is an existing bug and out of scope for this PR. Follow-up work will address this.

@mcastorina mcastorina merged commit ab5dd03 into main May 5, 2026
16 checks passed
@mcastorina mcastorina deleted the fix-renamed-secret-parts branch May 5, 2026 15:30
MuneebUllahKhan222 pushed a commit that referenced this pull request May 29, 2026
* Update linter to disallow assigning SecretParts later

This means all detectors.Result objects must be created with the
SecretParts field set.

* Update documentation

* Migrate existing detectors to always initialize SecretParts
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.

3 participants