feat(workflows): package fail-closed registry audit - #211
Conversation
Move the read-only workflow-registry auditor into pg_llm_batch so install, coverage, docstring, and compile gates see the production module. Type-check ref and SHA identity members before equality so a hostile str subclass cannot certify the caller commit while resolving another tree. Add the pg-llm-batch-workflow-audit console script, operator/ADR/doctoring/CHANGELOG contract, and realistic fail-closed regressions. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Blocking evidence-integrity/boundedness defect on exact head 4b1ca74bff9a996ffec5d674c1a36216a54bfb93: _validate_captured_at() only requires captured_at to be an exact built-in str, and both public audit functions then copy that value verbatim into the machine receipt. Consequently captured_at="not-a-time", an empty string, or an arbitrarily large string is accepted before GitHub reads and emitted as the audit timestamp. That contradicts the module's own receipt contract and bounded-evidence goal; an operator/library caller can produce syntactically invalid or unbounded audit evidence while the receipt otherwise looks successful.
Please repair test-first without weakening the read-only design: add RED regressions for invalid/non-UTC/non-canonical and oversized captured_at values; then either remove caller timestamp injection from the public evidence API or validate it to the same finite canonical UTC RFC 3339 shape generated by _utc_timestamp() (including real calendar validity) before any GitHub access. Keep deterministic testing through a private/internal clock seam if needed. Prove GREEN plus existing exact-type, output-bound, CLI, full coverage/docstring, and Python 3.14 gates.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Additional integration blocker on this exact head: this PR adds docs/adr/0016-workflow-registry-audit.md, while concurrently active Ready PR #212 at exact head 63a89c7b39d5dad87c7186bdbeaae2c2043f1b66 independently adds docs/adr/0016-postgres-logical-restore-seek.md. Protected main@d2f1e32271910a6db98a0757d67194ddadca4566 currently tops out at ADR 0015, so these two branches have allocated the same ADR number to different decisions. They would not file-conflict because the slugs differ, which makes the duplicate identifier especially easy to ship silently and corrupt ADR references/index semantics.
Before this lane is Ready, allocate a collision-free ADR identifier after a fresh inventory of protected main plus all active/no-PR writer branches, and update every reference/test/index assertion in this branch consistently. Do not simply assume the next integer: active recovery branches already advertise 0017/0018 on their own non-main heads. Add a repository regression that rejects duplicate ADR numeric prefixes across the integrated tree so future parallel branches fail deterministically during CI.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
What to do with #211
Do not merge this head (0e3fa51). The captured_at gate is in place: empty, non-UTC, non-canonical, invalid-calendar, and oversized timestamps fail before any GitHub read. The remaining merge blocker is ADR numbering.
This branch still adds docs/adr/0016-workflow-registry-audit.md. Open #212 already adds docs/adr/0016-postgres-logical-restore-seek.md. The slugs differ, so git will not conflict, and the integrated tree would carry two different decisions as ADR 0016.
Live ADR inventory (protected main + open writers)
- main tops out at 0015
- #212: 0016 logical restore seek
- #215: 0017 recovery evidence binding
- #216: 0018 restore catalog acceptance
- #219: 0019 physical PITR profile
- #221: 0020 recovery receipt verification
Next free number is 0021.
Landing vehicle
#222 keeps this packaged detector, moves the record to ADR 0021, cites RFC 3339 for receipt timestamps, and adds tests/test_adr_numeric_prefixes.py so a later merge of two same-number ADRs fails CI.
Do not merge #199 at cc4362a. That head is still unpackaged and still compares identity members before an exact-type check.
Sent by Cursor Automation: Fix Issues
| @@ -0,0 +1,84 @@ | |||
| # ADR 0016: Read-only exact-SHA workflow registry audit | |||
There was a problem hiding this comment.
2 participants
What to do with this PR
Install
pg-llm-batchand runpg-llm-batch-workflow-auditagainst an exact protected SHA. Reviewactive_absent_workflowsyourself. Do not disable workflows from this tool. Do not merge #199 atcc4362a— that head still compares identity members before an exact-type check and is not installable.Why this exists
#199 added a read-only GitHub Actions registry detector. A buyer still could not install it, the 100% coverage gate never saw it, and a hostile
strsubclass could certify the caller commit SHA while the auditor resolved a different tree. This PR keeps the detector read-only, moves it intopg_llm_batch, and closes that identity-member hole.Operator contract
0: protected ref stayed on the supplied SHA; no repository-backed orphan candidates.2: JSON receipt withactive_absent_workflowsfor a separate review.1: fail-closed audit error. Fix the selector, SHA, token scope, rate limit, or truncated tree and rerun.dynamic/identities are receipted and never orphans.See
docs/doctoring/workflow-registry-audit.mdand ADR 0016.Verification
pytest tests/test_workflow_registry_audit*.py— 73 passed--cov=pg_llm_batch.workflow_registry_audit --cov-fail-under=100— 100%interrogate --fail-under 100 pg_llm_batch— 100%ruff checkon the new module and audit tests — cleanRefs #158. Completes the #199 product gap without mutating Actions state.