From 08e510f761d74745cf4ee2d79296004cd46d2aca Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 19 Oct 2026 15:50:07 +0000 Subject: [PATCH] Validate repo policy overrides and the safety config maps A repo's .github/ion.yml `policy` block was accepted as any object, and anyone who can push to that repo can edit it. A value such as `required_files: 1` or `branch_protection: true` then crashed compute_analysis for the whole org on every run, and `required_approvals: two` crashed the execute preflight. The org and repo `policy` now share one schema that types the keys ion reads, so a malformed override is ignored and reported as `bad_override` like any other invalid override. A falsy value still turns a check off. `quorum_by_kind` was missing from the schema, and `writes.caps` and `guards` accepted any key. A misspelt kind, write type or guard silently kept the weaker default, and `quorum_by_kind: {merge: 0}` loaded and then raised IndexError in evaluate() for every awaiting proposal of that kind. These are now validated. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016PpXZJdMN5FExq9THy5VNg --- ion/config.py | 28 ++++++++++++++++++++-------- tests/test_misc.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/ion/config.py b/ion/config.py index 377565f..2b48ad1 100644 --- a/ion/config.py +++ b/ion/config.py @@ -9,7 +9,9 @@ import jsonschema import yaml +from .tracker import KINDS from .util import home +from .writes import TYPES as WRITE_TYPES DEFAULTS: dict = { "org": None, @@ -59,6 +61,17 @@ } _STR_LIST = {"type": "array", "items": {"type": "string"}} +# The org policy, and a repo's `policy` override (which anyone who can push to that repo can edit). Only the types of +# the keys ion reads are checked; a falsy value still turns a check off, as before. +_POLICY = {"type": "object", "properties": { + "default_branch": {"type": "string"}, + "branch_protection": {"anyOf": [{"enum": [False, None]}, {"type": "object", "properties": { + "require_pr": {"type": "boolean"}, "required_approvals": {"type": "integer", "minimum": 0}, + "allow_force_pushes": {"type": "boolean"}, "allow_deletions": {"type": "boolean"}, + "enforce_admins": {"type": "boolean"}}}]}, + "dependabot_alerts": {"type": ["boolean", "null"]}, + "secret_scanning": {"type": ["string", "boolean", "null"]}, + "required_files": {"type": ["array", "null"], "items": {"type": "string"}}}} SCHEMA = { "type": "object", "properties": { @@ -72,11 +85,7 @@ "backend": {"enum": ["claude_cli", "anthropic_api", "none"]}, "max_calls_per_run": {"type": "integer", "minimum": 0}}}, "budget": {"type": "object", "properties": {"graphql_points_per_run": {"type": "integer", "minimum": 1}}}, - "policy": {"type": "object", "properties": { - "branch_protection": {"type": "object", "properties": { - "require_pr": {"type": "boolean"}, "required_approvals": {"type": "integer", "minimum": 0}, - "allow_force_pushes": {"type": "boolean"}, "allow_deletions": {"type": "boolean"}, - "enforce_admins": {"type": "boolean"}}}}}, + "policy": _POLICY, "auto_apply": {"type": "array", "items": {"enum": ["branch_protection", "dependabot_alerts", "secret_scanning", "health_fix"]}}, "policy_actions": {"type": "object", "additionalProperties": {"enum": ["log", "issue", "fix"]}, @@ -90,14 +99,17 @@ "rules": {"type": "array", "items": {"type": "object", "required": ["from", "allow"], "properties": { "from": {"type": "string"}, "allow": _STR_LIST}}}, "scope_prefix": {"type": "string"}, "block_cross_scope_merge": {"type": "boolean"}}}, - "writes": {"type": "object", "properties": {"caps": {"type": "object", "additionalProperties": {"type": "integer", "minimum": 0}}}}, - "guards": {"type": "object", "properties": { + "writes": {"type": "object", "properties": {"caps": {"type": "object", "propertyNames": {"enum": sorted(WRITE_TYPES)}, + "additionalProperties": {"type": "integer", "minimum": 0}}}}, + "guards": {"type": "object", "additionalProperties": False, "properties": { "enabled": {"type": "boolean"}, "max_repo_fraction": {"type": "number", "minimum": 0, "maximum": 1}, "min_repos": {"type": "integer", "minimum": 1}, "max_archives": {"type": "integer", "minimum": 0}, "max_writes": {"type": "integer", "minimum": 1}, "forbid_protection_lowering": {"type": "boolean"}}}, "thresholds": {"type": "object", "additionalProperties": {"type": "number"}}, "approval_ttl_days": {"type": "integer", "minimum": 1}, "quorum": {"type": "integer", "minimum": 1}, + "quorum_by_kind": {"type": "object", "propertyNames": {"enum": KINDS}, + "additionalProperties": {"type": "integer", "minimum": 1}}, "archive_requires_second_approval": {"type": "boolean"}, "exclude": _STR_LIST, "git_identity": {"type": "object", "required": ["name", "email"]}, @@ -106,7 +118,7 @@ REPO_OVERRIDE_SCHEMA = { "type": "object", - "properties": {"maintainers": _STR_LIST, "ignore": {"type": "boolean"}, "policy": {"type": "object"}, + "properties": {"maintainers": _STR_LIST, "ignore": {"type": "boolean"}, "policy": _POLICY, "exclude_paths": _STR_LIST, "enabled": {"type": "boolean"}, "policy_actions": {"type": "object", "additionalProperties": {"enum": ["log", "issue", "fix"]}}}, "additionalProperties": False, diff --git a/tests/test_misc.py b/tests/test_misc.py index afa9ab9..25c5fdb 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -44,6 +44,41 @@ def test_invalid_config_rejected(): load_config(None, {"org": "o", "policy": {"branch_protection": {"require_pr": "yes"}}}) +@pytest.mark.parametrize("over", [ + {"quorum_by_kind": {"archives": 2}}, # misspelt kind: archive quorum would silently stay at `quorum` + {"quorum_by_kind": {"merge": 0}}, # `quorum: 0` is refused, so the per-kind spelling must be too + {"writes": {"caps": {"archive": 0}}}, # misspelt write type: the archive cap would silently stay at 1 + {"guards": {"max_archive": 0}}, # misspelt guard: the breaker's archive limit would stay at 2 + {"policy": {"required_files": "LICENSE"}}, + {"policy": {"branch_protection": True}}, +]) +def test_safety_config_typos_rejected(over): + with pytest.raises(ConfigError): + load_config(None, {"org": "o", **over}) + + +@pytest.mark.parametrize("text", ["policy: {required_files: 1}\n", "policy: {required_files: [1]}\n", + "policy: {branch_protection: true}\n", + "policy: {branch_protection: {required_approvals: two}}\n"]) +def test_malformed_repo_policy_is_reported_not_fatal(ctx, text): + """Anyone who can push to a repo can edit its .github/ion.yml. A malformed `policy` block there is reported for + that repo and ignored; it must not crash the analysis (or the preflight) for the whole org.""" + ctx.gh.pages = json.loads(json.dumps(ctx.gh.pages)) + for page in ctx.gh.pages: + for n in page["data"]["organization"]["repositories"]["nodes"]: + if n["name"] == "alpha-core": + n["ionYml"] = {"text": text} + run_collect(ctx) + a = run_analyze(ctx) + assert "bad_override" in [f["code"] for f in a["health"]["alpha-core"]] + assert "alpha-cli" in a["policy"] + + +def test_repo_policy_may_still_turn_a_check_off(): + from ion.config import parse_repo_override + assert parse_repo_override("policy: {branch_protection: false, required_files: null}\n")[1] is None + + def test_repo_override_can_lower_but_not_raise_policy_actions(): cfg = load_config(None, {"org": "o", "auto_apply": ["dependabot_alerts"], "policy_actions": {"branch_protection": "issue", "secret_scanning": "fix"}})