CyberCoveLLC/Triple-C
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 35 Wiki Activity

Fix root scrub deleting host files through symlinked parents (C1)

Browse Source 
SNAPSHOT_SCRUB_PATHS is expanded by /bin/sh inside the container as root.
For an entry ending `/*` the parent is a path *component*, resolved by both
the glob expansion and the `rm -rf`. The agent has passwordless sudo, so
`ln -s /workspace/myproject /var/log/apt` turned the next commit into a
recursive delete of the user's real files on the host — reproduced end to
end against a live container.

snapshot_scrub_script now routes every deletion through one `scrub_in`
function that validates the parent before touching anything inside it:
`cd -P` for a TOCTOU-free handle, `pwd -P` equality to reject a symlinked
component, a hardcoded containment allowlist that is deliberately not
derived from the path list, and an st_dev comparison against `/` so a bind
mount or a volume is refused even though it is not a symlink. It fails
closed when `stat` is missing.

Also:
- /tmp/triple-c-drops/* and /tmp/clipboard_*.png are age-limited to 14 days
  instead of scrubbed unconditionally. They hold the user's own files, and
  scrubbing them meant "drop a file, change a setting, lose it silently";
  removing them from the list would restore unbounded growth instead.
- M11: scrub_writable_layer returns a ScrubOutcome and skips cleanly when
  the container is not running, so a migration no longer logs "could not
  run … committing anyway" on every run.
- M12: the scrub exec is bounded by a 120s timeout; on expiry it logs and
  lets the commit proceed.
- The script is now fold-safe (self-terminating lines, no `#` comments).
  disk.rs folds it onto one `RUN` line and the previous form was a
  `"do" unexpected` syntax error there, so compaction had been scrubbing
  nothing at all.

Tests: the substring check on the script text is replaced by a behavioural
test that runs the real generated script with a real symlink planted in a
throwaway tree, plus structural tests over each containment construct and
a `sh -n` check of the folded form.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
This commit is contained in:
Claude
2026-08-23 11:11:57 -07:00
parent 0003793abb
commit ae3ca8cda4
1 changed files with 656 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src-tauri/src/docker/container.rs
+656 -47
View File
@@ -2041,32 +2041,73 @@ chmod 600 "$HOME/.aws/credentials""#;
///
/// A snapshot is the user's system layer: their packages, their `/opt`, their
/// `/var/lib/postgresql`. Nothing here may guess. Every entry is an absolute
/// path anchored to a directory Triple-C or a package manager owns, and the
/// three globs are anchored to `/tmp` specifically:
/// path anchored to a directory Triple-C or a package manager owns:
///
/// * `/workspace/{mount_name}` subtrees are **host bind mounts** — the user's
/// real project directories. No entry may ever reach one, which is why no
/// pattern here starts with `/workspace`.
/// * The only bind mounts under `/tmp` are `/tmp/.host-ca` and `/tmp/.host-aws`
/// (both read-only). A leading-dot name is not matched by a shell glob, and
/// none of the three patterns share their prefix, so neither can be selected
/// even by accident.
/// none of these patterns share their prefix.
/// * The apt entries keep their parent directory and remove only its contents
/// (`lists/*`, `archives/*.deb`, `apt/*`); `apt-get` is unhappy when the
/// directories themselves are missing.
///
/// A unit test pins the list, because the blast radius of a wrong entry here is
/// a user's data and the code that consumes it is a shell string.
/// ## Why a safe-looking *pattern* is not enough (C1)
///
/// The list used to be the whole of the defence, and that was wrong. These
/// patterns are expanded by `/bin/sh` inside the container running as **root**,
/// and for an entry ending `/*` the parent is a *path component*: both the glob
/// expansion and the `rm -rf` that follows resolve it. The agent in the
/// container has passwordless sudo, so anything able to run
/// `ln -s /workspace/myproject /var/log/apt` turns the next commit into a
/// recursive delete of the user's real files **on the host**. That was
/// reproduced end to end against a live container — a bind mount emptied by a
/// scrub whose path list named no `/workspace` anywhere.
///
/// (The entries whose glob is in the last component are the benign case: there
/// the *match* is the symlink, and `rm -rf -- link` unlinks the link and stops.
/// The distinction is not one a reviewer should have to make per entry, so the
/// script defends both alike.)
///
/// So these entries are only half the contract. The other half is
/// [`snapshot_scrub_script`], which validates the parent directory — not
/// reached through a symlink, not on another filesystem, inside
/// [`SCRUB_CONTAINMENT_PREFIXES`] — before it deletes anything inside it. That
/// is also why every entry here must keep its glob in the **final component**:
/// the parent has to be literal for the script to be able to check it at all. A
/// test enforces it.
///
/// A unit test also pins the list itself, because the blast radius of a wrong
/// entry here is a user's data and the code that consumes it is a shell string.
pub(crate) const SNAPSHOT_SCRUB_PATHS: &[&str] = &[
// Agent scratchpads. The user's global CLAUDE.md instructs every agent to
// put temporary files under a scratchpad directory in /tmp, so this is
// where a long-running project's writable layer actually goes.
//
// Not age-limited, and worth being explicit about why that is only *just*
// safe: the scrub runs as root against a container that is still running,
// so a live Claude Code session or a `triple-c-scheduler` task writing here
// has its scratchpad pulled out from under it mid-write. Both callers stop
// the container within a line or two — `start_project_container` in
// `project_commands.rs` stops and removes it, `migrate_project_to_base`
// stops it — so the process that would notice is about to be killed anyway.
// That is a property of those two call sites and not of this entry: a third
// caller scrubbing a container it means to keep running would be corrupting
// a live session, and would need to age-limit this the way the two below
// are.
"/tmp/claude-*",
// Files drag-dropped into a terminal, staged by
// `commands/terminal_commands.rs` at up to 256 MiB each. Nothing in the
// repo deletes them.
// `commands/terminal_commands.rs` at up to 256 MiB each, and one PNG per
// pasted image from the same module. Nothing else in the repo deletes
// either, so leaving them out of this list restores unbounded growth — but
// they are the user's *own* files, and often the only copy inside the
// container of something handed to the agent seconds ago by a path that is
// still sitting in the conversation. Scrubbing them unconditionally meant
// "drop a file, change any of the 24 settings that trigger a recreation,
// lose it silently". Both are therefore age-limited rather than removed —
// see [`SCRUB_MIN_AGE_DAYS`].
"/tmp/triple-c-drops/*",
// One PNG per pasted image, from the same module. Also never deleted.
"/tmp/clipboard_*.png",
// Runtime apt debris. `browser_view/install.rs` and
// `container/triple-c-playwright-heal` both run `apt-get install` inside a
@@ -2077,32 +2118,172 @@ pub(crate) const SNAPSHOT_SCRUB_PATHS: &[&str] = &[
"/var/log/dpkg.log",
];
/// Entries of [`SNAPSHOT_SCRUB_PATHS`] that are only deleted once a match's own
/// mtime is older than the given number of days. Anything not named here is
/// deleted whenever it is present.
///
/// Two weeks is chosen against the thing that goes wrong: a recreation can
/// happen seconds after a drop, and no conversation is still quoting a path it
/// was given a fortnight ago. It is a compromise, not a proof — the alternative
/// of dropping these patterns entirely would be silent unbounded growth in a
/// directory nothing else ever cleans.
const SCRUB_MIN_AGE_DAYS: &[(&str, u32)] = &[
("/tmp/triple-c-drops/*", 14),
("/tmp/clipboard_*.png", 14),
];
/// The only directory trees [`snapshot_scrub_script`] will operate in, checked
/// against the *resolved* parent directory at run time inside the container.
///
/// Deliberately **not** derived from [`SNAPSHOT_SCRUB_PATHS`]: it is the
/// backstop for the case where that list is itself wrong. An entry added under
/// `/workspace`, `/home/claude` or `/etc` fails this check and deletes nothing,
/// however plausible it looked in review.
const SCRUB_CONTAINMENT_PREFIXES: &[&str] = &["/tmp", "/var/log", "/var/lib/apt", "/var/cache/apt"];
/// Marker the scrub script prints so the byte total can be read back out of the
/// exec's interleaved stdout/stderr.
const SCRUB_MARKER: &str = "###TRIPLE-C-SCRUBBED ";
/// The `/bin/sh` program run inside the container to perform the scrub.
/// Split a [`SNAPSHOT_SCRUB_PATHS`] entry into the literal directory it is
/// anchored to and the glob to expand inside it.
///
/// Built here rather than inline so a test can read it. The path list is
/// interpolated **unquoted** on the `for` line, which is the whole point: the
/// shell expands the three globs there. An unmatched glob expands to itself,
/// the `[ -e ]` guard then fails, and the entry is skipped — so a pattern that
/// matches nothing is a no-op rather than an `rm` of a literal path.
/// Inside the loop `$p` is quoted, so a filename containing whitespace is one
/// argument.
/// `None` for anything that is not an absolute path with a non-empty final
/// component — which a test rules out, but the script generator must not have
/// to trust that, since what it emits runs as root.
fn split_scrub_pattern(pattern: &str) -> Option<(&str, &str)> {
let (dir, glob) = pattern.rsplit_once('/')?;
if !pattern.starts_with('/') || glob.is_empty() {
return None;
}
// A top-level entry such as `/dpkg.log` leaves an empty parent; anchor it.
Some((if dir.is_empty() { "/" } else { dir }, glob))
}
/// The `/bin/sh` program run inside the container to perform the scrub.
pub(crate) fn snapshot_scrub_script() -> String {
snapshot_scrub_script_under("")
}
/// [`snapshot_scrub_script`] with every absolute path re-anchored under `root`.
///
/// ## What the script does
///
/// One `scrub_in <pattern> <min-age-days, or `-`>` call per entry in
/// [`SNAPSHOT_SCRUB_PATHS`], with the pattern single-quoted so it reaches the
/// function unexpanded — unquoted, `/bin/sh` would expand the glob against the
/// script's own working directory before `scrub_in` ever ran. The function
/// splits it into parent and glob (`${1%/*}` / `${1##*/}`, which is why the
/// glob must live in the final component), and expands the glob only once the
/// working directory *is* the validated parent. Quoted everywhere it is used,
/// so a filename containing whitespace stays one argument; an unmatched glob
/// expands to itself and is skipped by the existence guard.
///
/// Passing the whole pattern rather than the two halves also keeps each entry
/// readable verbatim in the compaction `RUN` line — `disk.rs` asserts exactly
/// that, to catch a second forked copy of the list.
///
/// ## The containment guarantee (C1)
///
/// `scrub_in` is the only place in the script that deletes anything, and it
/// does so only after four checks. In order:
///
/// 0. `cd -P` into the parent **first**. Everything after that is relative to
/// the inode that gets validated, so re-pointing the *path* afterwards
/// cannot redirect the `rm`. A working directory is the only TOCTOU-free
/// handle `/bin/sh` offers.
/// 1. `pwd -P` — the fully resolved path — must equal what was asked for.
/// A symlinked component anywhere in the parent lands somewhere else, and
/// this is what catches `ln -s /workspace/myproject /var/log/apt`.
/// 2. The resolved path must be inside [`SCRUB_CONTAINMENT_PREFIXES`]. A
/// symlink is not the only way to name the wrong directory; this refuses to
/// operate outside the trees the scrub owns whatever the path list says.
/// 3. It must be on the same filesystem as the root. Check 1 does not see a
/// *bind mount*, which is not a symlink — but a bind mount and a named
/// volume each have a different `st_dev` from the overlay, and neither is
/// part of the writable layer the commit is about to capture. So anything on
/// another device is both dangerous to delete and pointless to. No `stat`
/// means no comparison, which means no deletion: this fails closed.
///
/// Inside the loop, an expansion carrying a path separator means the list has
/// changed shape and is skipped, and `rm --one-file-system` (probed, because
/// busybox's `rm` would reject it) refuses to recurse across a mount planted
/// *below* a validated directory.
///
/// ## Why every line ends in `;`
///
/// `disk.rs` folds this script onto a single `RUN` line for the compaction
/// build, joining non-blank lines with a space. That is only a join and not a
/// rewrite if each line already terminates its own statement — the previous
/// version did not, and its folded form was a `"do" unexpected` syntax error,
/// so compaction had been running no scrub at all. It also means the script
/// carries **no `#` comments**: folded, one would swallow the rest of the
/// program. A test pins both the multi-line and the folded form.
///
/// ## Why `root` exists
///
/// `""` in production, which leaves the paths exactly as written. A non-empty
/// root lets the real generated script be run by a real `/bin/sh`, with real
/// symlinks planted in it, inside a throwaway directory tree — because
/// containment is a *runtime* property and the test this replaces
/// (`!dockerfile.contains("/workspace")`) was a substring check over the script
/// text that the exploitable version passed.
fn snapshot_scrub_script_under(root: &str) -> String {
let calls: String = SNAPSHOT_SCRUB_PATHS
.iter()
.filter_map(|pattern| {
// Validate the shape here even though the shell re-derives it: an
// entry the script could not split is one it must not be handed.
split_scrub_pattern(pattern)?;
let age = SCRUB_MIN_AGE_DAYS
.iter()
.find(|(p, _)| p == pattern)
.map(|(_, days)| days.to_string())
.unwrap_or_else(|| "-".to_string());
Some(format!("scrub_in '{root}{pattern}' '{age}';\n"))
})
.collect();
let allowed: String = SCRUB_CONTAINMENT_PREFIXES
.iter()
.map(|p| format!("{root}{p}|{root}{p}/*"))
.collect::<Vec<_>>()
.join("|");
format!(
r#"total=0
for p in {paths}; do
[ -e "$p" ] || continue
sz=$(du -sb "$p" 2>/dev/null | cut -f1)
case "$sz" in ''|*[!0-9]*) sz=0 ;; esac
rm -rf -- "$p" 2>/dev/null && total=$((total + sz))
done
echo "{marker}$total"
r#"total=0;
rootdev=$(stat -c %d '{root}/' 2>/dev/null);
rmopt=;
rm --one-file-system --help >/dev/null 2>&1 && rmopt=--one-file-system;
scrub_in() {{
n=$(
d=${{1%/*}}; [ -n "$d" ] || d=/;
g=${{1##*/}};
cd -P "$d" 2>/dev/null || exit 0;
[ "$(pwd -P)" = "$d" ] || exit 0;
case "$d" in {allowed}) ;; *) exit 0 ;; esac;
[ -n "$rootdev" ] || exit 0;
[ "$(stat -c %d . 2>/dev/null)" = "$rootdev" ] || exit 0;
acc=0;
for p in $g; do
case "$p" in */*|.|..) continue ;; esac;
{{ [ -e "$p" ] || [ -L "$p" ]; }} || continue;
[ "$2" = "-" ] || [ -n "$(find "$p" -maxdepth 0 -mtime +"$2" -print 2>/dev/null)" ] || continue;
sz=$(du -sb -- "$p" 2>/dev/null | cut -f1);
case "$sz" in ''|*[!0-9]*) sz=0 ;; esac;
rm -rf $rmopt -- "$p" 2>/dev/null && acc=$((acc + sz));
done;
echo "$acc";
);
case "$n" in ''|*[!0-9]*) n=0 ;; esac;
total=$((total + n));
}};
{calls}echo "{marker}$total";
exit 0
"#,
paths = SNAPSHOT_SCRUB_PATHS.join(" "),
root = root,
allowed = allowed,
calls = calls,
marker = SCRUB_MARKER,
)
}
@@ -2117,24 +2298,118 @@ fn parse_scrub_total(output: &str) -> Option<u64> {
.find_map(|line| line.trim().strip_prefix(SCRUB_MARKER)?.trim().parse().ok())
}
/// How long [`scrub_writable_layer`] waits for its exec before the commit goes
/// ahead without it.
///
/// The scrub is a `du -sb` plus an `rm -rf` over a tree a running agent may
/// still be writing to, and it sits on the critical path of every recreate and
/// every migration with the UI parked on "Saving container state…" and no way
/// to cancel. Two minutes is an order of magnitude more than the measured cost
/// on a 4.48 GB layer, and far less than the point where a user force-quits.
const SCRUB_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(120);
/// What [`scrub_writable_layer`] actually did.
///
/// The distinction that earns this type is [`ScrubOutcome::NotRunning`] versus
/// [`ScrubOutcome::Failed`]: "there was nothing to exec into" is routine, while
/// "the exec ran and broke" is the one case worth a warning in the log.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ScrubOutcome {
/// The scrub ran to completion and freed this many bytes — possibly zero,
/// which is a real answer.
Reclaimed(u64),
/// The container is not running, so there is no `docker exec` to run in.
/// Not a failure: a stopped container's writable layer is not growing, and
/// on the migration path it was already scrubbed while it was.
NotRunning,
/// The container was running, but the scrub did not finish within
/// [`SCRUB_TIMEOUT`].
TimedOut,
/// The container was running and the scrub genuinely failed.
Failed,
}
impl ScrubOutcome {
/// Bytes reclaimed. Zero for every outcome that is not a completed scrub.
pub fn bytes(&self) -> u64 {
match self {
Self::Reclaimed(bytes) => *bytes,
_ => 0,
}
}
}
/// Delete the throwaway files listed in [`SNAPSHOT_SCRUB_PATHS`] from a
/// container's writable layer so the commit that follows does not bake them in.
///
/// Runs as **root**: the apt debris is root-owned while the scratchpads belong
/// to `claude`, and root can remove both.
/// to `claude`, and root can remove both. That is also what makes
/// [`snapshot_scrub_script`]'s containment checks load-bearing rather than
/// decorative.
///
/// **Never fails the caller, by design.** A scrub is an optimisation; a commit
/// is the only copy of the user's system layer. Losing some disk is a strictly
/// better outcome than refusing to snapshot, so every failure here is a log
/// line and nothing more. Note that this is a `docker exec` and therefore only
/// works while the container runs: `migrate_project_to_base` stops its
/// container before the pre-swap commit, so it calls this itself beforehand
/// rather than relying on the call inside [`commit_container_snapshot`].
pub async fn scrub_writable_layer(container_id: &str) -> u64 {
/// line and nothing more — including the timeout, which stops waiting and lets
/// the commit proceed rather than leaving the UI on "Saving container state…"
/// with no way out.
///
/// ## Why it checks whether the container is running (M11)
///
/// This is a `docker exec`, so it only works on a running container.
/// `migrate_project_to_base` knows that: it scrubs the container itself and
/// *then* stops it, because the pre-swap commit is the largest snapshot
/// Triple-C ever takes. The call inside [`commit_container_snapshot`] therefore
/// arrives at a stopped container and could only ever fail, which logged
/// "could not run … committing anyway" on every single migration — training
/// the reader to ignore the one line that would matter if the scrub had really
/// broken. Asking first turns that into a debug line and keeps the warning
/// meaningful.
pub async fn scrub_writable_layer(container_id: &str) -> ScrubOutcome {
match is_container_running(container_id).await {
Ok(true) => {}
Ok(false) => {
log::debug!(
"Skipping pre-commit scrub of container {}: it is not running, so there is nothing to exec into and its writable layer is not growing",
container_id
);
return ScrubOutcome::NotRunning;
}
Err(e) => {
// Only Docker being unreachable reaches here, in which case the
// commit this precedes is about to fail on its own — but say so
// rather than reporting a skip that never happened.
log::warn!(
"Could not tell whether container {} is running ({}); skipping the pre-commit scrub and committing anyway",
container_id,
e
);
return ScrubOutcome::Failed;
}
}
let script = snapshot_scrub_script();
let cmd = vec!["/bin/sh".to_string(), "-c".to_string(), script];
let exec = crate::docker::exec::exec_oneshot_as(container_id, "root", cmd, Vec::new());
match crate::docker::exec::exec_oneshot_as(container_id, "root", cmd, Vec::new()).await {
// M12: nothing under `docker/` has a timeout, and this is the one call on
// the critical path of every recreate. Dropping the future stops us
// *waiting*; the `sh` keeps running inside the container and its deletions
// are still valid, they just stop counting towards the total. That is the
// right trade — the commit that follows is merely a little larger.
let exec_result = match tokio::time::timeout(SCRUB_TIMEOUT, exec).await {
Ok(result) => result,
Err(_) => {
log::warn!(
"Pre-commit scrub of container {} did not finish within {}s; committing anyway",
container_id,
SCRUB_TIMEOUT.as_secs()
);
return ScrubOutcome::TimedOut;
}
};
match exec_result {
Ok((output, _exit_code)) => match parse_scrub_total(&output) {
Some(bytes) => {
if bytes > 0 {
@@ -2144,7 +2419,7 @@ pub async fn scrub_writable_layer(container_id: &str) -> u64 {
bytes as f64 / 1_048_576.0
);
}
bytes
ScrubOutcome::Reclaimed(bytes)
}
None => {
log::warn!(
@@ -2152,7 +2427,7 @@ pub async fn scrub_writable_layer(container_id: &str) -> u64 {
container_id,
output.trim()
);
0
ScrubOutcome::Failed
}
},
Err(e) => {
@@ -2161,7 +2436,7 @@ pub async fn scrub_writable_layer(container_id: &str) -> u64 {
container_id,
e
);
0
ScrubOutcome::Failed
}
}
}
@@ -2198,6 +2473,10 @@ pub async fn scrub_writable_layer(container_id: &str) -> u64 {
///
/// See [`SNAPSHOT_SCRUB_PATHS`]. Every commit stacks a layer, so a file present
/// here is a file the project's image carries for the rest of its life.
///
/// The scrub is a no-op on a container that is already stopped — the migration
/// path scrubs it itself while it is still running and stops it before getting
/// here — and it can never fail this function; see [`scrub_writable_layer`].
pub async fn commit_container_snapshot(container_id: &str, project: &Project) -> Result<(), String> {
let docker = get_docker()?;
let image_name = get_snapshot_image_name(project);
@@ -2206,7 +2485,7 @@ pub async fn commit_container_snapshot(container_id: &str, project: &Project) ->
// stacks a layer and never rewrites one, so anything present at this
// instant is paid for permanently — see [`SNAPSHOT_SCRUB_PATHS`]. Failure
// is swallowed inside; a scrub must never be able to block a snapshot.
scrub_writable_layer(container_id).await;
let scrub = scrub_writable_layer(container_id).await;
// Parse repo:tag
let (repo, tag) = match image_name.rsplit_once(':') {
@@ -2232,7 +2511,13 @@ pub async fn commit_container_snapshot(container_id: &str, project: &Project) ->
.await
.map_err(|e| format!("Failed to commit container snapshot: {}", e))?;
log::info!("Committed container {} as snapshot {}:{}", container_id, repo, tag);
log::info!(
"Committed container {} as snapshot {}:{} ({:.2} MB dropped by the pre-commit scrub)",
container_id,
repo,
tag,
scrub.bytes() as f64 / 1_048_576.0
);
Ok(())
}
@@ -3576,13 +3861,18 @@ mod tests {
assert!(blind.left_something_behind());
}
// ── Pre-commit scrub (A1) ────────────────────────────────────────────────
// ── Pre-commit scrub (A1, C1) ────────────────────────────────────────────
#[test]
fn no_scrub_path_can_reach_a_host_bind_mount() {
// `/workspace/{mount_name}` is the user's own project directory, bound
// in from the host. Nothing in this list may ever name one — and the
// two read-only host mounts under /tmp must be equally unreachable.
//
// Necessary, not sufficient: a pattern that looks like this can still
// resolve into a bind mount through a symlinked parent, which is what
// the script's own checks exist for. See
// `the_scrub_script_refuses_a_symlinked_parent_...` below.
for path in SNAPSHOT_SCRUB_PATHS {
assert!(
path.starts_with('/'),
@@ -3626,6 +3916,69 @@ mod tests {
}
}
#[test]
fn every_scrub_pattern_keeps_its_glob_in_the_final_component() {
// The script can only validate a parent directory it can name, so the
// parent has to be literal. An entry like `/var/*/apt/*` would put a
// glob in a path *component*, which is the shape that made C1
// exploitable in the first place.
for pattern in SNAPSHOT_SCRUB_PATHS {
let (dir, glob) = split_scrub_pattern(pattern)
.unwrap_or_else(|| panic!("{} has no literal parent directory", pattern));
assert!(
!dir.contains(['*', '?', '[']),
"{}: the parent {} is itself a glob, so the script cannot check it",
pattern,
dir
);
assert!(!glob.is_empty(), "{} has an empty final component", pattern);
// Both halves are single-quoted into the script; a quote or a blank
// would break out of that quoting and change what runs as root.
assert!(
!pattern.contains('\'') && !pattern.contains(char::is_whitespace),
"{} cannot be safely single-quoted into the scrub script",
pattern
);
}
}
#[test]
fn every_scrub_parent_is_inside_the_scripts_containment_allowlist() {
// The allowlist is hardcoded in the script and deliberately not derived
// from this list, so the two can drift apart — in which case the entry
// silently stops being scrubbed. Fail here instead.
for pattern in SNAPSHOT_SCRUB_PATHS {
let (dir, _) = split_scrub_pattern(pattern).expect("a literal parent");
assert!(
SCRUB_CONTAINMENT_PREFIXES
.iter()
.any(|p| dir == *p || dir.starts_with(&format!("{}/", p))),
"{} is anchored at {}, which the script's containment check would refuse",
pattern,
dir
);
}
}
#[test]
fn the_containment_allowlist_cannot_reach_anything_of_the_users() {
for prefix in SCRUB_CONTAINMENT_PREFIXES {
assert!(prefix.starts_with('/'), "{} is not absolute", prefix);
assert!(!prefix.ends_with('/'), "{} would match the wrong things", prefix);
for forbidden in [
"/", "/workspace", "/home", "/etc", "/usr", "/opt", "/srv", "/root",
"/var/lib/postgresql", "/var/lib/docker",
] {
assert!(
!(forbidden == *prefix || forbidden.starts_with(&format!("{}/", prefix))),
"the allowlist entry {} contains {}",
prefix,
forbidden
);
}
}
}
#[test]
fn the_scrub_list_covers_every_measured_source_of_writable_layer_growth() {
// Each of these was measured in a real container's pending commit.
@@ -3648,21 +4001,199 @@ mod tests {
}
#[test]
fn the_scrub_script_expands_globs_but_quotes_the_match() {
fn the_users_own_files_are_only_scrubbed_once_they_are_stale() {
// Regression: these two hold files the user handed the agent by hand,
// and deleting them at commit time turned "change a setting" into
// "silently lose the file you just dropped". They stay in the list —
// nothing else ever reclaims them — but only with an age gate.
for user_owned in ["/tmp/triple-c-drops/*", "/tmp/clipboard_*.png"] {
assert!(
SNAPSHOT_SCRUB_PATHS.contains(&user_owned),
"{} was removed from the list instead of age-limited, so nothing reclaims it",
user_owned
);
let age = SCRUB_MIN_AGE_DAYS.iter().find(|(p, _)| *p == user_owned);
assert!(
age.is_some_and(|(_, days)| *days >= 7),
"{} is scrubbed without a meaningful age limit, which loses a just-dropped file",
user_owned
);
}
// The scratchpads are machine debris and are not age-limited; if that
// ever changes, the comment explaining why must change with it.
assert!(!SCRUB_MIN_AGE_DAYS.iter().any(|(p, _)| *p == "/tmp/claude-*"));
// An age limit on a pattern that is not scrubbed at all does nothing
// and reads as though it does.
for (pattern, _) in SCRUB_MIN_AGE_DAYS {
assert!(SNAPSHOT_SCRUB_PATHS.contains(pattern), "{} is not scrubbed", pattern);
}
}
#[test]
fn the_scrub_script_validates_every_directory_before_deleting_in_it() {
// The structural half of the C1 fix, for the environments where the
// `/bin/sh` test below cannot run. Each assertion here pins one of the
// four checks; deleting any of them from the script fails this test.
let script = snapshot_scrub_script();
// Unquoted on the `for` line — that is what makes the shell expand the
// globs at all.
assert!(script.contains("for p in /tmp/claude-* /tmp/triple-c-drops/*"));
// Quoted everywhere it is *used*, so a filename with a space is one
// argument and not two paths.
assert!(script.contains(r#"[ -e "$p" ] || continue"#));
assert!(script.contains(r#"rm -rf -- "$p""#));
// Exactly one deletion in the whole script, inside `scrub_in` and
// downstream of all four checks. A future edit that adds a bare `rm`
// at the top level — which is what the vulnerable version was — fails
// here.
assert_eq!(
script.matches("rm -rf").count(),
1,
"the scrub deletes somewhere other than inside the validated block:\n{}",
script
);
// The handle: after this, paths are relative to a validated inode, so
// re-pointing the directory cannot redirect the delete.
assert!(script.contains(r#"cd -P "$d" 2>/dev/null || exit 0;"#));
// 1. no symlinked component anywhere in the parent
assert!(script.contains(r#"[ "$(pwd -P)" = "$d" ] || exit 0;"#));
// 2. positive containment against a hardcoded allowlist
assert!(script.contains("/tmp|/tmp/*|/var/log|/var/log/*"));
assert!(!script.contains("/workspace"));
// 3. the same filesystem as the root — a bind mount or volume is not
// part of the writable layer and must never be touched. Fails
// closed when `stat` is unavailable.
assert!(script.contains(r#"[ -n "$rootdev" ] || exit 0;"#));
assert!(script.contains(r#"[ "$(stat -c %d . 2>/dev/null)" = "$rootdev" ] || exit 0;"#));
// 4. an expansion carrying a separator means the list changed shape
assert!(script.contains(r#"case "$p" in */*|.|..) continue ;; esac;"#));
}
#[test]
fn the_scrub_script_hands_each_glob_over_unexpanded() {
let script = snapshot_scrub_script();
// Single-quoted at the call site: an unquoted `*` would be expanded
// against the script's own working directory before `scrub_in` ran.
assert!(script.contains("scrub_in '/tmp/claude-*' '-';"));
assert!(script.contains("scrub_in '/var/log/apt/*' '-';"));
assert!(script.contains("scrub_in '/tmp/triple-c-drops/*' '14';"));
// The parent/glob split happens in the shell, so every entry stays
// readable verbatim — `disk.rs` folds this onto one `RUN` line and
// asserts each pattern appears there rather than a forked copy.
for pattern in SNAPSHOT_SCRUB_PATHS {
assert!(script.contains(pattern), "{} is not named in the script", pattern);
}
// Expanded inside the function, where the cwd is the validated
// directory, and quoted everywhere it is *used* so a filename with a
// space stays one argument.
assert!(script.contains("for p in $g; do"));
assert!(script.contains(r#"rm -rf $rmopt -- "$p""#));
// `rm -rf /` would be catastrophic and is exactly what a botched
// interpolation produces.
assert!(!script.contains("rm -rf -- /\n"));
assert!(!script.contains(" / "));
}
/// The behavioural half of the C1 fix.
///
/// The test this replaces asserted `!script.contains("/workspace")`, which
/// the exploited version passed: containment is a property of what the
/// paths resolve to at run time. Docker is not available everywhere the
/// suite runs, so a throwaway directory tree stands in for the container
/// and `snapshot_scrub_script_under` re-anchors the real generated script
/// onto it. Linux-only because the script uses GNU `stat -c` and `du -b`,
/// which is what it will always run against inside the image.
#[cfg(target_os = "linux")]
#[test]
fn the_scrub_script_refuses_a_symlinked_parent_and_still_reclaims_a_real_one() {
use std::fs;
use std::process::Command;
// Canonicalised: a TMPDIR that is itself a symlink would trip check 1
// and make every case pass by doing nothing at all.
let base = fs::canonicalize(std::env::temp_dir()).expect("a real temp dir");
let root = base.join(format!("triple-c-scrub-{}", uuid::Uuid::new_v4().simple()));
let root_str = root.to_str().expect("a UTF-8 temp path").to_string();
let mk = |rel: &str| fs::create_dir_all(root.join(rel)).expect("mkdir");
mk("tmp/triple-c-drops");
mk("var/log");
mk("var/lib/apt/lists");
mk("var/cache/apt/archives");
// Stands in for /workspace/{mount_name}: the user's own project,
// bind-mounted from the host.
mk("workspace/myproject/sub");
fs::write(root.join("workspace/myproject/precious.txt"), "do not delete").unwrap();
fs::write(root.join("workspace/myproject/sub/nested.txt"), "nor this").unwrap();
// Debris the scrub is supposed to take, so this cannot pass by
// refusing everything.
mk("tmp/claude-scratch");
fs::write(root.join("tmp/claude-scratch/blob"), vec![0u8; 64 * 1024]).unwrap();
fs::write(root.join("var/lib/apt/lists/deb.list"), vec![0u8; 32 * 1024]).unwrap();
// The attack, in the two shapes that were verified against a real
// container: a symlinked *parent* (a path component, resolved by both
// the glob and the `rm -rf`) and a symlinked *match* (safe already —
// `rm -rf -- link` unlinks the link — and pinned here so it stays so).
std::os::unix::fs::symlink(root.join("workspace/myproject"), root.join("var/log/apt"))
.unwrap();
std::os::unix::fs::symlink(
root.join("workspace/myproject"),
root.join("tmp/claude-link"),
)
.unwrap();
// The age gate on the user's own files: the one dropped a moment ago
// survives a recreation, the one from last month does not.
fs::write(root.join("tmp/triple-c-drops/fresh.txt"), "just dropped").unwrap();
fs::write(root.join("tmp/triple-c-drops/stale.txt"), vec![0u8; 8 * 1024]).unwrap();
let touched = Command::new("touch")
.arg("-d")
.arg("30 days ago")
.arg(root.join("tmp/triple-c-drops/stale.txt"))
.status()
.map(|s| s.success())
.unwrap_or(false);
let script = snapshot_scrub_script_under(&root_str);
let output = Command::new("/bin/sh")
.arg("-c")
.arg(&script)
.output()
.expect("run the generated script");
let stdout = String::from_utf8_lossy(&output.stdout).into_owned();
let stderr = String::from_utf8_lossy(&output.stderr).into_owned();
let exists = |rel: &str| root.join(rel).exists();
let survived = exists("workspace/myproject/precious.txt")
&& exists("workspace/myproject/sub/nested.txt");
let debris_gone =
!exists("tmp/claude-scratch") && !exists("var/lib/apt/lists/deb.list");
let link_removed = fs::symlink_metadata(root.join("tmp/claude-link")).is_err();
let fresh_kept = exists("tmp/triple-c-drops/fresh.txt");
let stale_gone = !exists("tmp/triple-c-drops/stale.txt");
let total = parse_scrub_total(&stdout);
fs::remove_dir_all(&root).ok();
assert!(
survived,
"the scrub followed a symlinked parent into a bind mount.\nstdout: {}\nstderr: {}\nscript:\n{}",
stdout, stderr, script
);
assert!(
debris_gone,
"the scrub stopped reclaiming anything.\nstdout: {}\nstderr: {}",
stdout, stderr
);
assert!(link_removed, "a symlinked match was left behind instead of unlinked");
assert!(fresh_kept, "a file dropped moments ago was scrubbed anyway");
if touched {
assert!(stale_gone, "an age-limited file well past its limit was kept");
}
let total = total.expect("the marker line is missing");
assert!(
total >= 96 * 1024,
"reported total {} is too small to have removed the planted debris",
total
);
}
#[test]
fn the_scrub_total_is_read_back_from_the_marker_line() {
assert_eq!(
@@ -3677,6 +4208,84 @@ mod tests {
assert_eq!(parse_scrub_total(""), None);
}
#[test]
fn only_a_completed_scrub_reports_bytes() {
// M11: a stopped container is a skip, not a failure, and every outcome
// that is not a completed run contributes nothing to the log's total.
assert_eq!(ScrubOutcome::Reclaimed(4096).bytes(), 4096);
assert_eq!(ScrubOutcome::Reclaimed(0).bytes(), 0);
assert_eq!(ScrubOutcome::NotRunning.bytes(), 0);
assert_eq!(ScrubOutcome::TimedOut.bytes(), 0);
assert_eq!(ScrubOutcome::Failed.bytes(), 0);
assert_ne!(ScrubOutcome::NotRunning, ScrubOutcome::Failed);
}
#[test]
fn the_scrub_cannot_hold_a_snapshot_up_indefinitely() {
// M12: the scrub is a `du -sb` plus an `rm -rf` over a tree a running
// agent may still be writing to, on the critical path of every
// recreate with the UI on "Saving container state…" and no cancel.
assert!(SCRUB_TIMEOUT.as_secs() >= 30, "too tight to survive a slow but healthy scrub");
assert!(SCRUB_TIMEOUT.as_secs() <= 300, "long enough that a user would force-quit first");
}
/// `disk.rs` folds this script onto one `RUN` line for the compaction
/// build by joining its non-blank lines with a space, so the script has to
/// be a sequence of self-terminating statements and carry no `#` comments.
/// The previous version was neither: its folded form was a `"do"
/// unexpected` syntax error, which means compaction had been scrubbing
/// nothing at all. The fold is reproduced here rather than imported
/// because it is private to the other module — a divergence would show up
/// as this test passing while the real Dockerfile broke, so it is pinned
/// against the same wording in `fold_shell_script`.
#[cfg(unix)]
#[test]
fn the_scrub_script_survives_being_folded_onto_one_run_line() {
use std::io::Write;
use std::process::{Command, Stdio};
let script = snapshot_scrub_script();
for line in script.lines() {
// A `#` only starts a comment at the beginning of a word, so the
// quoted `###TRIPLE-C-SCRUBBED` marker is fine; anything else is a
// comment that eats the rest of the program once folded.
assert!(
!line.trim_start().starts_with('#') && !line.contains(" #"),
"a `#` comment swallows the rest of the program once folded: {}",
line
);
}
let folded = script
.lines()
.map(str::trim)
.filter(|line| !line.is_empty())
.collect::<Vec<_>>()
.join(" ");
assert!(!folded.contains('\n'));
for candidate in [script.as_str(), folded.as_str()] {
let mut child = Command::new("/bin/sh")
.arg("-n")
.stdin(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.expect("spawn /bin/sh -n");
child
.stdin
.take()
.expect("stdin")
.write_all(candidate.as_bytes())
.expect("write the script");
let out = child.wait_with_output().expect("wait");
assert!(
out.status.success(),
"the scrub script does not parse: {}\n---\n{}",
String::from_utf8_lossy(&out.stderr),
candidate
);
}
}
// ── Container log rotation (A2) ──────────────────────────────────────────
#[test]