From 1004d64dc41d2bba9cca13db9122ce3a1adb5b6b Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Thu, 5 Mar 2026 06:32:54 -0500 Subject: [PATCH] ci(security): add pre-push trivy gate and workflow-script safety checks --- .../release/ghcr-vulnerability-policy.json | 1 - .github/workflows/pub-docker-img.yml | 121 ++++++++++++++++-- docs/actions-source-policy.md | 21 +++ docs/operations/ghcr-vulnerability-policy.md | 9 +- scripts/ci/ci_change_audit.py | 55 +++++++- scripts/ci/tests/test_ci_scripts.py | 66 ++++++++++ 6 files changed, 259 insertions(+), 14 deletions(-) diff --git a/.github/release/ghcr-vulnerability-policy.json b/.github/release/ghcr-vulnerability-policy.json index 64209b060..e4e56e3ab 100644 --- a/.github/release/ghcr-vulnerability-policy.json +++ b/.github/release/ghcr-vulnerability-policy.json @@ -6,7 +6,6 @@ "latest" ], "blocking_severities": [ - "HIGH", "CRITICAL" ], "max_blocking_findings_per_tag": 0, diff --git a/.github/workflows/pub-docker-img.yml b/.github/workflows/pub-docker-img.yml index 1a6520e29..937321772 100644 --- a/.github/workflows/pub-docker-img.yml +++ b/.github/workflows/pub-docker-img.yml @@ -33,6 +33,7 @@ env: GIT_CONFIG_VALUE_0: /dev/null REGISTRY: ghcr.io IMAGE_NAME: ${{ github.repository }} + TRIVY_IMAGE: aquasec/trivy:0.58.2 jobs: pr-smoke: @@ -83,8 +84,8 @@ jobs: tags: zeroclaw-pr-smoke:latest labels: ${{ steps.meta.outputs.labels || '' }} platforms: linux/amd64 - cache-from: type=gha - cache-to: type=gha,mode=max + cache-from: type=gha,scope=pub-docker-pr-${{ github.event.pull_request.number || 'dispatch' }} + cache-to: type=gha,scope=pub-docker-pr-${{ github.event.pull_request.number || 'dispatch' }},mode=max - name: Verify image run: docker run --rm zeroclaw-pr-smoke:latest --version @@ -174,6 +175,107 @@ jobs: echo "latest_tag=${LATEST_SUFFIX}" } >> "$GITHUB_OUTPUT" + - name: Build release candidate image (pre-push scan) + uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 + with: + context: . + push: false + load: true + tags: zeroclaw-release-candidate:${{ steps.meta.outputs.release_tag }} + platforms: linux/amd64 + cache-from: type=gha,scope=pub-docker-release-${{ steps.meta.outputs.release_tag }} + cache-to: type=gha,scope=pub-docker-release-${{ steps.meta.outputs.release_tag }},mode=max + + - name: Pre-push Trivy gate (CRITICAL blocks, HIGH warns) + shell: bash + run: | + set -euo pipefail + mkdir -p artifacts + + LOCAL_SCAN_IMAGE="zeroclaw-release-candidate:${{ steps.meta.outputs.release_tag }}" + + docker run --rm \ + -v "$PWD/artifacts:/work" \ + "${TRIVY_IMAGE}" image \ + --quiet \ + --ignore-unfixed \ + --severity CRITICAL \ + --format json \ + --output /work/trivy-prepush-critical.json \ + "${LOCAL_SCAN_IMAGE}" + + critical_count="$(python3 - <<'PY' + import json + from pathlib import Path + + report = Path("artifacts/trivy-prepush-critical.json") + if not report.exists(): + print(0) + raise SystemExit(0) + + data = json.loads(report.read_text(encoding="utf-8")) + count = 0 + for result in data.get("Results", []): + vulns = result.get("Vulnerabilities") or [] + count += len(vulns) + print(count) + PY + )" + + docker run --rm \ + -v "$PWD/artifacts:/work" \ + "${TRIVY_IMAGE}" image \ + --quiet \ + --ignore-unfixed \ + --severity HIGH \ + --format json \ + --output /work/trivy-prepush-high.json \ + "${LOCAL_SCAN_IMAGE}" + + docker run --rm \ + -v "$PWD/artifacts:/work" \ + "${TRIVY_IMAGE}" image \ + --quiet \ + --ignore-unfixed \ + --severity HIGH \ + --format table \ + --output /work/trivy-prepush-high.txt \ + "${LOCAL_SCAN_IMAGE}" + + high_count="$(python3 - <<'PY' + import json + from pathlib import Path + + report = Path("artifacts/trivy-prepush-high.json") + if not report.exists(): + print(0) + raise SystemExit(0) + + data = json.loads(report.read_text(encoding="utf-8")) + count = 0 + for result in data.get("Results", []): + vulns = result.get("Vulnerabilities") or [] + count += len(vulns) + print(count) + PY + )" + + { + echo "### Pre-push Trivy Gate" + echo "- Candidate image: \`${LOCAL_SCAN_IMAGE}\`" + echo "- CRITICAL findings: \`${critical_count}\` (blocking)" + echo "- HIGH findings: \`${high_count}\` (advisory)" + } >> "$GITHUB_STEP_SUMMARY" + + if [ "${high_count}" -gt 0 ]; then + echo "::warning::Pre-push Trivy found ${high_count} HIGH vulnerabilities (advisory only)." + fi + + if [ "${critical_count}" -gt 0 ]; then + echo "::error::Pre-push Trivy found ${critical_count} CRITICAL vulnerabilities." + exit 1 + fi + - name: Build and push Docker image uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6 with: @@ -183,8 +285,8 @@ jobs: ZEROCLAW_CARGO_ALL_FEATURES=true tags: ${{ steps.meta.outputs.tags }} platforms: linux/amd64,linux/arm64 - cache-from: type=gha - cache-to: type=gha,mode=max + cache-from: type=gha,scope=pub-docker-release-${{ steps.meta.outputs.release_tag }} + cache-to: type=gha,scope=pub-docker-release-${{ steps.meta.outputs.release_tag }},mode=max - name: Set GHCR package visibility to public shell: bash @@ -271,7 +373,7 @@ jobs: if-no-files-found: ignore retention-days: 21 - - name: Scan published image for vulnerabilities (Trivy) + - name: Scan published image for policy evidence (Trivy) shell: bash run: | set -euo pipefail @@ -298,7 +400,7 @@ jobs: docker run --rm \ -v "$PWD/artifacts:/work" \ - aquasec/trivy:0.58.2 image \ + "${TRIVY_IMAGE}" image \ --quiet \ --ignore-unfixed \ --severity HIGH,CRITICAL \ @@ -308,7 +410,7 @@ jobs: docker run --rm \ -v "$PWD/artifacts:/work" \ - aquasec/trivy:0.58.2 image \ + "${TRIVY_IMAGE}" image \ --quiet \ --ignore-unfixed \ --severity HIGH,CRITICAL \ @@ -319,7 +421,7 @@ jobs: docker run --rm \ -v "$PWD/artifacts:/work" \ - aquasec/trivy:0.58.2 image \ + "${TRIVY_IMAGE}" image \ --quiet \ --ignore-unfixed \ --severity HIGH,CRITICAL \ @@ -419,5 +521,8 @@ jobs: artifacts/trivy-sha-*.json artifacts/trivy-latest.txt artifacts/trivy-latest.json + artifacts/trivy-prepush-critical.json + artifacts/trivy-prepush-high.json + artifacts/trivy-prepush-high.txt if-no-files-found: ignore retention-days: 14 diff --git a/docs/actions-source-policy.md b/docs/actions-source-policy.md index 675d1b971..1941bcaaa 100644 --- a/docs/actions-source-policy.md +++ b/docs/actions-source-policy.md @@ -57,6 +57,27 @@ Because this repository has high agent-authored change volume: - Expand allowlist only for verified missing actions; avoid broad wildcard exceptions. - Keep rollback instructions in the PR description for Actions policy changes. +## `pull_request_target` Safety Contract + +The repository intentionally uses `pull_request_target` for PR intake/label automation. +Those workflows run with base-repo token scope, so script-level safety rules are strict. + +Required controls: + +- Keep `pull_request_target` limited to trusted automation workflows (`pr-intake-checks.yml`, `pr-labeler.yml`, `pr-auto-response.yml`). +- Run only repository-owned helper scripts from `.github/workflows/scripts/`. +- Treat PR-controlled strings as data only; never execute or evaluate them. +- Block dynamic execution primitives in workflow helper scripts: + - `eval(...)` + - `Function(...)` + - `vm.runInContext(...)`, `vm.runInNewContext(...)`, `vm.runInThisContext(...)`, `new vm.Script(...)` + - `child_process.exec(...)`, `execSync(...)`, `spawn(...)`, `spawnSync(...)`, `execFile(...)`, `execFileSync(...)`, `fork(...)` + +Enforcement: + +- `.github/workflows/ci-change-audit.yml` runs `scripts/ci/ci_change_audit.py` with policy-fail mode. +- The audit policy blocks new unsafe workflow-script JS patterns and new `pull_request_target` triggers in CI/security workflow changes. + ## Validation Checklist After allowlist changes, validate: diff --git a/docs/operations/ghcr-vulnerability-policy.md b/docs/operations/ghcr-vulnerability-policy.md index 92412b9ac..152a051a3 100644 --- a/docs/operations/ghcr-vulnerability-policy.md +++ b/docs/operations/ghcr-vulnerability-policy.md @@ -18,17 +18,24 @@ For each stable release publish, Trivy scan evidence must exist for all required The policy requires these scan reports to be machine-readable and validated before publish is considered complete. +Publish flow also runs a pre-push Trivy gate on a local release-candidate image: + +- `CRITICAL` findings block image publish +- `HIGH` findings are reported as advisory warnings + ## Blocking Rule Policy field `blocking_severities` defines which severities are merge-blocking for publish. Default policy: -- `HIGH` - `CRITICAL` `max_blocking_findings_per_tag` is `0`, so any blocking finding fails the gate. +`HIGH` findings are still collected and published in Trivy artifacts and run summaries, +but are advisory-only to avoid blocking urgent patch releases on non-critical CVEs. + ## Parity Rules To keep tags consistent and auditable, the gate can enforce parity checks: diff --git a/scripts/ci/ci_change_audit.py b/scripts/ci/ci_change_audit.py index 7455e90d1..c2932a993 100755 --- a/scripts/ci/ci_change_audit.py +++ b/scripts/ci/ci_change_audit.py @@ -9,6 +9,7 @@ The report is designed for change-control traceability and light policy checks: - detect risky pipe-to-shell commands (e.g. `curl ... | sh`) - detect newly introduced `pull_request_target` triggers in supported YAML forms - detect broad `permissions: write-all` grants +- detect unsafe JS execution patterns in workflow helper scripts - detect newly introduced `${{ secrets.* }}` references """ @@ -46,12 +47,27 @@ WORKFLOW_PATH_PREFIXES = ( ) WORKFLOW_EXTENSIONS = (".yml", ".yaml") SHELL_EXTENSIONS = (".sh", ".bash") +JS_EXTENSIONS = (".js", ".cjs", ".mjs") USES_RE = re.compile(r"^\+\s*(?:-\s*)?uses:\s*([^\s#]+)") SECRETS_RE = re.compile(r"\$\{\{\s*secrets\.([A-Za-z0-9_]+)\s*}}") SHA_PIN_RE = re.compile(r"^[0-9a-f]{40}$") PIPE_TO_SHELL_RE = re.compile(r"\b(?:curl|wget)\b.*\|\s*(?:sh|bash)\b") PERMISSION_WRITE_RE = re.compile(r"^\+\s*([a-z-]+):\s*write\s*$") PERMISSIONS_WRITE_ALL_RE = re.compile(r"^\+\s*permissions\s*:\s*write-all\s*$", re.IGNORECASE) +UNSAFE_JS_PATTERNS: tuple[tuple[str, re.Pattern[str]], ...] = ( + ("eval()", re.compile(r"\beval\s*\(")), + ("Function()", re.compile(r"\bFunction\s*\(")), + ( + "vm.* execution", + re.compile(r"\bvm\.(?:runInContext|runInNewContext|runInThisContext|Script)\b"), + ), + ( + "child_process dynamic execution", + re.compile( + r"\bchild_process\.(?:exec|execSync|spawn|spawnSync|execFile|execFileSync|fork)\s*\(" + ), + ), +) def line_adds_pull_request_target(added_text: str) -> bool: @@ -90,6 +106,18 @@ def is_shell_path(path: str) -> bool: return path.endswith(SHELL_EXTENSIONS) or path.startswith(".githooks/") +def is_workflow_script_js_path(path: str) -> bool: + return path.startswith(".github/workflows/scripts/") and path.endswith(JS_EXTENSIONS) + + +def detect_unsafe_js_patterns(added_text: str) -> list[str]: + stripped = added_text.lstrip() + # Ignore comments for this policy check to reduce false positives in docs/comments. + if stripped.startswith("//") or stripped.startswith("/*") or stripped.startswith("*"): + return [] + return [label for label, pattern in UNSAFE_JS_PATTERNS if pattern.search(added_text)] + + @dataclass class FileAudit: path: str @@ -102,6 +130,7 @@ class FileAudit: added_pipe_to_shell: list[str] = field(default_factory=list) added_write_permissions: list[str] = field(default_factory=list) added_pull_request_target: int = 0 + added_unsafe_js_patterns: list[str] = field(default_factory=list) @property def risk_level(self) -> str: @@ -109,6 +138,7 @@ class FileAudit: self.unpinned_actions or self.added_pipe_to_shell or self.added_pull_request_target + or self.added_unsafe_js_patterns or "write-all" in self.added_write_permissions ): return "high" @@ -179,7 +209,8 @@ def build_markdown( lines.append( f"- Policy violations: `{len(violations)}` " "(currently: unpinned `uses:`, pipe-to-shell commands, broad " - "`permissions: write-all`, and new `pull_request_target` triggers)" + "`permissions: write-all`, unsafe workflow-script JS execution patterns, " + "and new `pull_request_target` triggers)" ) lines.append("") @@ -197,14 +228,15 @@ def build_markdown( lines.append("") lines.append( "| Path | Status | +Lines | -Lines | New Actions | New Secret Refs | " - "Pipe-to-Shell | New `*: write` | New `pull_request_target` | Risk |" + "Pipe-to-Shell | Unsafe JS Patterns | New `*: write` | New `pull_request_target` | Risk |" ) - lines.append("| --- | --- | ---:| ---:| ---:| ---:| ---:| ---:| ---:| --- |") + lines.append("| --- | --- | ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| --- |") for audit in sorted(audits, key=lambda x: x.path): lines.append( f"| `{audit.path}` | `{audit.status}` | {audit.added} | {audit.deleted} | " f"{len(audit.added_actions)} | {len(audit.added_secret_refs)} | " - f"{len(audit.added_pipe_to_shell)} | {len(set(audit.added_write_permissions))} | " + f"{len(audit.added_pipe_to_shell)} | {len(set(audit.added_unsafe_js_patterns))} | " + f"{len(set(audit.added_write_permissions))} | " f"{audit.added_pull_request_target} | " f"`{audit.risk_level}` |" ) @@ -228,6 +260,10 @@ def build_markdown( lines.append("- Added pipe-to-shell commands (high risk):") for cmd in audit.added_pipe_to_shell: lines.append(f" - `{cmd}`") + if audit.added_unsafe_js_patterns: + lines.append("- Added unsafe workflow-script JS patterns (high risk):") + for pattern_name in sorted(set(audit.added_unsafe_js_patterns)): + lines.append(f" - `{pattern_name}`") if audit.added_write_permissions: lines.append("- Added write permissions:") for permission_name in sorted(set(audit.added_write_permissions)): @@ -272,6 +308,7 @@ def main() -> int: audit = FileAudit(path=path, status=status, added=added, deleted=deleted) workflow_yaml = is_workflow_yaml_path(path) shell_script = is_shell_path(path) + workflow_script_js = is_workflow_script_js_path(path) for line in parse_patch_added_lines(args.base_sha, args.head_sha, path): added_text = line[1:].strip() @@ -296,6 +333,14 @@ def main() -> int: f"{path}: pipe-to-shell command introduced -> `{command}`" ) + if workflow_script_js: + unsafe_matches = detect_unsafe_js_patterns(added_text) + for pattern_name in unsafe_matches: + audit.added_unsafe_js_patterns.append(pattern_name) + violations.append( + f"{path}: unsafe workflow-script JS pattern introduced -> `{pattern_name}`" + ) + permission_match = PERMISSION_WRITE_RE.match(line) if permission_match and workflow_yaml: audit.added_write_permissions.append(permission_match.group(1)) @@ -323,6 +368,7 @@ def main() -> int: "new_unpinned_actions": sum(len(a.unpinned_actions) for a in audits), "new_secret_references": sum(len(a.added_secret_refs) for a in audits), "new_pipe_to_shell_commands": sum(len(a.added_pipe_to_shell) for a in audits), + "new_unsafe_js_patterns": sum(len(set(a.added_unsafe_js_patterns)) for a in audits), "new_write_permissions": sum(len(set(a.added_write_permissions)) for a in audits), "new_pull_request_target_triggers": sum(a.added_pull_request_target for a in audits), "violations": len(violations), @@ -342,6 +388,7 @@ def main() -> int: "unpinned_actions": a.unpinned_actions, "added_secret_refs": sorted(set(a.added_secret_refs)), "added_pipe_to_shell": a.added_pipe_to_shell, + "added_unsafe_js_patterns": sorted(set(a.added_unsafe_js_patterns)), "added_write_permissions": sorted(set(a.added_write_permissions)), "added_pull_request_target": a.added_pull_request_target, "risk_level": a.risk_level, diff --git a/scripts/ci/tests/test_ci_scripts.py b/scripts/ci/tests/test_ci_scripts.py index 19294a483..8ce2f8e11 100644 --- a/scripts/ci/tests/test_ci_scripts.py +++ b/scripts/ci/tests/test_ci_scripts.py @@ -1157,6 +1157,71 @@ class CiScriptsBehaviorTest(unittest.TestCase): self.assertGreaterEqual(report["summary"]["new_write_permissions"], 1) self.assertIn("write-all", "\n".join(report["violations"])) + def test_ci_change_audit_blocks_unsafe_workflow_script_patterns(self) -> None: + repo = self.tmp / "repo" + repo.mkdir(parents=True, exist_ok=True) + run_cmd(["git", "init"], cwd=repo) + run_cmd(["git", "config", "user.name", "Test User"], cwd=repo) + run_cmd(["git", "config", "user.email", "test@example.com"], cwd=repo) + + workflow_scripts_dir = repo / ".github" / "workflows" / "scripts" + workflow_scripts_dir.mkdir(parents=True, exist_ok=True) + helper = workflow_scripts_dir / "unsafe_helper.js" + helper.write_text( + textwrap.dedent( + """ + module.exports = async function runSafe() { + const value = "ok"; + return value; + }; + """ + ).strip() + + "\n", + encoding="utf-8", + ) + run_cmd(["git", "add", "."], cwd=repo) + run_cmd(["git", "commit", "-m", "base"], cwd=repo) + base_sha = run_cmd(["git", "rev-parse", "HEAD"], cwd=repo).stdout.strip() + + helper.write_text( + textwrap.dedent( + """ + module.exports = async function runUnsafe() { + const output = child_process.exec("echo unsafe"); + return output; + }; + """ + ).strip() + + "\n", + encoding="utf-8", + ) + run_cmd(["git", "add", "."], cwd=repo) + run_cmd(["git", "commit", "-m", "head"], cwd=repo) + head_sha = run_cmd(["git", "rev-parse", "HEAD"], cwd=repo).stdout.strip() + + out_json = self.tmp / "audit-unsafe-workflow-script.json" + out_md = self.tmp / "audit-unsafe-workflow-script.md" + proc = run_cmd( + [ + "python3", + str(SCRIPTS_DIR / "ci_change_audit.py"), + "--base-sha", + base_sha, + "--head-sha", + head_sha, + "--output-json", + str(out_json), + "--output-md", + str(out_md), + "--fail-on-violations", + ], + cwd=repo, + ) + self.assertEqual(proc.returncode, 3) + report = json.loads(out_json.read_text(encoding="utf-8")) + self.assertGreaterEqual(report["summary"]["new_unsafe_js_patterns"], 1) + self.assertIn("unsafe workflow-script JS pattern introduced", "\n".join(report["violations"])) + def test_ci_change_audit_ignores_fixture_signatures_in_python_ci_tests(self) -> None: repo = self.tmp / "repo" repo.mkdir(parents=True, exist_ok=True) @@ -1211,6 +1276,7 @@ class CiScriptsBehaviorTest(unittest.TestCase): self.assertEqual(report["violations"], []) self.assertEqual(report["summary"]["new_unpinned_actions"], 0) self.assertEqual(report["summary"]["new_pipe_to_shell_commands"], 0) + self.assertEqual(report["summary"]["new_unsafe_js_patterns"], 0) self.assertEqual(report["summary"]["new_write_permissions"], 0) self.assertEqual(report["summary"]["new_pull_request_target_triggers"], 0)