Skip to content

Validate repo policy overrides and the safety config maps - #61

Merged
orda-aska merged 1 commit into
mainfrom
fix/validate-policy-and-safety-config
Oct 29, 2026
Merged

orda-aska merged 1 commit into
mainfrom
fix/validate-policy-and-safety-config

Conversation

@ahrazzle

@ahrazzle ahrazzle commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Found in a third audit pass. Each change has a test that fails without it.

Changes

ion/config.py (REPO_OVERRIDE_SCHEMA): a repo's .github/ion.yml policy block was accepted as any object. Per the README, anyone who can push to a repo can edit that file, and a malformed value took down the whole org:

  • policy: {required_files: 1} or {required_files: [1]} → TypeError/AttributeError in compute_analysis (policy.py:127-128). ion run prints analyze failed: ... and skips propose, sync and execute for every repo, every run, until someone finds the file. ion plan-config crashes the same way.
  • policy: {branch_protection: true} → AttributeError at policy.py:84 (bp.get on a bool).
  • policy: {branch_protection: {required_approvals: two}} with org branch_protection: fix → ValueError in the execute preflight (writes.py, int("two")), so no approved proposal runs.

The org policy and the repo override now share one schema. It types the keys ion reads: default_branch, branch_protection and its five fields, dependabot_alerts, secret_scanning, required_files. A malformed override is ignored and reported as bad_override, like any other invalid override. A falsy value (branch_protection: false, required_files: null) still turns a check off, as before; unknown keys are still ignored.

ion/config.py (SCHEMA): three safety-relevant maps silently kept the weaker default on a typo.

  • quorum_by_kind (in the README key table) was not in the schema at all. {archives: 2} or {Archive: 2} left archive quorum at quorum. {merge: 0} loaded, although quorum: 0 is refused, and then evaluate raised IndexError at core.py:70 for every awaiting merge, which failed sync-approvals and the execute preflight. Kinds are now limited to tracker.KINDS, with values ≥ 1.
  • writes.caps: {archive: 0} (for archive_repo) left the archive cap at 1. Keys are now limited to writes.TYPES.
  • guards: {max_archive: 0} left the breaker's archive limit at 2. Unknown keys are now refused.

A config with one of these typos now fails at load with invalid config: .... ion doctor reports it too.

Not in this PR: a repo override can still validly ask for weaker branch protection than the org (for example require_pr: false). With the org at branch_protection: fix, the breaker then refuses the lowering, and that aborts the whole --apply run. That is the same root cause as #64 (repos stricter than the policy are reported as drift).

Validation

uv run pytest -q: 261 passed (250 existing, 11 new). 9 of the new tests fail with the config.py change reverted. The other 2 are regression guards: the org config already refused branch_protection: true, and the falsy opt-out must keep working. scripts/check_licenses.py: 0 not allowed.

Automated posting by agentic team with human oversight.

🤖 Generated with Claude Code

https://claude.ai/code/session_016PpXZJdMN5FExq9THy5VNg

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016PpXZJdMN5FExq9THy5VNg

Copy link
Copy Markdown
Contributor Author

The ci workflow on this PR ends in startup_failure with no jobs started. That is not this PR: every run in the repo has failed the same way since 2026-10-18, including the pushes of #50 and #59 to main, and the workflow files have not changed since 2026-10-13. It looks like a repository or organisation Actions setting. Details are in #73. There is no code fix to port. Locally, the full suite passes (261) and scripts/check_licenses.py is clean. Once Actions runs again, a re-run of this PR's checks should pick it up.

Automated posting by agentic team with human oversight.


Generated by Claude Code

@ahrazzle

Copy link
Copy Markdown
Contributor Author

Independent review by a different agent than the drafter. Verdict: approve.

What I checked. The diff touches only ion/config.py and tests. The org policy schema and the per-repo override schema now share one _POLICY definition, so an override can no longer smuggle untyped policy keys past validation.

writes.caps keys are restricted to the known write types, quorum_by_kind keys to the known proposal kinds with a minimum of 1, and guards now rejects unknown keys. A falsy value still turns a check off, as before: this is type validation, not a behaviour change.

Suite on the branch: 261 passed, against 250 on main. The 11 new tests cover the new schema paths.

This PR and #62 both touch ion/config.py, in different regions. Whichever merges second may need a small rebase there.

This is posted as a comment because the reviewer's account is the same GitHub account that authored the PR, so GitHub will not accept a formal approving review from it. Review is requested from the other maintainer accounts. Under the repo rule (a PR merges once a different agent than its drafter has reviewed it), this review is the cross-agent pass.

Automated posting by agentic team with human oversight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants