fix(sast-semgrep): fold the changed-scope gate into its single consumer job

[seonghobae]

What changed

sast-semgrep.yml no longer has a standalone changed-scope job. The "Classify changed paths" step (byte-identical script) now runs inside the single consumer semgrep, right after harden-runner (which must audit the classifier's own gh api egress). The four expensive steps — checkout, revision verify, manifest verify, Run Semgrep — carry if: steps.scope.outputs.code == 'true', and so does the final "Enforce Semgrep gate" step. The job keeps if: github.event.action != 'closed' with no needs. term and gains pull-requests: read (the permission the gate job carried).

Measured in #1904 ("Items 15/16/17 measurement", docs/product-technical-gap-baseline.md): the gate was one full runner allocation per PR spent computing two booleans for one downstream job. Net: one runner per PR for this workflow instead of two, org-wide (the workflow is ruleset-dispatched into ~74 repositories).

Safety conditions, checked explicitly rather than assumed

• Job-level decision is preserved. Org ruleset 18156473 ignores every on: filter, and .github's classic protection would leave a trigger-filtered context Pending forever (docs/doctoring/required-workflow-path-filter-boundary.md). The semgrep job still runs on every PR and concludes success on a doc-only PR (steps skip, job succeeds). tests/test_docs_only_pr_runner_admission.py::test_each_gate_workflow_keeps_an_always_admitted_job semantics hold via the new dedicated test.
• No .github classic context disappears. Neither Detect changed scope nor Semgrep (multi-language SAST) is among the 14 named contexts in .github's branch protection (list in the boundary doc), so nothing goes Pending there.
• The trap the first draft would have shipped: the enforce step's always() && (... || steps.semgrep.outputs.rc != '0') evaluates rc as the empty string when Run Semgrep is step-skipped, and '' != '0' is true — every doc-only PR would have failed. The added steps.scope.outputs.code == 'true' && on that step is what makes the fold safe.
• strix.yml, the other single-consumer gate, is deliberately untouched — it is a documented multi-PR hot-file collision zone. security-scan.yml's gate has four consumers and is legitimately amortized.

Contract tests

• tests/test_docs_only_pr_runner_admission.py: GATE_WORKFLOWS drops sast-semgrep.yml; the doc-pattern byte-identity check still includes it (the case line is unchanged); new test_sast_semgrep_folds_the_gate_into_its_single_consumer_at_step_level pins: no changed-scope: job / needs, exactly one runs-on, job if exactly the close guard, pull-requests: read, five step guards, the enforce-step guard, harden-runner before the classifier.
• tests/test_required_security_runner_image_contract.py: runs-on: ubuntu-24.04 count 2 → 1 with the reason in the docstring.
• docs/product-technical-gap-baseline.md: status line + dated paragraph under the Items 15/16/17 entry.

Verification

coverage run -m pytest tests on this head → 3040 passed, 1 skipped (macOS platform skip), 0 warnings; coverage 100%; interrogate 100%; yaml.safe_load succeeds. Live confirmation that a doc-only PR's Semgrep (multi-language SAST) check concludes success needs this on protected main (the ruleset dispatches main's copy).

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 47 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 87ffb5da-2fb4-4d64-8e3d-1b91028222f8

📥 Commits

Reviewing files that changed from the base of the PR and between 828eaae and aa52e2a.

📒 Files selected for processing (4)

• .github/workflows/sast-semgrep.yml
• docs/product-technical-gap-baseline.md
• tests/test_docs_only_pr_runner_admission.py
• tests/test_required_security_runner_image_contract.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cwl-noema-review]

Noema LLM review

The fold of the changed-scope classifier into the single semgrep job is correct and safe. The classifier logic is unchanged (same step ID scope, same gh api logic), all expensive steps plus the final enforce step gate on steps.scope.outputs.code == 'true', and the job-level if: has no needs. term, so a doc-only PR still runs one job that concludes success. The enforce-step guard explicitly prevents the empty-rc false failure, and byte-level contract tests cover all five guard occurrences plus the exact enforce-step expression.

Reviewed changed lines

• .github/workflows/sast-semgrep.yml:46 (RIGHT): Job-level if: remains github.event.action != 'closed' with no needs. term, preserving the always-admitted job property required for doc-only PRs.
• .github/workflows/sast-semgrep.yml:121 (RIGHT): The enforce step's guard includes steps.scope.outputs.code == 'true' before the rc != '0' check, preventing a skipped Run Semgrep from failing on empty rc.
• .github/workflows/sast-semgrep.yml:215 (RIGHT): The complete condition always() && steps.scope.outputs.code == 'true' && (steps.semgrep_sarif.outputs.finding_count != '0' || steps.semgrep.outputs.rc != '0') short-circuits correctly, making the fold safe.
• tests/test_docs_only_pr_runner_admission.py:224 (RIGHT): Byte-level test asserts exactly 5 steps.scope.outputs.code == 'true' occurrences and the exact enforce-step guard string, covering all gated steps.
• tests/test_required_security_runner_image_contract.py:31 (RIGHT): Runner-image contract updated to expect exactly one runs-on: ubuntu-24.04, confirming no second job was reintroduced by the fold.

