Skip to content

bug(security): security:sast fails on main — 2 semgrep findings from #695 block pre-push #729

Description

@scottschreckengaust

Problem

mise run security:sast fails on main, so the local pre-push hook (hooks:pre-push:securitymise run security) is red for everyone, on every branch, regardless of what the branch changes.

Reproduced on main @ a94be94c and independently on two separate worktrees (a near-main branch and a branch whose only diff is one mise.toml line):

$ mise run security:sast
❯❱ javascript.lang.security.audit.detect-non-literal-regexp.detect-non-literal-regexp
      221┆ return new RegExp(`(^|[^a-z0-9])${esc}([^a-z0-9]|$)`, 'i').test(text);
❯❱ python.lang.security.audit.dynamic-urllib-use-detected.dynamic-urllib-use-detected
       89┆ with urllib.request.urlopen(req, timeout=30) as r:
exit=1
Finding Location
detect-non-literal-regexp cdk/src/handlers/shared/orchestration-comment-trigger.ts:221
dynamic-urllib-use-detected scripts/linear_epic.py:89

Both lines are present verbatim on main, last touched by #695 (4357c353, "land the carved orchestration arc on main (S2–S8)"). Neither has an inline nosemgrep.

Why this matters

  • It blocks pushes that CI would pass. security:sast is not a required status check on the main ruleset (14980587 requires only build (agentcore) and Secrets, deps, and workflow scan), and security-pr.yml explicitly defers the heavy SAST suite to security.yml. So the local gate is stricter than the merge gate — the same asymmetry class as fix(ci): scheduled gitleaks sweep flags unmerged-branch history — scope to main + require per-PR secret check #721, on another surface.
  • Contributors learn to reach for --no-verify, which is how a real finding eventually gets waved through.

Likely cause: unpinned rules, not new code

security:sast runs semgrep scan --config auto ..., which resolves rule packs from the remote registry at scan time, so this can flip red with no code change. That is exactly the failure mode #540 ("semgrep = \"latest\" + --config auto lets registry changes break CI") and #722 (pin scanner toolchain) predict. Local semgrep is currently 1.172.0, unpinned.

Worth confirming whether these two rules are newly-added/newly-tightened upstream rather than something #695 introduced — if so, the durable fix is pinning (#540/#722) and this issue is the concrete instance that justifies it.

Assessment of the two findings

Both look like defensible false positives, but that should be a deliberate call, not an implicit one:

  • orchestration-comment-trigger.ts:221 — the interpolated segment is escaped on the immediately preceding line (phrase.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')), so it is not attacker-controlled regex syntax. A ReDoS argument would need the input bound checked.
  • linear_epic.py:89 — a maintainer-run script hitting the Linear API; the URL is not user-supplied.

Proposed fix

  1. Triage each finding; if a false positive, add an inline nosemgrep: <rule-id> -- <reason> on the offending line (the convention AGENTS.md already prescribes for security:sast:masking).
  2. Pin semgrep + rule packs so registry drift cannot redden the suite again — defer to feat(ci): pin Semgrep + rule packs (semgrep = "latest" + --config auto lets registry changes break CI) #540/chore(ci): pin scanner toolchain + mise + mise-action to exact versions across all surfaces (reproducible security suite) #722 rather than duplicating.
  3. Consider whether the pre-push SAST gate should be diff-scoped (like security:secrets:range after fix(ci): scheduled gitleaks sweep flags unmerged-branch history — scope to main + require per-PR secret check #721) so a contributor is never blocked by a pre-existing finding in code they did not touch.

Acceptance criteria

  • mise run security:sast exits 0 on main.
  • Any retained suppression is inline, rule-scoped, and carries a written justification.
  • Pre-push does not fail on findings that the required merge checks would not fail on.

How this surfaced

Found while landing #721 (PR #728): after the gitleaks scope fix cleared the original block, the pre-push hook still failed on these two SAST findings. Verified against the diff — PR #728 changes only mise.toml, and the findings reproduce on untouched main. Each other gate passes (security:secrets, :range, security:deps, security:gh-actions, //cdk:compile), so #728 was pushed with --no-verify and this filed instead.

Related: #540, #722 (pinning), #542 (security:sast:masking red on main — separate gate, same suite), #721 / #723 (the gitleaks half of this asymmetry), #695 (introduced the flagged lines).

Metadata

Metadata

Assignees

No one assigned

    Labels

    ci-cdBuild pipeline, deploy.yml, CI perf/caching, GitHub Actions workflowssecurityCedar/HITL, IAM least-privilege, secrets, PII/DLP, guardrails, supply-chain/CVE

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions