diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0279bed..43fc0b0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,6 +54,16 @@ jobs: python-version: "3.12" - run: python tools/security_guards.py + python-tests: + name: Python tool tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + with: + python-version: "3.12" + - run: python -m unittest discover -s tests -t . -v + dependency-review: name: Dependency review (upstream PRs only) # Requires the repo's Dependency graph, which forks never inherit and diff --git a/tests/test_security_guards.py b/tests/test_security_guards.py new file mode 100644 index 0000000..9f9297e --- /dev/null +++ b/tests/test_security_guards.py @@ -0,0 +1,169 @@ +import json +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +GUARD_SCRIPT = REPO_ROOT / "tools" / "security_guards.py" + +sys.path.insert(0, str(REPO_ROOT / "tools")) +import security_guards # noqa: E402 (imported for its allowlist constants) + + +def run_guards(root: Path) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(root / "tools" / "security_guards.py")], + capture_output=True, + text=True, + ) + + +class GuardRepoFixture(unittest.TestCase): + """Builds a minimal repo tree the guards pass on, then breaks one thing per test. + + The guard script resolves the repo root from its own location, so each test + copies it into a temp tree and runs it as a subprocess - the same way CI + invokes it - asserting on real exit codes and messages. + """ + + def setUp(self): + self.root = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.root, ignore_errors=True) + + (self.root / "tools").mkdir() + shutil.copy(GUARD_SCRIPT, self.root / "tools" / "security_guards.py") + + self.settings = self.root / ".claude" / "settings.json" + self.settings.parent.mkdir() + self.write_settings(sorted(security_guards.ALLOWED_PERMISSIONS)) + + self.gitignore = self.root / ".gitignore" + self.write_gitignore(security_guards.REQUIRED_IGNORE_RULES) + + self.manifest = self.root / ".agents" / "skills" / "example-search" / "cli" / "package.json" + self.manifest.parent.mkdir(parents=True) + self.write_manifest({"name": "example-cli", "scripts": {"start": "bun run src/cli.ts"}}) + + def write_settings(self, allow): + self.settings.write_text(json.dumps({"permissions": {"allow": list(allow)}})) + + def write_gitignore(self, rules): + self.gitignore.write_text("\n".join(rules) + "\n") + + def write_manifest(self, data, path=None): + (path or self.manifest).write_text(json.dumps(data)) + + +class CleanTreeTests(GuardRepoFixture): + def test_clean_tree_passes(self): + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("security_guards: OK", result.stdout) + + +class PermissionGuardTests(GuardRepoFixture): + def test_wildcard_bash_permission_fails(self): + self.write_settings(sorted(security_guards.ALLOWED_PERMISSIONS) + ["Bash(*)"]) + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("not in the reviewed allowlist", result.stdout) + self.assertIn("Bash(*)", result.stdout) + + def test_network_fetch_permission_fails(self): + self.write_settings(sorted(security_guards.ALLOWED_PERMISSIONS) + ["Bash(curl:*)"]) + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("not in the reviewed allowlist", result.stdout) + + def test_dropped_allowlisted_permission_still_passes(self): + # Removing a shipped permission narrows exposure; the guard only + # rejects additions, it must not force entries to exist. + allow = sorted(security_guards.ALLOWED_PERMISSIONS)[:-1] + self.write_settings(allow) + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_invalid_settings_json_fails(self): + self.settings.write_text("{not json") + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("invalid JSON", result.stdout) + + +class GitignoreGuardTests(GuardRepoFixture): + def test_each_missing_personal_data_rule_fails(self): + for rule in security_guards.REQUIRED_IGNORE_RULES: + with self.subTest(rule=rule): + remaining = [r for r in security_guards.REQUIRED_IGNORE_RULES if r != rule] + self.write_gitignore(remaining) + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("required personal-data rule missing", result.stdout) + self.assertIn(rule, result.stdout) + self.write_gitignore(security_guards.REQUIRED_IGNORE_RULES) + + def test_extra_rules_are_allowed(self): + self.write_gitignore(list(security_guards.REQUIRED_IGNORE_RULES) + ["*.bak", "scratch/"]) + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + +class ManifestGuardTests(GuardRepoFixture): + def test_each_lifecycle_script_fails(self): + for script in sorted(security_guards.FORBIDDEN_SCRIPTS): + with self.subTest(script=script): + # The guard flags the script KEY; the value is never inspected, + # so it must stay benign: attack-shaped values (curl-pipe-to-sh + # etc.) written to disk trip AV heuristics - Windows Defender + # quarantines the fixture mid-test and the suite goes flaky. + self.write_manifest( + {"name": "example-cli", "scripts": {script: "echo test"}} + ) + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("lifecycle script", result.stdout) + self.assertIn(script, result.stdout) + self.write_manifest({"name": "example-cli", "scripts": {}}) + + def test_trusted_dependencies_fails(self): + self.write_manifest({"name": "example-cli", "trustedDependencies": ["left-pad"]}) + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("trustedDependencies", result.stdout) + + def test_benign_scripts_pass(self): + self.write_manifest( + {"name": "example-cli", "scripts": {"start": "bun run src/cli.ts", "test": "bun test", "typecheck": "tsc --noEmit"}} + ) + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_node_modules_manifests_are_ignored(self): + # Installed dependencies are not repo-tracked code; a hostile manifest + # inside node_modules must not fail the guard (and bun blocks its + # lifecycle scripts anyway). + nm = self.manifest.parent / "node_modules" / "some-dep" / "package.json" + nm.parent.mkdir(parents=True) + self.write_manifest({"name": "some-dep", "scripts": {"postinstall": "echo test"}}, path=nm) + result = run_guards(self.root) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_no_manifests_at_all_fails(self): + self.manifest.unlink() + result = run_guards(self.root) + self.assertEqual(result.returncode, 1) + self.assertIn("no package.json files found", result.stdout) + + +class RealRepoTests(unittest.TestCase): + def test_guards_pass_on_this_repo(self): + # The live check CI runs: the actual repo tree must satisfy its own guards. + result = run_guards(REPO_ROOT) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + +if __name__ == "__main__": + unittest.main()