chore(security): masking rule precision — annotate correct fail-closed sites + fix nosemgrep placement (#790)

[ClintEastman02]

Closes #790 (Category 0 of the #756 silent-success-masking gate).

What changed vs. what the issue proposed

The issue proposed tightening the rule with a pattern-not so a catch/except that classifies an error and rethrows it stops being flagged as masking. An earlier revision of this PR argued that couldn't be delivered as a clean precision win and annotated the sites instead. @isadeks disproved that in review — the tightening is deliverable, and this PR now ships it.

The precision win the rule now encodes

cli/src/linear-oauth.ts's correct classify-then-rethrow —

catch (err) {
  if (isNotFound(err)) return undefined; // "not found" legitimately means nothing
  throw err;                             // everything else fails closed
}

— was previously structurally indistinguishable to the rule from the conditional-rethrow masking it was built to catch (the maskedConditionalRethrow / masked_conditional_reraise fixtures from #311):

catch (err) { if (cond) throw err; return null; } // sometimes throws, otherwise masks

The distinguishing signal is statement ordering: the correct pattern returns before it throws (the empty-success is the early classify branch; the throw is the fallthrough). The mask throws before it returns (the throw is the early branch; the mask is the fallthrough). The new pattern-not anchors on return $RET; ... throw $ERR; (return-before-throw) in both the TS and Python rules, so the legitimate pattern clears while #311's masking pattern still fires.

The trade-off (documented in the rule header)

The ... between the return and the throw is intentionally loose, so the exemption also clears an ambiguous shape — catch (e) { if (benign(e)) return null; throw e; } where benign() might be wrong. This is a deliberate call (see the CLASSIFY-THEN-RETHROW EXEMPTION block in silent-success-masking.yaml): a catch that rethrows on any path is fail-closed by construction, and per #730 we do not want a rule that trains contributors to annotate correct fail-closed code away. Genuine masks — which never rethrow — are unaffected.

Changes

• .semgrep/silent-success-masking.yaml — added the return-before-throw pattern-not to both ts- and py-silent-success-masking, with a header block documenting the exemption and its trade-off. Also documented the placement footgun (focus-metavariable: $RET anchors the finding on the return; a nosemgrep token binds only on the return line or the line immediately above).
• .semgrep/silent-success-masking.ts / .py — added okClassifyThenRethrow / ok_classify_then_reraise fixtures so the exemption is regression-tested, alongside the retained maskedConditionalRethrow fixtures that prove feat(security): custom semgrep rules for silent-success masking (AI004) #311 detection is preserved.
• cli/src/linear-oauth.ts — removed the nosemgrep on both readExistingWebhookSecret and its twin readExistingOauthTokens: the rule now clears these classify-then-rethrow sites on its own, so the annotations are no longer needed.
• agent/src/observability.py — the existing nosemgrep never bound (a multi-line comment left the token two lines above the return None); collapsed to a single token line directly above the return. This site is genuine best-effort masking (a tracer fault must not fail the write path) with no rethrow, so it stays annotated.
• cli/src/commands/jira.ts — removed an inert nosemgrep on an empty catch (no masked return, so nothing to suppress); kept the human comment.
• scripts/check-masking-suppression-placement.mjs (new) + mise.toml — a guard wired into security:sast:masking and :masking:range that fails if any masking nosemgrep token drifts off the return line, so the observability-style mis-binding cannot recur.
• AGENTS.md — documented that the token must sit on the flagged return line or the line immediately above.

cdk/src/handlers/registry-publish.ts (parseBody) keeps its annotation: it returns null on malformed JSON with no rethrow, so it is structurally a mask (the caller turns null into a 400 VALIDATION_ERROR) and no rule tightening clears it — annotation is correct there.

Verification

• semgrep test .semgrep/ — 2/2 fixtures pass (both the new ok-path and the retained feat(security): custom semgrep rules for silent-success masking (AI004) #311 masking detection).
• node scripts/check-masking-suppression-placement.mjs — all masking suppressions correctly placed.
• Ratcheted PR gate (security:sast:masking:range vs main) — zero new findings; the two cli/src/linear-oauth.ts sites now clear via the rule rather than via annotation.
• eslint (cli + cdk) clean; agent observability tests green.

Rebased onto current main (Cedar/budget/osv fixes merged); all checks re-run post-merge.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@52fae0a). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
cli/src/commands/jira.ts	33.33%	2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #862   +/-   ##
=======================================
  Coverage        ?   92.57%           
=======================================
  Files           ?      338           
  Lines           ?    98026           
  Branches        ?     9735           
=======================================
  Hits            ?    90744           
  Misses          ?     7282           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[isadeks]

1. Verdict

REQUEST_CHANGES — every code change in this PR is correct and I would happily take all five files as-is; but the load-bearing justification for not delivering the approved issue's headline deliverable ("no pattern-not can clear the false positive without regressing the #311 fixtures") is empirically false, and #790 is scope-frozen under ADR-003. Fix the record (or deliver the rule change) and this is an approve.

2. Vision alignment

• Tenet 4 (fail closed on risk; fail open only where safety allows) — this PR is squarely in service of tenet 4 rather than trading it away. It does not weaken the ts/py-silent-success-masking gate at all: I diffed .semgrep/silent-success-masking.yaml against the merge-base and the change is 100% comment lines — zero patterns, metavariable-regex, or focus-metavariable deltas. Detection power is byte-identical to main.
• The #775 regression check passes. Because the rule body is unchanged, the shape that blocked CI on PR #775 at cdk/src/handlers/shared/budgets.ts is still matched exactly as before (that site now sits at cdk/src/handlers/shared/budgets.ts:236 with its own justified on-line suppression). No blind spot introduced.
• Reviewable outcomes — the annotate-rather-than-weaken choice actually preserves the gate's forcing function on the classify-then-rethrow shape, which is exactly where #775's review found a question worth asking. That is a good instinct and worth stating in the PR body as the real reason (see blocking issue 1).
• No tenet is traded, so no ADR is required.

3. Blocking issues

1. The stated reason for dropping deliverables 1 and 2 of scope-frozen #790 does not hold — a viable pattern-not exists

.semgrep/silent-success-masking.yaml:16-22 (and the PR body / commit message, which become the permanent record)

Approved issue #790's Fix section lists four deliverables. Items 3 (move the mis-bound observability.py suppression) and 4 (document the placement footgun) shipped. Items 1 ("tighten the rule with a pattern-not for a catch/except block that contains a throw/raise") and 2 (fixtures) did not, and the issue explicitly pre-rejected the route this PR took:

Why not just suppress — Per the #730 precedent, suppressing correct code is the wrong shape of fix — it trains contributors to annotate correctness away. These are false positives; fix the rule.

ADR-003 line 32: "After approval, the issue is considered scope-frozen: further revisions that change deliverables require re-approval." The deviation was disclosed on #790 in the "Starting implementation" comment, which is the right instinct — but no maintainer replied and the issue was not re-approved, so the PR currently changes an approved deliverable on the author's own authority.

That would be a soft process nit if the technical claim behind it were true. It is not. The PR asserts:

any pattern-not broad enough to clear the linear-oauth false positive also silences the #311 detection

I tested that. The formulation the PR appears to have tried does regress — adding

- pattern-not: |
    try { ... } catch ($E) { ... throw $ERR; }

clears cli/src/linear-oauth.ts:428 and silences the maskedConditionalRethrow fixture at .semgrep/silent-success-masking.ts:103. Confirmed: findings on the fixture file drop from [13, 22, 32, 41, 50, 87, 103] to [13, 22, 32, 41, 50, 87].

But the two shapes are not structurally indistinguishable — they differ in the order of the return and the throw inside the catch. In the correct classify-then-rethrow the rethrow is the fallthrough and comes after the classified return; in maskedConditionalRethrow the throw is nested in an if and comes before the masking return. Anchoring on that ordering works:

- pattern-not: |
    try { ... } catch ($E) { ... return $RET; ... throw $ERR; }

Measured results with that single added clause, everything else identical:

scan	PR rule	rule + ordering pattern-not
.semgrep/silent-success-masking.ts fixtures	[13, 22, 32, 41, 50, 87, 103]	[13, 22, 32, 41, 50, 87, 103] — all true positives kept, incl. #311's maskedConditionalRethrow
cli/src/linear-oauth.ts (merge-base version, unannotated)	[428]	[] — false positive cleared, no annotation needed
full worktree, --exclude '.semgrep/*'	16 findings	the same 16 findings — nothing else lost

So deliverable 1 was achievable, and the "impossible" framing is what makes this blocking rather than a nit: the PR body and commit message are the artefact the next contributor reads when they pick up the remaining #756 categories, and they currently record a false impossibility result about the project's own security rule. That is how a gate quietly stops being tightened.

Risk: (a) an approved, scope-frozen deliverable is dropped on a disproven premise with no re-approval; (b) the false claim is now cited in the commit message and will be inherited by #756's other categories; (c) cli/src/linear-oauth.ts:428 keeps a permanent suppression that a rule fix would have made unnecessary — and a suppression is a blind spot forever, whereas a rule predicate is reviewable.

Fix — either is fine, both are cheap:

1. Deliver it. Add the ordering pattern-not to ts-silent-success-masking (plus the python analogue), add the classify-then-rethrow shape as an ok-annotated fixture in .semgrep/silent-success-masking.{ts,py}, and drop the cli/src/linear-oauth.ts:428 suppression (and, for consistency, the pre-existing twin at :497). Note this does trade away detection of the genuinely ambiguous catch (e) { if (benign(e)) return null; throw e; } shape — call that out in the rule header so the trade is on the record. cdk/src/handlers/shared/budgets.ts:236 is that shape and already carries its own suppression, so no live coverage is lost today.
2. Or keep annotating, but correct the record. Replace "any pattern-not … also silences the #311 detection" with the accurate version: a pattern-not that clears this FP is achievable via return/throw ordering, but it would blanket-exempt every conditional-return-then-rethrow catch — including sites like budgets.ts:236 that we specifically want a human to justify — so we prefer an explicit, justified annotation per site. That is a stronger argument than impossibility and it is true. Then get #790 re-approved (or amended) for the changed deliverable per ADR-003 line 32 before merge.

Also note the header comment block currently documents only the placement footgun; whichever route is chosen, the "why we did not tighten the pattern" reasoning belongs in that header too, not only in a PR description that nobody greps.

4. Non-blocking suggestions

1. AGENTS.md:23 — the entry point contributors actually read still says only "allowlist intentional fallbacks with an inline nosemgrep: <rule-id> -- <reason> comment". That sentence is exactly what produced the observability.py bug this PR fixes: it does not say which line. Add four words — "on the flagged return line (or the line immediately above)". Progressive disclosure means the footgun note in the rule header will only be read by someone already editing the rule.
2. No automated guard against the footgun recurring. The fix here is a hand-placed comment; nothing prevents the next multi-line justification from drifting one line up and silently un-suppressing (or, worse, appearing to suppress). I checked the other 16 live findings for this exact failure — sed-ing 4 lines above each and grepping for nosemgrep returns 0 hits everywhere, so observability.py was genuinely the only mis-bound suppression and the sweep is complete. Worth a small scripts/ check (or a self-referential semgrep rule) asserting every nosemgrep: *silent-success-masking token sits on a return/raise line or the line directly above it, so this cannot regress unnoticed.
3. agent/src/observability.py:84-86 — the new prose comment ("The trace id is a graceful-missing correlation field; a tracer fault here must not fail the caller's write path") and the justification on the token line ("tracer fault must not fail the write path") now say the same thing twice, and both restate the function docstring above. Collapse to the single token line; the docstring already carries the contract.

5. Documentation

• What shipped: the rule-header PLACEMENT FOOTGUN block in .semgrep/silent-success-masking.yaml:16-22. It is accurate — I verified the mechanism empirically rather than taking it on trust: scanning the merge-base copy of agent/src/observability.py with this PR's rule still reports the finding at :86 despite the pre-existing suppression, and it disappears on the PR head. The footgun was real and the explanation of why (the focus-metavariable: $RET anchor, continuation line lacking the token) is correct.
• What is missing: the AGENTS.md:23 pointer (nit 1) and, per blocking issue 1, the "why not tighten the pattern" rationale in the rule header.
• Starlight mirror: not applicable. The diff touches no docs/guides/, docs/design/, docs/decisions/, or CONTRIBUTING.md file — the five changed paths are .semgrep/, agent/src/, cdk/src/, cdk/test/, cli/src/ — so docs/src/content/docs/ needs no regeneration, and no hand-edit to the mirror is present.

6. Tests & CI

Coverage — good, and the one test added is the right test. cdk/test/handlers/registry-handlers.test.ts:112 proves the exact claim the new parseBody suppression makes. It is not tautological: it drives the real publishHandler with body: '{not json' (non-empty, so it must traverse the JSON.parse catch rather than the if (!raw) early return), and asserts three independent things — statusCode === 400, error.code === 'VALIDATION_ERROR', and mockPublish not called. I checked the assertions against the real response shape in cdk/src/handlers/shared/response.ts:127-144 ({ error: { code, message, request_id } }) and the real mapping at cdk/src/handlers/registry-publish.ts:55-58; both match, so this should pass. Per constraint on fork code I did not execute it.

The other two annotated sites need no new tests:

• cli/src/linear-oauth.ts:428 — the fail-closed half the suppression cites is already covered at cli/test/linear-oauth.test.ts:512-560: AccessDenied rethrows, an arbitrary error rethrows, a corrupt-JSON bundle rethrows, and the not-found/absent/malformed cases return undefined. The suppression's claim is test-backed today.
• agent/src/observability.py — comment-only; behaviour unchanged.

Semgrep verification I ran myself (permitted: parses and pattern-matches text, executes nothing):

• semgrep test .semgrep/ → 2/2: ✓ All tests passed.
• semgrep scan --config .semgrep/silent-success-masking.yaml --exclude '.semgrep/*' . on the PR head → 16 findings, none in the three touched files.
• Same rule against the merge-base copies of the three files → 3 findings (agent/src/observability.py:86, cdk/src/handlers/registry-publish.ts:116, cli/src/linear-oauth.ts:428). So the 19 → 16 claim in the PR body checks out, and all three removals are true false-positives — I read each site independently:
  • parseBody — null is the documented failure encoding and the sole caller converts it to a 400 four lines later; malformed client JSON is an input class, not a swallowed fault. Correct suppression, accurate justification.
  • readExistingWebhookSecret — only isNotFound(err) returns undefined; every other error rethrows on the next line, and the docblock at :397-419 spells out the #611 clobber this guards. The justification's attribution to the #612-B1 contract matches the docblock. Correct suppression, and consistent with the pre-existing twin at :497.
  • current_otel_trace_id — str | None return, None already means "no recording span" on the non-error path, and the docstring explains callers read it inside DDB-write try-blocks where a raised tracer error would be misclassified as a DDB failure and trip the progress circuit breaker. Genuine graceful-missing field. Correct suppression.
• No suppression here hides a real silent-success. That was my primary concern going in and it is cleared.

Actual CI status at time of review: all four checks still pending — Dead-code detection (advisory), Secrets/deps/workflow scan, Validate PR title, build (agentcore). I am not claiming green. Codecov reports all modified coverable lines covered (BASE report missing, so the delta is unreliable).

CDK test-perf rule #366: not applicable. The new test is a pure handler unit test — no Template.fromStack(), no synth, no postCliContext, no re-enabled Lambda bundling.

Bootstrap policy coverage (ADR-002): not applicable. I checked the changed-file list against cdk/src/constructs/ and cdk/src/stacks/ — the diff touches neither, and introduces no new CloudFormation resource type, so cdk/src/bootstrap/policies/*.ts, resource-action-map.ts, BOOTSTRAP_VERSION, the regenerated artifacts, and the DEPLOYMENT_ROLES.md golden baseline all correctly stay untouched.

Security review scope: omitted deliberately. No IAM, Cedar, network, secrets-handling, or input-gateway behaviour changes — the registry-publish.ts and linear-oauth.ts edits are comment-only insertions and the observability.py edit rearranges comments. The only executable-behaviour delta in the entire PR is the new test.

7. Human heuristics

• Proportionality — pass. +25/-2 across 5 files for three annotations, one comment relocation, one header note and one test. No new abstraction, no helper invented for a one-off. Notably restrained given the temptation to refactor the rule.
• Coherence — concern. The changes belong here and reuse the established vocabulary (nosemgrep: <rule-id> -- <why>, mirroring the twin at cli/src/linear-oauth.ts:497), but the PR title and commit subject say "masking rule precision" while the rule's matching behaviour is untouched — the only rule delta is a comment (.semgrep/silent-success-masking.yaml:16-22). "Annotate correct fail-closed sites + document the placement footgun" is what actually happened; the title over-claims and, combined with blocking issue 1, could leave a future reader thinking the rule was already tuned.
• Clarity — pass. The justifications name the concrete contract rather than gesturing ("null IS the failure encoding and the caller turns it into a 400 VALIDATION_ERROR"; "the fail-closed half of the #612-B1 contract"). Nothing is hidden behind a plausible default — each annotated site was already surfacing its failure to the caller, which is precisely why the annotations are legitimate. No magic values, so no contracts/constants.json question arises.
• Appropriateness — concern. Maintainable and idiomatic for this team, and the new test drives the real handler rather than asserting against a self-written stand-in (only the registry client and ulid are mocked, which is correct). The concern is the same one as blocking issue 1: the decision record is not verified — the "cannot be delivered" conclusion was asserted rather than demonstrated, and it does not survive testing. In a repo whose whole premise is autonomous agents producing reviewable outcomes, a PR narrative that reads as a verified negative result but is not one is the kind of thing that must be caught here.

[ClintEastman02]

@isadeks — thank you, this was the right catch and the disproof was correct. I took your Option 1 (deliver the rule change) rather than correcting the record, pushed as b23cb77a and reflected in the rewritten PR description.

Blocking issue 1 — resolved by delivering, not re-arguing

The "any pattern-not also silences #311" claim was empirically false, exactly as you showed. The rule now carries the ordering clause you measured:

- pattern-not: |
    try { ... } catch ($E) { ... return $RET; ... throw $ERR; }

on both ts- and py-silent-success-masking (the Python analogue matches except $EXC as $E: ... return $RET ... raise, covering bare raise and raise $ERR). Concretely:

• .semgrep/silent-success-masking.yaml — ordering pattern-not added to both rules, plus a CLASSIFY-THEN-RETHROW EXEMPTION header block that states the trade you flagged on the record: it blanket-exempts the ambiguous catch (e) { if (benign(e)) return null; throw e; } shape, and per fix(security): clear the two security:sast findings blocking pre-push (#729) #730 that is the deliberate choice over training contributors to annotate correct fail-closed code away.
• Fixtures — okClassifyThenRethrow / ok_classify_then_reraise added to .semgrep/silent-success-masking.{ts,py}; the maskedConditionalRethrow / masked_conditional_reraise feat(security): custom semgrep rules for silent-success masking (AI004) #311 fixtures are retained and still fire. semgrep test .semgrep/ → 2/2.
• cli/src/linear-oauth.ts — dropped the suppression on readExistingWebhookSecret and the pre-existing twin readExistingOauthTokens (:497), as you suggested for consistency. Both now clear via the rule; no annotation, no permanent blind spot.

On the ADR-003 scope-freeze: delivering items 1 & 2 puts the PR back inside the approved #790 Fix section rather than deviating from it, so this route closes the process gap rather than needing a re-approval for a changed deliverable.

cdk/src/handlers/shared/budgets.ts:236 is the ambiguous shape you named; it keeps its own on-line suppression, so no live coverage is lost today — now explicitly noted as the trade in the rule header.

Non-blocking suggestions — all three taken

1. AGENTS.md:23 — added "on the flagged return line (or the line immediately above) — the rule anchors on the return, so a token placed higher does not bind."
2. Automated guard — added scripts/check-masking-suppression-placement.mjs: asserts every nosemgrep: *silent-success-masking token sits on, or one line above, a return/raise/throw. Wired into both security:sast:masking and :masking:range so it runs at pre-push and per-PR. It already earned its keep — it flagged an inert token on an empty catch in cli/src/commands/jira.ts, which I removed.
3. agent/src/observability.py — collapsed the duplicated prose to the single # nosemgrep: token line directly above the return None; the docstring already carries the contract. This site keeps its annotation (no rethrow → genuine best-effort masking).

Coherence concern — resolved

The title/subject now match reality: the rule's matching behaviour is tightened, so "masking rule precision" is accurate rather than over-claiming.

Verification (re-run after rebasing onto current main)

• semgrep test .semgrep/ → 2/2 (feat(security): custom semgrep rules for silent-success masking (AI004) #311 detection preserved, new ok-path green).
• node scripts/check-masking-suppression-placement.mjs → all placements correct.
• security:sast:masking:range vs main → 0 new findings; both linear-oauth.ts sites now clear via the rule.
• eslint (cli + cdk) clean; agent observability tests green.

Ready for another look.

another reviewer will review

[scottschreckengaust]

1. Verdict

APPROVE. The blocker from the prior (now-dismissed) isadeks review has been delivered, not re-argued: the ts/py-silent-success-masking rule now carries the return-before-throw pattern-not, the two cli/src/linear-oauth.ts false positives clear via the rule instead of via permanent suppressions, and the change is regression-tested and empirically verified below. Backing issue #790 carries the approved label, so the ADR-003 scope-freeze concern is also resolved (delivering items 1 & 2 puts the PR back inside the approved Fix section).

2. Vision alignment

Squarely in service of Tenet 4 (fail closed on risk) and reviewable outcomes. It converts two permanent per-site suppressions (blind spots forever) into a reviewable rule predicate, while preserving detection of genuine masks. The one deliberate detection trade — the rule now also blanket-exempts the ambiguous catch (e) { if (benign(e)) return null; throw e; } shape — is a documented tenet trade-off (rule-header CLASSIFY-THEN-RETHROW EXEMPTION block, .semgrep/silent-success-masking.yaml), was explicitly recommended by a MEMBER reviewer, and loses no live coverage today (cdk/src/handlers/shared/budgets.ts:236, the one known ambiguous site, retains its own on-line suppression). No undocumented tenet trade; no ADR required.

3. Blocking issues

None. The prior REQUEST_CHANGES (review 5171938278) is DISMISSED and I independently re-verified its single blocker against the current head:

• Rule tightening delivered. Both ts- and py-silent-success-masking gained the ordering pattern-not (return $RET; ... throw/raise). Verified empirically at head 1d6cb14c:
  • semgrep test .semgrep/ → 2/2 pass.
  • Scanning .semgrep/silent-success-masking.ts: line 103 (maskedConditionalRethrow, the #311 fixture) still fires; the new okClassifyThenRethrow (line 119) does NOT — order is load-bearing as claimed. Same for the Python twin.
  • Full-repo masking scan (excluding fixtures) → 16 findings, unchanged from the baseline the prior reviewer measured; both cli/src/linear-oauth.ts classify-then-rethrow sites now clear via the rule with no annotation.
• Placement guard works. node scripts/check-masking-suppression-placement.mjs → all suppressions correctly placed.

4. Non-blocking suggestions / nits

1. scripts/check-masking-suppression-placement.mjs:21 — the ANCHOR heuristic (/\b(return|raise|throw)\b/) matches the bare word, so it will accept a token whose next line merely mentions return/throw/raise in a comment or string while the actual anchored return is further down — a rare false-negative for the guard. Acceptable for a defensive lint (it is belt-and-suspenders over semgrep test, not the correctness gate), but worth a comment noting the limitation. Not blocking.
2. git ls-files -z walks every tracked source file each run; fine at current repo size, just noting it grows with the tree.

5. Documentation

• AGENTS.md:23 updated to name which line the nosemgrep token must sit on — closes the exact doc gap that produced the observability.py mis-binding. AGENTS.md (repo root) is not a Starlight-mirrored source (docs/guides/, docs/design/, CONTRIBUTING.md are), so no docs:sync regeneration is required — confirmed the diff touches no mirrored path.
• The PLACEMENT FOOTGUN and CLASSIFY-THEN-RETHROW EXEMPTION rule-header blocks put the rationale and the trade-off on the record where a rule editor will read them. Good.
• Backing issue #790 is approved and this PR completes its Fix section.

6. Tests & CI

• Fixtures: okClassifyThenRethrow / ok_classify_then_reraise added to .semgrep/silent-success-masking.{ts,py}; the retained maskedConditionalRethrow / masked_conditional_reraise fixtures prove #311 detection survives. semgrep test .semgrep/ 2/2.
• New handler test cdk/test/handlers/registry-handlers.test.ts drives the real publishHandler with body: '{not json' and asserts 400 + VALIDATION_ERROR + mockPublish not called — a real fail-closed assertion for the retained parseBody suppression, not a tautology.
• CI: build (agentcore) SUCCESS, Secrets, deps, and workflow scan SUCCESS, Validate PR title SUCCESS. Mergeable.
• Bootstrap policy coverage (ADR-002): not applicable — the diff touches no cdk/src/constructs/ or cdk/src/stacks/ and introduces no new CloudFormation resource type, so bootstrap policies, resource-action-map.ts, BOOTSTRAP_VERSION, regenerated artifacts, and the DEPLOYMENT_ROLES.md golden baseline correctly stay untouched.
• CDK synth-perf rule #366: not applicable — the new test is a pure handler unit test (no Template.fromStack(), no synth, no re-enabled bundling).

7. Review agents run

Execution context cannot spawn nested pr-review-toolkit / security-review sub-agents, so I performed the equivalent analysis by hand and state that limitation explicitly:

• code-reviewer (by hand): style/routing conform to AGENTS.md; changes land in the correct packages.
• silent-failure-hunter (by hand): this PR is about the silent-failure gate — I verified the loosening exempts only fail-closed (rethrow-on-fallthrough) shapes and that all 16 live masking findings are preserved; no swallowed fault introduced.
• type-design-analyzer: omitted — no new types (comment/rule/test changes only).
• comment-analyzer (by hand): the new rule-header prose and per-site justifications are accurate against the code they describe.
• pr-test-analyzer (by hand): the added test covers the fail-closed path the parseBody suppression claims; the linear-oauth contract is already covered at cli/test/linear-oauth.test.ts.
• /security-review (by hand): the only security-relevant delta is the deliberate, documented, MEMBER-endorsed loosening of a SAST detection rule — no IAM/Cedar/network/secrets/input-gateway behaviour changes; the registry-publish.ts and linear-oauth.ts edits are comment-only and observability.py rearranges comments.

8. Human heuristics

• Proportionality — pass. Small, targeted change; a rule predicate + fixtures + a one-file guard for a recurring footgun. No over-abstraction.
• Coherence — pass. The earlier over-claim concern ("rule precision" while the rule was untouched) is now resolved — the rule matching behaviour genuinely is tightened. Reuses established nosemgrep: <rule-id> -- <why> vocabulary.
• Clarity — pass. Justifications name concrete contracts ("null IS the failure encoding … caller turns it into a 400"); the ordering rationale is explicit in both the fixtures and the rule header.
• Appropriateness — pass. Maintainable and idiomatic; the decision record is now verified rather than asserted, which was the core of the prior reviewer's appropriateness concern.