ci: declare branch protection as rulesets and audit effective drift (#298) - #344
parthrohit22 wants to merge 2 commits into
Conversation
…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>
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>
| connection = http.client.HTTPSConnection( # nosemgrep: python.lang.security.audit.httpsconnection-detected.httpsconnection-detected # noqa: E501 | ||
| API_HOST, timeout=30, context=ssl.create_default_context() | ||
| ) |
m-khan-97
left a comment
There was a problem hiding this comment.
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.
What does this PR do?
Declares
devandmainbranch 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
Changes
Declared state:
.github/branch-protection/{dev,main}.json, in GitHub's ruleset import formatCI Summary,DCO sign-off,dependency-review,Analyze (python),Analyze (javascript); no standing bypass actors.devrequires 1 approval andmainrequires 2. The existingEnforce dev to main sourcejob already runs insideCI Summary.Effective state:
scripts/check_branch_protection.pyplus.github/workflows/branch-protection-audit.ymlGET /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.branch-protection-evidenceJSON artifact (kept 90 days).Post-merge assurance
ci.ymlandcodeql.ymlnow also run onpushtodevandmain. 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
devupdate-learn-page.ymlhas been failing withGH006: Protected branch update failed. It now force-updatesdocs/refresh-learn-page-statsand opens or refreshes a PR through the normal protected flow. It uses theSTATS_BOT_TOKENsecret if one is set, because PRs opened withGITHUB_TOKENdon't start workflows.Docs:
docs/ci-pipeline.mdnow 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/openshieldtoday: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)
gh api -X POST repos/OWASP/openshield/rulesets --input .github/branch-protection/dev.json(repeat withmain.json), then retire the classic protection.BRANCH_PROTECTION_AUDIT_TOKENsecret (ruleset read-admin) so the audit can verify bypass actors.STATS_BOT_TOKENsecret and allow GitHub Actions to create PRs.Testing
tests/test_check_branch_protection.py: 15 tests. They cover:mainrequires 2 approvalstest_devops_client.pyare local-only (azure-devopsisn't installed on my machine) and also fail on unmodifieddev.ruff check .andruff format --check .are clean, and all workflow YAML parses.OWASP/openshield(output above).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
Signed-off-bytrailerinfra/description