Skip to content

fix(receiver): confine the --delay-updates staging path to the module #7659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
oferchen merged 1 commit into from
Sep 4, 2026
Merged

fix(receiver): confine the --delay-updates staging path to the module #7659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion crates/transfer/src/disk_commit/process/commit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>/<basename>`, 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<bool> {
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<bool> {
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
Expand Down
2 changes: 1 addition & 1 deletion crates/transfer/src/receiver/transfer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
9 changes: 4 additions & 5 deletions tools/ci/upstream-3.5.0-expect.macos.nonroot.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading