Skip to content

Populate SecretParts on all detectors#4919

Merged
mcastorina merged 2 commits into
mainfrom
polecat/nitro-mob4i8yf
Apr 24, 2026
Merged

Populate SecretParts on all detectors#4919
mcastorina merged 2 commits into
mainfrom
polecat/nitro-mob4i8yf

Conversation

@mcastorina

@mcastorina mcastorina commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Populates Result.SecretParts on every detector package that constructs
detectors.Result{} without it. This is the last migration step in the
SecretParts effort — after this, the checksecretparts static check
reports zero findings across all 898 detector packages and can be flipped
from warning-only to hard-fail in a one-line follow-up.

Stacks on #4913 (which introduces the checksecretparts linter and is
what surfaces the list of unmigrated detectors this PR works through).
Depends transitively on #4911 (rename) and #4912 (docs).

Approach

Two commits:

  1. Single-part detectors (~695 packages) — the common case where a
    detector captures a single secret value. Adds
    SecretParts: map[string]string{"key": <value>} alongside the
    existing Raw: []byte(<value>).

  2. Multi-part detectors (~125 packages) — detectors that capture
    multiple components (key/secret, client_id/client_secret, host/user/
    password, etc.). Each gets a multi-entry SecretParts map keyed by
    the component's role.

Test plan

  • go build ./...
  • go vet ./pkg/detectors/...
  • go run ./hack/checksecretparts -fail — 898 packages scanned, 0 findings
  • go test ./pkg/detectors/... — 895 packages ok, 0 FAIL

Note

Medium Risk
Broad mechanical change across hundreds of detector implementations; while mostly additive metadata, mistakes in per-detector SecretParts keys/values could affect downstream analyzers or deduping/verification that expects specific component naming.

Overview
Populates detectors.Result.SecretParts for detectors that previously only set Raw/RawV2, including single-value detectors (adding { "key": <value> }) and multi-part detectors (adding role-keyed maps like key/secret, id/token, url/password, etc.).

This standardizes structured secret component output across the detector suite (including some results that also use RawV2, Redacted, and ExtraData) without changing the underlying match/verification flows.

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

@mcastorina mcastorina requested review from a team April 23, 2026 21:35

@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 57093b7. Configure here.

SecretParts: map[string]string{
"url": portalLink,
"email": email,
},

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.

ClickHelp SecretParts omits the API key component

Medium Severity

The SecretParts map for the ClickHelp detector includes "url" and "email" but omits apiKey, which is the actual secret credential. The detector iterates over three nested loops (portalLink, email, apiKey), and the apiKey is used for authentication via SetBasicAuth(email, apiKey) and is even explicitly flagged via s1.SetPrimarySecretValue(apiKey). Omitting it from SecretParts means the most sensitive component of the credential is not tracked.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 57093b7. 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 a good callout and makes me question why apiKey isn't part of the Raw??

@mcastorina mcastorina Apr 24, 2026

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.

The SecretParts match what is used in RawV2, so for consistency I want to keep this as is, even if it's wrong.

It will be a good guinea pig for supporting hash migrations in enterprise once that work is complete.

Base automatically changed from polecat/rust-moae37t5 to main April 24, 2026 15:15
@mcastorina mcastorina requested a review from a team as a code owner April 24, 2026 15:15
Adds SecretParts: map[string]string{"key": <secret>} to every detector
package that constructs detectors.Result with a single captured secret
value. This is the single-part half of the SecretParts migration (the
linter's common case, ~695 packages).
@mcastorina mcastorina force-pushed the polecat/nitro-mob4i8yf branch from 57093b7 to 5d5a825 Compare April 24, 2026 15:20
@mcastorina mcastorina merged commit ad2f9a4 into main Apr 24, 2026
16 checks passed
@mcastorina mcastorina deleted the polecat/nitro-mob4i8yf branch April 24, 2026 15:32
MuneebUllahKhan222 pushed a commit that referenced this pull request May 29, 2026
* Populate SecretParts on single-part detectors

Adds SecretParts: map[string]string{"key": <secret>} to every detector
package that constructs detectors.Result with a single captured secret
value. This is the single-part half of the SecretParts migration (the
linter's common case, ~695 packages).

* Populate SecretParts on multi-part detectors
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.

4 participants