From 036861fbdcf6c3dac66597bf1e6e730501c4c3ae Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Fri, 4 Sep 2026 08:37:54 +0300 Subject: [PATCH] fix(receiver): confine the --delay-updates staging path to the module On a daemon receiver `--partial-dir` is peer-supplied, and `--delay-updates` stages every file through `<--partial-dir>/` before renaming it onto the destination. Both of those renames walked the ownership walk in its `Ancillary` spelling, which leaves `AbsPathTracker` disabled and so never consults the session confinement root at all. A symlink standing at the partial-dir name therefore sent the staged file - a complete copy of the source - outside the served module, and the delayed sweep then renamed it back over the destination. Measured against the cell's own fixture (module `mod` with `blink -> /secret` planted inside it, `-a --delay-updates --partial-dir=/blink src/ mod/`), macOS exited 23 with `mod/f0` replaced and the outside `secret/f0` destroyed. Both endpoints now take `operator_rename_confined`, and the run exits 11 with the destination untouched and the victim intact - the same refusal the Linux baseline already encodes. Each half is load-bearing, measured in isolation: neither confined exit 23, dest REPLACED, victim DESTROYED sweep only exit 23, dest untouched, victim DESTROYED both exit 11, dest untouched, victim intact The staging endpoint deliberately does not reuse `rename_config_sandboxed`: its `renameat_via_sandbox_or_fallback` helper drops to a path-based rename when the anchored open fails, which is the drop-to-unconfined upstream names as wrong - `syscall.c` `open_dir_secure()` signals policy with `errno == 0` rather than letting a runtime errno select the unconfined arm. upstream: util1.c:1518-1530 `handle_partial_dir(..., PDIR_CREATE)` runs the whole retention under `operator_path_resolve`; syscall.c:1891 `do_rename_at()` and receiver.c:546 `do_rename(partialptr, fname)` are the two renames. The macOS nonroot expect manifest flips `operator-path-partial-dir-daemon` fail -> pass in the same commit. macOS leg 235/5 -> 237/3; the three residual rows are the ones the manifest header already classifies (two unsatisfiable on this host with real rsync 3.5.0 landing on the same result, one owned). --- .../src/disk_commit/process/commit.rs | 37 ++++++++++++++++++- crates/transfer/src/receiver/transfer.rs | 2 +- .../upstream-3.5.0-expect.macos.nonroot.txt | 9 ++--- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/crates/transfer/src/disk_commit/process/commit.rs b/crates/transfer/src/disk_commit/process/commit.rs index 3d8bd0d607..b1f4324cd5 100644 --- a/crates/transfer/src/disk_commit/process/commit.rs +++ b/crates/transfer/src/disk_commit/process/commit.rs @@ -148,7 +148,7 @@ pub(super) fn commit_file( clear_partial_dir_obstruction(parent)?; create_dir_all_sandboxed(config.backup_env(), parent)?; } - let result = rename_config_sandboxed(config, cleanup_guard.path(), &staging_path) + let result = stage_into_partial_dir(config, cleanup_guard.path(), &staging_path) .map_err(|e| { crate::temp_guard::attach_commit_op( crate::temp_guard::CommitOp::Rename, @@ -503,6 +503,41 @@ pub(super) fn rename_config_sandboxed( } } +/// Moves the received temp into the `--delay-updates` staging path through the +/// ownership walk, bound to the session's confinement root. +/// +/// The staging path is `<--partial-dir>/`, and on a daemon receiver +/// `--partial-dir` is PEER-SUPPLIED: a symlink standing at that name sends the +/// staged file - a complete copy of the source - outside the served module, and +/// the later sweep then renames it back over the destination. So this endpoint +/// takes the confined walk rather than [`rename_config_sandboxed`], whose +/// sandbox helper falls back to a path-based rename when the anchored open +/// fails - exactly the drop-to-unconfined that upstream names as wrong. +/// +/// # Upstream Reference +/// +/// - `rsync-3.5.0/util1.c:1518-1530` `handle_partial_dir(..., PDIR_CREATE)` - +/// the whole retention runs under `operator_path_resolve`. +/// - `rsync-3.5.0/syscall.c:1891` `do_rename_at()` under that flag. +#[cfg(unix)] +fn stage_into_partial_dir( + _config: &DiskCommitConfig, + old_path: &Path, + new_path: &Path, +) -> io::Result { + fast_io::operator_rename_confined(old_path, new_path, true)?; + Ok(false) +} + +#[cfg(not(unix))] +fn stage_into_partial_dir( + config: &DiskCommitConfig, + old_path: &Path, + new_path: &Path, +) -> io::Result { + rename_config_sandboxed(config, old_path, new_path) +} + /// Returns `true` when an I/O error represents a cross-device link (EXDEV). /// /// Forwards to [`fast_io::is_cross_device`], the single source of truth shared diff --git a/crates/transfer/src/receiver/transfer.rs b/crates/transfer/src/receiver/transfer.rs index e87b18a571..05e8bee008 100644 --- a/crates/transfer/src/receiver/transfer.rs +++ b/crates/transfer/src/receiver/transfer.rs @@ -466,7 +466,7 @@ pub(in crate::receiver) enum DeletePassPhase { /// glibc's `rename()` lowers to on x86_64. #[cfg(unix)] fn sweep_rename(old_path: &Path, new_path: &Path) -> io::Result<()> { - fast_io::operator_rename(old_path, new_path, true) + fast_io::operator_rename_confined(old_path, new_path, true) } #[cfg(not(unix))] diff --git a/tools/ci/upstream-3.5.0-expect.macos.nonroot.txt b/tools/ci/upstream-3.5.0-expect.macos.nonroot.txt index 1d1bd416c2..383537dff7 100644 --- a/tools/ci/upstream-3.5.0-expect.macos.nonroot.txt +++ b/tools/ci/upstream-3.5.0-expect.macos.nonroot.txt @@ -15,20 +15,19 @@ # run's emitted `expect-result.macos.nonroot.txt` artifact and say so; do NOT # edit individual rows to make the job green. # -# The 4 `fail` rows were classified 2026-09-04 by running each cell twice on +# The 3 `fail` rows were classified 2026-09-04 by running each cell twice on # one host, varying ONLY $RSYNC between oc and the real 3.5.0 binary. That # upstream arm is the control; without it a failure cannot be attributed. # # chmod-setid upstream FAILS too - unsatisfiable here # partial-protected-regular-retry-policy upstream FAILS too - unsatisfiable here -# operator-path-partial-dir-daemon upstream PASSES - real oc defect # filter-merge-content-echo owned by task 1011 # -# Two of the four are therefore NOT oc divergences: the platform (or this +# Two of the three are therefore NOT oc divergences: the platform (or this # host's sysctl state) refuses what the cell asserts, and real rsync lands on # the same result. They stay `fail` because the assertion is unsatisfiable # here, not because oc is right - re-baselining them would hide the row rather -# than resolve it. None of the four appears in upstream's own +# than resolve it. None of the three appears in upstream's own # testsuite/skiplist/macos.txt. 00-hello pass acl-symlink-race skip @@ -250,7 +249,7 @@ operator-path-insecure-links-refused pass operator-path-link-dest pass operator-path-log-file pass operator-path-partial-dir pass -operator-path-partial-dir-daemon fail +operator-path-partial-dir-daemon pass operator-path-partial-dir-exclude-daemon pass operator-path-temp-dir pass operator-path-traversal-backup-dir-daemon pass