Skip to content

feat(scan): harden the publish scan with 20 detectors, a rule corpus, and a labeled fixture gate - #183

Merged
A1igator merged 9 commits into
mainfrom
A1igator/scan-hardening-corpus
Aug 26, 2026
Merged

feat(scan): harden the publish scan with 20 detectors, a rule corpus, and a labeled fixture gate#183
A1igator merged 9 commits into
mainfrom
A1igator/scan-hardening-corpus

Conversation

@A1igator

@A1igator A1igator commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Scope

Hardens the deterministic publish scan with 20 new detectors and a labeled fixture corpus that measures per-detector precision and recall in CI, so a detector edit shows its false-positive cost before it merges.

  • src/lib/scan-rules.json: the detector set moves out of code into data (id, tier, pattern, description, per-rule attribution). src/lib/scan.ts compiles it and throws on an unknown handler rather than silently dropping a block-tier rule. The tenjin server-side ingest gate vendors this same file.
  • src/lib/scan.ts: block tier gains BIP-39 seed phrases, otpauth:// TOTP URIs, framing-free OpenSSH keys, and 12 provider token shapes; warn tier gains RFC1918/loopback endpoints, collaboration links, cloud resource ids, .env dump blocks, and a Shannon-entropy catch-all. Placeholder suppression drops docs-shaped matches (sk-xxxx, <YOUR_KEY>, user@example.com) including for block-tier detectors, which is the one bypass risk here; it is narrowly scoped to x runs, angle/brace templates, your… labels, and the RFC 2606 reserved domains, and pinned by tests.
  • src/lib/scan-corpus.json + src/lib/scan.corpus.test.ts: 121 labeled samples (positives and benign lookalikes for every detector, plus an adversarial transcript-shaped sample) hold precision and recall at 1.0, enforce the redaction invariant, and pin a ReDoS budget against transcript-scale input.

Notes

The ReDoS audit found and fixed three quadratic patterns: email, internal-hostname, and db-connection-uri each had an unbounded run that backtracked against a following obligation. On a 200KB single line (the shape a JSONL transcript record takes) the worst detector now costs 39ms; before, two of them took 25 and 31 seconds.

The rule port was diffed against secretlint's preset-recommend. Eight shapes present there and absent from the gitleaks subset are now included and credited in NOTICE.md, along with the BIP-39 wordlist. Nothing derives from TruffleHog (AGPL).

Credential literals in the corpus are stored split into thirds and rejoined at test time, so the committed bytes carry no contiguous token shape. GitHub push protection rejected the first version of this branch, which is the correct behaviour and the reason for the split.

src/skills-text.test.ts had a guard tying the publish skill's warn-triage lists to the detector set. It fired on this change; the skill's two lists now name the five new warn detectors, and the guard reads scan-rules.json directly instead of scraping source and inferring each tier from proximity.

Testing

  • pnpm lint, pnpm typecheck, pnpm format:check: clean.
  • pnpm test: 2179 passed, 10 skipped, 69 files.
  • pnpm build and bash scripts/pack-smoke.sh: pass (the corpus JSON is bundled into dist, not shipped loose).

Closes #45. Refs #182.

Greptile Summary

The PR moves publish-scan definitions into a shared data corpus and substantially expands secret, credential, private-context, and entropy detection while adding labeled precision, recall, redaction, and performance fixtures.

  • Adds new blocking and warning detectors, including algorithmic BIP-39, environment-dump, and entropy checks.
  • Uses rule metadata to keep team-shelf filtering aligned across publish and edit.
  • Adds a labeled scanner corpus and adversarial performance checks.
  • Updates publish guidance, safety documentation, attribution notices, and release notes.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/lib/scan.ts Compiles the shared rule corpus, dispatches algorithmic detectors and redaction handlers, applies suppressions, and exposes consistent team-shelf filtering.
src/lib/scan-rules.json Defines detector tiers, patterns, attribution, redaction policies, suppressions, and team-shelf behavior as shared data.
src/lib/scan.corpus.test.ts Enforces labeled detector precision and recall, secret-redaction behavior, and transcript-scale runtime budgets.
src/lib/scan-corpus.json Supplies positive and benign samples for detector evaluation while splitting credential-shaped fixture values.
src/commands/publish.ts Applies data-driven team-shelf warning filtering while preserving non-bypassable hard blocks before wallet and network access.
src/commands/edit.ts Keeps edit scanning and team-shelf narrowing aligned with the publish path.
skills/tenjin-publish/SKILL.md Updates agent-facing guidance for the expanded detector set and team-shelf survivor rules.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Publish or edit content] --> B[Load embedded scan rule corpus]
    B --> C[Run regex and algorithmic detectors]
    C --> D[Apply placeholder suppressions and deduplicate]
    D --> E{Team shelf?}
    E -->|Yes| F[Keep blocks and teamSurvives warnings]
    E -->|No| G[Keep all findings]
    F --> H{Blocking finding?}
    G --> H
    H -->|Yes| I[Refuse publication or edit]
    H -->|No| J{Consent required for warnings?}
    J -->|Yes| K[Return findings for confirmation]
    J -->|No| L[Proceed to signer and remote write]
Loading

Reviews (2): Last reviewed commit: "fix(scan): move the team-shelf survivor ..." | Re-trigger Greptile

Context used (3)

… and a labeled fixture gate

Move the detector set out of code into src/lib/scan-rules.json as data (id,
tier, pattern, description, attribution) so the tenjin server-side ingest gate
can vendor the same corpus. scan.ts compiles that data and implements the
handlers and algorithmic detectors it names, refusing to start on an unknown
handler rather than silently dropping a block-tier rule.

Twenty new detectors. Block: BIP-39 seed phrases, otpauth:// TOTP URIs,
framing-free OpenSSH private keys, and Supabase/Twilio/SendGrid/Hugging Face/
Vercel/Notion/Linear/Figma/GitLab/Docker/Cloudflare/Databricks token shapes.
Warn: RFC1918 and loopback endpoints, collaboration workspace links, cloud
resource ids, pasted .env blocks, and a Shannon-entropy catch-all. Placeholder
suppression drops docs-shaped matches before they reach the findings list.

A labeled fixture corpus holds per-detector precision and recall at 1.0 in CI,
enforces the redaction invariant, and pins a ReDoS budget against
transcript-scale input, which caught three quadratic patterns (email,
internal-hostname, db-connection-uri).

Closes #45. Refs #182.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator added the priority: critical Blocks other PRs; merge-priority label Aug 18, 2026
@A1igator

Copy link
Copy Markdown
Contributor Author

