feat(baseline): ruleset-aware branch-protection verdict (closes #343) - #384
feat(baseline): ruleset-aware branch-protection verdict (closes #343)#384mlieberman85 wants to merge 1 commit into
Conversation
|
CI is red on Root cause: CNCF added Note the test is Feature-032 diff itself is green: 82 new + updated tests pass, other 2733 workspace tests pass. Happy to rebase after #385 lands or (if reviewers prefer) chain a small hash-only bump into this branch as a stopgap once we confirm the RepositoryEntry object form isn't in the wild yet. |
… don't false-FAIL (closes darnitdevorg#343) Feature 019 taught the sieve to treat a 404 from `/repos/{owner}/{repo}/branches/{branch}/protection` as a definitive FAIL for the four branch-protection controls (OSPS-AC-03.01, OSPS-AC-03.02, OSPS-QA-03.01, OSPS-QA-07.01). Repository Rulesets -- the newer protection mechanism -- also produce a 404 on that endpoint while genuinely protecting the branch, so the shipped fix produces false FAILs for repos protected via rulesets. Reported by @justaugustus as a follow-up to the 019 fix. Adds a new sieve handler `github_branch_protection` registered by `darnit-baseline` that reconciles the two surfaces: - Classic 200 with the required signal -> PASS from classic (rulesets not consulted; fast path). - Classic did not carry the signal (404 or 200 without the specific field) -> consult rulesets. If any active ruleset whose `conditions.ref_name` covers the audited branch carries a rule of the right type/parameter -> PASS from ruleset. - Both surfaces respond and neither protects -> FAIL. Locks the 019 invariant on the true-negative path. - Either surface returns 401/403/429/5xx or a mid-pagination fetch fails -> INCONCLUSIVE, which falls through the trailing manual pass to WARN. Preserves the "WARN means unknown" semantic on ambiguous responses. Two-surface layering (Q1 clarification): a repo whose classic protection requires PRs but delegates status-checks to a ruleset correctly PASSes OSPS-QA-03.01 via the ruleset without changing the other controls' verdicts. Adds a shared helper `gh_api_with_status` in `darnit.core.utils` that parses `HTTP <code>:` from `gh`'s stderr on non-zero exit so callers can distinguish 404 from 403 from 5xx. Existing `gh_api()` and `gh_api_safe()` are refactored as thin wrappers preserving their exact call contracts (~30 existing callers unchanged). Uses `gh api --paginate` for the rulesets list so repos with more rulesets than a single page get full enumeration; a mid-page fetch failure resolves the affected control to WARN with source `partial-fetch`, not a silent truncation. No new runtime dependencies. Zero touches under `packages/(darnit-gittuf|darnit-reproducibility|darnit-hello)/src/`. The default-branch value used for `~DEFAULT_BRANCH` include-list matching is consumed from `HandlerContext.default_branch` (populated by the audit driver) -- no extra `GET /repos/{owner}/{repo}` call is introduced, matching SC-004's exact API-call budget. Test coverage: - 48 handler unit tests (ruleset matching, PASS/FAIL/WARN paths per requirement, config validation). - 19 unit tests for `gh_api_with_status` (status parsing across 2xx/4xx/5xx, paginate flag, wrapper contracts). - 7 integration tests through the sieve orchestrator (one per TOML control PASS via ruleset, one FAIL when no protection, one exact API-call budget assertion, one WARN via manual fallback). - 8 pre-existing feature-019 tests updated to the two-surface semantics without weakening FAIL invariants. Full workspace: 2815 pass, 17 skip, 0 fail. Ruff clean. `validate_sync.py` PASS. Structure decision and FR-013 no-new-dep guards satisfied. Non-goals deferred to v0.1: - Organization-level inherited rulesets (repo-level check only). - Evaluate-mode rulesets (only `enforcement = "active"` counts). - Glob-pattern ref-name matching (globs treated as non-matching and surfaced in `considered_rulesets` for the operator). Spec: `specs/032-ruleset-branch-protection/` (63 tasks, all closed). Closes darnitdevorg#343.
abd000d to
0dbcc7f
Compare
Summary
Follow-up to feature 019, addresses @justaugustus's comment on #343.
Feature 019 taught the sieve to treat a 404 from
/repos/{owner}/{repo}/branches/{branch}/protectionas a definitive FAIL for the four branch-protection controls (OSPS-AC-03.01, OSPS-AC-03.02, OSPS-QA-03.01, OSPS-QA-07.01). GitHub Repository Rulesets -- the newer protection mechanism -- also produce a 404 on the classic endpoint while genuinely protecting the branch, so the shipped fix produces false FAILs for repos protected via rulesets.This PR introduces a new sieve handler
github_branch_protection(registered bydarnit-baseline) that reconciles the two surfaces:conditions.ref_namecovers the audited branch and carries the right rule type/parameter -> PASS from ruleset. Preserves cross-surface layering.A shared helper
gh_api_with_statusis added todarnit.core.utilsso callers can distinguish 404 from 403 from 5xx by parsing theHTTP <code>:prefix fromgh's stderr. Existinggh_api()andgh_api_safe()become thin wrappers with unchanged call contracts.Uses
gh api --paginatefor the rulesets list; a mid-page fetch failure resolves to WARN with sourcepartial-fetch, not a silent truncation. Default-branch value used for~DEFAULT_BRANCHinclude-list matching is consumed fromHandlerContext.default_branch-- no extraGET /repos/{owner}/{repo}call is introduced (SC-004 API-call budget invariant, locked by a dedicated integration test).Zero new runtime dependencies. Zero product-source changes under
packages/(darnit-gittuf|darnit-reproducibility|darnit-hello)/src/.Spec:
specs/032-ruleset-branch-protection/(63 tasks, all closed; three clarifications recorded during/speckit-clarify).Closes #343.
Non-goals deferred to v0.1
enforcement = \"active\"counts as protection).considered_rulesets).Test plan
pytest tests/darnit_baseline/test_branch_protection_handler.py-- 48 pass (ruleset matching, PASS/FAIL/WARN per requirement, config validation).pytest tests/darnit_baseline/test_branch_protection_integration.py-- 7 pass (one per TOML control PASS via ruleset, one FAIL when no protection, one exact API-call budget assertion locking SC-004, one WARN via manual fallback).pytest tests/darnit_baseline/controls/test_branch_protection.py-- 8 pre-existing feature-019 tests updated to the two-surface semantics; FAIL invariant preserved for the true-negative path.pytest tests/darnit/core/test_gh_api_status.py-- 19 pass (status parsing across 2xx/4xx/5xx, paginate flag, wrapper contracts, gh-not-found).pytest tests/ --deselect .../test_upstream_spec_unchanged-- 2815 pass, 17 skip, 0 fail.ruff checkon all feature-touched files -- clean.python scripts/validate_sync.py --verbose-- PASS.src/.pyproject.tomldiffs.specs/032-ruleset-branch-protection/quickstart.mddocuments the recipe.Backward compatibility: additive. The
github_branch_protectionhandler is new; the four TOML controls' passes swap fromexecto the new handler while keeping their trailing manual passes unchanged. Every non-branch-protection control's verdict is untouched. Thegh_api/gh_api_safepublic contracts are preserved.