Fix P0 correctness issues in jail enforcement, merge, and mutation log#6
Merged
Conversation
- ReadWriteFilesystem: reject writes whose destination's final component is a symlink resolving outside the root. The parent-only containment check let appends follow a pre-existing symlink out of the jail. - Workspace.merge: record the parent's mutation sequence at branch time and refuse to merge over tracked-but-uncheckpointed parent edits with the new WorkspaceError.mergeUncheckpointedChanges, instead of silently clobbering them. The checkpoint-head comparison alone cannot see writes that never advanced the head. - FileCheckpointStore: tolerate a torn trailing mutations.jsonl line left by a crashed append (skip on read, truncate before the next append) instead of failing every subsequent load. Mid-file corruption still surfaces as an error. - FileCheckpointStore: derive the next mutation sequence from the log's final record rather than re-reading the entire log on every append, which was O(n^2) across a session. Adds regression tests for all four fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G29SXqzHqDycqXGa4dRr4x
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four correctness fixes surfaced by a review of the filesystem and checkpoint layers, each with regression tests.
ReadWriteFilesystem: close a jail escape through destination symlinksWrite paths only containment-checked the parent directory of the destination. A pre-existing symlink at the final component pointing outside the configured root was therefore followed by the append path (
FileHandle(forWritingTo:)), writing outside the jail. Destinations whose final component is a symlink are now resolved and containment-checked; symlinks that stay inside the root keep working.Workspace.merge: stop silently discarding uncheckpointed parent editsThe merge guard compared checkpoint heads only, and tracked writes never move the head — so a parent that edited files after
branch()without checkpointing passed the guard and had those edits clobbered by the branch snapshot restore.branch()now records the parent's mutation cursor, andmerge()throws the newWorkspaceError.mergeUncheckpointedChangeswhen the parent has tracked-but-uncheckpointed changes. Writes made before the branch still merge fine.FileCheckpointStore: survive a tornmutations.jsonltailA crash mid-append left a partial trailing line that made every subsequent log read throw, poisoning the whole history. Reads now skip a torn tail (an undecodable final line with no trailing newline), and the next append truncates it before writing. Corruption anywhere before the final line still surfaces as an error.
FileCheckpointStore: O(1) sequence derivation on appendappendMutationre-read the entire log to computemax(sequence) + 1on every append — O(n²) across a session. It now reads only the log's final record (backwards chunked scan, with a full-scan fallback for unusual tails). Appends still happen under the existingflocksidecar lock, so cross-process monotonicity is unchanged.Tests
Six new regression tests:
Full suite: 124 tests in 9 suites passed on macOS CI against this same content.
🤖 Generated with Claude Code
https://claude.ai/code/session_01G29SXqzHqDycqXGa4dRr4x
Generated by Claude Code