Skip to content

fix(journal): close run-journal hardening gaps - #633

Merged
solomonneas merged 6 commits into
mainfrom
codex/632-journal-hardening
Jul 31, 2026
Merged

fix(journal): close run-journal hardening gaps#633
solomonneas merged 6 commits into
mainfrom
codex/632-journal-hardening

Conversation

@solomonneas

Copy link
Copy Markdown
Collaborator

Fixes #632.

This follow-up closes four hardening gaps found while grading #568. It keeps existing event canonicalization, envelope fields, and projector output unchanged for existing event types. No existing golden fixture was regenerated.

Fix and regression map

Item Fix Named regression tests
Mid-run enrollment shedding record_run_start now reports lifecycle or authority enrollment. Later status writes inspect run.json without following symlinks and raise refusing to overwrite unknown durable enrollment state for missing, unreadable, non-object, or non-regular receipts. test_record_run_start_treats_lifecycle_only_enrollment_as_durable
test_run_payload_fails_closed_when_enrolled_run_json_becomes_corrupt
Chained redaction record Adds status-neutral run.redaction.recorded events with the five reference-only fields from #632. Anchors hash the exact record.json bytes, survive later overlapping rewrites, bind child records to the parent anchor observed in the child's quarantined input journal, and are checked against the record inventory after rewrite and cleanup. Structural-only ranges now fail before mutation. test_overlapping_redactions_preserve_both_chained_operation_anchors
test_replaced_redaction_record_set_fails_chained_anchor_validation
test_redaction_after_cleaned_parent_uses_active_anchor_reference
test_redaction_inventory_rejects_tampered_parent_record_anchor_reference
test_redaction_rejects_preserved_structural_only_range_without_mutation
Atomic text-write directory durability localio.write_text_atomic fsyncs the parent directory after os.replace on supported platforms, opens it with directory and no-follow flags, and preserves the primary fsync error if close also fails. test_write_text_atomic_fsyncs_parent_after_replace
test_write_text_atomic_opens_parent_without_following_symlinks
test_write_text_atomic_reports_directory_fsync_failure_after_replace
test_write_text_atomic_skips_parent_fsync_when_platform_has_no_support
Partial-tail recovery serialization recover_partial_tail now owns the same append lock and SIGTERM deferral critical section as append_event. test_recover_partial_tail_reentrancy_matches_append_event

Verification

brigade work verify run --target . --command "./scripts/verify" --capture brigade-work
receipt: 20260731-164708-work-verify-193280
5554 passed, 3 skipped in 542.02s
Total coverage: 82.99%

The follow-up independent review found no remaining Critical or Important findings. Its two Minor findings, structural-only no-op redactions and duplicate resume verification, were fixed before the full gate above.

solomonneas and others added 4 commits July 31, 2026 13:02
Co-Authored-By: Codex <codex@openai.com>
Co-Authored-By: Codex <codex@openai.com>
Co-Authored-By: Codex <codex@openai.com>
Co-Authored-By: Codex <codex@openai.com>
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • coderabbit-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b953c511-9ae6-4ab3-b788-7b0350bcae03

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@solomonneas solomonneas self-assigned this Jul 31, 2026
@solomonneas

Copy link
Copy Markdown
Collaborator Author

Grader review: APPROVE-WITH-CHANGES. All four #632 items are correctly implemented and the closeout map checks out (receipt 20260731-164708-work-verify-193280 verified on disk, 5,554 passed; no golden fixture regenerated, confirmed by captured check 20260731-183223-work-verify-a31ae9). Two changes required before merge, two minors that can ride.

Required before merge:

  1. localio._fsync_parent_directory opens the parent with O_NOFOLLOW on the final component, so a target directory reached through a symlink now raises ELOOP after a successful os.replace. write_text_atomic has 30+ call sites outside the journal, and symlinked repo or workspace directories are a normal deployment shape. Resolve the parent before opening (or catch ELOOP and retry on path.parent.resolve()), and add a regression test that writes through a symlinked parent directory. The journal-side _fsync_directory keeps its strict no-follow contract; this generic helper should not inherit it.
  2. Add the missing test for redaction of a range that contains a prior run.redaction.recorded anchor event. That case is the core attribution guarantee this PR exists to provide; it needs a pinned regression test either proving the anchor is preserved or proving the operation is refused.

Ride-along follow-ups (no merge block): idempotent-cleanup journal churn in _refresh_chained_anchors, and the dropped split-retirement digest cross-check flagged in review.

Re-request review after the two changes with a fresh full-suite receipt.

solomonneas and others added 2 commits July 31, 2026 15:05
Co-Authored-By: Codex <codex@openai.com>
Co-Authored-By: Codex <codex@openai.com>
@solomonneas

Copy link
Copy Markdown
Collaborator Author

Implemented the two required changes from the grader review.

  1. b81ed9ac (fix(io): support symlinked atomic-write parents)

    • localio._fsync_parent_directory resolves the generic atomic-write parent before opening it with the existing directory and no-follow flags.
    • run_journal._fsync_directory is unchanged and still refuses a symlinked final directory.
    • Regression tests:
      • test_write_text_atomic_allows_directory_symlink_parent
      • test_run_journal_fsync_directory_refuses_directory_symlink
  2. 54bce3db (test(redaction): pin overlapping anchor preservation)

    • The regression reads the first operation's actual run.redaction.recorded sequence and includes that sequence in the second redaction range.
    • It requires a complete chain with no chain errors or partial tail, both operation anchors, matching record_sha256 values, and the expected parent operation link.
    • Regression test: test_redaction_preserves_first_anchor_when_second_range_contains_it

Verification:

  • Focused receipt 20260731-185508-work-verify-2ca7c7: 3 passed.
  • Full receipt 20260731-185534-work-verify-67a837: ./scripts/verify exited 0, with 5,556 passed, 3 skipped, and 82.99% coverage.

No fixture, golden, event-canonicalization, projector, run_journal.py, or run_redaction.py changes were made. The two ride-along minors remain out of scope.

@solomonneas

Copy link
Copy Markdown
Collaborator Author

Grader final pass: both required changes verified. The localio fix resolves the parent before the no-follow open (one line, correct minimal blast radius), the journal-side strict helper is untouched and now has a pinning test, and the overlapping-anchor regression proves the first redaction anchor survives a second redaction whose range contains it, with matching record_sha256 and parent link. Receipts verified on disk: focused 2ca7c7 (3 passed) and full 67a837 (5,556 passed, 3 skipped, 82.99% coverage) at head 54bce3d with a clean tree. CI fully green. APPROVE - merging.

@solomonneas
solomonneas merged commit a3b2fba into main Jul 31, 2026
28 checks passed
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.

run-journal: four hardening residuals from the #568 grading review

1 participant