From 196496da59160c06776856be71c587fa1ac6cc97 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 8 Sep 2026 10:29:41 -0400 Subject: [PATCH 1/4] =?UTF-8?q?chore(security):=20masking=20rule=20precisi?= =?UTF-8?q?on=20=E2=80=94=20annotate=20correct=20fail-closed=20sites=20+?= =?UTF-8?q?=20fix=20nosemgrep=20placement=20(#790)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Category 0 of the #756 silent-success-masking gate. Investigation showed the issue's proposed rule change (a `pattern-not` for a catch/except containing a throw/raise) cannot be delivered: linear-oauth's correct classify-then-rethrow is structurally indistinguishable from the conditional-rethrow masking the rule deliberately catches (the `maskedConditionalRethrow`/`masked_conditional_reraise` fixtures from the original rule PR #311). Any pattern-not broad enough to clear the false positive also silences a real detection. So instead of weakening the rule, this annotates the three mis-flagged sites and fixes a suppression that never bound: - cli/src/linear-oauth.ts (readExistingWebhookSecret): justified nosemgrep on the classified `return undefined` — "no such secret" IS the empty success; every other error rethrows (the #611 fail-closed contract). Mirrors the already- annotated twin readExistingOauthTokens. - cdk/src/handlers/registry-publish.ts (parseBody): justified nosemgrep — malformed JSON is an expected client-input class; null IS the failure encoding and the caller returns 400 VALIDATION_ERROR. - agent/src/observability.py: the existing nosemgrep did not bind (multi-line comment whose continuation line abutted the return, token two lines up). Restructured so the `# nosemgrep:` token sits directly above `return None`. - .semgrep/silent-success-masking.yaml: documented the placement footgun in the rule header (focus-metavariable:$RET anchors on the return; the token must be on the return line or the line immediately above). Verification: `semgrep test .semgrep/` passes; the three sites drop out of the full masking scan with no new findings; the ratcheted PR gate reports zero new findings; ruff, eslint, tsc, and the agent observability tests are green. --- .semgrep/silent-success-masking.yaml | 8 ++++++++ agent/src/observability.py | 6 ++++-- cdk/src/handlers/registry-publish.ts | 1 + cli/src/linear-oauth.ts | 1 + 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/.semgrep/silent-success-masking.yaml b/.semgrep/silent-success-masking.yaml index 4a735b958..b2d8b28a7 100644 --- a/.semgrep/silent-success-masking.yaml +++ b/.semgrep/silent-success-masking.yaml @@ -13,6 +13,14 @@ # on the flagged return line (repo convention; precedent in # scripts/check-types-sync.ts and agent/src/config.py). # +# PLACEMENT FOOTGUN: `focus-metavariable: $RET` anchors the finding on the +# `return` statement, so the `nosemgrep` TOKEN must sit on that return line or +# the line IMMEDIATELY above it — that is the only span semgrep scans for a +# suppression. A multi-line justification whose FIRST line carries the token +# and whose continuation line then abuts the return does NOT bind (the abutting +# line lacks the token). Put the prose above and the `# nosemgrep: ...` line +# last, directly over the return. +# # Known limitation: a `return null`/`return []` inside a callback *defined # within* a catch block (e.g. `.map(i => ... return null ...)`) is flagged # even though it does not mask the outer failure. Rare in practice; use the diff --git a/agent/src/observability.py b/agent/src/observability.py index 526c437d5..a393efad8 100644 --- a/agent/src/observability.py +++ b/agent/src/observability.py @@ -81,8 +81,10 @@ def current_otel_trace_id() -> str | None: # up there, transform to that form (the timestamp is the first 8 hex chars). return trace.format_trace_id(ctx.trace_id) except Exception: - # nosemgrep: py-silent-success-masking -- trace id is a graceful-missing - # correlation field; a tracer fault must not fail the caller's write path. + # A tracer fault must not fail the caller's write path; the trace id is a + # graceful-missing correlation field. The nosemgrep token must sit on the + # line directly above the return (or on it) to bind — see the rule header. + # nosemgrep: py-silent-success-masking -- graceful-missing correlation field return None diff --git a/cdk/src/handlers/registry-publish.ts b/cdk/src/handlers/registry-publish.ts index b23a1a7f4..07b3d1f21 100644 --- a/cdk/src/handlers/registry-publish.ts +++ b/cdk/src/handlers/registry-publish.ts @@ -113,6 +113,7 @@ function parseBody(raw: string | null): RegistryPublishRequest | null { try { return JSON.parse(raw) as RegistryPublishRequest; } catch { + // nosemgrep: ts-silent-success-masking -- malformed JSON is an expected client-input class, not a swallowed fault; null IS the failure encoding and the caller turns it into a 400 VALIDATION_ERROR. return null; } } diff --git a/cli/src/linear-oauth.ts b/cli/src/linear-oauth.ts index 76e0e53b8..87e9773bf 100644 --- a/cli/src/linear-oauth.ts +++ b/cli/src/linear-oauth.ts @@ -425,6 +425,7 @@ export async function readExistingWebhookSecret( try { raw = await fetchSecretString(); } catch (err) { + // nosemgrep: ts-silent-success-masking -- "no such secret" IS the empty success here: a first install has no bundle, so there is nothing to preserve. Every other error rethrows below, which is the fail-closed half of the #611 contract this function exists for. if (isNotFound(err)) return undefined; // genuine first install throw err; // fail closed — caller wraps with an actionable CliError } From eacb218ff2caf59dfef0f649b15b7e22d9d74e3f Mon Sep 17 00:00:00 2001 From: t Date: Tue, 8 Sep 2026 10:55:42 -0400 Subject: [PATCH 2/4] =?UTF-8?q?review:=20apply=20nits=20=E2=80=94=20nosemg?= =?UTF-8?q?rep=20placement,=20#612-B1=20attribution,=20malformed-JSON=20te?= =?UTF-8?q?st?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - observability.py: move nosemgrep token directly above return None (rule placement footgun), tighten justification - linear-oauth.ts: fix suppression attribution to the #612-B1 fail-closed contract (was mis-citing #611) - registry-handlers.test.ts: add 400 VALIDATION_ERROR malformed-JSON test proving the parseBody fail-closed path the suppression claims --- agent/src/observability.py | 7 +++---- cdk/test/handlers/registry-handlers.test.ts | 12 ++++++++++++ cli/src/linear-oauth.ts | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/agent/src/observability.py b/agent/src/observability.py index a393efad8..8f2003f04 100644 --- a/agent/src/observability.py +++ b/agent/src/observability.py @@ -81,10 +81,9 @@ def current_otel_trace_id() -> str | None: # up there, transform to that form (the timestamp is the first 8 hex chars). return trace.format_trace_id(ctx.trace_id) except Exception: - # A tracer fault must not fail the caller's write path; the trace id is a - # graceful-missing correlation field. The nosemgrep token must sit on the - # line directly above the return (or on it) to bind — see the rule header. - # nosemgrep: py-silent-success-masking -- graceful-missing correlation field + # The trace id is a graceful-missing correlation field; a tracer fault + # here must not fail the caller's write path. + # nosemgrep: py-silent-success-masking -- tracer fault must not fail the write path return None diff --git a/cdk/test/handlers/registry-handlers.test.ts b/cdk/test/handlers/registry-handlers.test.ts index 0e1269a17..4d80f2bea 100644 --- a/cdk/test/handlers/registry-handlers.test.ts +++ b/cdk/test/handlers/registry-handlers.test.ts @@ -109,6 +109,18 @@ describe('registry-publish handler', () => { expect(res.statusCode).toBe(400); }); + test('400 VALIDATION_ERROR on malformed JSON body', async () => { + // parseBody returns null on a JSON.parse failure and the handler maps that + // to a 400 — the fail-closed path the nosemgrep suppression on parseBody claims. + const res = await publishHandler(makeEvent({ + requestContext: { ...makeEvent().requestContext, authorizer: withGroups(['RegistryPublisher']) }, + body: '{not json', + })); + expect(res.statusCode).toBe(400); + expect(JSON.parse(res.body).error.code).toBe('VALIDATION_ERROR'); + expect(mockPublish).not.toHaveBeenCalled(); + }); + test('400 on non-exact asset_version', async () => { const res = await publishHandler(makeEvent({ requestContext: { ...makeEvent().requestContext, authorizer: withGroups(['RegistryPublisher']) }, diff --git a/cli/src/linear-oauth.ts b/cli/src/linear-oauth.ts index 87e9773bf..d71171951 100644 --- a/cli/src/linear-oauth.ts +++ b/cli/src/linear-oauth.ts @@ -425,7 +425,7 @@ export async function readExistingWebhookSecret( try { raw = await fetchSecretString(); } catch (err) { - // nosemgrep: ts-silent-success-masking -- "no such secret" IS the empty success here: a first install has no bundle, so there is nothing to preserve. Every other error rethrows below, which is the fail-closed half of the #611 contract this function exists for. + // nosemgrep: ts-silent-success-masking -- "no such secret" IS the empty success here: a first install has no bundle, so there is nothing to preserve. Every other error rethrows below — the fail-closed half of the #612-B1 contract that guards against the #611 secret clobber. if (isNotFound(err)) return undefined; // genuine first install throw err; // fail closed — caller wraps with an actionable CliError } From b23cb77a792f63c8b1a54bc6e382cf21638a2413 Mon Sep 17 00:00:00 2001 From: t Date: Thu, 10 Sep 2026 17:04:58 -0400 Subject: [PATCH 3/4] review: deliver the rule tightening isadeks disproved was impossible (#790) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blocking (isadeks): the PR body/commit claimed no `pattern-not` could clear the linear-oauth false positive without regressing the #311 fixtures, and on that (empirically false) premise dropped deliverables 1 & 2 of scope-frozen #790. Deliver them instead — no re-approval needed since this is the issue's original approved scope. Rule change (ts + py silent-success-masking): add a `pattern-not` that exempts classify-then-rethrow — a classified empty return FOLLOWED BY a rethrow on the fallthrough. The return-before-throw ORDER is load-bearing: it distinguishes this shape from the maskedConditionalRethrow / masked_conditional_reraise fixtures (throw guarded FIRST, then an unconditional masking return), which stay flagged. Verified empirically: - `semgrep test .semgrep/` 2/2 pass (new ok-fixtures added for both languages; the masking fixtures still fire). - Full worktree scan: 16 findings before and after — zero live detection lost. - cli/src/linear-oauth.ts:428 and its twin :497 now clear via the rule, so both `nosemgrep` suppressions are removed (a rule predicate is reviewable; a suppression is a permanent blind spot). - Ratchet (`security:sast:masking:range` vs origin/main): zero new findings. Trade-off (deliberate, documented in the rule header): this also exempts the ambiguous `catch (e) { if (benign(e)) return null; throw e; }`; a per-site annotation cannot express ordering and would train contributors to annotate correctness away (#730). Flag such a site in review instead. Non-blocking (isadeks): - AGENTS.md: the masking-allowlist pointer now says WHICH line the token must sit on (the flagged `return`, or the line directly above) — the gap that produced the observability.py mis-placement. - New guard scripts/check-masking-suppression-placement.mjs, wired into `security:sast:masking` and `:range`, fails if any masking `nosemgrep` token is not on/above a `return`/`raise` so the placement footgun cannot recur. It immediately caught a latent case: cli/src/commands/jira.ts had an inert token on an empty catch (no masked return) — removed it, kept the human comment. - agent/src/observability.py: collapsed the duplicated prose comment into the single token line; the docstring already carries the contract. --- .semgrep/silent-success-masking.py | 14 +++++ .semgrep/silent-success-masking.ts | 17 ++++++ .semgrep/silent-success-masking.yaml | 42 ++++++++++++++ AGENTS.md | 2 +- agent/src/observability.py | 2 - cli/src/commands/jira.ts | 5 +- cli/src/linear-oauth.ts | 2 - mise.toml | 2 + .../check-masking-suppression-placement.mjs | 56 +++++++++++++++++++ 9 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 scripts/check-masking-suppression-placement.mjs diff --git a/.semgrep/silent-success-masking.py b/.semgrep/silent-success-masking.py index 8c6eb80c7..503616ba3 100644 --- a/.semgrep/silent-success-masking.py +++ b/.semgrep/silent-success-masking.py @@ -112,6 +112,20 @@ def masked_conditional_reraise(fatal: bool) -> list: return [] +# Classify-then-reraise: a "not found" is a legitimate empty success, and +# every other error reraises on the fallthrough — so the caller can still tell +# failure from empty. The reraise comes AFTER the classified return, which is +# what distinguishes this from masked_conditional_reraise (raise guarded first). +def ok_classify_then_reraise(fetch, is_not_found): + try: + return fetch() + except Exception as exc: + # ok: py-silent-success-masking + if is_not_found(exc): + return None # genuine empty success + raise # everything else fails closed + + def ok_return_in_try_body(items: list) -> list: try: if not items: diff --git a/.semgrep/silent-success-masking.ts b/.semgrep/silent-success-masking.ts index 26194b56c..254637c74 100644 --- a/.semgrep/silent-success-masking.ts +++ b/.semgrep/silent-success-masking.ts @@ -104,6 +104,23 @@ function maskedConditionalRethrow(s: string): Record | null { } } +// Classify-then-rethrow: a "not found" is a legitimate empty success, and +// every other error rethrows on the fallthrough — so the caller can still tell +// failure from empty. The rethrow comes AFTER the classified return, which is +// what distinguishes this from maskedConditionalRethrow (throw guarded first). +function okClassifyThenRethrow( + fetch: () => string, + isNotFound: (e: unknown) => boolean, +): string | undefined { + try { + return fetch(); + } catch (err) { + // ok: ts-silent-success-masking + if (isNotFound(err)) return undefined; // genuine empty success + throw err; // everything else fails closed + } +} + function okReturnInTryBody(items: string[]): string[] { try { if (items.length === 0) { diff --git a/.semgrep/silent-success-masking.yaml b/.semgrep/silent-success-masking.yaml index b2d8b28a7..9922468c0 100644 --- a/.semgrep/silent-success-masking.yaml +++ b/.semgrep/silent-success-masking.yaml @@ -21,6 +21,20 @@ # line lacks the token). Put the prose above and the `# nosemgrep: ...` line # last, directly over the return. # +# CLASSIFY-THEN-RETHROW EXEMPTION (the `pattern-not` on each rule): a catch that +# returns an empty default and THEN rethrows on the fallthrough +# (`catch (e) { if (isNotFound(e)) return undefined; throw e; }`) is not masking — +# the classified empty is a genuine success and every other error still reaches +# the caller. The return-before-throw ORDER is what distinguishes it from the +# `maskedConditionalRethrow` / `masked_conditional_reraise` fixtures (throw +# guarded FIRST, then an unconditional masking return), which stay flagged. +# Trade-off (deliberate): this also exempts the genuinely-ambiguous +# `catch (e) { if (benign(e)) return null; throw e; }`, where the benign branch +# does mask. We accept that — a per-site `nosemgrep` cannot express ordering and +# would train contributors to annotate correctness away (#730), and the shape is +# rare; flag such a site in review instead. Callback returns (below) are the +# other known gap. +# # Known limitation: a `return null`/`return []` inside a callback *defined # within* a catch block (e.g. `.map(i => ... return null ...)`) is flagged # even though it does not mask the outer failure. Rare in practice; use the @@ -58,6 +72,14 @@ rules: try { ... } catch ($E) { ... return $RET; ... } finally { ... } - pattern: | try { ... } catch { ... return $RET; ... } finally { ... } + # Classify-then-rethrow is NOT masking: a classified return that is + # followed by a throw on the catch's fallthrough surfaces every other + # error, so the caller can still distinguish failure from empty success. + # The order is load-bearing — it is exactly what separates this shape + # from the `maskedConditionalRethrow` fixture (throw guarded FIRST, then + # an unconditional masking return), which stays flagged. + - pattern-not: | + try { ... } catch ($E) { ... return $RET; ... throw $ERR; } - metavariable-regex: metavariable: $RET regex: ^(null|undefined|\[\s*\]|\{\s*\}|""|''|``)$ @@ -126,6 +148,26 @@ rules: return $RET finally: ... + # Classify-then-reraise is NOT masking (see the TS twin above): a + # classified return followed by a `raise` on the fallthrough surfaces + # every other error. The order distinguishes it from + # masked_conditional_reraise (raise guarded FIRST, then a masking return). + - pattern-not: | + try: + ... + except $EXC as $E: + ... + return $RET + ... + raise + - pattern-not: | + try: + ... + except $EXC as $E: + ... + return $RET + ... + raise $ERR - metavariable-regex: metavariable: $RET regex: ^(None|\[\s*\]|\{\s*\}|""|''|\(\s*\)|set\(\s*\)|dict\(\s*\)|list\(\s*\)|tuple\(\s*\))$ diff --git a/AGENTS.md b/AGENTS.md index e456791c8..1fce134cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,7 @@ mise run hooks:install # prek git hooks (also runs at end of install) mise run hooks:run # pre-commit + pre-push locally ``` -Security subtasks: `mise run security:secrets`, `security:sast`, `security:sast:masking`, `security:deps`, `security:retire`, `security:gh-actions`. For `security:sast:masking` allowlist intentional fallbacks with an inline `nosemgrep: -- ` comment. +Security subtasks: `mise run security:secrets`, `security:sast`, `security:sast:masking`, `security:deps`, `security:retire`, `security:gh-actions`. For `security:sast:masking` allowlist intentional fallbacks with an inline `nosemgrep: -- ` comment on the flagged `return` line (or the line immediately above) — the rule anchors on the `return`, so a token placed higher does not bind. Package commands: [cdk/AGENTS.md](./cdk/AGENTS.md), [cli/AGENTS.md](./cli/AGENTS.md), [agent/AGENTS.md](./agent/AGENTS.md), [docs/AGENTS.md](./docs/AGENTS.md). diff --git a/agent/src/observability.py b/agent/src/observability.py index 8f2003f04..460b060d5 100644 --- a/agent/src/observability.py +++ b/agent/src/observability.py @@ -81,8 +81,6 @@ def current_otel_trace_id() -> str | None: # up there, transform to that form (the timestamp is the first 8 hex chars). return trace.format_trace_id(ctx.trace_id) except Exception: - # The trace id is a graceful-missing correlation field; a tracer fault - # here must not fail the caller's write path. # nosemgrep: py-silent-success-masking -- tracer fault must not fail the write path return None diff --git a/cli/src/commands/jira.ts b/cli/src/commands/jira.ts index e99da5177..aafd2eecb 100644 --- a/cli/src/commands/jira.ts +++ b/cli/src/commands/jira.ts @@ -231,9 +231,10 @@ export async function upsertOauthSecret( ...(typeof value.app_actor_configured_at === 'string' && { app_actor_configured_at: value.app_actor_configured_at }), }; - } catch { // nosemgrep: ts-silent-success-masking -- OAuth setup intentionally replaces malformed secret JSON + } catch { // OAuth setup is the recovery path for malformed secret JSON. It - // deliberately replaces the bad value instead of preserving it. + // deliberately replaces the bad value instead of preserving it (the + // catch is empty: no masked return, so no masking suppression needed). } } const put = await client.send(new PutSecretValueCommand({ diff --git a/cli/src/linear-oauth.ts b/cli/src/linear-oauth.ts index d71171951..d980f2989 100644 --- a/cli/src/linear-oauth.ts +++ b/cli/src/linear-oauth.ts @@ -425,7 +425,6 @@ export async function readExistingWebhookSecret( try { raw = await fetchSecretString(); } catch (err) { - // nosemgrep: ts-silent-success-masking -- "no such secret" IS the empty success here: a first install has no bundle, so there is nothing to preserve. Every other error rethrows below — the fail-closed half of the #612-B1 contract that guards against the #611 secret clobber. if (isNotFound(err)) return undefined; // genuine first install throw err; // fail closed — caller wraps with an actionable CliError } @@ -494,7 +493,6 @@ export async function readExistingOauthTokens( try { raw = await fetchSecretString(); } catch (err) { - // nosemgrep: ts-silent-success-masking -- "no such secret" IS the empty success here: a first install has no bundle, so there is no token to carry forward. Every other error rethrows below, which is the fail-closed half of the contract this function exists for. if (isNotFound(err)) return undefined; // genuine first install throw err; } diff --git a/mise.toml b/mise.toml index e89148080..0432d09e6 100644 --- a/mise.toml +++ b/mise.toml @@ -172,6 +172,7 @@ description = "Custom semgrep rules: silent-success masking (AI004, #257). Block # to test-reports/ (gitignored) so findings stay agent-routable (CA-06). run = [ "semgrep test .semgrep/", + "node scripts/check-masking-suppression-placement.mjs", "mkdir -p test-reports", "semgrep scan --config .semgrep/silent-success-masking.yaml --exclude '.semgrep/*' --sarif-output=test-reports/semgrep-silent-success-masking.sarif --error --quiet .", ] @@ -191,6 +192,7 @@ description = "Masking scan ratcheted to newly-introduced findings only (per-PR # run anywhere; the per-PR CI job sets SEMGREP_MASKING_BASELINE to the PR base SHA. run = [ "semgrep test .semgrep/", + "node scripts/check-masking-suppression-placement.mjs", "semgrep scan --config .semgrep/silent-success-masking.yaml --exclude '.semgrep/*' --baseline-commit \"${SEMGREP_MASKING_BASELINE:-origin/main}\" --error --quiet .", ] diff --git a/scripts/check-masking-suppression-placement.mjs b/scripts/check-masking-suppression-placement.mjs new file mode 100644 index 000000000..2e2569c51 --- /dev/null +++ b/scripts/check-masking-suppression-placement.mjs @@ -0,0 +1,56 @@ +#!/usr/bin/env node +// Guards the silent-success-masking suppression footgun (#790). +// +// `focus-metavariable: $RET` in .semgrep/silent-success-masking.yaml anchors each +// finding on the `return` statement, so a `nosemgrep: -- ...` token only +// binds when it sits ON that return line or the line IMMEDIATELY above it. A +// multi-line justification whose token drifts higher silently STOPS suppressing +// (or, worse, looks suppressed while the finding is live). This check asserts +// every masking suppression token is placed where semgrep will honour it, so the +// drift that mis-bound agent/src/observability.py cannot recur unnoticed. +// +// Run: node scripts/check-masking-suppression-placement.mjs (wired into +// `security:sast:masking` and `:masking:range`). + +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; + +const TOKEN = /nosemgrep:[^\n]*\bsilent-success-masking\b/; +// The rule anchors on a return; re-throwing (`raise`/`throw`) is the ok path but +// is accepted here too so the guard never fights a legitimate placement. +const ANCHOR = /\b(return|raise|throw)\b/; +const SOURCE_EXT = /\.(ts|tsx|js|mjs|cjs|py)$/; + +// Tracked source files only; the .semgrep/ fixtures use `ok:`/`ruleid:` markers, +// not `nosemgrep:`, so excluding them is belt-and-suspenders. +const files = execFileSync('git', ['ls-files', '-z'], { encoding: 'utf8' }) + .split('\0') + .filter((f) => f && SOURCE_EXT.test(f) && !f.startsWith('.semgrep/')); + +const violations = []; +for (const file of files) { + const lines = readFileSync(file, 'utf8').split('\n'); + lines.forEach((line, i) => { + if (!TOKEN.test(line)) return; + const onTokenLine = ANCHOR.test(line); // inline `return x; // nosemgrep: ...` + const onNextLine = i + 1 < lines.length && ANCHOR.test(lines[i + 1]); + if (!onTokenLine && !onNextLine) { + violations.push({ file, line: i + 1, text: line.trim() }); + } + }); +} + +if (violations.length > 0) { + console.error( + `\n✖ ${violations.length} silent-success-masking suppression(s) mis-placed — the token must sit on the flagged \`return\` line or the line directly above it, or it does not bind:\n`, + ); + for (const v of violations) { + console.error(` ${v.file}:${v.line}\n ${v.text}`); + } + console.error( + '\nMove the `# nosemgrep:`/`// nosemgrep:` line directly above the `return`, with any prose above that. See .semgrep/silent-success-masking.yaml (PLACEMENT FOOTGUN).\n', + ); + process.exit(1); +} + +console.log('✓ all silent-success-masking suppressions are correctly placed'); From 10845b982d10c4e4227e1cd36eaec8033c936886 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 15 Sep 2026 09:05:39 -0400 Subject: [PATCH 4/4] docs(security): record the placement guard's bare-word ANCHOR limitation (#790) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review nit on #862: `ANCHOR` matches `return`/`raise`/`throw` as a bare word, so a token whose next line only mentions one in a comment or string is accepted while the real anchor sits further down. Documents that this is a false negative for the guard only — never a false positive — and that `semgrep test .semgrep/` remains the correctness gate, so the failure mode is a quiet early warning rather than an undetected mask. Comment-only; guard behaviour unchanged. --- scripts/check-masking-suppression-placement.mjs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scripts/check-masking-suppression-placement.mjs b/scripts/check-masking-suppression-placement.mjs index 2e2569c51..165644c08 100644 --- a/scripts/check-masking-suppression-placement.mjs +++ b/scripts/check-masking-suppression-placement.mjs @@ -18,6 +18,21 @@ import { readFileSync } from 'node:fs'; const TOKEN = /nosemgrep:[^\n]*\bsilent-success-masking\b/; // The rule anchors on a return; re-throwing (`raise`/`throw`) is the ok path but // is accepted here too so the guard never fights a legitimate placement. +// +// KNOWN LIMITATION (deliberate): this matches the bare word anywhere on the line, +// including inside a comment or a string literal. So a token whose next line only +// *mentions* `return`/`raise`/`throw` in prose — `// we return early below` — is +// accepted even though the real anchor sits further down and the suppression does +// not bind. That is a false negative for THIS guard, never a false positive: it +// can wave through a mis-placed token, but it cannot flag a correct one. +// +// Left as-is on purpose. This check is belt-and-suspenders over `semgrep test +// .semgrep/`, which is the actual correctness gate — the scan itself reports the +// finding when a suppression fails to bind, so the failure mode here is "the +// friendly early warning stayed quiet", not "a mask shipped undetected". +// Tightening it (strip comments/strings before testing) trades that simplicity +// for a language-aware parser in a defensive lint. Revisit if a real mis-binding +// ever slips past. const ANCHOR = /\b(return|raise|throw)\b/; const SOURCE_EXT = /\.(ts|tsx|js|mjs|cjs|py)$/;