diff --git a/.github/pluginctl/src/pluginctl/capabilities.py b/.github/pluginctl/src/pluginctl/capabilities.py index a7b0f477..00f6922a 100644 --- a/.github/pluginctl/src/pluginctl/capabilities.py +++ b/.github/pluginctl/src/pluginctl/capabilities.py @@ -121,6 +121,49 @@ }, } +SANDBOX_BYPASS_RULES: dict[str, dict[str, int | None]] = { + "plugin/sandbox-bypass/asyncio-process-api": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/builtins-mutation": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/ctypes-usage": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/direct-builtins-access": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/filesystem-api": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/importlib-wrapped-module": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/os-process-api": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/requests-session-api": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/socket-convenience-api": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, + "plugin/sandbox-bypass/sys-modules-tamper": { + "min_manifest_version": 2, + "max_manifest_version": None, + }, +} + MANIFEST_SCHEMA_POLICIES: dict[int, dict[str, Any]] = { 0: { "min_app_version": None, @@ -220,6 +263,16 @@ def capability_contract_rule_applies(rule_id: str, manifest_version: int) -> boo return False +def sandbox_bypass_rule_applies(rule_id: str, manifest_version: int) -> bool: + """Whether a CodeQL sandbox-bypass rule applies to this schema version.""" + policy = SANDBOX_BYPASS_RULES.get(rule_id) + return bool( + policy + and manifest_version_enforces_sandbox(manifest_version) + and rule_applies_to_manifest_version(policy, manifest_version) + ) + + def declared_capabilities(manifest: dict[str, Any]) -> set[str]: return effective_capabilities(manifest) diff --git a/.github/pluginctl/src/pluginctl/validate/sarif.py b/.github/pluginctl/src/pluginctl/validate/sarif.py index 0f2fb53c..dac3fd91 100644 --- a/.github/pluginctl/src/pluginctl/validate/sarif.py +++ b/.github/pluginctl/src/pluginctl/validate/sarif.py @@ -149,6 +149,17 @@ def contract_result_requires_review(result: dict) -> bool: ) +def sandbox_result_requires_review(result: dict) -> bool: + """Only sandbox-bypass findings in enforcing runtime manifests are findings.""" + manifest = _runtime_manifest(_uri_plugin_name(result)) + if manifest is None: + return False + if not capabilities.manifest_version_is_valid(manifest): + return True + version = capabilities.parse_manifest_version(manifest) + return capabilities.sandbox_bypass_rule_applies(_rule_id(result), version) + + @dataclass class Counts: blocking: int = 0 @@ -174,7 +185,10 @@ def classify(sarif_objs: list[dict]) -> Counts: for result in (run.get("results") or []): is_sandbox = is_sandbox_bypass_rule(_rule_id(result)) is_contract = is_capability_contract_rule(_rule_id(result)) - if is_sandbox and not feature_flags.SANDBOX_BYPASS_DETECTION: + if is_sandbox and ( + not feature_flags.SANDBOX_BYPASS_DETECTION + or not sandbox_result_requires_review(result) + ): continue if is_contract and ( not feature_flags.CAPABILITY_CONTRACT_DETECTION @@ -252,7 +266,11 @@ def findings_table(sarif_objs: list[dict], predicate, repo: str, sha: str, continue rid = _rule_id(result) if is_sandbox_bypass_rule(rid): - if not feature_flags.SANDBOX_BYPASS_DETECTION or exclude_sandbox_bypass: + if ( + not feature_flags.SANDBOX_BYPASS_DETECTION + or exclude_sandbox_bypass + or not sandbox_result_requires_review(result) + ): continue if is_capability_contract_rule(rid) and ( not feature_flags.CAPABILITY_CONTRACT_DETECTION @@ -285,7 +303,10 @@ def sandbox_findings_table(sarif_objs: list[dict], repo: str, sha: str, if is_suppressed(result): continue rid = _rule_id(result) - if not is_sandbox_bypass_rule(rid): + if ( + not is_sandbox_bypass_rule(rid) + or not sandbox_result_requires_review(result) + ): continue uri, line = _location(result) msg = process_message((result.get("message") or {}).get("text")) @@ -335,7 +356,10 @@ def suppressed_findings_table(sarif_objs: list[dict], repo: str, sha: str, if not is_suppressed(result): continue rid = _rule_id(result) - if is_sandbox_bypass_rule(rid) and not feature_flags.SANDBOX_BYPASS_DETECTION: + if is_sandbox_bypass_rule(rid) and ( + not feature_flags.SANDBOX_BYPASS_DETECTION + or not sandbox_result_requires_review(result) + ): continue if is_capability_contract_rule(rid) and ( not feature_flags.CAPABILITY_CONTRACT_DETECTION diff --git a/.github/pluginctl/tests/test_sarif.py b/.github/pluginctl/tests/test_sarif.py index f9e7fba9..e2924d80 100644 --- a/.github/pluginctl/tests/test_sarif.py +++ b/.github/pluginctl/tests/test_sarif.py @@ -12,6 +12,12 @@ def _load(): return [json.load(fh)] +def _write_runtime_manifest(tmp_path, manifest): + plugin_dir = tmp_path / "plugins" / "demo" + plugin_dir.mkdir(parents=True) + (plugin_dir / "plugin.json").write_text(json.dumps(manifest), encoding="utf-8") + + def test_extension_rules_override_driver(): objs = _load() secmap = sarif.build_secmap(objs[0]["runs"][0]) @@ -45,7 +51,9 @@ def test_sandbox_bypass_is_ignored_when_disabled(monkeypatch): assert counts.total == 4 -def test_sandbox_bypass_is_informational_when_enabled(monkeypatch): +def test_sandbox_bypass_is_informational_when_enabled(tmp_path, monkeypatch): + _write_runtime_manifest(tmp_path, {"manifest_version": 2}) + monkeypatch.chdir(tmp_path) monkeypatch.setattr(sarif.feature_flags, "SANDBOX_BYPASS_DETECTION", True) counts = sarif.classify(_load()) assert counts.blocking == 1 @@ -118,7 +126,9 @@ def test_suppressed_findings_table_includes_only_suppressed(): assert table.count("`py/sql-injection`") == 1 -def test_sandbox_findings_table_when_enabled(monkeypatch): +def test_sandbox_findings_table_when_enabled(tmp_path, monkeypatch): + _write_runtime_manifest(tmp_path, {"manifest_version": 2}) + monkeypatch.chdir(tmp_path) monkeypatch.setattr(sarif.feature_flags, "SANDBOX_BYPASS_DETECTION", True) table = sarif.sandbox_findings_table(_load(), "org/repo", "abc123", []) assert "`plugin/sandbox-bypass/ctypes-usage`" in table @@ -150,6 +160,67 @@ def test_capability_contract_requires_an_enforcing_manifest(tmp_path, monkeypatc assert sarif.classify([{"runs": [{"results": [result]}]}]).capability_contract == 1 +def test_sandbox_bypass_requires_an_enforcing_manifest(tmp_path, monkeypatch): + plugin_dir = tmp_path / "plugins" / "demo" + plugin_dir.mkdir(parents=True) + monkeypatch.chdir(tmp_path) + result = { + "ruleId": "plugin/sandbox-bypass/filesystem-api", + "locations": [{"physicalLocation": {"artifactLocation": { + "uri": "plugins/demo/plugin.py"}, "region": {"startLine": 1}}}], + } + + for manifest in ({}, {"manifest_version": 0}, {"manifest_version": 1}): + (plugin_dir / "plugin.json").write_text(json.dumps(manifest), encoding="utf-8") + assert not sarif.sandbox_result_requires_review(result) + + monkeypatch.setattr(sarif.feature_flags, "SANDBOX_BYPASS_DETECTION", True) + (plugin_dir / "plugin.json").write_text(json.dumps({"manifest_version": 0}), encoding="utf-8") + sarif_objects = [{"runs": [{"results": [result]}]}] + assert sarif.classify(sarif_objects).sandbox_bypass == 0 + assert "filesystem-api" not in sarif.sandbox_findings_table( + sarif_objects, "org/repo", "abc123", [] + ) + + (plugin_dir / "plugin.json").write_text( + json.dumps({"manifest_version": 2}), encoding="utf-8" + ) + assert sarif.sandbox_result_requires_review(result) + + +def test_sandbox_bypass_rules_respect_schema_ranges(tmp_path, monkeypatch): + plugin_dir = tmp_path / "plugins" / "demo" + plugin_dir.mkdir(parents=True) + monkeypatch.chdir(tmp_path) + v3_rule = "plugin/sandbox-bypass/v3-only" + retired_rule = "plugin/sandbox-bypass/retired-in-v3" + monkeypatch.setitem(sarif.capabilities.SANDBOX_BYPASS_RULES, v3_rule, { + "min_manifest_version": 3, + "max_manifest_version": None, + }) + monkeypatch.setitem(sarif.capabilities.SANDBOX_BYPASS_RULES, retired_rule, { + "min_manifest_version": 2, + "max_manifest_version": 2, + }) + result = lambda rule: { + "ruleId": rule, + "locations": [{"physicalLocation": {"artifactLocation": { + "uri": "plugins/demo/plugin.py"}, "region": {"startLine": 1}}}], + } + + for manifest_version in (0, 1, 2): + (plugin_dir / "plugin.json").write_text( + json.dumps({"manifest_version": manifest_version}), encoding="utf-8" + ) + assert not sarif.sandbox_result_requires_review(result(v3_rule)) + + (plugin_dir / "plugin.json").write_text( + json.dumps({"manifest_version": 3}), encoding="utf-8" + ) + assert sarif.sandbox_result_requires_review(result(v3_rule)) + assert not sarif.sandbox_result_requires_review(result(retired_rule)) + + def test_capability_contract_accepts_declared_or_legacy_manifests(tmp_path, monkeypatch): plugin_dir = tmp_path / "plugins" / "demo" plugin_dir.mkdir(parents=True) diff --git a/docs/plugin-security.md b/docs/plugin-security.md index 38f69bd3..d605a20e 100644 --- a/docs/plugin-security.md +++ b/docs/plugin-security.md @@ -39,7 +39,9 @@ instead reported as potential wrapper-bypass attempts for maintainer review. ## Sandbox Bypass Queries The pack separately detects concrete attempts to undermine the current -plugin-local builtins and import wrappers: +plugin-local builtins and import wrappers. These checks apply only to enforcing +runtime manifests (v2 and later), and each rule declares its supported manifest +version range: | Rule | Detection | |---|---|