Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 24 additions & 31 deletions .github/workflows/sast-semgrep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,36 @@ permissions:
contents: read

jobs:
changed-scope:
name: Detect changed scope
semgrep:
name: Semgrep (multi-language SAST)
# The org ruleset IGNORES every `on:` filter (paths, branches, types) when it
# runs this workflow in another repository, and a trigger-level skip would
# leave `.github`'s classic required contexts Pending forever. Both
# mechanisms honour a JOB-level skip, so the doc/image-only decision is made
# here and consumed through `needs`. See
# mechanisms honour a job that runs and concludes on its own, so the
# doc/image-only decision is made by the classifier step below and consumed
# by the expensive steps' `if:` guards. See
# docs/doctoring/required-workflow-path-filter-boundary.md.
# Fails OPEN: an unreadable, empty, or truncated file list scans everything.
# The gate lives inside this job as a step-level guard (one runner, not two).
if: github.event.action != 'closed'
runs-on: ubuntu-24.04
timeout-minutes: 5
permissions:
contents: read
pull-requests: read
outputs:
code: ${{ steps.scope.outputs.code }}
deps: ${{ steps.scope.outputs.deps }}
security-events: write
actions: read
env:
# Deterministic, no telemetry: registry rules are fetched but no scan data
# is sent back.
SEMGREP_SEND_METRICS: "off"
# Semgrep OSS 1.169.0. Keep the immutable manifest reference in one
# place so hosted scans and local reproduction cannot drift.
SEMGREP_IMAGE: "semgrep/semgrep@sha256:2b33f46ba66cf8cc2ad59ccfa7d22951fd00c632c38f1339e84ec8e6e641a942"
steps:
- name: Harden the runner (Audit all outbound calls)
uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1
with:
egress-policy: audit
- name: Classify changed paths
id: scope
env:
Expand Down Expand Up @@ -99,35 +110,15 @@ jobs:
echo "code=${code}" >> "$GITHUB_OUTPUT"
echo "deps=${deps}" >> "$GITHUB_OUTPUT"
echo "changed-scope code=${code} deps=${deps}"

semgrep:
name: Semgrep (multi-language SAST)
needs: changed-scope
if: github.event.action != 'closed' && needs.changed-scope.outputs.code == 'true'
runs-on: ubuntu-24.04
permissions:
contents: read
security-events: write
actions: read
env:
# Deterministic, no telemetry: registry rules are fetched but no scan data
# is sent back.
SEMGREP_SEND_METRICS: "off"
# Semgrep OSS 1.169.0. Keep the immutable manifest reference in one
# place so hosted scans and local reproduction cannot drift.
SEMGREP_IMAGE: "semgrep/semgrep@sha256:2b33f46ba66cf8cc2ad59ccfa7d22951fd00c632c38f1339e84ec8e6e641a942"
steps:
- name: Harden the runner (Audit all outbound calls)
uses: step-security/harden-runner@b09bb98e06d4d774595224525879c09bc6e98c40 # v2.20.1
with:
egress-policy: audit
- name: Checkout exact submitted revision
if: steps.scope.outputs.code == 'true'
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
with:
repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
ref: ${{ github.event.pull_request.head.sha || github.sha }}
persist-credentials: false
- name: Verify exact submitted revision
if: steps.scope.outputs.code == 'true'
env:
EXPECTED_CHECKOUT_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name || github.repository }}
EXPECTED_CHECKOUT_SHA: ${{ github.event.pull_request.head.sha || github.sha }}
Expand All @@ -140,6 +131,7 @@ jobs:
fi
echo "SAST_CHECKOUT scanner=semgrep repository=${EXPECTED_CHECKOUT_REPOSITORY} expected_sha=${EXPECTED_CHECKOUT_SHA} actual_sha=${actual_sha}"
- name: Verify pinned Semgrep manifest
if: steps.scope.outputs.code == 'true'
run: |
set -euo pipefail
if [[ "${SEMGREP_IMAGE}" =~ ^semgrep/semgrep@sha256:[0-9a-f]{64}$ ]]; then
Expand All @@ -151,6 +143,7 @@ jobs:
fi
- name: Run Semgrep (SARIF)
id: semgrep
if: steps.scope.outputs.code == 'true'
run: |
set +e
echo "Using ${SEMGREP_IMAGE}"
Expand Down Expand Up @@ -219,7 +212,7 @@ jobs:
echo "SEMGREP_ENGINE_FAILURE rc=${SEMGREP_RC:-missing}: Semgrep failed without a WARNING/ERROR SARIF result; inspect the scan command output above."
fi
- name: Enforce Semgrep gate (fail on Medium+ findings)
if: always() && (steps.semgrep_sarif.outputs.finding_count != '0' || steps.semgrep.outputs.rc != '0')
if: always() && steps.scope.outputs.code == 'true' && (steps.semgrep_sarif.outputs.finding_count != '0' || steps.semgrep.outputs.rc != '0')
env:
SEMGREP_RC: ${{ steps.semgrep.outputs.rc }}
SEMGREP_FINDING_COUNT: ${{ steps.semgrep_sarif.outputs.finding_count }}
Expand Down
21 changes: 19 additions & 2 deletions docs/product-technical-gap-baseline.md
Original file line number Diff line number Diff line change
Expand Up @@ -3237,8 +3237,8 @@ intended contract before rewriting the assertion — left for a dedicated follow

## Items 15/16/17 measurement: `Detect changed scope` gate jobs — 2 of 3 are pure runner overhead — 2026-09-05

**Status:** Measured, not yet fixed. Recorded so the fix is grounded in real numbers rather than the intuition
this measurement partly refuted.
**Status:** Measured 2026-09-05; `sast-semgrep.yml` fixed 2026-09-13 (below); `strix.yml` deferred. Recorded so
the fix is grounded in real numbers rather than the intuition this measurement partly refuted.

**Why measured.** Items 15/16/17 ask to remove needlessly-triggered workflows, consolidate workflow files
("bootup에도 시간이 듦"), and cut redundant steps; the standing complaint is the org's 60-concurrent-job
Expand Down Expand Up @@ -3353,3 +3353,20 @@ queries the check-runs API at its own time, order-independently. The implementin
their change was safe because they had scoped it narrowly, not because they had checked for the name
collision — which is the more useful lesson: **a job name is unique only within one workflow file, and the
same name in another file can carry the opposite safety property.**

**Fixed for `sast-semgrep.yml`, 2026-09-13.** The standalone `changed-scope` job is gone; its
"Classify changed paths" step now runs inside the single consumer `semgrep` (after `harden-runner`,
which must audit the classifier's own `gh api` egress) and the four expensive steps plus the final
"Enforce Semgrep gate" step carry `steps.scope.outputs.code == 'true'`. The job keeps
`if: github.event.action != 'closed'` with no `needs.` term, so a doc-only PR's run still executes one
job that concludes `success` -- the load-bearing property from
[`required-workflow-path-filter-boundary.md`](doctoring/required-workflow-path-filter-boundary.md) is
preserved, and neither `Detect changed scope` nor `Semgrep (multi-language SAST)` is among `.github`'s
classic required contexts, so nothing goes Pending there. One 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, which is `!= '0'` and would have failed every doc-only PR;
the guard on that step is what makes the fold safe. Net: one runner allocation per PR for this
workflow instead of two, org-wide. `strix.yml` (the other single-consumer gate) is deliberately left
alone -- it is a documented multi-PR hot-file collision zone. Contract:
`tests/test_docs_only_pr_runner_admission.py::test_sast_semgrep_folds_the_gate_into_its_single_consumer_at_step_level`,
`tests/test_required_security_runner_image_contract.py`.
50 changes: 44 additions & 6 deletions tests/test_docs_only_pr_runner_admission.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@
WORKFLOWS_DIR = REPO_ROOT / ".github/workflows"

# The required workflows that keep the canonical `changed-scope` gate job.
# `sast-semgrep.yml` has only one consumer job, so it folds the classifier
# into that job as a step-level guard instead of a standalone job -- see
# GATED_JOBS below.
GATE_WORKFLOWS = (
"security-scan.yml",
"sast-semgrep.yml",
"strix.yml",
)

Expand All @@ -52,7 +54,6 @@
# output, keyed by workflow filename.
GATED_JOBS = {
"security-scan.yml": ("osv-scan", "dependency-review", "trivy-fs", "scorecard"),
"sast-semgrep.yml": ("semgrep",),
"strix.yml": ("strix",),
}

Expand Down Expand Up @@ -86,7 +87,7 @@ def _on_block(workflow: str) -> str:


def test_gate_job_is_byte_identical_across_the_five_workflows_apart_from_if():
"""The `changed-scope` block must not drift between its five copies."""
"""The `changed-scope` block must not drift between every gate copy."""
normalized_blocks = set()
for filename in GATE_WORKFLOWS:
workflow = _read(filename)
Expand All @@ -110,7 +111,7 @@ def test_gate_job_and_codeql_scope_step_share_one_doc_pattern_line():
`COPYING.txt`/`NOTICE`/`NOTICE.txt` names.
"""
doc_pattern_lines = set()
for filename in (*GATE_WORKFLOWS, "codeql-pr.yml"):
for filename in (*GATE_WORKFLOWS, "sast-semgrep.yml", "codeql-pr.yml"):
workflow = _read(filename)
matches = [
line for line in workflow.splitlines() if "*.md|*.markdown" in line
Expand Down Expand Up @@ -208,8 +209,8 @@ def test_codeql_pr_gates_analyze_head_at_step_level_not_job_level():
def test_each_gate_workflow_keeps_an_always_admitted_job():
"""A fully-skipped run must conclude `success`, never `skipped`.

Every one of the five workflows needs at least one job with no `needs:`
and no needs-output-dependent `if:` -- the `changed-scope` job itself
Every gate workflow needs at least one job with no `needs:` and no
needs-output-dependent `if:` -- the `changed-scope` job itself
qualifies -- so a doc-only PR's run still has a job that runs and
succeeds instead of every job skipping and the run itself reporting
`skipped` (an undocumented conclusion for a required check).
Expand All @@ -220,3 +221,40 @@ def test_each_gate_workflow_keeps_an_always_admitted_job():
job_if = re.search(r"(?m)^ if: (.*)$", block)
assert job_if is not None, filename
assert "needs." not in job_if.group(1), filename


def test_sast_semgrep_folds_the_gate_into_its_single_consumer_at_step_level():
"""`sast-semgrep.yml` has one consumer, so the gate is a step, not a job.

A standalone `changed-scope` job cost a second runner allocation per PR
purely to compute two booleans for one downstream job (measured in
docs/product-technical-gap-baseline.md, "Items 15/16/17 measurement").
Folding it into `semgrep` keeps the load-bearing property -- the job
still runs and concludes `success` on a doc-only PR -- while the
expensive steps gate on the classifier step's output. The final gate
step must also carry that guard: a step-skipped `Run Semgrep` leaves
`steps.semgrep.outputs.rc` empty, which is `!= '0'`.
"""
workflow = _read("sast-semgrep.yml")
# The classifier's own log lines keep saying "changed-scope" (byte-for-byte
# verbatim across every copy, see test_gate_job_and_codeql_scope_step_share_
# one_doc_pattern_line); what must be gone is the standalone JOB.
assert "changed-scope:" not in workflow
assert "needs: changed-scope" not in workflow
assert "needs.changed-scope" not in workflow
assert workflow.count("runs-on: ubuntu-24.04") == 1

semgrep = _top_level_job_block(workflow, "semgrep")
assert not re.search(r"(?m)^ needs:", semgrep)
job_if = re.search(r"(?m)^ if: (.*)$", semgrep)
assert job_if is not None
assert job_if.group(1) == "github.event.action != 'closed'"
assert "pull-requests: read" in semgrep
assert "id: scope" in semgrep
assert semgrep.count("steps.scope.outputs.code == 'true'") == 5
assert (
"if: always() && steps.scope.outputs.code == 'true' && "
"(steps.semgrep_sarif.outputs.finding_count != '0' || steps.semgrep.outputs.rc != '0')"
) in semgrep
# Harden-runner audits egress and must precede the classifier's gh api call.
assert semgrep.index("Harden the runner") < semgrep.index("Classify changed paths")
8 changes: 5 additions & 3 deletions tests/test_required_security_runner_image_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ def test_sast_semgrep_uses_explicit_supported_image(self) -> None:

`#1656` removed the sibling `cancel-closed-pr-runs` no-op job (it
only duplicated PR-stable workflow concurrency), leaving one runner
job in this workflow instead of two. It is 2, not 1, again after the
job in this workflow instead of two. It was 2, not 1, again after the
`changed-scope` gate job was added to skip doc-only PR scope (org
ruleset 18156473 ignores trigger-level path filters).
ruleset 18156473 ignores trigger-level path filters). The count
returned to 1 when that `changed-scope` job was folded into the
`semgrep` job as a step-level guard (one consumer, one runner).
"""
workflow = SAST_SEMGREP.read_text(encoding="utf-8")
self.assertNotIn("runs-on: ubuntu-latest", workflow)
self.assertEqual(workflow.count("runs-on: ubuntu-24.04"), 2)
self.assertEqual(workflow.count("runs-on: ubuntu-24.04"), 1)


if __name__ == "__main__":
Expand Down
Loading