handle_unlink(path, is_dir = true) accepts a directory that is not empty and answers Ok(true), where rmdir(2) would return ENOTEMPTY. The whiteout is recorded, and commit() then removes the directory and everything under it.
Reproducer:
let wd = workdir.path().canonicalize().unwrap();
fs::create_dir(wd.join("d")).unwrap();
fs::write(wd.join("d/inner.txt"), "DATA").unwrap();
let mut b = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap();
b.handle_unlink(&format!("{}/d", wd.display()), true); // Ok(true)
b.commit(); // Ok(())
Observed: rmdir = Ok(true), commit = Ok(()), and d/inner.txt is gone afterwards.
Two separate problems sit on top of each other:
- Divergence from the syscall. A child that calls
rmdir() on a directory it believes to be empty gets success instead of ENOTEMPTY. Code that relies on that errno to detect leftovers — a build script cleaning a tree, a test asserting a directory was drained — takes the wrong branch, and inside the sandbox it cannot tell.
- Destructive follow-through. Because the whiteout is path-based,
commit() applies it with remove_dir_all_in_root, so contents the child never asked to delete are destroyed. changes() reports a single Deleted "d", so neither the dry run nor the recovery report names them.
The emptiness check has to happen against the merged view rather than either layer alone: a directory can be empty in the upper while the lower still has entries, and vice versa when the upper holds whiteouts. list_merged_dir (cow/seccomp.rs:1008) already computes exactly that view, so the check is available — the rmdir path just does not consult it.
Related, and probably wanting one fix between them: #159 (a whiteout is not recursive, so a deleted directory's contents stay readable and can be republished). Both come from the whiteout being a plain path string with no notion of a subtree.
Reproduced on main at 44b1c65; found while writing COW merge tests for #148, unrelated to that PR's subject. As in #160, these handlers take the absolute path as the child sees it — a relative argument makes safe_rel return None and the handler answers Ok(false), which hides the behaviour.
handle_unlink(path, is_dir = true)accepts a directory that is not empty and answersOk(true), wherermdir(2)would returnENOTEMPTY. The whiteout is recorded, andcommit()then removes the directory and everything under it.Reproducer:
Observed:
rmdir = Ok(true),commit = Ok(()), andd/inner.txtis gone afterwards.Two separate problems sit on top of each other:
rmdir()on a directory it believes to be empty gets success instead ofENOTEMPTY. Code that relies on that errno to detect leftovers — a build script cleaning a tree, a test asserting a directory was drained — takes the wrong branch, and inside the sandbox it cannot tell.commit()applies it withremove_dir_all_in_root, so contents the child never asked to delete are destroyed.changes()reports a singleDeleted "d", so neither the dry run nor the recovery report names them.The emptiness check has to happen against the merged view rather than either layer alone: a directory can be empty in the upper while the lower still has entries, and vice versa when the upper holds whiteouts.
list_merged_dir(cow/seccomp.rs:1008) already computes exactly that view, so the check is available — the rmdir path just does not consult it.Related, and probably wanting one fix between them: #159 (a whiteout is not recursive, so a deleted directory's contents stay readable and can be republished). Both come from the whiteout being a plain path string with no notion of a subtree.
Reproduced on
mainat 44b1c65; found while writing COW merge tests for #148, unrelated to that PR's subject. As in #160, these handlers take the absolute path as the child sees it — a relative argument makessafe_relreturnNoneand the handler answersOk(false), which hides the behaviour.