Skip to content

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

Open
parthrohit22 wants to merge 2 commits into
OWASP:devfrom
parthrohit22:infra/enforce-branch-protection
Open

parthrohit22 wants to merge 2 commits into
OWASP:devfrom
parthrohit22:infra/enforce-branch-protection

Conversation

@parthrohit22

Copy link
Copy Markdown
Collaborator

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

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

…WASP#298)

Declare dev and main protection as versioned GitHub rulesets and add a
scheduled audit that compares the rules GitHub actually enforces with them,
fails on drift, and retains a JSON evidence artifact.

- dev: 1 approval, main: 2; code owner review, last-push approval, stale
  review dismissal, conversation resolution, strict required checks
  (CI Summary, DCO sign-off, dependency-review, CodeQL), no bypass actors
- tests fail if a declared ruleset is weakened or names a check no job runs
- run CI and CodeQL on pushes to dev and main for post-merge assurance
- stop pushing statistics straight to protected dev; open a PR instead
- document admin apply steps, evidence, and the emergency procedure

Signed-off-by: parthrohit22 <parthrohit60@gmail.com>
Comment thread scripts/check_branch_protection.py Fixed
Semgrep flagged the dynamic urllib call. Requests now go to a fixed
api.github.com host over a verified default TLS context, and API error
statuses or connection failures are reported as drift.

Signed-off-by: parthrohit22 <parthrohit60@gmail.com>
Comment on lines +184 to +186
connection = http.client.HTTPSConnection( # nosemgrep: python.lang.security.audit.httpsconnection-detected.httpsconnection-detected # noqa: E501
API_HOST, timeout=30, context=ssl.create_default_context()
)
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