Skip to content

chore(security): masking rule precision — stop flagging correct fail-closed code + fix mis-placed suppression (#756 Cat 0) #790

Description

@ClintEastman02

Follow-up to #756 (Category 0 from @scottschreckengaust's triage on #756). Split out per that triage so #756 can track each category independently. Part of #756.

Problem

The py/ts-silent-success-masking rule (AI004, #257) flags several sites that are correct fail-closed code, which teaches contributors that correct code needs annotations. The rule's patterns match try { ... } catch ($E) { ... return $RET; } with no awareness of a sibling throw, so a classifier that returns empty for a classified case and re-throws everything else trips it.

Sites (verified on the PR #788 branch)

  • cli/src/linear-oauth.ts:382if (isNotFound(err)) return undefined; immediately followed by throw err;. The surrounding docblock exists because of the fix(cli): linear setup clobbers a second workspace's webhook signing secret with the stack-wide one → 401 Invalid signature #611 secret-clobber incident and specifies "ANY OTHER error → THROW". This is the exemplar of correct handling, and the scanner flags it.
  • cdk/src/handlers/registry-publish.ts:116parseBody returning null is the failure encoding; the caller converts it to 400 VALIDATION_ERROR. Malformed client JSON is an expected input class, not an exception.
  • agent/src/observability.py:86 — already has a justified nosemgrep, but on the wrong line (the except line, not the return). The rule uses focus-metavariable: $RET, so the suppression never binds. Moving the identical comment onto the return None line drops the file from 1 active finding to 0.

Fix

  1. Tighten the rule with a pattern-not for a catch/except block that contains a throw/raise.
  2. Add the three shapes above as fixtures under .semgrep/ (keep semgrep test .semgrep/ green).
  3. Fix the mis-placed observability.py:86 suppression (move it onto the return line).
  4. Add a line to the rule header documenting the return-line placement footgun — the existing inline suppressions depend on getting it right.

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.

Line numbers verified against the PR #788 branch (masking-scan SARIF). Credit: triage by @scottschreckengaust on #756.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

P2lowest priorityapprovedWhen an issue has been approved and readysecurityCedar/HITL, IAM least-privilege, secrets, PII/DLP, guardrails, supply-chain/CVEtooling

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions