From f1aacad2dd55062b48bdf084a790a6f75ed4098f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 07:58:11 +0000 Subject: [PATCH] [oblt-aw][security] Fix SEC-010 semgrep rule mapping Correct semgrep check_id to SEC mapping in scripts/obs/security-scan.sh so secret/credential findings are not misclassified as SEC-010 injection findings. Add regression coverage and update detector docs/ruleset traceability accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/workflows/gh-aw-security-detector.md | 2 +- docs/workflows/security-scanning-ruleset.md | 6 +- scripts/obs/security-scan.sh | 4 +- tests/test_security_scan_semgrep_mapping.py | 76 +++++++++++++++++++++ 4 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/test_security_scan_semgrep_mapping.py diff --git a/docs/workflows/gh-aw-security-detector.md b/docs/workflows/gh-aw-security-detector.md index 54e086f..f2cb668 100644 --- a/docs/workflows/gh-aw-security-detector.md +++ b/docs/workflows/gh-aw-security-detector.md @@ -42,7 +42,7 @@ jobs: |-------|-----------| | SEC-002, SEC-021, SEC-030, SEC-040, SEC-043, and other workflow rules | **zizmor** (offline audits; `ident` mapped to SEC IDs in [scripts/obs/security-scan.sh](../../scripts/obs/security-scan.sh)) | | SEC-010, SEC-002 (expression), SEC-020 (credentials) | **actionlint** JSON output (security-related kinds / messages only) | -| SEC-010, SEC-012 | **semgrep** `p/github-actions` on `.github/workflows` | +| SEC-002, SEC-010, SEC-012, SEC-020 | **semgrep** `p/github-actions` on `.github/workflows` (mapped by `check_id` patterns in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh)) | | SEC-011 | shellcheck on `*.sh` / `*.bash`; actionlint also runs shellcheck on embedded `run:` scripts | | SEC-032 | curl/wget in scripts without checksum/signature helpers in-file (custom heuristic) | | SEC-033 | `npm audit` when lockfile + npm available | diff --git a/docs/workflows/security-scanning-ruleset.md b/docs/workflows/security-scanning-ruleset.md index 5f9e171..edb1583 100644 --- a/docs/workflows/security-scanning-ruleset.md +++ b/docs/workflows/security-scanning-ruleset.md @@ -40,12 +40,12 @@ The table below documents how each rule ID is currently represented in the detec | Rule ID | Implemented in detector | Primary implementation path | |---------|-------------------------|-----------------------------| | SEC-001 | No | Not currently emitted by [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | -| SEC-002 | Yes | `actionlint` secret message mapping and `zizmor` `secrets-outside-env` mapping in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | +| SEC-002 | Yes | `actionlint` secret message mapping, `zizmor` `secrets-outside-env` mapping, and `semgrep` `check_id` mappings for non-hardcoded secret/token/credential findings in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | | SEC-003 | No | Not currently emitted by [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | -| SEC-010 | Yes | `actionlint` expression mapping, `zizmor` template/github-env mappings, and `semgrep` injection mapping in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | +| SEC-010 | Yes | `actionlint` expression mapping, `zizmor` template/github-env mappings, and `semgrep` injection/template mappings in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | | SEC-011 | Yes | `shellcheck` and `actionlint` shellcheck mappings in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | | SEC-012 | Yes | `zizmor` default and targeted mappings plus `semgrep` non-injection workflow mappings in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | -| SEC-020 | Yes | `actionlint` credentials mapping and `zizmor` hardcoded credentials mapping in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | +| SEC-020 | Yes | `actionlint` credentials mapping, `zizmor` hardcoded credentials mapping, and `semgrep` hardcoded secret/token/credential mappings in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | | SEC-021 | Yes | `zizmor` `unredacted-secrets` mapping in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | | SEC-022 | Yes | `zizmor` `overprovisioned-secrets` and `secrets-inherit` mappings in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | | SEC-030 | Yes | `zizmor` unpinned/ref-integrity mappings in [`scripts/obs/security-scan.sh`](../../scripts/obs/security-scan.sh) | diff --git a/scripts/obs/security-scan.sh b/scripts/obs/security-scan.sh index b59354b..28426f6 100755 --- a/scripts/obs/security-scan.sh +++ b/scripts/obs/security-scan.sh @@ -184,7 +184,9 @@ if [ -d "$REPO_ROOT/.github/workflows" ] && command -v semgrep >/dev/null 2>&1; (if ($sv == "ERROR" or $sv == "error") then "high" elif ($sv == "WARNING" or $sv == "warning") then "medium" else "low" end) as $sev | - (if ($cid | test("injection|insecure|secret|credential"; "i")) then "SEC-010" + (if ($cid | test("hardcoded.*(secret|token|credential)"; "i")) then "SEC-020" + elif ($cid | test("secret|token|credential"; "i")) then "SEC-002" + elif ($cid | test("injection|template|insecure"; "i")) then "SEC-010" else "SEC-012" end) as $rule | "\($p)|\($ln)|\($rule)|\($sev)|semgrep [\($cid)]: \($msg)" ' >>"$FINDINGS_TMP" 2>/dev/null || true diff --git a/tests/test_security_scan_semgrep_mapping.py b/tests/test_security_scan_semgrep_mapping.py new file mode 100644 index 0000000..6de3e37 --- /dev/null +++ b/tests/test_security_scan_semgrep_mapping.py @@ -0,0 +1,76 @@ +from __future__ import annotations + +import json +import os +import pathlib +import stat +import subprocess + +import pytest + +ROOT = pathlib.Path(__file__).parent.parent +SECURITY_SCAN = ROOT / "scripts" / "obs" / "security-scan.sh" + + +def _write_fake_semgrep(bin_dir: pathlib.Path, result: dict[str, object]) -> None: + semgrep = bin_dir / "semgrep" + payload = json.dumps({"results": [result]}) + semgrep.write_text( + "#!/usr/bin/env sh\n" + f"printf '%s\\n' '{payload}'\n", + encoding="utf-8", + ) + semgrep.chmod(semgrep.stat().st_mode | stat.S_IEXEC) + + +def _run_scan(tmp_path: pathlib.Path, check_id: str) -> str: + repo = tmp_path / "repo" + workflow_dir = repo / ".github" / "workflows" + workflow_dir.mkdir(parents=True) + (workflow_dir / "test.yml").write_text( + "name: test\non: workflow_dispatch\njobs: {}\n", encoding="utf-8" + ) + + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + _write_fake_semgrep( + fake_bin, + { + "path": ".github/workflows/test.yml", + "start": {"line": 3}, + "check_id": check_id, + "extra": {"message": "synthetic semgrep finding", "severity": "ERROR"}, + }, + ) + + env = os.environ.copy() + env["PATH"] = f"{fake_bin}:/usr/bin:/bin" + completed = subprocess.run( + ["bash", str(SECURITY_SCAN), str(repo)], + check=True, + capture_output=True, + text=True, + env=env, + ) + return completed.stdout + + +@pytest.mark.parametrize( + ("check_id", "expected_rule"), + [ + ("github-actions.hardcoded-secret", "SEC-020"), + ("github-actions.secret-in-command", "SEC-002"), + ("github-actions.template-injection", "SEC-010"), + ("github-actions.workflow-oddity", "SEC-012"), + ], +) +def test_semgrep_check_id_maps_to_expected_sec_rule( + tmp_path: pathlib.Path, check_id: str, expected_rule: str +) -> None: + output = _run_scan(tmp_path, check_id) + line = next( + row + for row in output.splitlines() + if row and "|3|" in row and "semgrep [" in row + ) + assert f"|{expected_rule}|" in line