feat(publication): enforce research safety scan in required Gate - #569
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds fail-closed scanning, staged redaction, allowlist validation, tests, and CI enforcement for publication artifacts. The Gate now requires a successful publication guard result. ChangesPublication safety enforcement
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant publication_guard
participant check_publication_safety
participant summary
PullRequest->>publication_guard: trigger publication guard
publication_guard->>check_publication_safety: run scanner
check_publication_safety-->>publication_guard: return scan status
publication_guard-->>summary: provide publication guard result
summary->>summary: fail Gate when result is not successful
Merge Risk: 🟡 Moderate · up to Malformed JSON containing NaN or Infinity can be retained in prepared publication output. Validate structured files before the clean-file shortcut before merging. 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Full details: Linked Issues checkExplanation Issue Full details: Out of Scope Changes checkExplanation Issue ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Workflow state fingerprint for Keepalive Loop Reporter. Do not edit. |
🤖 Keepalive Loop StatusPR #569 | Agent: Codex | Iteration 12+8 🚀 extended Current State
🔍 Failure Classification| Error type | infrastructure |
|
Keepalive Work Log (click to expand)
|
|
Workflow state fingerprint for Agents Gate Followups. Do not edit. |
|
Runner dispatch state for codex on PR #569. Do not edit. |
|
Autofix attempts exhausted for this head. Latest Gate summary: Please investigate manually. |
|
Autofix attempts exhausted for this head. Latest Gate summary: Please investigate manually. |
|
Autofix attempts exhausted for this head. Latest Gate summary: Please investigate manually. |
There was a problem hiding this comment.
🟡 Changes recommended
Critical workflow and scanner issues, plus allowlist persistence concerns, remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a dependency-free publication safety scanner with strict allowlisting and required Gate integration.
Changes:
- Scans publication artifacts for disclosure patterns.
- Adds fail-closed diagnostics and subprocess coverage.
- Integrates standalone and required CI workflows.
File summaries
| File | Summary | Review notes |
|---|---|---|
tests/test_publication_safety.py |
Scanner and Gate tests | No additional findings. |
scripts/check_publication_safety.py |
Scanner implementation | Critical (1 vote): Reject .publication-allow as an allowlist target and add regression coverage. |
research-program/.publication-allow |
Allowlist template | Moderate (1 vote): The machine-owned mirror may overwrite this policy file. |
README.md |
Usage and policy documentation | Moderate (1 vote): Documented exceptions may not persist in the machine-owned mirror. |
.github/workflows/publication-guard.yml |
Standalone and reusable guard workflow | Critical (2 votes): Move workflow permissions to the job level so reusable invocation can start. |
.github/workflows/pr-00-gate.yml |
Required Gate integration | No additional findings. |
Review details
Suppressed comments (2)
README.md:43
- The documented exception file is inside
research-program/, butREADME.md:21says this entire tree is a deletion-synced machine-owned mirror whose added or edited files are silently removed or overwritten on the next tick. A future exception added here therefore will not persist, and the new README simultaneously tells maintainers to edit a file the engine owns. Preserve.publication-allowin the mirror's source/sync contract or store the policy outside the generated tree before relying on these exceptions.
For an intentional example, add an exact path relative to `research-program/`
to `.publication-allow`, in the form `example.md:credential # Synthetic example
reviewed for publication.` Each entry needs a reason and one of `home-path`,
`credential`, `private-key`, `scratch-path`, or `internal-host`. Wildcards and
paths outside the research tree are rejected. An exception covers only that
file and rule; it does not suppress other rules or files. No findings are
automatically redacted or allowlisted. Correct the source that produces an
research-program/.publication-allow:1
README.md:21states that everything underresearch-program/is a deletion-enabled machine-owned mirror and that changes there are overwritten on the next tick. Keeping the exception file in that tree means a future justified exception can disappear during synchronization, changing guard behavior without a source change; put this file in the engine/source of truth or add an explicit sync-preserved exception before documenting edits here.
# Exact paths relative to research-program, followed by a rule and a reason:
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d7be9f6 to
5b41c28
Compare
|
Closer review-fix push
Removed stale Remaining blocker (not a human gate): |
Closer lane — fifth base refresh (2026-09-14T10:43:44Z)Source: #554 | Branch: What changed this round
Current state (post-refresh)
Review floorConservative 7-minute floor: 2026-09-14T10:50:44Z (from commit Next closer action (after floor + green CI)
Automation-owned scheduling contention from frequent main status writes; no human decision requested. |
|
Opener drain refresh: merged current main into this existing branch using GitHub update-branch guarded by the expected previous head. Incoming compared change was research-program/STATUS.md only. Current head: cc4a8b2; incorporated base: 901121a. Fresh CI is pending. Conservative review floor: 2026-09-14T11:16:17.237460+00:00; any later push resets the window. Closer owns fresh exact-head/current-base/required-and-expected-check/all-review-thread validation, then merge and verify:compare/source-issue disposition. No implementation completion or fresh local test result is claimed by this status-only refresh. |
…hanging prepare_copy() called read_bytes() on every walked filename without checking it is a regular file. read_bytes() on a FIFO blocks forever waiting for a writer, so a staging tree containing one hangs preparation rather than being refused by it. check_publication_safety.py already rejects non-regular files. Preparation disagreeing with the scanner means the stricter of the two is the one that never gets to run, which is the worst way for a safety pair to differ. Deliberate-break gate: removing the is_file() check makes test_preparation_rejects_a_named_pipe_instead_of_hanging hang until killed (verified at a 60s timeout); restoring it passes in 0.11s. Found by adversarial review of this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Closer recovery for source #554 at exact head
Fresh validation: the current implementation snapshot's publication tests report 169 passed ( Current head is OPEN, non-draft, CLEAN/MERGEABLE, with zero active non-outdated review threads (complete pagination), 53 reported contexts successful or intentionally skipped, and 50 check runs with no failures. Both publication-safety jobs, Python 3.12/3.13, Ruff, mypy, gate-summary and required The seven-minute floor is 2026-09-14T13:34:35Z, using the observed cleanup-push time conservatively. No merge or auto-merge is armed. At or after that floor, the closer must re-read the unchanged exact head, current base, complete required/expected checks and suites, and review threads before merging. Any further push restarts the floor. If merged, apply |
Closer checkpoint — base refresh (2026-09-14T13:53Z)Action:
Gates pending on new head: Gate, publication-safety, Python 3.12/3.13, Ruff, mypy, CodeRabbit. Review floor: 2026-09-14T14:00:05Z (7 min from merge commit Next (closer): After floor + all required/expected checks green + 0 active non-outdated threads → guarded merge → Implementation unchanged from prior green run; this refresh is status-only base incorporation. |
|
Opener cap-drain: main advanced again after the clean audit; expected-head guarded refresh incorporated only research-program/STATUS.md. New head 39185cb; state OPEN, draft=False. Fresh CI/review floor required: 2026-09-14T14:20:40.744318+00:00. No repeated refresh loop this round. Closer owns exact head/current base/all checks and zero active review threads before merge and verify:compare/source554 disposition. |
|
Closer recovery for source #554: updated the existing branch with main through an expected-head guarded GitHub update. The incoming merge-base-relative diff contains only Previous head: The prior head had zero active non-outdated review threads and successful Gate, both publication-safety jobs, Python 3.12/3.13, Ruff and mypy. These results do not establish the new head's CI result. Retained acceptance and mutation evidence: #569 (comment) and #569 (comment). Conservative review floor: 2026-09-14T14:32:44.010443+00:00 from observation of the new head. Any further push restarts it. Do not merge or arm auto-merge before that floor. Next: unchanged exact head, current strict base, complete required and expected checks/suites, and zero active review threads, then guarded merge and The recurring blocker is scheduled status commits advancing main before strict-base readiness and the seven-minute review floor overlap. This is automation-owned scheduling contention. No branch-protection bypass or human approval is requested. Source scheduling recovery should arrange a stable main interval long enough for CI plus the review floor without weakening either merge gate. |
Closer checkpoint — 2026-09-14T14:49:27ZHead: Gates observed on new head: Gate/gate-summary, both publication-safety guards, Python 3.12/3.13, Ruff, mypy, CodeRabbit all PASS; 0 active non-outdated review threads. Review floor: Next closer action after floor: unchanged exact head + current strict base + complete required/expected checks/suites + zero active threads → guarded merge → Repeated scheduled STATUS-only main commits continue to starve strict-base readiness before the review floor can elapse. Automation-owned scheduling should allow a stable main interval long enough for CI plus seven minutes without weakening either gate. |
|
Closer recovery for source #554: repaired the local publication writer that repeatedly advanced main during this PR's strict-base review window. The existing writer deferred mirror publication for sync reviews only. It now supports an explicit target-scoped closer lease, limited to two hours, with automatic expiry and early clearance. It does not pause research queue execution or alter branch protection. Invalid, future, expired, cross-repository, and overlong leases do not hold publication. Local source: Guarded branch update: Conservative review floor: 2026-09-14T15:19:51.633213+00:00. Any subsequent head change restarts the floor. Snapshot: OPEN, non-draft, MERGEABLE; zero active non-outdated threads (complete page); Python 3.12 still in progress, Python 3.13/Ruff/mypy/both publication guards successful. This snapshot is not merge authority. Branch protection remains strict and requires Next: after the floor, confirm unchanged exact head/current base, every expected check/suite and required Gate reported successfully, and zero active review threads immediately before a guarded merge. Then apply |
|
Closer exact-head merge evidence for source #554 at The conservative seven-minute review floor (2026-09-14T15:19:51.633213Z) has elapsed. The head remains unchanged and includes current main The prescribed top-level absent-check reporter is missing, so it was not counted as PASS. A direct Ready-specific completeness audit fetched every workflow file at this exact head, enumerated all runs/checks/suites, and compared the previous exact head's complete context list: 53 current contexts, zero missing versus the prior head. All nine applicable synchronize-event workflows reported. All nine GitHub Actions suites completed successfully or with intentional skips. Gate/gate-summary, both publication-safety jobs, Python 3.12/3.13, Ruff, mypy, conformance and CodeRabbit status are successful. The 50 check runs contain no failure or pending job. Auto-pilot and verifier are label/closed-event workflows, not missing synchronize checks; Dependabot Auto-Lock does not apply to this branch. Seven third-party app suites have zero jobs and remain queued; these placeholders are not represented as completed reviews, and the actual CodeRabbit commit status reports SUCCESS. Merge still requires an immediate unchanged-head, clean/up-to-date base, complete checks and zero active non-outdated review-thread requery. After guarded squash merge: apply verify:compare, clear the temporary local publication hold, and leave source #554 open until its durable verifier report is dispositioned. |
Provider Comparison ReportProvider Summary
📋 Full Provider Details (click to expand)openai
anthropic
Agreement
Disagreement
Unique Insights
🔍 LangSmith Traces |
|
Workflow state fingerprint for Agents Verifier. Do not edit. |
|
Closer audited the durable provider comparison against merged commit The allowlist-path concern is actionable: without The exporter concern is dispositioned from the source issue's later verification-follow-up section: it explicitly requires shared PEM patterns, exporter tests and an accurate integration claim. Current README says preparation is an explicit operator step and CI runs only the checker; no CI auto-redaction claim is made. Existing tests cover the full PEM family, read preservation and nonzero examined-file counts. No exporter removal or workflow rewrite is required. Validation for #574: 193 full-suite tests pass, 176 focused scanner/exporter tests pass. Removing This records a substantive follow-up, not a dual-provider PASS. Keep #554 open. Next closer: after the seven-minute floor on #574's unchanged head, audit all required/expected checks and active review threads, merge only with zero active non-outdated threads, apply |
Closes #554
Automated Status Summary
Scope
research-program/artifacts/work-bundle/is prepared to be handed to colleagues in the owner's work environment, and the rest ofresearch-program/is published to a public repository. Two disclosure-class defects have already reached this tree, both invisible on a casual read:clones/<Repo>/...), which made twenty-two issues unactionable across the fleet and had to be repaired one by one.Both were found by a person looking. Neither would have been caught by any check in this repository, because there is none. The bundle's own redaction pass is a one-time human-reviewed step, not a gate, so nothing prevents the next artifact from reintroducing what the last one removed.
Evidence:
research-program/artifacts/work-bundle/REDACTIONS.mdrecords the manual redaction pass and its scope reduction;README.mdstates the disclosure standard this issue enforces;research-program/CHECKPOINT.mdrecords the repair of the path-citation defect.Tasks
scripts/check_publication_safety.pyscanning every file underresearch-program/for: an absolute home path (/Users/), a credential prefix (sk-ant-,sk-proj-,ghp_,github_pat_,lsv2_,crsr_,AIza), a private-key header (BEGIN RSA PRIVATE KEY,BEGIN OPENSSH PRIVATE KEY), a scratchpad or drafting-agent path (clones/,/private/tmp/,scratchpad/), and an internal hostname pattern (.local:or a barelocalhost:port reference).research-program/.publication-allow, holding onepath:rulepair per line with a required trailing comment giving the reason, so a deliberate exception is visible and justified rather than a silent skip..github/workflows/publication-guard.ymlrunning the script on pull requests and on pushes to the default branch.tests/test_publication_safety.pywith fixtures for each rule, an allowlisted exception, and a clean tree.Acceptance criteria
python scripts/check_publication_safety.pyexits non-zero on a fixture containing a token prefix and names the file, the line and the rule.Summary by CodeRabbit
New Features
Documentation