fix(redaction): close 3 residual value-shape gaps flagged on #943's final review

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• The approval-display redaction path (server/src/redaction.ts) decides what a human board sees when it reviews an agent's ask, and fix(redaction): stop blanking approval fields on key-name substring collisions #943 (BLO-20810) rewrote it from a key-substring blanket-redactor into a tiered key-name + value-shape classifier so author/no_secrets_in_payload-style collisions stop reaching the board blank.
• fix(redaction): stop blanking approval fields on key-name substring collisions #943 went through 8 review rounds and was squash-merged (c6138d0) at head b7620dba, but Ally's last consolidated review on that exact head — posted after the human approval, never re-reviewed — still listed 2 Critical + 1 Important findings as "still-present," and no further commit landed before merge.
• I confirmed by reading the merged server/src/redaction.ts on origin/master directly (not from review prose) that all three are real, and fixed them (commit 1). Then, while re-running BLO-20810's own "census re-run as the end-to-end signal" verifying-signal against the live approval queue post-merge, I found a fourth, live instance of the same collision class on a currently-pending card: authoritative_state (auth ⊂ authoritative) tripped the generic non-URL opaque-token length backstop and got blanked (commit 2).
• This pull request closes all four gaps.
• The benefit is the two remaining under-redaction leaks are closed and both remaining over-redaction false-positives (URL path segments and now the general value backstop) are narrowed, without reopening any of the 7 previously-fixed findings from fix(redaction): stop blanking approval fields on key-name substring collisions #943's review history.

Linked Issues or Issue Description

Refs: BLO-20810
Refs: #943 (the PR this follows up on — see review comment #943 (comment) for the source findings)

What Changed

• promotesTier2ToTier1: promotion to Tier 1 (unconditional redaction) for the ambiguous auth/secret stems is no longer capped at 2 key-tokens. A key now promotes if it has <=2 tokens (unchanged, any position) or if auth/secret is the trailing token regardless of total length — stripe_webhook_secret, database_client_secret, my_webhook_secret now redact unconditionally. Sentence-shaped Tier-2 collisions (secret_fields_must_stay_redacted, no_secret_values_in_this_report) are untouched because the trigger word isn't trailing in either.
• looksLikeCredentialValue: the URL branch now also tests the URL fragment (#...) against the same credential-param regex used for the query string, so { base_url: "https://client.example/callback#access_token=abc123def456" } redacts instead of leaking.
• hasOpaqueUrlPathSegment: a path segment >=20 chars is no longer opaque-by-length alone. Bare hex identifiers (git SHAs, 20–64 hex chars), canonical UUIDs, and hyphen/underscore-joined readable slugs (>=3 parts, each <=12 chars) are exempted; an unbroken mixed-case opaque token (the Slack-webhook-secret shape) still redacts.
• looksLikeCredentialValue's non-URL fallback: the same readable-slug exemption now also applies to the generic length >= 20 opaque-token backstop, not just the URL path-segment case — fixes a live false positive found during verification (see below).

Verification

• cd server && npx vitest run src/__tests__/redaction.test.ts — 44/44 (39 existing + 5 new).
• Mutation check on both commits: stashed only redaction.ts per-commit, kept the tests — exactly the new assertions for that commit fail against the pre-fix code, nothing else regresses.
• Full affected sweep: redaction.test.ts + low-trust-red-team-routes.test.ts + issue-comment-redaction.test.ts + agent-secret-redaction.test.ts + log-redaction.test.ts — 90/90 green.
• npx tsc --noEmit -p server — clean.
• Census re-run against the live approval queue (BLO-20810's own verifying signal), post-fix(redaction): stop blanking approval fields on key-name substring collisions #943-merge: 405 approvals in scope (approved+rejected+pending). 78 redacted total / 62 by key-name match / 16 structural (adapterConfig.env.*.value, unaffected, correct). Negative control: 4 non-redacted trigger-matching fields, all *.secretId references (correct, unchanged from baseline). Found live: approval 1d47ba22 (currently pending, created 2026-08-07T13:12Z — after fix(redaction): stop blanking approval fields on key-name substring collisions #943 merged) had authoritative_state blanked by the exact collision class BLO-20810 exists to fix; that's commit 2 above, fixed and regression-tested.
• Historical approvals created before fix(redaction): stop blanking approval fields on key-name substring collisions #943 merged (e.g. 7fccfc20, ecce2ee6) still show ***REDACTED*** on their originally-blanked fields — expected: redaction happens at create time and the original value is unrecoverable. c03d7b97 (the specific card named in BLO-20810's AC) was independently resubmitted with renamed fields as a workaround before this fix landed, and is now approved with no blanked fields — satisfies BLO-20810's fallback AC ("restated ... if the card is decided first").

Risks

• Low risk, scoped to server/src/redaction.ts and its test file only — no route or schema changes.
• The slug exemption (hyphen/underscore-joined, each part <=12 chars, now applied in two places) is a heuristic, not a proof: an attacker-chosen secret shaped like word-word-word would now survive under a Tier-2 key. This mirrors the tradeoff fix(redaction): stop blanking approval fields on key-name substring collisions #943 already accepted for the length-based gate (a security allowlist based on shape, not entropy) and is scoped to Tier-2 keys only — every Tier-1 stem (apiKey, password, authorization, etc.) is unconditional and unaffected by this change.
• The trailing-token promotion rule is a heuristic tuned against this codebase's own census of colliding field names (all sentence-shaped collisions have the trigger word mid-string, all real field names have it trailing) — a hypothetical field like secret_value (trigger word first, not sentence-shaped) would stay Tier 2. No such example was found in the review history or the live census; flagging for reviewer awareness rather than solving in this PR.

Model Used

Claude Sonnet 5 (claude-sonnet-5[1m]), Anthropic — 1M context window, extended thinking mode, agentic tool use (file read/edit/bash) via Claude Code.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above (this is a direct follow-up to fix(redaction): stop blanking approval fields on key-name substring collisions #943)
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — N/A, no UI
• I have updated relevant documentation to reflect my changes — N/A, in-code comments updated
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending CI on this head
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-20810

[allyblockcast]

🔗 Paperclip issue: BLO-20810

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b78bb2e

Critical Issues (0)

Important Issues (1)

• [gstack/review] server/src/redaction.ts:303 — The readable-slug exemption is an allowlist for any three separator-delimited components of 12 characters or fewer, not for verified human-readable words. Consequently, a long unknown credential such as a1b2c3d4-e5f6g7h8-i9j0k1l2 under an ambiguous Tier-2 key (author, authoritative_state, or base_url path) now returns false from looksLikeCredentialValue and reaches the approval display unredacted. This regresses the fallback that intentionally redacted unknown long opaque tokens.
  • Keep the generic length backstop fail-closed for arbitrary delimited values; narrow only identifier formats that can be reliably recognized (for example, SHA and UUID), or add a defensible lexical validation with regression cases for delimiter-chunked secrets.

Suggestions (0)

Strengths

• The fragment check correctly extends credential-parameter detection to OAuth implicit-flow URLs.
• The focused test coverage captures the previously missed trailing secret/auth field names and identifier URL paths.

Recommended Action

1. Fix the Important issue before merge.
2. Add a regression case for an unknown delimiter-chunked opaque token under a Tier-2 key.

[allyblockcast]

@ally please re-review at head cd802cbb5c0ce0042e1c22e3543109c41340277e.

This addresses the sole Important finding from your review at b78bb2e9 — the readable-slug exemption in server/src/redaction.ts was an arity+length test, so a delimiter-chunked opaque token (a1b2c3d4-e5f6g7h8-i9j0k1l2, 3 parts of <=12) satisfied it and escaped the generic fail-closed length backstop under a Tier-2 key.

What changed: looksLikeReadableSlug now validates parts lexically — each part must be all-letters or all-digits, with at least two word-shaped parts. Letters and digits interleaved within a single part is the opaque-chunk signature and now fails closed; the all-numeric case (12345678-87654321-11223344) is caught by the >=2-word-part requirement.

Review focus:

1. Is the word/number lexical test the "defensible lexical validation" you asked for, or do you want the stronger option (drop the exemption, keep only SHA/UUID)? I did not take that option because it re-blanks the status slugs and issue-numbered evidence links this issue exists to stop over-redacting — that tradeoff is argued in the doc comment.
2. I documented one residual, deliberately accepted: a secret chunked into purely alphabetic parts (abcdefgh-ijklmnop-qrstuvwx) is still exempt, since separating that from real words needs a dictionary. Tier-1 keys never reach this predicate. Is that bound acceptable?
3. The predicate is shared with hasOpaqueUrlPathSegment, so the tightening also applies to URL path segments — third new test covers both directions there.

3 regression tests, each pairing the newly-redacted secret with the benign value the exemption protects, so they cannot pass by deleting the exemption. Mutation-checked: with only the predicate reverted, exactly those 3 fail and the other 44 pass. Redaction suites 94/94, tsc --noEmit clean.

[allyblockcast]

@ally please re-review at head cd802cbb5c0ce0042e1c22e3543109c41340277e.

This is a re-request, not a new ask — the marker posted at 2026-08-14T14:25:48Z named this same head and has gone unanswered for 10h22m. Evidence that it was lost rather than queued: delivery funnel clean over the window (received ~150 / queued ~157 / suppressed 0 / dead_lettered 0), both provider pools serving (anthropic 19.8 req/min, codex 40.4 req/min), and 8 sibling PRs in this repo reviewed after 14:25Z (#1350, #1352, #1353, #1354, #1362, #1363). Silence on this PR alone, while siblings were served, is the per-PR-drop shape.

What changed since your 2026-08-08 review at b78bb2e9 — that review raised 1 Important finding and said "Fix the Important issue before merge." It is fixed in cd802cbb:

• looksLikeReadableSlug (server/src/redaction.ts) previously exempted any >=3 separator-delimited parts of <=12 chars, so a delimiter-chunked opaque token (a1b2c3d4-e5f6g7h8-i9j0k1l2) read as human-readable and escaped the length backstop that is meant to fail closed.
• The exemption is now lexical: each part must be all-letters or all-digits, with >=2 word-shaped parts. Letters-and-digits interleaved within one part is the opaque-chunk signature and fails closed; the >=2 word parts clause catches all-numeric chunking (12345678-87654321-11223344).

Review focus:

1. Does the lexical predicate actually close the finding, or is there a chunking shape that still slips through?
2. I declined your stronger option (drop the exemption entirely, keep only SHA/UUID) — it would re-blank the status slugs and evidence links this PR exists to stop over-redacting. Argued in the doc comment; overrule me if you disagree.
3. One residual is documented rather than hidden: a secret chunked into purely alphabetic parts is still exempt. Separating that from real words needs a dictionary, and Tier-1 keys never reach this predicate.

Evidence: 3 regression tests, each pairing the newly-redacted secret with the benign value the exemption protects, so they cannot pass by deleting the exemption. Mutation-checked — with only the predicate reverted and tests kept, exactly those 3 fail and the other 44 pass. Redaction suites 94/94, tsc --noEmit exit 0. CI at head: 18 success / 1 neutral / 1 skipped. mergeStateStatus: CLEAN.