test(shelfundo): keep the stranger's inode distinct from the move's - #184
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe test now keeps the destination file open before deletion and recreation. This prevents inode reuse from making the replacement appear to be the original file. ChangesShelf undo test
Priority: ➖ Normal Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other · Severity of issue fixed: Medium Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
3ddc1a2 to
3ba2eaf
Compare
undo_refuses_to_walk_a_stranger_back removed the landed file and wrote a replacement at the same name, expecting the recorded dev/ino/kind to refuse it. On a filesystem that hands the just-freed inode to the replacement -- tmpfs and ext4 both do, and the Omarchy builder did -- the stranger carried the move's own identity, put_back did not refuse, and it fell through to move_back with the source directory absent, failing with "could not go back (file or folder not found)". Hold the landed file open across the unlink so its inode stays allocated and the replacement is provably a different item. dev+ino+kind stays the identity: ctime is deliberately not part of it, because permission repair changes it (see backend/opsreq.rs), so the test has to make the two items differ rather than trust the allocator. This is the one failure that aborts Omarchy's check() for flea 0.3.0 and 0.3.1.
3ba2eaf to
72b0497
Compare
|
Shipped in v0.3.3. Thanks @felixzsh! https://github.com/thisisgm/flea/releases/tag/v0.3.3 |
2 participants
shelfundo::tests::undo_refuses_to_walk_a_stranger_backremoves the landed file, writesa replacement at the same name, and expects the recorded
dev/ino/kindto refuse it.On a filesystem that hands the just-freed inode to the replacement -- ext4 does, and the
Omarchy package builder's
/tmpdid -- the stranger carries the move's own identity,put_backdoes not refuse, and it falls through tomove_backwith the source directoryabsent, failing with "could not go back (file or folder not found)".
That one failure aborted
check()and kept flea 0.3.0/0.3.1 from publishing. Omarchyworked around it in the package by running the
shelfundosuite withTMPDIRon/dev/shm(tmpfs, which allocates inode numbers from a counter and never reuses one),which is exactly the allocator behaviour this test was silently assuming. This makes the
test independent of that assumption, so it holds on ext4, tmpfs and anywhere else.
Hold the landed file open across the unlink, so its inode stays allocated and the
replacement is provably a different item.
dev+ino+kindremains the identity:ctimeis deliberately not part of it because permission repair changes it (see
backend/opsreq.rs), so the test has to make the two items differ rather than trust theallocator.
Verified: the
shelfundosuite is 8 passed on btrfs and tmpfs; the original failure doesnot reproduce here (this box's filesystems do not recycle the inode), which is why it only
showed on the builder.
Fixes #180.
Summary by CodeRabbit