Adversarial validation

• .github/workflows/sast-semgrep.yml:46 (RIGHT) falsified: A doc-only PR would leave the run skipped or fail due to empty rc on the enforce step. — Job-level if: at line 46 has no needs. term, so the job runs; the enforce guard at line 215 includes steps.scope.outputs.code == 'true' before the rc comparison, short-circuiting to false when code is false.
• tests/test_required_security_runner_image_contract.py:33 (RIGHT) falsified: A second runner job remains, costing an extra allocation per PR. — The contract test asserts workflow.count("runs-on: ubuntu-24.04") == 1, and the diff shows exactly one job block with that image.
• tests/test_docs_only_pr_runner_admission.py:224 (RIGHT) falsified: Some expensive step is missing the classifier-output guard and would run on doc-only PRs. — The byte-level test asserts exactly 5 occurrences of steps.scope.outputs.code == 'true', covering checkout, verify-submitted-revision, verify-pinned-manifest, Run Semgrep, and Enforce-gate steps.
• Residual risk: No residual risk identified; the fold is covered by byte-level contract tests and the job-level if: preserves required-check behavior.

Findings

• No blocking findings.

• Result: APPROVE
• Head SHA: aa52e2abfc5a9f4766c7088abdd4556f22178659
• Reviewer credential: noema-review-github-app-refresh
• Actor: cwl-noema-review[bot]

[seonghobae]

Bypass-merge evidence (directive §11 infrastructure exception)

Head merged: aa52e2abfc5a9f4766c7088abdd4556f22178659 (contains protected main@ebc69a401; base merged non-force on 2026-09-13 18:10Z, 0 conflicts).

Change under review: partial #1976 — sast-semgrep.yml: fold the separate changed-scope job into the semgrep job as a step-level guard (one runner per PR instead of two; measured in #1904). Scan/checkout steps are guarded by steps.scope.outputs.code, post-scan steps by hashFiles(...), the gate by both, so a docs-only PR still concludes success on the required Semgrep (multi-language SAST) context. Detect changed scope is not a required context on main (verified via branch protection API). Contract tests test_docs_only_pr_runner_admission.py / test_required_security_runner_image_contract.py updated.

Local exact-head verification (detached worktree at this head): coverage run -m pytest tests -W error::DeprecationWarning → 3079 passed / 3 host-skipped / 0 warnings; coverage report → 99% overall — the only gap is scripts/ci/noema_review_document.py (73%), merged to main today by #2172/#2178 and untouched by this PR; the 2 host-skipped tests in tests/test_noema_document_review_context.py require NOEMA_HWP_MCP_SOURCE fixtures that this host lacks. Every file this PR touches is at 100%.; interrogate → 100.0%.

Non-passing required checks on this head at 01:05Z and their class:

check	state @01:05Z	class
CodeQL compatibility analysis (python/actions)	fail	fail-closed placeholder: DISPATCH_OUTCOME: success, VERDICT_STATE: pending; auto-rerun after the dispatch verdict (same pattern passed on the previous head)
coverage-evidence / coverage-source-tree / opencode-review	queued since 23:52Z (>1h)	runner-pool saturation (#712/#1531)
scan-pr-queue	queued since ~23:00Z (2h)	runner-pool saturation (scheduler self-run)
strix	in_progress since ~23:30Z	orchestrator/free gateway; ≥2h per model is policy
noema-review	(no current-head run yet)	gateway dispatch pending

Security/contract checks that did complete on this head all pass (Bandit, Semgrep, gitleaks, pip-audit, trivy-fs, scorecard, CodeRabbit, coverage where finished). The remaining blockers are the orchestrator/free review gateway (noema/opencode/strix, #2165/#2148/#1915) and runner-pool saturation (jobs queued for hours, #712/#1531); CodeQL "compatibility analysis" failures are the documented fail-closed placeholders (DISPATCH_OUTCOME: success, VERDICT_STATE: pending). Same-account approval is impossible (422 Can not approve your own pull request).

Post-merge: the required review workflows re-run on main; a real finding is repaired or reverted on main. Nothing is skipped, only deferred. Merged via gh pr merge --admin --merge (no branch mutation) by the owner-authorised session, following the same standard applied to #2123 and the owner's 12 manual merges on 2026-09-13.

🤖 Generated with Claude Code