From 389264cd8fe92af0aa14f493bd0f65ff81e670d4 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Thu, 17 Sep 2026 23:08:27 +0800 Subject: [PATCH 1/2] fix(runtime): stage uploads outside the writable workspace --- CONTRIBUTING.md | 11 +++-- packages/codex-executor/README.md | 41 +++++++++++-------- packages/codex-executor/src/bin/write.rs | 10 ++++- packages/codex-executor/src/write_file.rs | 25 ++++++++--- .../codex-executor/src/write_file_tests.rs | 38 +++++++++++++++++ 5 files changed, 98 insertions(+), 27 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 89d6cf82..abda7e49 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -640,10 +640,13 @@ the demonstrated native hard-link overwrite and whole-message size gaps; it is not a second filesystem service or public admission. Reuse held-directory traversal and existing rustix directory-relative operations for replacement. Keep preparation, caller authorization and uncertain mutation recovery in their existing owning layers. -The caller must prevent concurrent workspace writers during installation; native -tools and background processes can otherwise interfere with staging. Public upload -admission must establish this prerequisite or provide a separately verified safe -commit mechanism. Temporary-file cleanup is best effort. +Require an existing disjoint staging directory on the destination filesystem. +The operator must protect that directory and its ancestors from native tool and +background-process writes; same-user mode bits alone do not do so. Use separate +native filesystem policies for the installer and workspace tools, with a dedicated +staging directory per Environment. The helper cannot attest that placement rule; +public admission must establish it. Concurrent workspace writers need not be +globally stopped to protect staged bytes. Temporary-file cleanup is best effort. A queued stdin receipt, missing helper result or process termination is not a file commit receipt. See the [installer contract](packages/codex-executor/README.md#scoped-file-installer) for private limits, cleanup, metadata and concurrency semantics. diff --git a/packages/codex-executor/README.md b/packages/codex-executor/README.md index 6265bf23..5ce36fbe 100644 --- a/packages/codex-executor/README.md +++ b/packages/codex-executor/README.md @@ -73,10 +73,15 @@ limitations: hard-link targets are modified in place, and base64 encoding a 50 MiB file exceeds the native 64 MiB message bound. Install this helper outside the writable workspace and invoke it directly through the native process API, with restricted network and a filesystem policy limited to that workspace and -required helper/runtime reads. It does not authorize callers or enable Files.create. - -Arguments are the authorized absolute root, a nonempty relative file path and -its declared byte count (0–50 MiB). Stream those bytes in bounded native stdin +required helper/runtime reads, plus write access to the private staging directory. +It does not authorize callers or enable Files.create. + +Arguments are the authorized absolute root, a nonempty relative file path, +its declared byte count (0–50 MiB), and an existing absolute staging directory. +The staging directory must be outside the workspace, not its ancestor, and on +the destination filesystem. Symlink traversal and cross-filesystem replacement +are rejected; there is no copy fallback. The former three-argument private CLI +is no longer accepted. Stream those bytes in bounded native stdin chunks, followed by their 32-byte binary SHA-256 digest. This is one private frame; there is no second request on that process. The native process protocol has no stdin-close method, so the digest terminates the frame without waiting for EOF. @@ -85,25 +90,29 @@ means queued input, not committed file contents. The helper reuses held-directory no-follow traversal, rejects existing nonregular targets and requires an existing parent. It writes a fresh mode-0600 temporary -file with the existing rustix openat/renameat operations, checks the declared byte count and +file in the held staging directory with the existing rustix openat/renameat operations, checks the declared byte count and digest, syncs the file, then replaces the destination directory entry and syncs -the parent. Existing hard links retain their original inode and contents. This +both directories. Existing hard links retain their original inode and contents. This private replacement policy does not preserve destination mode/ownership metadata -or establish official overwrite semantics. The caller must prevent concurrent -workspace writers throughout installation, including native tools and background -processes that can modify the staging file or its directory entry. The digest -checks streamed input; it does not protect against another process replacing the -staging name or modifying its inode before commit. This prerequisite is not yet -established for public uploads. Later writers can change the installed file; -no snapshot or exactly-once guarantee is implied. +or establish official overwrite semantics. The operator must protect staging and +its ancestors from native tools and background processes. A dedicated staging +directory per Environment, with native tool write access limited to the workspace +and separate installer access, is the qualified mechanism. Directory naming or +mode 0700 alone does not isolate processes running as the same user. The helper +cannot verify other processes' policies; public admission must bind and validate +this condition. Broad read permission may still expose staging bytes; confidentiality +requires its own placement policy. Concurrent workspace changes do not gain access +to protected staging, but later writers can change the installed file. No snapshot +or exactly-once guarantee is implied. One version-1 JSON response reports `outcome: completed` with `size_bytes`, -`failed` before replacement, or `unknown` if the parent sync fails after replacement. +`failed` before replacement, or `unknown` if either directory sync fails after replacement. Errors contain only a fixed safe code. Require a complete response plus observed native exit/output close; exit zero alone is insufficient. Input errors preserve -the old destination under that concurrency prerequisite. Temporary-file cleanup +the old destination provided staging remains protected; independent workspace +writers can still change that destination themselves. Temporary-file cleanup is best effort: permission or I/O errors, as well as forced termination, can leave -a `.parsar-upload-*` staging file. Never interpret it as a completed upload. +a `.parsar-upload-*` file in the private staging directory. Never interpret it as a completed upload. A missing receipt remains unknown and must not trigger automatic replay. This helper does not fence a replacement owner after remote transport or service loss; public admission still needs operation ownership and recovery handling. diff --git a/packages/codex-executor/src/bin/write.rs b/packages/codex-executor/src/bin/write.rs index 05de5b04..bd3f0c92 100644 --- a/packages/codex-executor/src/bin/write.rs +++ b/packages/codex-executor/src/bin/write.rs @@ -9,13 +9,19 @@ use std::path::Path; fn run() -> Result { let args: Vec<_> = std::env::args().skip(1).collect(); - if args.len() != 3 { + if args.len() != 4 { return Err(io::Error::from(io::ErrorKind::InvalidInput).into()); } let size = args[2] .parse::() .map_err(|_| io::Error::from(io::ErrorKind::InvalidInput))?; - write_file::install(Path::new(&args[0]), &args[1], size, io::stdin().lock())?; + write_file::install( + Path::new(&args[0]), + &args[1], + size, + io::stdin().lock(), + Path::new(&args[3]), + )?; Ok(size) } diff --git a/packages/codex-executor/src/write_file.rs b/packages/codex-executor/src/write_file.rs index 24b930a2..f650b57a 100644 --- a/packages/codex-executor/src/write_file.rs +++ b/packages/codex-executor/src/write_file.rs @@ -1,5 +1,5 @@ use crate::workspace_path::{anchor, directory}; -use rustix::fs::{AtFlags, FileType, Mode, OFlags, openat, renameat, statat, unlinkat}; +use rustix::fs::{AtFlags, FileType, Mode, OFlags, fstat, openat, renameat, statat, unlinkat}; use sha2::{Digest, Sha256}; use std::fs::File; use std::io::{self, Read, Write}; @@ -28,8 +28,11 @@ pub(crate) fn install( relative: &str, size: u64, mut input: impl Read, + staging_root: &Path, ) -> Result<(), Failure> { if size > MAX_BYTES + || staging_root.starts_with(root) + || root.starts_with(staging_root) || relative.is_empty() || relative.len() > 4096 || relative @@ -41,13 +44,19 @@ pub(crate) fn install( let (parent, leaf) = relative.rsplit_once('/').unwrap_or(("", relative)); let root = anchor(root)?; let parent = directory(&root, parent)?; + let staging_parent = directory(&anchor(staging_root)?, "")?; + if fstat(&parent).map_err(io::Error::from)?.st_dev + != fstat(&staging_parent).map_err(io::Error::from)?.st_dev + { + return Err(io::Error::from(io::ErrorKind::InvalidInput).into()); + } match statat(&parent, leaf, AtFlags::SYMLINK_NOFOLLOW) { Ok(metadata) if FileType::from_raw_mode(metadata.st_mode) == FileType::RegularFile => (), Ok(_) => return Err(io::Error::from(io::ErrorKind::InvalidInput).into()), Err(rustix::io::Errno::NOENT) => (), Err(error) => return Err(io::Error::from(error).into()), } - let staging = Staging::new(&parent)?; + let staging = Staging::new(&staging_parent)?; let mut file = &staging.file; let mut digest = Sha256::new(); let mut remaining = size; @@ -66,11 +75,17 @@ pub(crate) fn install( return Err(io::Error::from(io::ErrorKind::InvalidData).into()); } staging.file.sync_all()?; - staging.persist(leaf)?; + staging.persist(&parent, leaf)?; File::from(parent).sync_all().map_err(|error| Failure { committed: true, error, })?; + File::from(staging_parent) + .sync_all() + .map_err(|error| Failure { + committed: true, + error, + })?; Ok(()) } @@ -98,8 +113,8 @@ impl<'a> Staging<'a> { }) } - fn persist(mut self, leaf: &str) -> io::Result<()> { - renameat(self.parent, self.name.as_str(), self.parent, leaf)?; + fn persist(mut self, destination: &OwnedFd, leaf: &str) -> io::Result<()> { + renameat(self.parent, self.name.as_str(), destination, leaf)?; self.committed = true; Ok(()) } diff --git a/packages/codex-executor/src/write_file_tests.rs b/packages/codex-executor/src/write_file_tests.rs index 6dcedfb8..c14fab04 100644 --- a/packages/codex-executor/src/write_file_tests.rs +++ b/packages/codex-executor/src/write_file_tests.rs @@ -14,6 +14,44 @@ fn frame(data: &[u8]) -> impl Read + '_ { Cursor::new(data).chain(Cursor::new(Sha256::digest(data).to_vec())) } +fn install(root: &Path, relative: &str, size: u64, input: impl Read) -> Result<(), Failure> { + let staging = fixture(); + let result = super::install(root, relative, size, input, staging.path()); + assert_eq!(fs::read_dir(staging.path()).unwrap().count(), 0); + result +} + +#[test] +fn requires_existing_disjoint_nofollow_staging_before_consuming_input() { + let f = fixture(); + let root = f.path().join("workspace"); + let staging = f.path().join("private"); + fs::create_dir_all(root.join("nested")).unwrap(); + fs::create_dir(&staging).unwrap(); + fs::write(root.join("file"), b"old").unwrap(); + symlink(&staging, f.path().join("link")).unwrap(); + struct Unread; + impl Read for Unread { + fn read(&mut self, _: &mut [u8]) -> io::Result { + panic!("invalid staging must be rejected before reading input") + } + } + for invalid in [ + root.clone(), + root.join("nested"), + f.path().to_path_buf(), + f.path().join("missing"), + f.path().join("link"), + staging.join("../private"), + Path::new("relative").to_path_buf(), + ] { + let failure = super::install(&root, "file", 3, Unread, &invalid).unwrap_err(); + assert!(!failure.committed); + assert_eq!(fs::read(root.join("file")).unwrap(), b"old"); + } + assert_eq!(fs::read_dir(&staging).unwrap().count(), 0); +} + #[test] fn replaces_only_destination_hard_link_and_keeps_exact_binary_bytes() { let f = fixture(); From b30208d29b0e6b13bd0d7942bd17b75903e2f0b9 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Thu, 17 Sep 2026 23:11:10 +0800 Subject: [PATCH 2/2] docs: specify qualified native staging mount layout --- CONTRIBUTING.md | 6 +++++- packages/codex-executor/README.md | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index abda7e49..fa59b9f6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -644,7 +644,11 @@ Require an existing disjoint staging directory on the destination filesystem. The operator must protect that directory and its ancestors from native tool and background-process writes; same-user mode bits alone do not do so. Use separate native filesystem policies for the installer and workspace tools, with a dedicated -staging directory per Environment. The helper cannot attest that placement rule; +staging directory per Environment. The qualified installer sees one writable parent +containing only that Environment's workspace and staging; tools retain workspace-only +write access. Keep history, credentials and other tenants outside that parent. +Separate sandbox bind mounts may reject rename even on the same backing filesystem; +never fall back to copying. The helper cannot attest that placement rule; public admission must establish it. Concurrent workspace writers need not be globally stopped to protect staged bytes. Temporary-file cleanup is best effort. A queued stdin receipt, missing helper result or process termination is not a file diff --git a/packages/codex-executor/README.md b/packages/codex-executor/README.md index 5ce36fbe..8682e2d6 100644 --- a/packages/codex-executor/README.md +++ b/packages/codex-executor/README.md @@ -72,8 +72,12 @@ The optional `agents-api-codex-write` helper addresses two pinned native write limitations: hard-link targets are modified in place, and base64 encoding a 50 MiB file exceeds the native 64 MiB message bound. Install this helper outside the writable workspace and invoke it directly through the native process API, -with restricted network and a filesystem policy limited to that workspace and -required helper/runtime reads, plus write access to the private staging directory. +with restricted network and the required helper/runtime reads. The qualified +installer policy grants write access to one dedicated per-Environment parent +containing only workspace and staging; ordinary native tools can write only the +workspace. Keep credentials, native history and other Environments outside that +parent. Separate writable mount entries can make rename fail with EXDEV even +when their backing filesystem matches; the helper must reject that layout. It does not authorize callers or enable Files.create. Arguments are the authorized absolute root, a nonempty relative file path, @@ -90,9 +94,10 @@ means queued input, not committed file contents. The helper reuses held-directory no-follow traversal, rejects existing nonregular targets and requires an existing parent. It writes a fresh mode-0600 temporary -file in the held staging directory with the existing rustix openat/renameat operations, checks the declared byte count and -digest, syncs the file, then replaces the destination directory entry and syncs -both directories. Existing hard links retain their original inode and contents. This +file in the held staging directory with existing rustix openat/renameat operations, +checks the declared byte count and digest, syncs the file, then replaces the +destination directory entry and syncs both directories. Existing hard links retain +their original inode and contents. This private replacement policy does not preserve destination mode/ownership metadata or establish official overwrite semantics. The operator must protect staging and its ancestors from native tools and background processes. A dedicated staging @@ -112,7 +117,8 @@ native exit/output close; exit zero alone is insufficient. Input errors preserve the old destination provided staging remains protected; independent workspace writers can still change that destination themselves. Temporary-file cleanup is best effort: permission or I/O errors, as well as forced termination, can leave -a `.parsar-upload-*` file in the private staging directory. Never interpret it as a completed upload. +a `.parsar-upload-*` file in the private staging directory. Never interpret it as a +completed upload. A missing receipt remains unknown and must not trigger automatic replay. This helper does not fence a replacement owner after remote transport or service loss; public admission still needs operation ownership and recovery handling.