mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-17 05:31:16 +08:00
* fix(cli): cp-style sandbox download and workspace-boundary check `openshell sandbox download` produced a directory at the host destination instead of writing the source bytes as a regular file, and did not refuse sandbox-side paths that resolved outside `/sandbox` (only `creds.json` was rejected, by filesystem policy rather than by an explicit check). Mirrors the cp-style design from the upload-side fixes (#667 / #694 and #885 / #952): destination resolution now respects trailing slashes and existing-directory destinations, single-file sources land as regular files via a staged extraction and atomic rename, and the function delegates the SSH tar stream to a shared helper. Adds an explicit lexical workspace-boundary check on the sandbox-side source — required on the download side because filesystem policy alone is not a substitute for an explicit check when crossing the trust boundary in the leak direction. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * fix(cli): canonicalise sandbox download path remotely and quote tar member The lexical check in `validate_sandbox_source_path` clears any path whose components stay under `/sandbox` after `.` and `..` are folded out, but it cannot see symlinks. A workspace-relative symlink such as `/sandbox/etc-link -> /etc` lets `openshell sandbox download <name> /sandbox/etc-link/passwd /tmp/passwd` slip through, then `tar -C /sandbox/etc-link passwd` follows the link on the sandbox side and streams `/etc/passwd` back to the host. Add `canonicalize_sandbox_source_path` which runs `realpath -e --` over SSH and rejects the result unless it still equals `/sandbox` or starts with `/sandbox/`. The lexical guard stays as a cheap fail-fast for obviously-bad inputs, and the rest of the download flow now uses the canonical path everywhere a tar invocation or probe might dereference a symlink. While editing the single-file branch, also add the `--` separator before the basename in `tar cf - -C <parent> -- <name>`. Without it, a sandbox file whose basename starts with `-` (e.g. `--checkpoint-action=...`) is parsed by GNU tar as an option rather than a member, which at best fails the wrap and at worst triggers tar option behaviour. The directory branch passes literal `.` and is unaffected. Cover both fixes with pure-function tests: `is_under_sandbox_workspace` gets accept/reject pairs (the same predicate the post-realpath validator uses) and the new `build_single_file_tar_cmd` helper asserts the `--` separator and shell escaping survive any future refactor. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * chore(cli): drop redundant tempfile dev-dependency entry `tempfile` was promoted from `[dev-dependencies]` to `[dependencies]` once the cp-style single-file download path started using `TempDir::new_in` from production code. The original `[dev-dependencies]` line stayed behind by accident — entries under `[dependencies]` are already available to test compilation, so the second declaration is dead weight. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * test(cli): add E2E coverage for sandbox download paths Adds four end-to-end tests for `openshell sandbox download` so the happy path and both Codex regressions have explicit coverage: * `sandbox_download_file_only` — seeds a single file via the new `SandboxGuard::exec` harness helper and pulls it back, exercising `sandbox_sync_down_file` without an upload phase. * `sandbox_download_directory_only` — seeds a small directory tree and pulls it recursively, exercising `sandbox_sync_down_directory`. * `sandbox_download_rejects_symlinks_pointing_outside_workspace` — regression for the high-severity finding. Plants three escape shapes (`/sandbox/etc-link -> /etc`, `/sandbox/passwd-link -> /etc/passwd`, and a path with a symlinked component `/sandbox/etc-link/passwd`) and asserts all three are refused with a workspace-boundary error before any tar streams. Also checks the host destination stays empty so a silent leak would surface. * `sandbox_download_handles_dash_leading_basename` — regression for the medium-severity finding. Seeds a file named `--checkpoint-action=evil` and verifies it downloads with byte-for-byte contents, proving the `--` separator stops tar parsing the basename as an option. The harness gains a single new method, `SandboxGuard::exec`, that wraps `openshell sandbox exec --name <name> --no-tty -- <argv>`. Existing tests preferred the upload path to populate sandbox state, but the regression cases need to plant filesystem state the upload flow cannot produce (symlinks, files whose basenames clap would otherwise reject). Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * docs(cli): note cp-style destination and workspace boundary on download `openshell sandbox download` now refuses any source path that resolves outside `/sandbox` (lexically or through a symlink), and the single-file branch follows `cp`-style placement rules. Document both so users hitting the workspace-boundary error or the "land at exact path vs land inside dir" decision can self-serve. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> * test(cli): tolerate miette line-wrapping in symlink-rejection assertions `sandbox_download_rejects_symlinks_pointing_outside_workspace` matched the substring `"outside the sandbox workspace"` against the CLI's error output. The check is correct in spirit but fragile in practice: miette wraps long single-line errors across multiple lines at terminal width and inserts a `│ ` continuation marker, so the substring breaks across the wrap and the test fails even though the canonicalisation step correctly refused the source. Split the match into three short markers — `resolves to`, `outside the`, `sandbox workspace` — each guaranteed to fit on one wrapped line. The combined check is stricter than the original (it now requires evidence that the remote `realpath -e` step actually ran, not just the boundary refusal) without inheriting the wrap fragility. Signed-off-by: Tinson Lai <tinsonl@nvidia.com> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
546 lines
21 KiB
Rust
546 lines
21 KiB
Rust
// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
|
|
// SPDX-License-Identifier: Apache-2.0
|
|
|
|
#![cfg(feature = "e2e")]
|
|
|
|
//! E2E test: bidirectional file upload/download with a sandbox.
|
|
//!
|
|
//! Prerequisites:
|
|
//! - A running openshell gateway (`mise run gateway:docker`)
|
|
//! - The `openshell` binary (built automatically from the workspace)
|
|
|
|
use std::fs;
|
|
use std::io::Write;
|
|
use std::process::Stdio;
|
|
|
|
use sha2::{Digest, Sha256};
|
|
|
|
use openshell_e2e::harness::sandbox::SandboxGuard;
|
|
|
|
/// Create a long-running sandbox, upload and download files, and verify
|
|
/// contents.
|
|
///
|
|
/// Covers:
|
|
/// 1. Directory round-trip (nested files)
|
|
/// 2. Large file round-trip (~512 KiB) with SHA-256 checksum verification
|
|
/// 3. Single-file round-trip
|
|
#[tokio::test]
|
|
async fn sandbox_file_upload_download_round_trip() {
|
|
// ---------------------------------------------------------------
|
|
// Step 1 — Create a sandbox with `--keep` running `sleep infinity`.
|
|
// ---------------------------------------------------------------
|
|
let mut guard =
|
|
SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready")
|
|
.await
|
|
.expect("sandbox create --keep");
|
|
|
|
let tmpdir = tempfile::tempdir().expect("create tmpdir");
|
|
|
|
// ---------------------------------------------------------------
|
|
// Step 2 — Upload: push a local directory into the sandbox.
|
|
// ---------------------------------------------------------------
|
|
let upload_dir = tmpdir.path().join("upload");
|
|
fs::create_dir_all(upload_dir.join("subdir")).expect("create upload dirs");
|
|
fs::write(upload_dir.join("greeting.txt"), "hello-from-local").expect("write greeting.txt");
|
|
fs::write(upload_dir.join("subdir/nested.txt"), "nested-content").expect("write nested.txt");
|
|
|
|
let upload_str = upload_dir.to_str().expect("upload path is UTF-8");
|
|
guard
|
|
.upload(upload_str, "/sandbox/uploaded")
|
|
.await
|
|
.expect("upload directory");
|
|
|
|
// ---------------------------------------------------------------
|
|
// Step 3 — Download: pull the uploaded files back and verify.
|
|
// ---------------------------------------------------------------
|
|
let download_dir = tmpdir.path().join("download");
|
|
fs::create_dir_all(&download_dir).expect("create download dir");
|
|
|
|
let download_str = download_dir.to_str().expect("download path is UTF-8");
|
|
guard
|
|
.download("/sandbox/uploaded/upload", download_str)
|
|
.await
|
|
.expect("download directory");
|
|
|
|
// Verify top-level file.
|
|
let greeting = fs::read_to_string(download_dir.join("greeting.txt"))
|
|
.expect("read greeting.txt after download");
|
|
assert_eq!(
|
|
greeting, "hello-from-local",
|
|
"greeting.txt content mismatch"
|
|
);
|
|
|
|
// Verify nested file.
|
|
let nested = fs::read_to_string(download_dir.join("subdir/nested.txt"))
|
|
.expect("read subdir/nested.txt after download");
|
|
assert_eq!(nested, "nested-content", "subdir/nested.txt content mismatch");
|
|
|
|
// ---------------------------------------------------------------
|
|
// Step 4 — Large-file round-trip (~512 KiB) to exercise multi-chunk
|
|
// SSH transport.
|
|
// ---------------------------------------------------------------
|
|
let large_dir = tmpdir.path().join("large_upload");
|
|
fs::create_dir_all(&large_dir).expect("create large_upload dir");
|
|
|
|
let large_file = large_dir.join("large.bin");
|
|
{
|
|
let mut f = fs::File::create(&large_file).expect("create large.bin");
|
|
let mut rng_data = vec![0u8; 512 * 1024]; // 512 KiB
|
|
rand::fill(&mut rng_data[..]);
|
|
f.write_all(&rng_data).expect("write large.bin");
|
|
}
|
|
|
|
let expected_hash = {
|
|
let data = fs::read(&large_file).expect("read large.bin for hash");
|
|
let mut hasher = Sha256::new();
|
|
hasher.update(&data);
|
|
hex::encode(hasher.finalize())
|
|
};
|
|
|
|
let large_dir_str = large_dir.to_str().expect("large_dir path is UTF-8");
|
|
guard
|
|
.upload(large_dir_str, "/sandbox/large_test")
|
|
.await
|
|
.expect("upload large file");
|
|
|
|
let large_down = tmpdir.path().join("large_download");
|
|
fs::create_dir_all(&large_down).expect("create large_download dir");
|
|
|
|
let large_down_str = large_down.to_str().expect("large_down path is UTF-8");
|
|
guard
|
|
.download("/sandbox/large_test/large_upload", large_down_str)
|
|
.await
|
|
.expect("download large file");
|
|
|
|
let actual_data = fs::read(large_down.join("large.bin")).expect("read large.bin after download");
|
|
let actual_hash = {
|
|
let mut hasher = Sha256::new();
|
|
hasher.update(&actual_data);
|
|
hex::encode(hasher.finalize())
|
|
};
|
|
|
|
assert_eq!(
|
|
expected_hash, actual_hash,
|
|
"large.bin SHA-256 mismatch after round-trip"
|
|
);
|
|
assert_eq!(
|
|
actual_data.len(),
|
|
512 * 1024,
|
|
"large.bin size mismatch: expected {} bytes, got {}",
|
|
512 * 1024,
|
|
actual_data.len()
|
|
);
|
|
|
|
// ---------------------------------------------------------------
|
|
// Step 5 — Single-file round-trip.
|
|
// ---------------------------------------------------------------
|
|
let single_file = tmpdir.path().join("single.txt");
|
|
fs::write(&single_file, "single-file-payload").expect("write single.txt");
|
|
|
|
let single_str = single_file.to_str().expect("single path is UTF-8");
|
|
guard
|
|
.upload(single_str, "/sandbox")
|
|
.await
|
|
.expect("upload single file");
|
|
|
|
let single_down = tmpdir.path().join("single_down");
|
|
fs::create_dir_all(&single_down).expect("create single_down dir");
|
|
|
|
let single_down_str = single_down.to_str().expect("single_down path is UTF-8");
|
|
guard
|
|
.download("/sandbox/single.txt", single_down_str)
|
|
.await
|
|
.expect("download single file");
|
|
|
|
let single_content = fs::read_to_string(single_down.join("single.txt"))
|
|
.expect("read single.txt after download");
|
|
assert_eq!(
|
|
single_content, "single-file-payload",
|
|
"single.txt content mismatch"
|
|
);
|
|
|
|
// ---------------------------------------------------------------
|
|
// Cleanup (guard also cleans up on drop).
|
|
// ---------------------------------------------------------------
|
|
guard.cleanup().await;
|
|
}
|
|
|
|
/// Verify that `sandbox upload` respects `.gitignore` by default.
|
|
///
|
|
/// Creates a temporary git repository with a `.gitignore` that excludes
|
|
/// `*.log` files, uploads the directory (without `--no-git-ignore`), and
|
|
/// confirms that tracked files arrive but ignored files do not.
|
|
#[tokio::test]
|
|
async fn upload_respects_gitignore_by_default() {
|
|
// ---------------------------------------------------------------
|
|
// Step 1 — Create a sandbox with `--keep`.
|
|
// ---------------------------------------------------------------
|
|
let mut guard =
|
|
SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready")
|
|
.await
|
|
.expect("sandbox create --keep");
|
|
|
|
// ---------------------------------------------------------------
|
|
// Step 2 — Set up a temp git repo with tracked + ignored files.
|
|
// ---------------------------------------------------------------
|
|
let tmpdir = tempfile::tempdir().expect("create tmpdir");
|
|
let repo = tmpdir.path().join("repo");
|
|
fs::create_dir_all(&repo).expect("create repo dir");
|
|
|
|
// Initialize git repo and add files.
|
|
let git_init = tokio::process::Command::new("git")
|
|
.args(["init"])
|
|
.current_dir(&repo)
|
|
.stdout(Stdio::null())
|
|
.stderr(Stdio::null())
|
|
.status()
|
|
.await
|
|
.expect("git init");
|
|
assert!(git_init.success(), "git init should succeed");
|
|
|
|
// Configure git user for the commit.
|
|
let _ = tokio::process::Command::new("git")
|
|
.args(["config", "user.email", "test@test.com"])
|
|
.current_dir(&repo)
|
|
.stdout(Stdio::null())
|
|
.stderr(Stdio::null())
|
|
.status()
|
|
.await;
|
|
let _ = tokio::process::Command::new("git")
|
|
.args(["config", "user.name", "Test"])
|
|
.current_dir(&repo)
|
|
.stdout(Stdio::null())
|
|
.stderr(Stdio::null())
|
|
.status()
|
|
.await;
|
|
|
|
// Create .gitignore, a tracked file, and an ignored file.
|
|
fs::write(repo.join(".gitignore"), "*.log\nbuild/\n").expect("write .gitignore");
|
|
fs::write(repo.join("tracked.txt"), "i-am-tracked").expect("write tracked.txt");
|
|
fs::write(repo.join("ignored.log"), "i-should-be-filtered").expect("write ignored.log");
|
|
fs::create_dir_all(repo.join("build")).expect("create build dir");
|
|
fs::write(repo.join("build/output.bin"), "build-artifact").expect("write build/output.bin");
|
|
|
|
// git add + commit so git ls-files works.
|
|
let _ = tokio::process::Command::new("git")
|
|
.args(["add", "."])
|
|
.current_dir(&repo)
|
|
.stdout(Stdio::null())
|
|
.stderr(Stdio::null())
|
|
.status()
|
|
.await
|
|
.expect("git add");
|
|
let _ = tokio::process::Command::new("git")
|
|
.args(["commit", "-m", "init"])
|
|
.current_dir(&repo)
|
|
.stdout(Stdio::null())
|
|
.stderr(Stdio::null())
|
|
.status()
|
|
.await
|
|
.expect("git commit");
|
|
|
|
// ---------------------------------------------------------------
|
|
// Step 3 — Upload WITH gitignore filtering (default).
|
|
// ---------------------------------------------------------------
|
|
let repo_str = repo.to_str().expect("repo path is UTF-8");
|
|
guard
|
|
.upload_with_gitignore(repo_str, "/sandbox/filtered", &repo)
|
|
.await
|
|
.expect("upload with gitignore filtering");
|
|
|
|
// ---------------------------------------------------------------
|
|
// Step 4 — Verify: tracked file exists, ignored files do not.
|
|
// ---------------------------------------------------------------
|
|
// Download the uploaded directory and verify contents.
|
|
let download_dir = tmpdir.path().join("verify");
|
|
fs::create_dir_all(&download_dir).expect("create verify dir");
|
|
let download_str = download_dir.to_str().expect("verify path is UTF-8");
|
|
|
|
guard
|
|
.download("/sandbox/filtered", download_str)
|
|
.await
|
|
.expect("download filtered upload");
|
|
|
|
// Filtered uploads of a directory preserve the source basename, so
|
|
// contents land under `<download>/repo/...` (matches `openshell sandbox
|
|
// upload <dir> <dest>` semantics from the unfiltered path).
|
|
let uploaded_root = download_dir.join("repo");
|
|
|
|
// tracked.txt should be present.
|
|
let tracked = fs::read_to_string(uploaded_root.join("tracked.txt"))
|
|
.expect("tracked.txt should exist after filtered upload");
|
|
assert_eq!(tracked, "i-am-tracked", "tracked.txt content mismatch");
|
|
|
|
// .gitignore itself should be present (it's tracked).
|
|
assert!(
|
|
uploaded_root.join(".gitignore").exists(),
|
|
".gitignore should be uploaded (it's a tracked file)"
|
|
);
|
|
|
|
// ignored.log should NOT be present.
|
|
assert!(
|
|
!uploaded_root.join("ignored.log").exists(),
|
|
"ignored.log should be filtered out by .gitignore"
|
|
);
|
|
|
|
// build/ directory should NOT be present.
|
|
assert!(
|
|
!uploaded_root.join("build").exists(),
|
|
"build/ directory should be filtered out by .gitignore"
|
|
);
|
|
|
|
// ---------------------------------------------------------------
|
|
// Cleanup.
|
|
// ---------------------------------------------------------------
|
|
guard.cleanup().await;
|
|
}
|
|
|
|
/// Verify that uploading a single tracked file from inside a git repo does not
|
|
/// expand to the entire repository.
|
|
#[tokio::test]
|
|
async fn upload_single_file_from_git_repo_only_uploads_that_file() {
|
|
let mut guard =
|
|
SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready")
|
|
.await
|
|
.expect("sandbox create --keep");
|
|
|
|
let tmpdir = tempfile::tempdir().expect("create tmpdir");
|
|
let repo = tmpdir.path().join("repo");
|
|
fs::create_dir_all(repo.join("nested")).expect("create repo dir");
|
|
|
|
let git_init = tokio::process::Command::new("git")
|
|
.args(["init"])
|
|
.current_dir(&repo)
|
|
.stdout(Stdio::null())
|
|
.stderr(Stdio::null())
|
|
.status()
|
|
.await
|
|
.expect("git init");
|
|
assert!(git_init.success(), "git init should succeed");
|
|
|
|
fs::write(repo.join(".gitignore"), "*.log\n").expect("write .gitignore");
|
|
fs::write(repo.join("nested/config.txt"), "single-file-from-repo").expect("write config.txt");
|
|
fs::write(repo.join("tracked.txt"), "should-not-upload").expect("write tracked.txt");
|
|
fs::write(repo.join("ignored.log"), "ignored").expect("write ignored.log");
|
|
|
|
let local_path = repo.join("nested/config.txt");
|
|
let local_str = local_path.to_str().expect("local path is UTF-8");
|
|
guard
|
|
.upload_with_gitignore(local_str, "/sandbox/single-file", &repo)
|
|
.await
|
|
.expect("upload single tracked file with gitignore");
|
|
|
|
let download_dir = tmpdir.path().join("single-file-download");
|
|
fs::create_dir_all(&download_dir).expect("create download dir");
|
|
let download_str = download_dir.to_str().expect("download path is UTF-8");
|
|
|
|
guard
|
|
.download("/sandbox/single-file", download_str)
|
|
.await
|
|
.expect("download uploaded single file");
|
|
|
|
let uploaded = fs::read_to_string(download_dir.join("config.txt")).expect("read config.txt");
|
|
assert_eq!(uploaded, "single-file-from-repo");
|
|
assert!(
|
|
!download_dir.join("tracked.txt").exists(),
|
|
"tracked.txt should not have been uploaded"
|
|
);
|
|
assert!(
|
|
!download_dir.join("ignored.log").exists(),
|
|
"ignored.log should not have been uploaded"
|
|
);
|
|
|
|
guard.cleanup().await;
|
|
}
|
|
|
|
/// Pre-populate a single file on the sandbox side via `sandbox exec` and pull
|
|
/// it back with `sandbox download`. Exercises `sandbox_sync_down_file` in
|
|
/// isolation (no upload phase).
|
|
#[tokio::test]
|
|
async fn sandbox_download_file_only() {
|
|
let mut guard =
|
|
SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready")
|
|
.await
|
|
.expect("sandbox create --keep");
|
|
|
|
guard
|
|
.exec(&["sh", "-c", "printf greeting-payload > /sandbox/greeting.txt"])
|
|
.await
|
|
.expect("seed greeting.txt inside the sandbox");
|
|
|
|
let tmpdir = tempfile::tempdir().expect("create tmpdir");
|
|
let dest = tmpdir.path().join("out");
|
|
fs::create_dir_all(&dest).expect("create download dir");
|
|
let dest_str = dest.to_str().expect("dest path is UTF-8");
|
|
|
|
guard
|
|
.download("/sandbox/greeting.txt", dest_str)
|
|
.await
|
|
.expect("download greeting.txt");
|
|
|
|
let actual = fs::read_to_string(dest.join("greeting.txt")).expect("read greeting.txt");
|
|
assert_eq!(
|
|
actual, "greeting-payload",
|
|
"downloaded file content mismatch"
|
|
);
|
|
|
|
guard.cleanup().await;
|
|
}
|
|
|
|
/// Pre-populate a small directory tree on the sandbox side via `sandbox exec`
|
|
/// and pull it recursively with `sandbox download`. Exercises
|
|
/// `sandbox_sync_down_directory` in isolation (no upload phase).
|
|
#[tokio::test]
|
|
async fn sandbox_download_directory_only() {
|
|
let mut guard =
|
|
SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready")
|
|
.await
|
|
.expect("sandbox create --keep");
|
|
|
|
guard
|
|
.exec(&[
|
|
"sh",
|
|
"-c",
|
|
"mkdir -p /sandbox/tree/sub \
|
|
&& printf top-level > /sandbox/tree/root.txt \
|
|
&& printf nested > /sandbox/tree/sub/child.txt",
|
|
])
|
|
.await
|
|
.expect("seed directory tree inside the sandbox");
|
|
|
|
let tmpdir = tempfile::tempdir().expect("create tmpdir");
|
|
let dest = tmpdir.path().join("out");
|
|
fs::create_dir_all(&dest).expect("create download dir");
|
|
let dest_str = dest.to_str().expect("dest path is UTF-8");
|
|
|
|
guard
|
|
.download("/sandbox/tree", dest_str)
|
|
.await
|
|
.expect("download directory tree");
|
|
|
|
let root = fs::read_to_string(dest.join("root.txt")).expect("read root.txt");
|
|
assert_eq!(root, "top-level", "root.txt content mismatch");
|
|
let child = fs::read_to_string(dest.join("sub/child.txt")).expect("read sub/child.txt");
|
|
assert_eq!(child, "nested", "sub/child.txt content mismatch");
|
|
|
|
guard.cleanup().await;
|
|
}
|
|
|
|
/// Assert that an error string carries the two markers of a remote-side
|
|
/// canonicalisation refusal: `resolves to` (proving `realpath -e` ran) and
|
|
/// `outside the` + `sandbox workspace` (proving the boundary check fired).
|
|
///
|
|
/// miette renders the long single-line error wrapped across multiple lines
|
|
/// at terminal width and inserts a `│ ` continuation marker, so a single
|
|
/// long substring like `"outside the sandbox workspace"` is fragile —
|
|
/// match the short markers individually instead.
|
|
fn assert_resolves_outside_workspace(err: &str, label: &str) {
|
|
assert!(
|
|
err.contains("resolves to") && err.contains("outside the") && err.contains("sandbox workspace"),
|
|
"expected resolves-outside-workspace error for {label}, got: {err}"
|
|
);
|
|
}
|
|
|
|
/// Regression for the Codex high-severity finding: `validate_sandbox_source_path`
|
|
/// is purely lexical, so a sandbox-side symlink that escapes `/sandbox` could
|
|
/// slip through and let `tar -C` follow the link into the host filesystem.
|
|
/// `resolve_sandbox_source_path` re-validates after `realpath -e` resolves
|
|
/// every component. Cover both shapes from the Codex example: the symlink as
|
|
/// the full source (directory case) and the symlink as a component of the
|
|
/// source path (file case).
|
|
#[tokio::test]
|
|
async fn sandbox_download_rejects_symlinks_pointing_outside_workspace() {
|
|
let mut guard =
|
|
SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready")
|
|
.await
|
|
.expect("sandbox create --keep");
|
|
|
|
guard
|
|
.exec(&[
|
|
"sh",
|
|
"-c",
|
|
"ln -s /etc /sandbox/etc-link && ln -s /etc/passwd /sandbox/passwd-link",
|
|
])
|
|
.await
|
|
.expect("plant escape symlinks inside the sandbox");
|
|
|
|
let tmpdir = tempfile::tempdir().expect("create tmpdir");
|
|
let dest = tmpdir.path().join("out");
|
|
fs::create_dir_all(&dest).expect("create download dir");
|
|
let dest_str = dest.to_str().expect("dest path is UTF-8");
|
|
|
|
// Directory-source symlink: /sandbox/etc-link -> /etc
|
|
let err = guard
|
|
.download("/sandbox/etc-link", dest_str)
|
|
.await
|
|
.expect_err("download of a directory symlink to /etc must be refused");
|
|
assert_resolves_outside_workspace(&err, "directory symlink");
|
|
|
|
// File-source symlink: /sandbox/passwd-link -> /etc/passwd
|
|
let err = guard
|
|
.download("/sandbox/passwd-link", dest_str)
|
|
.await
|
|
.expect_err("download of a file symlink to /etc/passwd must be refused");
|
|
assert_resolves_outside_workspace(&err, "file symlink");
|
|
|
|
// Path with a symlinked component: /sandbox/etc-link/passwd
|
|
let err = guard
|
|
.download("/sandbox/etc-link/passwd", dest_str)
|
|
.await
|
|
.expect_err("download via a symlinked path component must be refused");
|
|
assert_resolves_outside_workspace(&err, "symlinked component");
|
|
|
|
// Sanity check: the host destination should still be empty — no file from
|
|
// /etc should have leaked through on any of the three attempts.
|
|
assert!(
|
|
!dest.join("passwd").exists() && !dest.join("etc-link").exists(),
|
|
"no file should have been written when the source resolves outside the workspace"
|
|
);
|
|
|
|
guard.cleanup().await;
|
|
}
|
|
|
|
/// Regression for the Codex medium-severity finding: the single-file tar
|
|
/// invocation must wrap the basename behind a `--` separator, otherwise a
|
|
/// sandbox-side filename whose basename starts with `--` (e.g. created by a
|
|
/// malicious agent inside the sandbox) is parsed by GNU tar as an option
|
|
/// rather than a member to archive.
|
|
#[tokio::test]
|
|
async fn sandbox_download_handles_dash_leading_basename() {
|
|
let mut guard =
|
|
SandboxGuard::create_keep(&["sh", "-c", "echo Ready && sleep infinity"], "Ready")
|
|
.await
|
|
.expect("sandbox create --keep");
|
|
|
|
// Quoting the redirect target keeps the leading dashes intact across the
|
|
// shell parse; the sandbox-side filesystem ends up with a basename that
|
|
// would be parsed as an option by an unprotected tar invocation.
|
|
guard
|
|
.exec(&[
|
|
"sh",
|
|
"-c",
|
|
"printf dash-payload > '/sandbox/--checkpoint-action=evil'",
|
|
])
|
|
.await
|
|
.expect("seed dash-leading file inside the sandbox");
|
|
|
|
let tmpdir = tempfile::tempdir().expect("create tmpdir");
|
|
let dest = tmpdir.path().join("out");
|
|
fs::create_dir_all(&dest).expect("create download dir");
|
|
let dest_str = dest.to_str().expect("dest path is UTF-8");
|
|
|
|
guard
|
|
.download("/sandbox/--checkpoint-action=evil", dest_str)
|
|
.await
|
|
.expect("download dash-leading basename");
|
|
|
|
let actual =
|
|
fs::read_to_string(dest.join("--checkpoint-action=evil")).expect("read dash-leading file");
|
|
assert_eq!(
|
|
actual, "dash-payload",
|
|
"dash-leading file content mismatch — tar may have parsed the basename as an option"
|
|
);
|
|
|
|
guard.cleanup().await;
|
|
}
|