Skip to content

Scan sessions for exposed credentials and surface findings (#327) - #332

Merged
mando merged 11 commits into
mainfrom
mando-issue-327-secret-scanning
Aug 18, 2026
Merged

Scan sessions for exposed credentials and surface findings (#327)#332
mando merged 11 commits into
mainfrom
mando-issue-327-secret-scanning

Conversation

@mando

@mando mando commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Closes #327.

What

A deterministic, regex-based secret scanner runs at materialize time over each session's in-memory prompt/response text — no LLM call, no throttle, and independent of the retainText setting (the text is in memory at write time either way). Findings are stored in a new resolved_secret_findings table (schema v23 → v24 migration included).

A finding never contains the secret — only a category (aws_access_key, github_token, anthropic_api_key, openai_api_key, stripe_key, slack_token, private_key, jwt, generic_secret), a location (interaction + prompt/response slot), and a redacted hint (AKIA…WXYZ, the card-statement last-4 convention) so the user recognizes which credential it was.

Surfacing

  • Session detail: warning banner listing each finding (kind, hint, prompt vs response) with a Dismiss action. Dismissal is anchored to the finding set's digest (resolved_sessions.secret_scan_dismissed, carried forward by materialize like is_hidden), so it lapses automatically when a re-scan finds something different. A dismissed banner collapses to a muted line with "Show again".
  • Session list: red count badge on rows with undismissed findings.
  • Recommendations: an "N sessions may contain exposed credentials" warning that leads the list, so the signal reaches users who never open the session.

Explicit decisions (the issue's open questions)

  • Where it runs: inline in toMaterializeSessions, not a drain — it's free and deterministic, and a drain reading retained text back would go blind when retainText is off.
  • Sync: findings stay local-only in v1push.ts never reads the table (the same structural guarantee resolved_interaction_text gets), so even the redacted locators can't cross the wire. A count-level org signal is a possible future decision, documented in the design doc.
  • Rule set: small, high-precision subset in the spirit of gitleaks; the generic KEY=value rule is gated by length + Shannon entropy + a placeholder blocklist. Precision over recall so the banner stays trustworthy.
  • Dismissal: digest-anchored acknowledge, per above.

MCP (/mcp) strips findings alongside firstPrompt when transcript access is off — findings are derived from transcript text, so they inherit its gate.

Verification

  • 33 new tests: scanner rules/redaction/dedupe (test/secret-scan.test.ts), store persistence/dismissal/migration (test/secret-findings.test.ts, test/store.test.ts incl. a v23→v24 migration test), serve routes incl. CSRF + read-only drops (test/serve.test.ts), the recommendations rule (test/recommendations.test.ts).
  • Full suite green (756 tests), tsc --noEmit clean for root + web/, build:web succeeds.
  • End-to-end smoke test against a live serve: indexed a fixture session with synthetic keys, verified the detail payload, list badge, dismiss → banner/badge/recommendation all clear, undismiss, and the CSRF 403. Visually verified the banner and dismissed state in a browser.

Design doc: docs/internals/secret-scanning.md.

Fixes: #328

@mando

mando commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Low: Session-level secret deduplication is not implemented

Location: src/indexing/secret-scan.ts:124-141

scanTextForSecrets creates a fresh seen set for each text chunk. scanSessionForSecrets calls it separately for every prompt and response, then concatenates the results. As a result, the same credential repeated in two interactions is emitted twice, despite the comment at lines 100-101 saying that a key repeated across prompts is kept only at its first location.

Reproduction at the PR head:

const key = "AKIA" + "Q3G5X7BDFHJKLMNP";
scanSessionForSecrets({
  interactions: [
    { seq: 0, promptText: key },
    { seq: 1, promptText: key },
  ],
});
// returns two findings with the same category and hint

This can inflate warning counts and exhaust MAX_FINDINGS_PER_SESSION before distinct credentials are reported. Please keep a session-level seen set, or deduplicate the accumulated findings by a stable rule/value key while preserving the first location.

@mando

mando commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Low: Finding list has no explicit user-visible ordering

Location: web/src/components/SecretFindingsBanner.tsx:86-90

The banner renders a <ul> directly from findings.slice(0, 5). The source order is scanner rule order followed by interaction traversal, which is not an obvious ordering for users. This also conflicts with the repository UI rule that user-visible lists must be explicitly ordered and must not be presented as unordered lists.

Please choose and document an ordering, such as interaction sequence, prompt before response, then category and hint, and render an ordered or otherwise visibly ordered presentation.

@mando

mando commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in eca4cb2 — the dedupe seen set is now shared across the whole session (passed from scanSessionForSecrets into each scanTextForSecrets call) instead of being recreated per chunk, so a credential repeated across interactions flags once at its first location. Added a regression test covering the exact repro.

@mando

mando commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in 0a7b7a4 — the banner now sorts findings by interaction seq, then prompt before response, then category and hint (the scanner emits rule-order within a chunk; the user cares about where in the session a credential appeared), and renders an ordered <ol>. The ordering is documented on orderFindings.

mando and others added 11 commits August 18, 2026 15:50
Add a deterministic, regex-based secret scanner that runs at materialize
time over each session's in-memory prompt/response text (no LLM call, no
throttle, independent of text retention). Findings record only the
category, location, and a redacted hint (first/last few characters) —
never the secret value — and are stored in a new resolved_secret_findings
table (schema v24) that sync never reads, the same structural local-only
guarantee retained conversation text gets.

Surfaced in the web app three ways: a warning banner on the session
detail view listing each finding with a Dismiss action (anchored to the
finding set's digest, so it lapses when a re-scan finds something
different), a red count badge on session list rows with undismissed
findings, and an exposed-credentials warning that leads the
recommendations list. MCP gates findings behind the same
transcript-access setting as prompt text.

Rule set is a small high-precision subset (AWS/GitHub/Anthropic/OpenAI/
Stripe/Slack tokens, PEM private keys, JWTs, and an entropy-guarded
generic KEY=value rule) — precision over recall so the warning stays
trustworthy. Design decisions are documented in
docs/internals/secret-scanning.md.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the hand-written patterns with the gitleaks default config's rules
(https://github.com/gitleaks/gitleaks/blob/master/config/gitleaks.toml, MIT,
(c) Zachary Rice), pinned to upstream commit b58d3f102cf3. The patterns,
per-rule entropy floors, regex allowlists, and stopwords now live in a
data-only file, src/indexing/secret-scan-rules.ts, with full attribution and
a documented update procedure — a rule-set refresh is a data edit, not a
logic change. The scanner engine (secret-scan.ts) adopts gitleaks' matching
semantics: capture group 1 is the secret, entropy floors and allowlists drop
candidates.

Coverage is both stricter and broader: AWS's docs-example key is allowlisted
out, Anthropic/OpenAI keys require their real shapes (sk-ant-api03-…AA, the
T3BlbkFJ marker), and GitHub coverage grows to oauth/app/refresh/fine-grained
tokens plus Slack user/app/config tokens. Tests updated to synthetics that
satisfy each upstream rule.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
scanTextForSecrets created a fresh seen set per text chunk, so the same
credential pasted into two interactions was emitted twice — inflating
warning counts and burning MAX_FINDINGS_PER_SESSION on duplicates. Hoist
the seen set into scanSessionForSecrets and share it across every prompt
and response so a repeated credential flags once, at its first location.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The banner rendered findings in scanner rule order (then interaction
traversal), which is not an obvious ordering for a reader and left the
list unordered. Sort by interaction seq, prompt before response, then
category and hint, and render an ordered list.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
gitleaks' generic-api-key allowlist is written with regexTarget = "match":
its suppressions match the key NAME (key_name, csrf_token, public_token,
access_id, api_endpoint, ...), which only exists in the whole match. We
tested every allowlist regex against the extracted secret, so that entire
suppression list never fired and those assignments were reported as
exposed credentials.

Model allowlists the way gitleaks does - a list of blocks, each with its
own regexTarget - so the generic rule's two blocks (a secret-targeted
"bare identifier" check and the match-targeted key-name list) each test
what they were written against.
`ANTHROPIC_API_KEY="sk-ant-..."` matched both the precise rule and the
generic catch-all, and the dedupe key was per-rule, so one pasted key
produced two findings: the session badge read 2, the banner flipped to
the plural headline, and the list showed the same secret twice under two
categories. Quoted assignment is the common shape, so this was the normal
case rather than an edge one.

Run the precise rules ahead of the generic one and record the values they
claim (session-wide, alongside the existing dedupe set), then drop a
generic finding whose value overlaps a claimed one. Overlap rather than
equality: the generic rule's charset clips a JWT short.
Read-only mode (#281) drops the writes router, so POST
/api/sessions/:id/secret-findings/dismiss is never mounted and clicking
Dismiss failed with nothing to show for it. Every other write affordance
on the session page is already gated; the banner was not.

Gate the buttons, not the banner: the warning is worth showing on a
read-only server, the writes are not offered there.
The dismiss/undismiss mutation had no error path: a stopped Argus, or a
re-index that cleared the findings the open page still shows, left the
button re-enabled and the banner in place with no explanation.

Render the mutation's message the way the session page's other actions
do, in both the open and the dismissed state.
The Activity recommendations cache for 30 seconds, so after dismissing
the only flagged session the lead recommendation kept warning about it.
Invalidate that view alongside the session queries.
The 23 -> 24 step duplicated the resolved_secret_findings table body,
justified by a rescue case the migration chain makes impossible: a store
stamped 24 runs no migration at all, and validateOwnership rejects
anything past it. What the duplication does buy is a second copy of the
table shape to keep in sync.

Use the fresh-create constant, like the other table-adding steps, and
drop the rationale.
The exposed-credentials count includes sessions the user hid, and the
list leaves hidden sessions out, so a flagged hidden session was counted
and unreachable at the same time while the recommendation said to open
it.

Add a `flagged` filter to the sessions list (GET /api/sessions?flagged=1)
that narrows to undismissed findings and is the one filter that shows
hidden sessions, marked as hidden on the row so it doesn't look like the
hide failed. The recommendation now carries a link to it, spreading the
view's current search so the list lands scoped the way the count was.
@mando
mando force-pushed the mando-issue-327-secret-scanning branch from 33b1b81 to 7902765 Compare August 18, 2026 20:50
@mando
mando merged commit c3cff70 into main Aug 18, 2026
1 check passed
@mando
mando deleted the mando-issue-327-secret-scanning branch August 18, 2026 20:52
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.

Add GitHub Copilot CLI as an LLM provider Scan session content for exposed secrets/API keys and surface findings to the user

1 participant