Refuse a deployment download that climbs out of its own directory

[kevin9327]

What this changes

The desktop shell fetches the release tarball and lays it out under the directory it manages. The
check that was meant to keep an entry inside that directory does not:

// Nothing outside `root`, whatever the archive says. A tarball is somebody else's file.
let destination = root.join(&relative);
if !destination.starts_with(root) {
    return Err(format!("the download tried to write outside {}", root.display()));
}

Path::starts_with compares components and does not resolve .., so a path that climbs out still
begins with the root:

app/../../evil               -> /deploy/app/../../evil               starts_with(root)=true
server/../../../etc/x        -> /deploy/server/../../../etc/x        starts_with(root)=true
docker/../..                 -> /deploy/docker/../..                 starts_with(root)=true

The file filter above it does not stop this either: it only inspects the FIRST component, and
app/../../evil begins with app, which is one of the directories a deployment wants.

entry.unpack(&destination) is then given an absolute path and writes there. That is the tar-rs
call that does no containment checking of its own — unpack_in is the one that does — so nothing
between the archive and the disk asks the question the comment says is being asked.

Measured

A tarball whose only entry is OpenBot-0.0.8/app/../../escaped.txt, unpacked into a root one
directory below the scratch directory. Against main:

thread 'deployment::tests::a_download_that_climbs_out_of_the_root_is_refused' panicked at
src/deployment.rs:472:9:
a file was written above the root

The file is written and the call returns Ok.

The archive for that test is assembled from raw ustar bytes rather than through tar::Builder,
because the builder refuses such a path outright — "paths in archives must not have ..". That
refusal is the right behaviour for a writer and it is exactly why the reader cannot lean on it: an
archive shaped like this is not produced by a careful writer. A second test asserts the archive
really does carry the traversal, so the first cannot pass for the wrong reason.

How far this goes

Honestly: the tarball comes from github.com/CopilotKit/OpenBot/archive/refs/tags/{version}.tar.gz
over HTTPS at a pinned tag, so reaching it means controlling that release or the transport. This is
not something an ordinary user can trigger today.

I still think it is worth fixing rather than reasoning away. The code already decided the archive is
untrusted — "A tarball is somebody else's file" — and built a guard for it; the guard just does not
hold. A check that reads as protection and is not is worse than no check, because it is the reason
nobody looks again. This is also the one place in the app that writes files it did not author, and
it runs on a person's own machine with their own permissions.

Fix

The traversal is refused at the path rather than at where it landed. Every component of a path
inside the tree is an ordinary name, so anything else — .., an absolute path, a Windows drive
prefix — is refused, and the refusal names the entry.

The strip-the-wrapper-directory step, the wanted-list and the unpack are unchanged; they now live in
destination_in, which answers "where may this entry go, if anywhere" and is a pure function of the
root and the entry path.

fetch is split so the extraction loop is unpack(root, body). That is what makes the part
deciding where somebody else's archive may write testable without a network — the reason the fault
survived is that nothing could reach it.

Where it runs

• New state that outlives a request? None. This is the desktop shell, not the server; it
  writes to a directory it owns on one machine.
• What happens on the second replica? Not applicable — the shell runs on a person's own
  computer, one per install. The deployment it lays down is what the replicas later run.
• Anything serialised? No. The stamp is still written last, so an interrupted fetch is still
  retried rather than trusted.
• Anything fanned out to a browser? No.
• New listener, port, or schedule? None.

Boundary and audit

• Every acting call still goes through the gateway: no server path is touched.
• New refusals and new failures each write a row: not applicable here. This runs before any
  deployment exists, so there is no audit store; the refusal reaches the person through the
  setup screen's progress, the same way every other fetch failure does.
• Nothing new is trusted from the client: strictly less is trusted from the archive.

Changelog

• A line in CHANGELOG.md under Unreleased.

Tests

Three in desktop/src-tauri/src/deployment.rs:

• the test archive really carries the traversal, so the next test cannot pass for the wrong reason
• a download that climbs out of the root is refused, nothing is written above it, and the refusal
  says what it refused
• an ordinary download still lands where it should: app/index.ts and docker-compose.yml in
  place, docs/ skipped rather than refused

How I tested

Windows 11. cargo test --lib in desktop/src-tauri is 73 passed, 0 failed, up from 70 by the three
new tests. cargo fmt --all -- --check and cargo clippy --lib are clean. The crate has no
rust-toolchain.toml, so I built it with the 1.98.0 toolchain that was installed here.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.