Skip to content

verify_ruleset only checks that a ruleset exists, not that its rules match settings.yml #45

Description

@gavinbee

verify_ruleset() in scripts/apply-settings.py reports success on name presence alone:

def verify_ruleset(repo: str, ruleset_name: str) -> bool:
    existing = list_rulesets(repo)
    found = any(r["name"] == ruleset_name for r in existing)
    marker = "OK " if found else "FAIL"
    print(f"  {marker} ruleset '{ruleset_name}': {'present' if found else 'missing'}")
    return found

A ruleset named swimblocks-default with no rules at all reconciles as OK.

This is out of step with the same file's other two verifiers. verify() compares every key in the repository: block against the API's actual value and prints FAIL with both sides; verify_branch_protection() does the same, descending into nested dicts. Rulesets are the one thing applied but never checked.

Why it matters, given that apply_ruleset overwrites

apply_ruleset() sends the full desired ruleset by PUT on every run, so hand-edited drift is corrected whether or not it's detected. The gap is narrower than "drift goes unnoticed" — it is:

A payload the API does not fully honour is reported as OK. If a rule is rejected, silently dropped, or unsupported on the current plan, PUT still returns success, the ruleset still exists under that name, and the reconciler prints OK. Nothing tells you the rule isn't in force.

That bears directly on #43. If required_status_checks is dropped — wrong context string, plan limitation — the reconciler reports OK and we would believe CI is a required gate when it is not. #43's entire value is that certainty, and this is the check that would have to provide it.

Fix

Have verify_ruleset() re-read the ruleset by id and compare the rules it actually carries against settings.yml, printing actual vs expected on mismatch — the shape verify() already uses. At minimum: the set of rule types, and the parameters of each rule we specify.

Note GET /repos/{repo}/rulesets/{id} returns the rules; the list endpoint does not, so this needs the per-ruleset fetch list_rulesets() doesn't currently do.

Acceptance criteria

  • A ruleset whose rules differ from settings.yml reports FAIL with actual vs expected.
  • A rule present in settings.yml but absent from the API response reports FAIL.
  • Exit status reflects the failure, so the reconciler workflow goes red.

Out of scope

Refactoring the three verifiers onto a shared comparison helper. Worth doing; not worth entangling with #43.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions