fix(code-history): close apply_patch_proposal TOCTOU + crash window (Bonus 12 #7 / PR #135)

[Ghenghis]

Summary

Closes Bonus 12 finding #7. Tightens apply_patch_proposal from a verify-after-replace TOCTOU to a fully durable, recovery-friendly write.

Pre-fix: write tmp -> os.replace -> read -> hash -> rollback. No fsync, no inflight DB record, no parent-dir fsync. SIGKILL or power loss could leave a mutated file with no audit row to reconcile against (etcd #13839 family).

Post-fix: hash bytes -> reject if mismatch -> open("wb")+write+fsync(fd) -> record code_patch.replace_inflight in proof_events -> os.replace -> fsync(parent_dir on POSIX) -> defensive post-read hash check.

Tests added (6, 5 pass + 1 POSIX-skip on Windows host)

• Clean apply records inflight BEFORE replace, applied AFTER
• Hash mismatch blocks BEFORE replace; target unchanged; NO inflight written on rejection
• os.fsync called on tmp fd
• Simulated crash via os.replace raising OSError leaves target unchanged AND inflight marker in DB
• POSIX-only: parent-dir fsync (>=2 fsync calls)
• Windows-only: function does not crash trying to fsync a directory

Cross-platform fix during dev

Initial draft used os.open(..., O_WRONLY|O_CREAT|O_TRUNC) for the tmp file. On Windows that does NOT include O_BINARY by default and translated \n -> \r\n, breaking the post-replace hash. Tests caught this; final patch uses open(..., "wb") which is binary on every platform.

Verification

• py_compile: OK
• Focused tests: 5/5 pass + 1 skipped (POSIX-only)
• code_operator + apply_patch + recovery_ledger: 67/68 pass (1 POSIX-skip)
• Pre-push hook: passed

Scope

• Bonus 12 finding Wave B Auditor — proof bundle integrity report #7 only.
• v0.13.0 update remains formally deferred.
• RC v2 commits 2-5 remain paused.

Swarm provenance

Agent 8 of the 20-agent Blocker Elimination Swarm produced the diff.

References

• Python os.fsync: https://docs.python.org/3/library/os.html#os.fsync
• etcd #13839: Proposals should include a merkle root etcd-io/etcd#13839
• LWN "Ensuring data reaches disk": https://lwn.net/Articles/457667/

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 245f4880-60f8-4826-8075-958e8497c0d9

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/bonus12-toctou-fix

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request enhances the durability and atomicity of the apply_patch_proposal function by implementing a write-ahead pattern that includes pre-write hash verification, explicit fsync calls on temporary files and parent directories, and database-backed intent logging for crash recovery. Feedback suggests improving the portability of the directory fsync implementation by handling potential OSError exceptions on unsupported POSIX systems and ensuring the rollback path follows the same durability standards as the primary write path to prevent file corruption during failures.

[gemini-code-assist]

The directory fsync implementation is a significant durability improvement, but it may cause crashes on certain POSIX systems or filesystems (like macOS HFS+/APFS or some network mounts) where fsync on a directory handle is not supported and returns ENOTSUP or EINVAL.

Additionally, os.open should explicitly include an access mode (like os.O_RDONLY) when combined with os.O_DIRECTORY to ensure portability and correctness across all POSIX implementations.

Suggested change

	try:
	dir_fd = os.open(str(target.parent), os.O_DIRECTORY)
	except (AttributeError, OSError):
	dir_fd = None
	if dir_fd is not None:
	try:
	os.fsync(dir_fd)
	finally:
	os.close(dir_fd)
	try:
	# Explicitly include O_RDONLY; O_DIRECTORY is a safety flag
	dir_fd = os.open(str(target.parent), os.O_RDONLY | getattr(os, "O_DIRECTORY", 0))
	except (AttributeError, OSError):
	dir_fd = None
	if dir_fd is not None:
	try:
	os.fsync(dir_fd)
	except OSError:
	# Best-effort: some POSIX systems do not support directory fsync
	pass
	finally:
	os.close(dir_fd)

[gemini-code-assist]

While this defensive rollback restores the original file content if a post-replace hash mismatch is detected, it lacks the durability guarantees implemented in the primary write path. If the system crashes during this os.replace without an fsync of the rollback file and its parent directory, the target file could be left in a corrupted or inconsistent state. Given the high durability standards established in this PR, the rollback path should ideally follow the same fsync pattern.