The failing skill-drift (mirror is in sync) check is the known shared cause owned by #151: the mirror resync fix (97f8650) has not reached main yet, so every fresh branch inherits the drift. Not this PR's diff; no action here. It clears when #151 (or a split-out sync:skill fix) lands on main. All local gates on this PR are green (lint, typecheck, format, 2179 tests, build, pack-smoke).

@A1igator

Copy link
Copy Markdown
Contributor Author

Merged origin/main (d37524d, the #186 mirror resync) forward into this branch to clear the shared skill-drift red. Clean merge, no conflicts: this diff is scan rule data, fixtures, and their tests, and #180's hook work does not touch them. Re-ran lint, typecheck, format:check, and the full suite on the merge commit: 2232 passed, 10 skipped.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@vraspar vraspar 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.

Review: strong corpus and ReDoS work; the placeholder suppression bypasses the block tier on realistic input

Reviewed against docs/CONVENTIONS.md and the adversarial money/secrets model, verified at 8347a08, run at transcript scale.

What's solid:

  • The ReDoS budget test is real and meaningful: it exercises the exact quadratic shapes (uri separators, email locals, nested paths, base64 blobs) on a 200KB single line with a 1000ms budget. I reproduced the guarded email pattern at ~38ms on a 140K-token line, while an unbounded variant already hits ~102ms on a 4K-token line (quadratic, seconds at scale), so the budget reddens on the old pattern with a wide margin.
  • The corpus negatives are genuine hard lookalikes, not softballs: the 11-word BIP-39 boundary, getUserProfileByAccountIdentifier2 vs entropy, an all-lowercase git SHA, AKIALOOKSLIKE, sk-user-profile-..., SKU12345. Precision/recall 1.0 over 121 samples is earned on this set.
  • Licensing is honest: gitleaks (15 shapes), secretlint preset-recommend (exactly the 8 non-gitleaks shapes, each with a matching source field), BIP-39 wordlist credited to scure-bip39, and TruffleHog (AGPL) explicitly disclaimed. NOTICE matches the rule data.
  • The compile step throws on an unknown tier / algorithm / skip / excerpt handler rather than silently dropping a block rule (scan.ts compileLineDetectors / compileExcerpt).
  • The skills-text guard now reads scan-rules.json directly instead of scraping source and inferring tier by proximity — a strictly better coupling.

Major

  1. security: the docs-placeholder suppression bypasses the block tier on realistic secrets: isDocsPlaceholder runs DOCS_PLACEHOLDER.test(m[0]) — a SUBSTRING test over the whole match — ahead of every detector including block-tier ones (

    if (!isDocsPlaceholder(m[0]) && detector.skip?.(m) !== true) {
    ,
    const DOCS_PLACEHOLDER =
    /x{6,}|<[^<>]*>|\{\{[^{}]*\}\}|\byour[_-]?(?:key|token|secret|password)\b/i;
    function isDocsPlaceholder(match: string): boolean {
    return DOCS_PLACEHOLDER.test(match);
    }
    ). Because the pattern is x{6,}|<[^<>]*>|\{\{[^{}]*\}\}|... unanchored, any placeholder-shaped SUBSTRING inside a match suppresses the finding. I ran these against the real scan and every one produced ZERO block findings while the clean control blocked correctly:

    • postgres://appuser:Str0ng<Pw>Value@prod-db.acme.com:5432/main — password with angle brackets, no block.
    • mysql://root:Pa{{ss}}w0rd99@10.2.3.4:3306/app — password with braces, no block.
    • postgres://appuser:realpwxxxxxx99@prod-db.acme.com/main — password with a 6-x run, no block.
    • Authorization: Bearer ghp_realtokenxxxxxx0123456789abcdef — bearer token with an embedded 6-x run, no block.

    The db-connection-uri case is the sharp one: passwords legitimately contain <, >, {, }, and a connection string is one of the most common ways a live secret lands in a transcript — exactly the input the session-observer gate must hold on. This defeats the file's own stated guarantee that block findings "refuse a publish in every mode and are never --yes-clearable". Root cause: this is the same substring-vs-whole-value class the PR already fixed for isPlaceholder/PLACEHOLDER_WORD (whole-value anchored, scan.ts#L380-L395, pinned at scan.test.ts#L227-L238) but did NOT fix for isDocsPlaceholder. Fix: don't let the docs-placeholder test suppress a block-tier finding, or apply it to the captured secret value anchored (as the per-detector skip handlers already do) rather than as a substring of the whole match. The existing coverage misses this because the placeholder-suppression tests only assert an all-x or all-Z body, never a real block token/URI carrying an embedded placeholder substring (

    describe('scan — placeholder suppression', () => {
    it('drops docs-shaped placeholder matches even for block-tier detectors', () => {
    expect(checks(`the docs show ghp_${'x'.repeat(36)} as the shape`)).not.toContain(
    'github-token',
    );
    expect(checks('API_KEY=<YOUR_KEY>')).not.toContain('secret-assignment');
    expect(checks('set the header to A1b2C3xxxxxx4D5e6F7g8H9i0J1k2L3m4N5o')).not.toContain(
    'high-entropy-string',
    );
    });
    it('drops emails on the RFC 2606 reserved domains, but keeps real ones', () => {
    expect(checks('sample payloads use user@example.com throughout')).not.toContain('email');
    expect(checks('escalate to alice@corp.example when paging')).toContain('email');
    });
    it('does not treat a boring key body as a placeholder (no block bypass)', () => {
    // Only `x` runs and template braces are placeholders; a repeated-letter body
    // is still a live key shape.
    expect(checks(`t=ghp_${'Z'.repeat(36)}`)).toContain('github-token');
    expect(checks(`0x${HEX64}`)).toContain('raw-private-key');
    });
    });
    ) — add the embedded-substring case as a regression pin with the fix.

Minor

  1. backend/agent-usability: safety-model and changeset overstate the block guarantee: docs/safety-model.md now says the scan blocks "connection URIs with an embedded password ... in every mode" (

    Publishing and editing put content on a public marketplace under your identity. The local scan blocks obvious secrets and private-key material in every mode: provider token shapes, private keys in and out of PEM framing, connection URIs with an embedded password, BIP-39 recovery phrases, and TOTP provisioning URIs. Its rules live as data in `src/lib/scan-rules.json`. Other sensitive context can only be warned about, so the agent still has to use judgment before publishing.
    ), and the changeset frames block-tier placeholder suppression as a clean feature. Both are downstream of the Major: until the substring bypass is closed, the doc claims a non-bypassable block the code does not deliver for bracket/brace/x-run passwords. Fixing the Major makes the docs true; no separate doc change needed if it lands.

  2. hygiene: source comments cite ephemeral review rounds: scan.ts#L299 and scan.ts#L343 carry (review r5) provenance in code comments (the invariant they explain is load-bearing; the round citation is not). Per the anti-slop provenance rule, keep the "why" and drop the round tag. (Test-name (review rN) tags are fine — they document which regression each test pins.)

Nits (1), none blocking
  • scan.ts comment density is ~28% (198/696 non-blank), above the 12–15% target. Most of it is genuinely load-bearing "why" for security regexes (ReDoS bounds, charset exclusions, span-alignment fixes), so this is a light note, not a demand — a pass could thin the narration around the straightforward handlers.
Verified, not issues
  • Token-charset detectors (aws, github, stripe, jwt, npm, etc.) are NOT bypassable via <...>/{{...}} because their charsets exclude those characters; the only vector into them is a 6-x run, which a real working key cannot be made to contain on demand and hits by chance at negligible probability. The db-connection-uri and bearer detectors are the real exposure (broad value charsets), which is why the Major centers on them.
  • ReDoS: guarded email/db-uri/internal-hostname patterns all bounded; budget test runs at 200KB single-line scale and passes fast; reproduced the guarded-vs-unbounded gap directly.
  • Redaction invariant: block excerpts masked (pem armor the one header-only exception), corpus test asserts no excerpt carries the labeled secret; the email-inside-db-uri suppression prevents a password leaking through the email excerpt.
  • BIP-39: whitespace-only 12-word run; 11-word and punctuation-broken prose are negatives. Residual false-positive risk on transcript-scale prose or a crypto tutorial that lists 12+ wordlist words is an owner-accepted, documented tradeoff (scan.ts comment) and fail-safe (blocks publish, never leaks) — not a defect.
  • Pin: a13e98e is an ancestor of head; scan-rules.json byte-identical since the pin, so tenjin#723's vendored copy is current.
  • Local run: scan.test.ts + scan.corpus.test.ts + skills-text.test.ts green (168 tests); CI green at head including pack-smoke and skill-drift.

Verdict: comments-only. One Major — the placeholder suppression is a substring test that silently converts a block-tier finding into nothing for connection URIs whose password contains <, >, {, }, or a 6-x run, defeating the non-bypassable-block guarantee on exactly the transcript-scale input this corpus is being built to gate; the same bug class the PR fixed elsewhere. Two Minors (a doc that overstates until the Major lands, review-round provenance in two source comments) and one density nit. Corpus, ReDoS, and licensing work are solid.

The docs-placeholder rule is a substring test over the whole match and ran
ahead of every detector, so any secret merely CONTAINING a placeholder shape
was suppressed instead of blocked. Real passwords carry `<`, `>`, `{`, `}` and
letter runs, and a connection string is one of the commonest ways a live
credential reaches a transcript, so this defeated the non-bypassable-block
guarantee on exactly the input the observer gate exists to hold.

Suppression may now downgrade a warn, never a block. Block-tier suppression
stays per-detector and anchored to the captured secret value, which is the
whole-value form the rest of the file already uses. Adversarial fixtures pin
all four reported bypasses plus a `your…`-label variant, in scan.test.ts and in
the corpus; they are red without the one-line tier guard.

An all-x body in a block-tier shape now blocks rather than suppressing, which
is fail-safe: the corpus sample moves from negative to positive.

Also drops two review-round tags from source comments, corrects the changeset's
claim that suppression covered the block tier, and tightens the module header.

Refs #183 review 4965955596.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator

Copy link
Copy Markdown
Contributor Author

Thanks — the Major is correct and it was the sharp one. Reproduced your four inputs verbatim against the scan before touching anything: all four returned 0 block findings, the clean control returned 1. Fixed in 3e19464.

DOWNSTREAM: tenjin#723 vendored this corpus byte-identical and needs a re-vendor. scan-corpus.json changed in this commit (5 new adversarial samples + 1 relabel). scan-rules.json did NOT change, so if #723 vendors only the rules it is still current; if it vendors the corpus too, it needs the sync. Flagging loudly so the ordering does not get lost.

Major — placeholder suppression bypassed the block tier. Fixed as you framed it: the substring test may now only ever suppress a warn.

  • isDocsPlaceholder is gone; isSuppressedAsDocs(detector, m) gates on detector.severity === 'warn'. The block tier keeps only its per-detector skips, which were already whole-value anchored on the captured secret (exampleDbPasswordisPlaceholder(m[3]), placeholderGroup1isPlaceholder(m[1])) — the same anchored form you noted the PR had gotten right elsewhere.
  • I took the "don't let it suppress a block" branch rather than the "anchor it to the captured value" branch. Anchoring would also close your four cases, but it leaves the invariant as something a future reader has to re-derive per detector. A tier rule is cheaper to hold than an argument about charsets, and the cost of dropping it is one fail-safe false positive on documentation.
  • That cost is real and I took it deliberately: an all-x body inside a block-tier shape (ghp_ + 36 x) now blocks. The corpus sample moved from github-token/neg-placeholder to github-token/pos-placeholder-body. Fail-safe direction — it refuses a publish, it does not leak — and the operator edits their example.
  • Pins added, red without the fix (verified by reverting the one-line guard: 3 tests fail, all 97 pass with it). scan.test.ts gains a scan — the block tier is non-bypassable block with your four inputs plus a your…-label variant (redis://cache:yourkeyR3al99Value@…) and the ghp_+x case, asserting a block fires and that its excerpt is still [redacted-masked. The corpus gains the same five as labeled positives, so precision/recall now covers this class too (126 samples).
  • Your point about the exposure being db-connection-uri and bearer specifically is right — the charset-limited detectors were only reachable via a chance x run. The tier rule closes both anyway.

Minor 1 — safety-model / changeset overstated the guarantee. Agreed, and as you said the doc becomes true once the Major lands, so docs/safety-model.md is unchanged. The changeset did need editing: it framed block-tier placeholder suppression as a feature. It now says warn-tier only and states the anchoring rule.

Minor 2 — review-round tags in source comments. Fixed. Dropped (review r5) at both sites, kept the invariant each explains (the locale-sensitive toLowerCase span misalignment, and the d-flag splice vs. first-substring masking). Test-name tags left alone per your note.

Nit — comment density. Partially taken. I tightened the module header, which was the one place genuinely duplicating what NOTICE.md and the data file's source fields already say, and folded the invariants into a labelled list (the new TIER ORDERING invariant is now stated there rather than only at the call site). Density moved 29.4% → 29.2%, so: mostly not taken, on purpose. The remainder is ReDoS bounds, charset exclusions, span-alignment fixes, and documented accepted-gaps — the "why" that a regex cannot carry and that this review cycle just demonstrated is expensive to rediscover. I would rather carry it than hit the ratio.

Gates on 3e19464: pnpm lint, pnpm typecheck, pnpm format:check clean; full pnpm test 2234 passed, 10 skipped (69 files passed / 1 skipped).

…log carries

An advisory replay of the deterministic tier over 389 published posts
(tenjin#723) found the block tier's only hits were three false positives, all
public hex constants: a committee hash with a word between label and value, an
EIP `source_id =` identifier, and the ERC-20 Transfer event topic0 in a code
sample. Roughly 0.8% of the real catalog would have been refused at publish.

The demotion to the warn hex32-value now recognizes an id-class label set
(salt, id, topic, root, digest, commitment alongside hash/tx/blockhash), a
label sitting up to two short tokens before the value, and a data list of
universal public constants. Every constant is computed, not transcribed: the
topic0s are keccak256 of the event signature and the EIP-1967 slots are
keccak256(label) - 1.

The floor holds. Demotion is to warn, never silence, so a real key mislabeled
`hash` still surfaces. An unlabeled bare 64-hex, a label four tokens out, and a
word merely ending in a label all still block, each pinned.

Bounding the lookback to 256 characters also closes a fourth quadratic: the
label test re-sliced the whole line per match, so a line dense with 64-hex
values took 18.6s. It now takes 63ms, with a ReDoS fixture pinning it.

Refs #183, tenjin#723 comment 5335652890.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator

Copy link
Copy Markdown
Contributor Author

Implemented the detector fix proposed in the pre-flip replay (tenjin#723, comment 5335652890 — 389 published posts, 3 block-tier hits, all false positives). Commit 1f066b5.

DOWNSTREAM: tenjin#723 needs re-vendor round 2 — and this one DOES touch the rules. Unlike the last round, scan-rules.json changed: a new top-level publicHexConstants array plus the raw-private-key description. scan-corpus.json also changed (6 new fixtures). Both files need the sync; the engine-pin drift check should force it.

What changed, against your three triaged FPs:

your finding why it missed now
committee hash, hash was 0x… demotion required the label adjacent label may sit up to two short tokens before the value
EIP source_id = 0x… id-class labels absent label set gains salt, id, topic\d*, root, digest, commitment
ERC-20 Transfer topic0 no label at all publicHexConstants data list, consulted by the hex64 detector

The floor holds, exactly as you scoped it. Demotion is to the warn hex32-value, never silence — a real key mislabeled hash still renders and still needs an ack under enforce. Pinned negatives: an unlabeled bare 64-hex blocks; the hash of the private key is 0x… blocks (four tokens is out of window — two is the bound that keeps it honest); the grid 0x… and a valid 0x… block (words merely ending in a label are not labels); PRIVATE_KEY=0x… blocks. Your 0xddf252ad…523b3ef is in the constants list and confirmed my computed value.

Constants are computed, not transcribed. Generated with viem: topic0s are keccak256(utf8(signature)), the EIP-1967 slots are keccak256(label) - 1. Twelve entries — ERC-20/721 Transfer + Approval, ERC-721/1155 ApprovalForAll, ERC-1155 TransferSingle/TransferBatch, Ownable OwnershipTransferred, WETH Deposit/Withdrawal, both EIP-1967 slots, keccak256(""), and the zero hash. Data, not regex, per the ask.

Coverage: the three catalog shapes plus a CREATE2 salt: case are corpus fixtures expecting hex32-value (warn), and the two floor cases are corpus fixtures expecting raw-private-key (block). Red without the fix, verified by reverting the widened regex and the constants lookup: 5 tests fail, 102 pass with it. The scan — the block tier is non-bypassable suite from the previous round stays green throughout — a placeholder-lookalike REAL secret still blocks.

Bonus, and it would have bitten you at scale: adding a dense-64-hex ReDoS fixture exposed a fourth quadratic. The label test re-sliced the whole line per match, so a line carrying 3,000 hex values took 18.6s. Bounding the lookback to 256 chars makes it 63ms (295x), with the fixture pinning it. This predates the widening — the narrower regex just hid it — so it was latent in what you replayed.

Gates on 1f066b5: pnpm lint, pnpm typecheck, pnpm format:check clean; full pnpm test 2239 passed, 10 skipped.

On the operator call in your comment: with this landed there is no ~0.8% FP left to accept, so holding for the widening should no longer cost anything.

The live hosted skill moved again tonight (the #178 multi-searchId wire
form), so the drift gate reddened on this branch, which merged main
before the move. Mirror-only: scripts/sync-skill.mjs writes
skills/tenjin/SKILL.md and nothing else, so the scan rule data, the
corpus, and the tenjin-publish detector lists are untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@A1igator

Copy link
Copy Markdown
Contributor Author

Resynced the vendored hosted-skill mirror in b09a57f — the live tenjin.blog/skills.md moved again tonight (the #178 multi-searchId wire form, same drift main resynced at 576bd69), and this branch merged main before the move. Mirror-only: scripts/sync-skill.mjs writes skills/tenjin/SKILL.md and nothing else, so it never collided with the skills/tenjin-publish/SKILL.md warn-triage additions, and the scan rule data and corpus are untouched. Reproduced the gate steps locally (node scripts/sync-skill.mjs then git diff --exit-code -- skills/): clean. Full pnpm test 2239 passed, 10 skipped; lint, typecheck, format:check clean.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator

Copy link
Copy Markdown
Contributor Author

Post-b09a57f gap from the tenjin#723 catalog replay: the widened hex32 demotion still misses one real published post whose shape is the committee hash was `0xd4e6…` — a backtick sits between the label window and the value, and neither the intervening-token separator class nor the trailing class in HASH_LABEL_RE ([\s:=/,._-]) includes backticks or quotes, so markdown-formatted hash mentions stay block. The b09a57f fixture for this shape (hex32-value/pos-label-with-intervening-word) omitted the inline-code formatting the actual post carries. Proposal: add ` and straight quotes to both classes and extend that fixture to the backticked form (floor cases unaffected — the label set and two-token bound are unchanged). The tenjin port (BackTrackCo/tenjin#723) tracks this file by sha256 pin and will follow once it lands. Replay numbers: 389 posts, block hits 3 → 1 at b09a57f; this is the survivor.

… and value

The tenjin#723 replay re-run at b09a57f cut block-tier hits from 3 to 1. The
survivor is a real published post writing its hash as inline code — hash was
`0x…` — where a backtick sits between the label window and the value, and
neither separator class in HASH_LABEL_RE carried backticks or quotes. The
b09a57f fixture for that shape had dropped the formatting the post carries, so
the corpus agreed with the bug; it now keeps the backticks.

Only the two SEPARATOR classes gain ` ' " and the curly quotes. The label set
and the two-token bound are untouched, which is what keeps the floor: no amount
of punctuation tolerance can invent a label that is not in the set, so
PRIVATE_KEY=`0x…` in backticks still blocks, as does `the wallet key is "0x…"`.
Both are pinned, alongside the existing out-of-window and word-ending-in-a-label
cases.

Refs #183, tenjin#723 comment 5335789613.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator

Copy link
Copy Markdown
Contributor Author

Closed the last survivor from the replay re-run (comment 5335789613) in 68e6d2e. scan-rules.json changed, so advance the #723 sha256 pin to 68e6d2e.

The fix is separators only. Both classes in HASH_LABEL_RE gain `, ', " and the curly quotes. The label set and the two-token bound are untouched — which is exactly what keeps the floor, because no amount of punctuation tolerance can invent a label that is not in the closed set.

You were also right that the fixture was complicit: hex32-value/pos-label-with-intervening-word had dropped the inline-code formatting the real post carries, so the corpus agreed with the bug. It now keeps the backticks, and there is a second markdown-shaped positive ("id": "0x…").

Floor cases added, per your note that they should be unaffected — pinned rather than assumed:

  • PRIVATE_KEY=`0x…` → still blocks (raw-private-key + secret-assignment), as a corpus fixture and a unit test.
  • the wallet key is "0x…" → still blocks: quotes are separators, but key is not a label.
  • The existing out-of-window (the hash of the private key is 0x…), word-ending-in-a-label (grid, valid), and bare-unlabeled cases are unchanged and green.

Red without the fix, verified by reverting just the two character classes: 3 tests fail (the new backtick/quote test plus both corpus precision assertions), 103 pass with it. The scan — the block tier is non-bypassable suite stayed green throughout.

Gates on 68e6d2e: pnpm lint, pnpm typecheck, pnpm format:check clean; full pnpm test 2240 passed, 10 skipped. That should take your replay to 0 block-tier hits over the 389 — worth a confirming re-run before the flip.

@vraspar vraspar 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.

Re-review: the block-tier bypass is fixed and pinned on the embedded-substring cases; the catalog-driven demotion widening holds its floor, with one label-breadth decision worth a look

Reviewed at 68e6d2e, delta over 8347a08. Three fix commits (placeholder-tier confinement, then two 64-hex-demotion widenings from the tenjin#723 pre-flip replay) plus a mirror resync. Re-verified by reproducing the round-1 bypass inputs and a fresh adversarial set against the built scan.

Resolved since round 1

  1. [Major] Placeholder suppression confined to the warn tier. isDocsPlaceholder is gone; isSuppressedAsDocs(detector, m) gates the substring test on detector.severity === 'warn', so the block tier keeps only its per-detector skips, which are whole-value-anchored on the captured secret. Re-ran the four round-1 inputs plus the all-x body against the fixed scan: the angle-bracket password, brace password, 6-x-run password, embedded-x bearer token, and ghp_ + 36 x all now BLOCK (5/5), and each block excerpt is redacted (no leak). Warn-tier suppression survives — the corpus precision/recall stays 1.0, so the FP-reduction purpose is intact — and the all-x-in-a-block-shape sample was relabeled neg-placeholderpos-placeholder-body (fail-safe: refuses a publish, does not leak). The regression pin scan — the block tier is non-bypassable covers the embedded-substring cases explicitly, not just all-x, and 3 tests go red when the one-line guard is reverted.
  2. [Minor] docs and changeset now state the truth. safety-model.md and the changeset now say "Warn tier only: the block tier stays non-bypassable, and its own suppressions are anchored to the captured value," and spell out that a password containing </>/{/}/an x run, an unlabeled bare 64-hex, and a secret-named assignment all still block.
  3. [Minor] Review-round provenance removed. Both (review r5) code comments are gone (zero remain in scan.ts).

New — the two catalog-replay commits (1f066b5, 68e6d2e)

Both are false-positive fixes derived from real published content: the tenjin#723 pre-flip replay over 389 published posts found 3 block-tier hits, all genuine FPs on public hex constants (an ERC-20 Transfer topic0, an EIP source_id, a hash was 0x… mention), then 1 survivor (a backticked `0x…`), then 0. The fixes widen the 64-hex→warn hex32-value demotion: the label set gains salt/id/topic\d*/root/digest/commitment, a two-short-token window between label and value, a publicHexConstants data list (12 keccak values, computed via viem not transcribed), and markdown/quote punctuation as separators. Verified the floor holds against the real scan: bare 0x…, PRIVATE_KEY=0x… (+ backtick form), "privateKey": "0x…", and the wallet key is "0x…" all still BLOCK, while legitimate public values (a tx hash, the ERC-20 topic0 constant) demote to warn. Demotion is to warn, never silence.

Decision (non-blocking, owner's call)

  1. The demotion label set now includes broad, innocuous words — id most of all — so a 64-hex mislabeled with one rides down to warn. Confirmed against the real scan: id = 0x<64-hex privkey>, salt: 0x<privkey>, digest 0x<privkey>, my backup id is 0x<privkey>, and key_id: 0x<privkey> all demote from raw-private-key (block) to hex32-value (warn). This is the same ambiguity-class tradeoff accepted in round 1 — a private key and a 32-byte public value are byte-identical on Base, so the scan can only guess from context — now with a wider label set. It is not a silent-leak bug: demotion surfaces the finding and requires an ack under enforce, the common accidental shapes still block, and the widening is evidence-driven from real FPs. The one thing worth a deliberate look is that id (and key_id, which the _-boundary catches) is common and innocuous enough that an accidental key-under-an-id-field leak — {"id": "0x…"}, key_id: 0x… — demotes to warn, and under the DEFAULT advisory mode a warn publishes with only a logged finding. Options to tighten: drop id specifically, or withhold the demotion when the value also sits in a secret-named / *_key context. Not blocking on it — the ambiguity is fundamental and round 1 accepted the principle — but it is the residue of the widening, so here is the concrete behavior.

Corpus / pin lockstep with tenjin#723

scan-rules.json changed in this delta (the new publicHexConstants array), and scan.ts changed (the Major fix plus the demotion widening). Note for #723: its drift check pins only the JSON data files, so advancing its sha256 pin to 68e6d2e syncs the data, but #723's hand-ported scan.ts must independently receive BOTH the isSuppressedAsDocs tier fix and the HASH_LABEL_RE/publicHexConstants demotion logic — exactly the "code outside the drift check diverges silently" gap from round 1. Verification of #723's copy happens in that PR's re-review.

Verdict

Approve. The Major is fixed at the tier boundary and pinned red-without-fix on the embedded-substring cases; both Minors are closed; the demotion widening is evidence-driven with the floor preserved and demotion never silencing. The one decision (label breadth, esp. id) is a considered tradeoff the owner may want to revisit, not a defect. Ran scan + corpus + skills-text suites (176 passing); CI green at head.

vraspar
vraspar previously approved these changes Aug 19, 2026
Comment thread skills/tenjin/SKILL.md
Drops the per-detector genre examples from the first triage list, which
repeated "in a <kind> piece" six times to say one thing, and folds the
second list's trailing sentence into its lead. Every detector name the
skills-text coverage guard reads is unchanged.

Also corrects the intro while it is shorter: it claimed rights and
employer-internal content "have no detector at all", which was already
untrue of paid-content-marker and confidential-marker, and it now names
seed phrases in the blocking tier.

Refs #183 inline comment 3809528654.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator added priority: high High priority and removed priority: critical Blocks other PRs; merge-priority labels Aug 19, 2026
Resolves two conflicts against the alpha release, the hooks rename, and
the skill mirror resync on main.

- src/lib/scan.ts: keep both additions. This branch's BIP-39 and entropy
  helpers land beside main's survivesTeamDrop, whose three named warns
  (secret-assignment, hex32-value, embedded-instruction) all still exist
  in the rebuilt detector set.
- skills/tenjin-publish/SKILL.md: keep main's teamMode arm and its
  closing marker, with this branch's rewritten warn triage in the else
  arm.
- src/skills-text.test.ts: re-pin the public tenjin-publish render, which
  moves on purpose now that the triage names the new detectors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TE4zSrBc7Qw1qPodTGCT9
@A1igator A1igator added priority: medium Medium priority and removed priority: high High priority labels Aug 25, 2026

@vraspar vraspar 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.

Re-review: the merge is clean, but the two new credential catch-alls never joined survivesTeamDrop, so team mode drops them on the unattended path

Reviewed at 9a1e157, delta over 68e6d2e (round 2, approved): commit 4231f5a (publish-skill condensation) and the merge of origin/main, which brought #201's team mode into the file this PR rewrites. Code read only; CI, Greptile and skill-drift green at head; clean merge-tree against current main. Nothing from rounds 1 and 2 is re-raised.

Resolved since round 2

  1. The publish skill's scan triage is condensed as asked (4231f5a): the per-detector genre examples are gone, the block tier now names seed phrases, and the false "rights and employer-internal content have no detector at all" sentence is corrected (paid-content-marker and confidential-marker exist as warn rules).

Still open

  • The demotion-label breadth decision (id most of all) from round 2 stays the owner's call. One new input: the Stop hook now generates notes from transcripts and JSON tool output, where a 64-hex under a literal id: key is far likelier than in a hand-written post, and the dogfood runs full-auto, which clears the surviving hex32-value warn unseen. If id was ever going to leave the label set, this is the argument.

New

Major

  1. security: add high-entropy-string and env-dump-block to survivesTeamDrop: the predicate came in from main (#201) as a hardcoded list written against main's detector set (block + secret-assignment + hex32-value + embedded-instruction), and its docstring says the question that survives a team drop is "is this a live credential", justifying secret-assignment as "a live key whose shape no block detector matches". This PR adds exactly two warn detectors that answer that question and did not exist on main: high-entropy-string (the PR's own "catch-all behind the named shapes", which fires only when no named detector did) and env-dump-block (three or more KEY=VALUE lines with substantial values). secret-assignment does not backstop them, since it requires a secret-named key (API_KEY|SECRET|ACCESS_KEY|PRIVATE_KEY|PASSWORD|TOKEN|CREDENTIALS|AUTH_TOKEN), so SEGMENT_WRITE_KEY=<base62>, SENTRY_DSN=..., or Authorization: Basic <base64> in a team note yields only the two new warns, both filtered at publish.ts:247 and edit.ts:232 before warns is computed, so no prompt in review or auto either. The Stop-hook capture path is exactly the transcript-and-config-paste input these detectors were built for. Not a regression (main had no such detector), but a merge-time semantic gap in the file this PR rewrote, and nothing pins it: no test names survivesTeamDrop against the rule set, and the skills-text corpus guard renders teamMode=false. Fix options: add the two ids, or move the decision into the data as a teamSurvives field on scan-rules.json so the next credential warn joins by construction; either way docs/safety-model.md:43, docs/command-reference.md:380 and the skill's team arm ("exactly four survive") need the count updated.
    /**
    * Does this finding survive the team-shelf warn drop?
    *
    * A team shelf is not public, so the warn tier's "is this safe to make PUBLIC"
    * question stops applying: a repo slug is the POINT of a team note. Two OTHER
    * questions do not stop applying, so the block tier survives whole plus the three
    * warns that ask one of them rather than the public-safety one.
    *
    * "Is this a live credential?" — a team shelf is a hosted Postgres with logs and a
    * static shared door key, and a leaked key there is leaked.
    *
    * - `secret-assignment` — DEPLOY_API_KEY="pk_live_…" is a live key whose shape no
    * block detector matches.
    * - `hex32-value` — the SAME 0x+64-hex private-key detector as `raw-private-key`
    * above, demoted to warn only because a block finding is permanently
    * non-bypassable and a post carrying a tx hash must not be hard-blocked. Warn is
    * the surfaced-for-review tier there, not the safe tier, so on a team shelf the
    * finding still has to be seen.
    *
    * "Would this text steer the agent that reads it?" — injection risk does not move
    * with audience the way a rights or third-party-data concern does, because the body
    * is fed to a model either way, and team-shelf bodies are the ones the push sidecar
    * injects into teammates' agents unasked.
    *
    * - `embedded-instruction` — imperative "ignore all previous instructions" shapes
    * and BEGIN SYSTEM/HIDDEN PROMPT headers. The per-injection nonce fence
    * (push-scripts.ts) is the real reader-side boundary and is identical in both
    * arms; keeping the check is what puts a human in front of the laundering path,
    * where an already-poisoned agent captures at turn end and publishes to the very
    * shelf the sidecar re-injects (review r6).
    *
    * All three are kept BY NAME rather than promoted to block, so the consent cascade
    * still governs them: `review` and `auto` confirm, `full-auto` clears them unseen
    * exactly as it already does on the marketplace. Callers: commands/publish.ts and
    * commands/edit.ts, which must never disagree — this predicate is why they can't.
    */
    export function survivesTeamDrop(f: ScanFinding): boolean {
    return (
    f.severity === 'block' ||
    f.check === 'secret-assignment' ||
    f.check === 'hex32-value' ||
    f.check === 'embedded-instruction'
    );
    }

Minor

  1. integrity: pin the survivor names to real check ids: all three names resolve today (secret-assignment and embedded-instruction are rules; hex32-value is emitted only by the demotion literal at scan.ts:491 and has no rule entry, hand-added to the corpus id set at scan.corpus.test.ts:49). It is now named by string in five places; rename the demotion literal and team mode silently stops surfacing 64-hex key material with every test green, the same silent-shrinking-set failure this PR's skills-text guard rewrite was built to prevent. One assertion that every name in survivesTeamDrop is a corpus rule id or a known emitted check closes it.
    export function survivesTeamDrop(f: ScanFinding): boolean {
    return (
    f.severity === 'block' ||
    f.check === 'secret-assignment' ||
    f.check === 'hex32-value' ||
    f.check === 'embedded-instruction'
    );
    }
  2. agent-docs: the team arm's block-tier enumeration is now the stale one: 4231f5a corrected the public arm (109-110) to add seed phrases and safety-model.md:38 lists five families, but the team arm at 86-87 still reads "provider token formats, private keys, connection URIs with an embedded password". Three surfaces, three enumerations of one tier.
    Exactly four survive:
    - The whole BLOCKING tier: structured credential shapes — provider token formats,
    private keys, connection URIs with an embedded password. This shelf is a hosted
    database with logs and a door key the whole team holds, so a live credential
    published here is still a live credential loose. Exits 3 in every mode, and no
    `--yes` and no mode clears it.
    - `secret-assignment`: a secret-named assignment such as
    `DEPLOY_API_KEY="pk_live_…"`, whose shape no block detector matches.
    - `hex32-value`: a `0x` + 64-hex value in hash context — the same detector as the
    blocking raw private key, kept a warn only so a receipt or tx hash is not
    permanently unpublishable.
    - `embedded-instruction`: an "ignore all previous instructions" imperative or a
    `BEGIN SYSTEM PROMPT` header. The one survivor that is not about credentials.
    Injection risk does not shrink for being private the way a publicness concern
    does: a note here is fed to your teammates' agents, and the push sidecar injects
    it unasked. If the imperative is source material the note is ABOUT, say so and
Nits (2), none blocking
  • skills/tenjin-publish/SKILL.md:117-119: 4231f5a dropped the qualifiers ("high-entropy-string where the piece quotes an opaque handle", "private-network-endpoint in a local-dev walkthrough") from the "usually fine" bucket, so the bucket now reads as unconditional, five lines under "a secret with no recognizable shape is a prompt to look". The bucket membership itself predates this delta.
    secret with no recognizable shape is a prompt to look, not a stop.
    It matches patterns, so warnings split in two and only the second is worth the
    user's attention:
    - Usually fine when the piece is genuinely about them: `local-path`,
    `wallet-address`, `embedded-instruction`, `email`, `private-network-endpoint`,
    `high-entropy-string`.
  • src/lib/bip39-wordlist.json:4 is a single 2046-word wordlist line, so the repo ships a file that hard-blocks its own publish scan: a team note that pastes that line or a diff of it is permanently unpublishable to the shelf. Fail-safe and narrow; noted because the capture loop makes "write up what you just worked on" the common case and this PR is what it just worked on.
Verified, not issues
  • Merge fidelity: main's 45-line survivesTeamDrop block is present verbatim; git diff 4231f5a..9a1e157 -- src/lib/scan.ts is that block and nothing else, so no PR-side detector, skip handler, or round-2 tier fix was lost. scan.ts exports and signatures are unchanged main to head, and no post-68e6d2e main commit (sidecar, hooks, push-scripts) calls into scan.
  • SKILL.md: tenjin:when teamMode / else arms intact; the team survivor list matches the predicate exactly; the public arm's two triage lists cover all 17 warn ids with no extras.
  • BIP-39 prose false positives, measured: of ~50 common English function words only "a" is in the wordlist, capitals and attached punctuation break a run, and a token walk over ~93k lines of real prose-about-code in this workspace produced no run longer than 7 outside intentional test fixtures. The owner-accepted tradeoff from round 1 holds.
  • Block-tier FP survey on team-shaped content: openssh-private-key matches the base64 body magic, not the armor header; db-connection-uri skips the default passwords; bearer-token's placeholder skip is whole-value anchored. Only totp-uri has no skip and would block a note quoting the canonical Google Authenticator docs example; very narrow.
  • embedded-instruction patterns are byte-identical to main, and the warn-tier placeholder suppression cannot reach them, so team mode surfaces neither more nor less injection text than before.
  • A refused capture is loud, not silent: the Stop hook hands the agent the tenjin publish command, and PUBLISH_BLOCKED plus its fix lands in the agent's transcript; the block tier stays fail-safe. The dropped-warn path in the Major is the silent one.
  • The tenjin server's lib/ingest-scan/rules.json on main is byte-identical to this head's scan-rules.json and its drift check is pinned to this PR's head commit, so merging unblocks repointing that pin to main.

Verdict: comments-only for this round. The delta itself (skill text and the merge) is clean; the Major is what the merge exposed: the team-drop predicate was written before these detectors existed and now filters the two that matter most on the unattended path. One-line fix plus a pin, or the data-field form. Approve once the survivor set is updated and pinned.

@vraspar

vraspar commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Correction to the Major in the review above: "no prompt in review or auto either" overstates it. needsConfirmation confirms unconditionally under review, so the operator is still stopped once per note there; what they lose is the finding in the prompt body. The accurate statement is: promptless under auto (the mode the publish.ts comment advertises for teams), and unattended under the dogfooded full-auto. The fix and the rest of the finding stand.

survivesTeamDrop arrived from #201 as a hardcoded list written against
main's detector set, so this branch's two credential catch-alls never
joined it: a team note pasting SEGMENT_WRITE_KEY=<base62> or a .env
block surfaced nothing under `auto` and published unattended under
`full-auto`. The survivor set is now a `teamSurvives` flag on the rule
in scan-rules.json, so the next credential detector joins by marking
itself. hex32-value is named explicitly in scan.ts: it is emitted by the
hex64 demotion, not by a rule.

Pins: publish tests that a team-mode `auto` scan of an env dump and of
an unrecognized high-entropy token both stop; corpus tests that every
survivor name resolves to a rule id or a known emitted check, and that
the emitted one is still emitted.

Docs: safety-model.md, command-reference.md and the publish skill's team
arm carry the new count, and the block tier is now enumerated the same
five families on all three surfaces.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TE4zSrBc7Qw1qPodTGCT9
@A1igator

Copy link
Copy Markdown
Contributor Author

All four items addressed in 884df83. Every claim in the review verified against the tree before changing anything.

Major, survivesTeamDrop. Taken as the data-field form you offered, on the orchestrator's call: teamSurvives: true on the rule in scan-rules.json (src/lib/scan.ts:96-106, 757-767), so the predicate reads the corpus and the next credential warn joins by construction rather than by an edit somebody has to remember. Flagged: secret-assignment, high-entropy-string, env-dump-block, and both embedded-instruction entries. Block-tier rules carry no flag, since the tier survives whole, and compileLineDetectors now throws if one does.

hex32-value is the case the data cannot carry, exactly as you described: it is not a rule id, it is what the hex64 demotion emits. It is named explicitly in EMITTED_TEAM_SURVIVORS (src/lib/scan.ts:757) rather than inferred.

Your correction is the framing I used in the code and the tests: review still stops once per note either way and loses only the finding from the prompt body. What the gap actually cost was auto going promptless and full-auto publishing unattended, which is the mode the Stop hook's capture runs in. The docstring at src/lib/scan.ts:781-786 and the test comment at src/commands/publish.test.ts:1595-1601 say it that way.

Pins (src/commands/publish.test.ts:167-180, 1589-1628): two team-mode auto publishes, one on a SEGMENT_WRITE_KEY=<base62> env dump and one on the same token loose in prose. Each body produces exactly one warn, and it is the one under test: the key name is deliberately outside secret-assignment's set and the value is no block-tier shape, so nothing else rescues either body. Both reject NEEDS_CONFIRMATION and send nothing.

Minor 1, survivor names. src/lib/scan.corpus.test.ts:212-250. Three assertions: every name in the survivor set resolves to a corpus rule id or a known emitted check; the emitted check is still emitted by a real scan, so renaming the literal at scanHex64 fails there instead of shrinking the set in silence; and the five credential and injection checks are all present. The last one is a subset check, not an equality, so adding a survivor stays free and removing one has to fail.

Minor 2, block-tier enumeration. Unified across all three surfaces at five families, including TOTP provisioning URIs, which the public arm was missing too: skills/tenjin-publish/SKILL.md:86-91 (team) and :116-119 (public), docs/safety-model.md:38. That moves the public render, so the digest in src/skills-text.test.ts:948 is re-pinned with the reason.

Docs. docs/safety-model.md:43 and docs/command-reference.md:380 carry the new counts (five warn survivors, six things total), name the two added checks, and point at the teamSurvives flag as where the set lives. The publish skill's team arm goes from four to six, and the --yes warning at :322 lists all five warn checks.

Nits. Took the first: the "usually fine" bucket has its qualifiers back plus an explicit statement that "genuinely about them" is the whole condition (skills/tenjin-publish/SKILL.md:126-131). Skipped the second: bip39-wordlist.json blocking its own line is fail-safe and fixing it means a scan-side exemption for a repo path, which is a wider hole than the one it closes. Worth its own issue if the capture loop actually hits it.

Security invariant, checked: the public path cannot move, since survivesTeamDrop is only called under runtime.teamMode, and no detector pattern, tier, skip handler, or excerpt policy changed. The new survivor set is a strict superset of the old one, so the team path gains two surfaced findings and loses none.

Gates: pnpm check green (2706 passed, 10 skipped), plus lint, typecheck and format:check clean. No merge of origin/main into the branch.

One heads-up for whoever merges second: PR #185 also edits skills/tenjin-publish/SKILL.md and docs/safety-model.md on its own branch, so the later merge needs a small doc reconcile.

@A1igator

Copy link
Copy Markdown
Contributor Author

Correction to the line references in my previous comment. The content stands, the numbers were off. Accurate ones against 884df83:

  • teamSurvives on the rule type and the tier guard: src/lib/scan.ts:101-102 and :163-167.
  • The survivor set and the explicitly named emitted check: src/lib/scan.ts:731-743.
  • The predicate and the per-mode framing in its docstring: src/lib/scan.ts:745-796, with the cost sentence at :788-791.
  • Survivor-name assertions: src/lib/scan.corpus.test.ts:196-236.
  • Fixtures: src/commands/publish.test.ts:165-181. The two pins: :1611-1651, with the per-mode comment at :1613-1620.
  • The publish skill's --yes warning listing all five warn checks: skills/tenjin-publish/SKILL.md:328-336.

Everything else in that comment, including the file paths and the invariant and gate results, is as written.

@vraspar vraspar 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.

Re-review: survivor set is data now, pinned both ways; approve

Reviewed at 884df83, delta over 9a1e157 (one commit). Code read; CI, Greptile and skill-drift green.

Resolved since round 3

  1. [Major] survivor set: teamSurvives: true on secret-assignment, both embedded-instruction entries, env-dump-block, high-entropy-string; survivesTeamDrop is block || TEAM_SURVIVOR_CHECKS.has(check); compileLineDetectors throws on the flag outside the warn tier. Strict superset of the old set, survivesTeamDrop only called under teamMode, no detector or tier changed, so the public path cannot move. Two team-mode auto pins on a SEGMENT_WRITE_KEY env dump and the same token loose in prose, each producing exactly the warn under test.
  2. [Minor] survivor names pinned: corpus test resolves every name to a rule id or a known emitted check, asserts hex32-value is still emitted by a real scan, and floors the five credential/injection checks as a subset check.
  3. [Minor] block-tier enumeration: unified at five families across team arm, public arm and safety-model.md; public digest re-pinned with the reason.
  4. [Nit] "usually fine" qualifiers: restored.

Closed as decisions

  • The wordlist line self-blocking: left as fail-safe rather than a path exemption. Agreed.
  • Demotion label breadth (id): stays the owner's call, unchanged across rounds.

Verdict: approve. Note for the merge order: tenjin main's lib/ingest-scan/rules.json is pinned to this PR's head, and scan-rules.json changed in this commit (the flag), so the server pin should be advanced to main once this lands. #185 also edits skills/tenjin-publish/SKILL.md and docs/safety-model.md; whichever lands second needs a small doc reconcile.

@A1igator
A1igator merged commit 717e693 into main Aug 26, 2026
4 checks passed
@A1igator
A1igator deleted the A1igator/scan-hardening-corpus branch August 26, 2026 06:56
A1igator added a commit that referenced this pull request Aug 26, 2026
Second lander after #183 (scan hardening) and #187 (builder code). Three
conflicts, all resolved by uniting the two intents rather than picking a side.

docs/safety-model.md: keeps #183's masked-excerpt paragraph and its widened team
arm (five warn survivors, `teamSurvives` as data), keeps this branch's
local-first/`--excerpt` coverage sentence, its consent paragraph, and its
team-shelf-stays-advisory paragraph.

skills/tenjin-publish/SKILL.md: takes #183's five-check survivor list and keeps
this branch's correction to the same sentence, that the survivors are not the
only findings there are once the shelf scans at ingest.

skills-text.test.ts: both re-pin notes kept, digest re-pinned for the merged
render.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013TE4zSrBc7Qw1qPodTGCT9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Publish safety follow-up: RFC1918 / collaboration-URL / cloud-ARN warn checks

2 participants