ci: declare branch protection as rulesets and audit effective drift (#298)

[parthrohit22]

What does this PR do?

Declares dev and main branch protection as versioned GitHub rulesets and adds an automated audit that fails when the protection GitHub actually enforces is weaker than declared, so #298's controls are demonstrated rather than only documented.

Type of change

• New scan rule
• Remediation playbook
• Bug fix
• Dashboard/front-end work
• API endpoint
• Documentation
• Compliance mapping
• CI / infrastructure

Changes

Declared state: .github/branch-protection/{dev,main}.json, in GitHub's ruleset import format

• Both branches: branch deletion and force pushes blocked; a PR is required with code owner review, approval of the last push (the author of the last push cannot satisfy the rule), stale approvals dismissed, and conversations resolved; required checks are strict: CI Summary, DCO sign-off, dependency-review, Analyze (python), Analyze (javascript); no standing bypass actors.
• dev requires 1 approval and main requires 2. The existing Enforce dev to main source job already runs inside CI Summary.

Effective state: scripts/check_branch_protection.py plus .github/workflows/branch-protection-audit.yml

• Reads GET /repos/{repo}/rules/branches/{branch} and each applied ruleset, then reports missing rules, fewer approvals than declared, missing flags, missing or non-strict checks, and standing bypass actors.
• Runs weekly and on demand, and uploads a branch-protection-evidence JSON artifact (kept 90 days).
• Fails closed: an API error or unreachable API counts as drift. Bypass actors hidden from a read-only token are reported as unverified, never as compliant. Classic branch protection shows up in the output as a note but doesn't count as proof of enforcement.
• On PRs that touch the rulesets, the workflow only validates the declared files, offline.

Post-merge assurance

• ci.yml and codeql.yml now also run on push to dev and main. Post-merge runs are never cancelled (concurrency is keyed by SHA for pushes). This is compatible with the Mergify queue proposed in infra: add Mergify serial merge queue for dev branch #335.

Stats bot no longer pushes to protected dev

• update-learn-page.yml has been failing with GH006: Protected branch update failed. It now force-updates docs/refresh-learn-page-stats and opens or refreshes a PR through the normal protected flow. It uses the STATS_BOT_TOKEN secret if one is set, because PRs opened with GITHUB_TOKEN don't start workflows.

Docs: docs/ci-pipeline.md now separates declared state from effective state, and covers admin apply commands, how to verify each acceptance criterion, and an emergency procedure with no standing bypass.

Current effective state (evidence)

Running the audit against OWASP/openshield today:

[DRIFT] OWASP/openshield@dev
  - dev: 'deletion' rule is not enforced
  - dev: 'non_fast_forward' rule is not enforced
  - dev: 'pull_request' rule is not enforced
  - dev: 'required_status_checks' rule is not enforced
  ! dev: classic branch protection is enabled (status-check enforcement: off); ...
[DRIFT] OWASP/openshield@main
  (same four rules not enforced, plus the same classic-protection note)

This matches the #298 finding: no required check is enforced on either branch.

Admin actions needed after merge (I can't do these: my role is write, not admin)

1. gh api -X POST repos/OWASP/openshield/rulesets --input .github/branch-protection/dev.json (repeat with main.json), then retire the classic protection.
2. Optional: add a BRANCH_PROTECTION_AUDIT_TOKEN secret (ruleset read-admin) so the audit can verify bypass actors.
3. Add a STATS_BOT_TOKEN secret and allow GitHub Actions to create PRs.
4. Run Branch Protection Audit and confirm it's green, then run the three acceptance drills listed in the docs.

Testing

• tests/test_check_branch_protection.py: 15 tests. They cover:
  • the committed rulesets validate, and main requires 2 approvals
  • every required check name matches a real workflow job (a typo would otherwise block every merge)
  • validation rejects a weakened ruleset
  • no enforced rules, a missing or non-strict check, a self-approvable PR and a standing bypass are each reported as drift
  • hidden bypass actors are reported as unverified
  • an evidence record is written, and an unreachable API fails closed
• Full backend suite: 1058 passed. The 2 failures in test_devops_client.py are local-only (azure-devops isn't installed on my machine) and also fail on unmodified dev.
• ruff check . and ruff format --check . are clean, and all workflow YAML parses.
• Ran the audit live against OWASP/openshield (output above).
• No hardcoded credentials or secrets.

Related issue

Partially addresses #298. The in-repo work is complete, but the issue should stay open until an administrator applies the rulesets and the audit and acceptance drills pass.

Checklist

• Every commit includes a DCO Signed-off-by trailer
• Branch name follows the convention: infra/description
• I have not committed any real credentials

[m-khan-97]

I reviewed the current head end to end and verified the declared rules, audit behavior, workflow integration, and tests. The implementation is careful and the focused suite passes locally: 16 tests, plus validation of both committed rulesets. The required check names also match the checks currently emitted by the repository.

Approved from my side. One important operational boundary remains: this PR declares and audits the policy, but it does not make the live repository compliant by itself. The live rules API still reports no effective rulesets. Please keep #298 open until an organization administrator applies both rulesets, runs the audit successfully with bypass visibility, and completes the documented failure, stale-head, and self-approval drills.

[ritiksah141]

Approving. Reviewed end to end: read the full diff, checked the branch out locally, re-ran the suite, and reproduced the live audit against OWASP/openshield independently.

Local verification

• Full backend suite: 1064 passed, 6 skipped.
• tests/test_check_branch_protection.py: 16 passed (the description says 15; the file collects 16 with parametrization).
• ruff check and ruff format --check on the new script and tests: clean. All four changed workflow YAMLs and both ruleset JSONs parse.
• python scripts/check_branch_protection.py --validate-only: "Declared rulesets are valid for: dev, main".
• Ran the live audit with a read-only token: it reproduces the evidence in the description exactly (DRIFT on both branches with all four rule types unenforced, the classic-protection note with status-check enforcement off, exit 1), and GET /repos/OWASP/openshield/rulesets confirms no rulesets are applied yet. The evidence JSON record is well-formed.
• Push-trigger compatibility: enforce-source-branch no-ops on push events (github.base_ref is empty, the step skips, the job succeeds), and the CI Summary fail gate sees success from every job on push runs, so post-merge runs report green correctly. The per-SHA concurrency key means push runs never cancel each other.
• Action pins (checkout, setup-python, upload-artifact) match the SHAs used elsewhere in the repo.

Design notes

• test_required_checks_match_real_workflow_job_names is the right guard against the classic ruleset footgun: it parses every workflow (with CodeQL matrix expansion), runs inside pytest tests/ on every PR, so renaming a required job anywhere breaks CI immediately instead of blocking merges forever.
• Fail-closed layering is solid: declared-file validation gates the live comparison, API errors and unreachable API count as drift, and MIN_APPROVALS is hardcoded in the script so weakening main below 2 requires a second reviewed edit, not just a JSON tweak.
• PR mode is validate-only and offline, so fork PRs need no secrets; the fixed-host TLS client with path validation is careful.
• Docs are honest about the current state ("declared, not enforced") and the emergency procedure avoids standing bypass. Good.

Findings (non-blocking; #1 deserves a response or follow-up)

1. Bypass-actor visibility gap. GitHub's docs confirm bypass_actors is filtered from GET /rulesets/{id} responses for tokens without write access (the request does not 403). So with the default github.token, a standing bypass added later through the UI produces only an "unverified" note: the branch record and evidence JSON still say compliant: true and the weekly run stays green. The description's "never as compliant" overstates what the evidence shows. Minimal fixes: add a bypass_actors_verified boolean per branch to the evidence record, and/or treat unverified visibility as drift on scheduled runs. At minimum, the BRANCH_PROTECTION_AUDIT_TOKEN secret probably should not be labelled "Optional" in the admin actions, since without it the bypass half of the audit is decorative.
2. github_fetcher treats 3xx as success (only status >= 400 raises). A 301 (moved repo) returns a JSON message object, and downstream code then raises AttributeError with a traceback. Still non-zero exit, but confusing; if not 200 <= response.status < 300: raise GitHubApiError(response.status) is a one-line fix.
3. Ruleset details are always fetched via /repos/{repo}/rulesets/{id}. If an org-level ruleset ever applies to these branches (ruleset_source_type: Organization), that fetch 404s and the whole branch reports non-compliant. Fail-closed but misleading; routing by ruleset_source_type would future-proof it.
4. Admin action sequencing: applying the rulesets (action 1) before STATS_BOT_TOKEN exists (action 3) means the stats bot's PRs can never merge, because PRs opened with GITHUB_TOKEN start no workflows and the required checks never report. Suggest doing 3 before or together with 1.
5. Coordination note for #335: dismiss_stale_reviews_on_push plus strict checks means every "Update branch" or merge-queue rebase dismisses approvals, and require_last_push_approval bars the last pusher from re-approving. Mergify will wait for conditions to be re-met rather than deadlocking, but maintainer cadence changes noticeably on both branches once this is active. Worth verifying queue behavior when #335 lands.
6. Trivial: merging a stats PR re-triggers update-learn-page on dev, and if the stats drift between runs it immediately opens the next PR. Bounded by the changed-check, just something to expect.

Strong infrastructure PR: the declared-state vs effective-state split is exactly the right model for #298, the tests guard the guards, and the live audit confirms the evidence rather than trusting documentation.

[TFT444]

@parthrohit22 PR is approved and all CI checks are green, but there is a merge conflict with dev. Please rebase onto current dev and re-push to unblock the merge.

[parthrohit22]

@TFT444 conflict with dev is resolved — the branch now merges cleanly.

Merged current upstream/dev in rather than rebasing, matching how this branch has taken dev before. The only conflict was in CHANGELOG.md (additive): kept both the two #298 branch-protection entries and dev's AKS/OIDC entries. .github/workflows/ci.yml auto-merged.

Re-verified on the merged head, since dev added new CI jobs since the last run:

• pytest tests/test_check_branch_protection.py — 16 passed. test_required_checks_match_real_workflow_job_names is the one that matters here: the required check names in both rulesets still match the job names every workflow actually emits after dev's changes, so no drift was introduced by the merge.
• python scripts/check_branch_protection.py --validate-only — "Declared rulesets are valid for: dev, main".
• ruff check / ruff format --check on the script and tests — clean; all workflow YAMLs parse.

Nothing in the declared rulesets or the audit logic changed. The operational boundary @m-khan-97 raised still stands: this declares and audits the policy, it does not make the live repository compliant, so #298 should stay open until an administrator applies both rulesets and completes the drills.

@m-khan-97 @ritiksah141 your approvals were dismissed by the push, sorry for the churn — re-requesting.