sec (9/9): document that scanners must not print secrets into raw evidence

[jesse-merhi]

What changes

Documentation only. Adds a note to docs/scanners.md telling user-defined
scanner authors not to print secrets into their JSON evidence, and corrects
the scanner-id field description to match the validation added earlier in the
series (lowercase, ≤64 chars).

Why it matters (finding 9)

The hardening series found nine issues. Eight are code fixes (#25–#32). The
ninth is not something ClawScan can fix in code without breaking its core
contract:

• ClawScan preserves a scanner's stdout verbatim as raw evidence — that
  rawness is the point (auditable, unmodified scanner output).
• If a scanner prints its own API key into that JSON, ClawScan stores it as-is.
• ClawScan already redacts declared env values from scanner error text
  (sec (7/9): redact only declared scanner secretEnv values #31), but redacting inside raw evidence would corrupt the evidence.

So finding 9 is the scanner author's contract, documented here rather than
enforced in code. This was an explicit decision, not an oversight.

The note

Do not print secrets into evidence. Because ClawScan stores the scanner's
stdout verbatim as raw evidence, any secret the scanner writes into its own
JSON output is persisted as-is. ClawScan redacts declared env values from
scanner error text, but it cannot redact inside raw evidence without
corrupting it. A user-defined scanner must not echo its API keys, tokens, or
other credentials into the JSON report it emits.

Scope

sec 9 of 9; stacked on #32. Docs source only — generated dist/docs-site is
left to the release step per repo policy. Merge order:
main → #23 → #24 → #25 … #32 → this.

Verification

Prose + one table cell. No code paths touched; go build/test unaffected.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[clawsweeper]

Codex review: needs changes before merge. Reviewed July 25, 2026, 10:42 PM ET / July 26, 2026, 02:42 UTC.

ClawSweeper review

What this changes

This docs-only PR adds a warning that user-defined scanners must not emit secrets in raw JSON evidence and updates the documented scanner-ID rules to lowercase and 64 characters.

Merge readiness

⛔ Blocked by patch quality or review findings - 4 items remain

Keep this PR open: the raw-evidence warning is useful, but it currently documents declared-environment-value redaction as existing behavior while that implementation remains in the separate open PR at #31. The narrow documentation correction should land only once that dependency is on the base branch, or the sentence should be qualified/removed. Likely related people: jesse-merhi is the feature introducer and recent area contributor (high confidence).

Priority: P2
Reviewed head: e0dc38149ceba9e9659c7d247e347d9ea7ef2dbb

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The patch is concise and scoped, but its security contract currently overstates behavior available on the base branch.
Proof confidence	🌊 off-meta tidepool	Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Patch quality	🦪 silver shellfish (2/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Evidence reviewed	4 items	Current raw-evidence contract: The PR diff is anchored at the current-main scanner documentation section explaining that scanner stdout becomes raw evidence, so this is the correct public contract surface for warning scanner authors not to emit credentials. Unlanded dependency: The added sentence says ClawScan redacts declared environment values from scanner error text, but the supplied related-item record identifies that behavior as the purpose of still-open PR #31 rather than current main. Feature provenance: The user-defined-scanner/raw-evidence behavior appears to date to the merged BYOS implementation, whose merge commit is the most relevant origin for this documentation contract.
Findings	1 actionable finding	[P1] Do not claim redaction before it lands
Security	Needs attention	Unmerged error redaction is documented as current: The docs tell scanner authors that declared environment values are redacted from error text even though the supplied related PR that implements that protection remains open; this can create a false expectation about secret handling.

How this fits together

Config-defined user scanners execute through the runner, and their JSON stdout is persisted as raw evidence in run artifacts. The scanner documentation defines the configuration and evidence-handling contract that scanner authors and operators rely on.

flowchart TD
  A[Profile scanner configuration] --> B[User-defined scanner]
  B --> C[JSON stdout]
  C --> D[Raw run evidence]
  B --> E[Declared environment variables]
  E --> F[Scanner error text]
  F --> G[Declared-value redaction]

Loading

Before merge

• Do not claim redaction before it lands (P1) - The new wording says declared env values are redacted from scanner error text, but that behavior is still the subject of open sec (7/9): redact only declared scanner secretEnv values #31. Landing this on the current base would document a protection users do not have yet; remove or qualify the sentence until that dependency merges.
• Resolve security concern: Unmerged error redaction is documented as current - The docs tell scanner authors that declared environment values are redacted from error text even though the supplied related PR that implements that protection remains open; this can create a false expectation about secret handling.
• Resolve merge risk (P1) - If merged now, the scanner guide would promise declared-environment-value redaction in error text before that protection has landed, creating a misleading security expectation for user-defined scanners.
• Improve patch quality - Remove or qualify the error-text redaction claim until sec (7/9): redact only declared scanner secretEnv values #31 has merged, then refresh the documentation diff against current main.

Findings

• [P1] Do not claim redaction before it lands — docs/scanners.md:93-94
• [medium] Unmerged error redaction is documented as current — docs/scanners.md:93

Agent review details

Security

Needs attention: The patch introduces a security-sensitive documentation claim that is not yet backed by the current merge base.

Review metrics

Metric	Value	Why it matters
Documentation surface	1 file changed; 8 additions, 1 deletion	A small documentation-only patch makes a security contract claim, so its wording must match behavior already available on the merge base.

Merge-risk options

Maintainer options:

1. Align the warning with the current base (recommended)
   Remove or qualify the claim about declared-environment-value redaction until sec (7/9): redact only declared scanner secretEnv values #31 is merged, then retain the complete contract wording.

Technical review

Best possible solution:

Keep the raw-evidence warning, but state only landed behavior; add the error-text redaction sentence after #31 merges or rebase this PR on a base that contains it.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a documentation-only PR, and the relevant check is whether its security claims match behavior already landed on current main.

Is this the best way to solve the issue?

No. The raw-evidence warning is the right solution, but the added error-redaction assertion should not be published until its separate implementation is merged or the wording is limited to current behavior.

Full review comments:

• [P1] Do not claim redaction before it lands — docs/scanners.md:93-94
  The new wording says declared env values are redacted from scanner error text, but that behavior is still the subject of open sec (7/9): redact only declared scanner secretEnv values #31. Landing this on the current base would document a protection users do not have yet; remove or qualify the sentence until that dependency merges.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5cfa90b129f4.

Labels

Label changes:

• add P2: This is a focused security-documentation correctness issue with limited blast radius and a narrow repair.
• add merge-risk: 🚨 security-boundary: The added text could cause scanner authors to rely on an error-redaction protection that current main does not yet provide.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required because this PR only changes files under docs/.

Label justifications:

• P2: This is a focused security-documentation correctness issue with limited blast radius and a narrow repair.
• merge-risk: 🚨 security-boundary: The added text could cause scanner authors to rely on an error-redaction protection that current main does not yet provide.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🌊 off-meta tidepool and patch quality is 🦪 silver shellfish.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required because this PR only changes files under docs/.

Evidence

Security concerns:

• [medium] Unmerged error redaction is documented as current — docs/scanners.md:93
  The docs tell scanner authors that declared environment values are redacted from error text even though the supplied related PR that implements that protection remains open; this can create a false expectation about secret handling.
  Confidence: 0.98

Acceptance criteria:

• [P1] make docs-site.
• [P1] git diff --check.
• [P1] Review docs/scanners.md against the merged base and the final state of sec (7/9): redact only declared scanner secretEnv values #31.

What I checked:

• Current raw-evidence contract: The PR diff is anchored at the current-main scanner documentation section explaining that scanner stdout becomes raw evidence, so this is the correct public contract surface for warning scanner authors not to emit credentials. (docs/scanners.md:85, 5cfa90b129f4)
• Unlanded dependency: The added sentence says ClawScan redacts declared environment values from scanner error text, but the supplied related-item record identifies that behavior as the purpose of still-open PR sec (7/9): redact only declared scanner secretEnv values #31 rather than current main. (docs/scanners.md:93, e0dc38149ceb)
• Feature provenance: The user-defined-scanner/raw-evidence behavior appears to date to the merged BYOS implementation, whose merge commit is the most relevant origin for this documentation contract. (internal/runner/user_defined_scanner.go, 656e558b4ba5)
• ID-rule provenance: The lowercase and 64-character scanner-ID rule documented by this PR corresponds to the merged hardening change in commit 1ade9ae. (internal/profiles/resolver.go, 1ade9ae9be17)

Likely related people:

• jesse-merhi: Introduced the merged user-defined-scanner feature and the merged scanner-ID validation, and authored the adjacent open redaction PR that this documentation currently describes as already present. (role: feature introducer and recent area contributor; confidence: high; commits: 656e558b4ba5, 1ade9ae9be17; files: docs/scanners.md, internal/profiles/resolver.go, internal/runner/user_defined_scanner.go)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.