Stop the file panel refusing ordinary paths, and hanging on a FIFO

H6 (HIGH) — `resolve_host_path` canonicalised the host path and then re-ran
the *lexical* policy over the answer, hidden-component rule included. Because
canonicalisation resolves through symlinks, that rule started judging where a
path happens to live rather than where the user pointed: uploading out of a
dependency under pnpm (`node_modules/pkg` → `node_modules/.pnpm/…`) was
refused, and so was every download into, or upload out of, a visible directory
that leads to `~/.local/share`, `~/.cache`, `~/.var/app`, `~/.nvm` or
`~/.cargo`. None of it was refused before the H4 fix landed.

The two questions are now separate functions. `validate_host_path` judges the
string the user chose, unchanged. `validate_resolved_host_path` judges the
canonical form for the things only it can answer — the system roots (a Mac's
`/etc` *is* `/private/etc`), the login-item directories, and a new
`HOST_CREDENTIAL_DIRS` list. That last one is what keeps H4's escape closed:
`Downloads/pub` → `~/.ssh` with a leaf of `authorized_keys` is refused because
of where it lands, not because of how the directory is spelled. macOS handling
is untouched — `/private/tmp` stays out of `HOST_SYSTEM_ROOTS` and
`/var/folders` stays in the exceptions.

H8 (HIGH) — the upload reservation claimed its destination with
`sh -c 'set -C; : > "$0"'`, and the comment claiming that is `O_EXCL` was
wrong for a destination that is not a regular file. Against a FIFO the shell
opens it and blocks in `open(2)` forever; `exec_oneshot_raw` has no timeout, so
`upload_file_to_container` never returned and the Files pane sat on
"Uploading…" for the session with the rest of the batch abandoned. Verified in
a fresh ubuntu:24.04: the old form times out and the blocked `sh` stays in
`ps`; the new form answers in 35 ms.

The reservation is now a `link(2)` — it claims a name atomically, never opens
anything, and `EEXIST` is immediate whatever is in the way. A staging file at
an unguessable name in the same directory is linked into place and unlinked,
under a `trap … EXIT`. `exec_oneshot_as_within` adds a wall-clock ceiling as
the second line of defence, opt-in per call site so migration's `apt-get` is
unaffected. The upload contract is unchanged: default-refuse,
`overwrite: Option<bool>`, and `FILE_EXISTS: <full container path> already
exists`.

Also fixed, all in the same surface:

* A dangling symlink destination was a permanent dead end — `set -C` refused,
  the confirming `test -e` followed the link and said no, and raw shell text
  came back with no Replace on offer. `link(2)` does not follow the new-path
  link, and the script confirms with `[ -L ]`, so it reports as a collision.
* An upload through a symlink renamed the file: the leaf came off the
  *resolved* path, so `~/Downloads/latest.log` landed as `2026-08-23.log` and
  the collision prompt named a file the user never chose. The name now comes
  from the path the user gave; the resolved path is still what gets opened.
* `download_container_backup` leaked its partial file when the descriptor
  check fired. It now tracks `created` the way `stream_container_file_to_host`
  already did.
* The failed-upload cleanup was `rm -f` on a path that, the reservation having
  succeeded, held whatever was written in the interim — a host file under
  `/workspace/…`. It now removes only an empty regular file, and the comment
  says what it is doing.
* `resolve_container_dir` parsed a combined stdout+stderr buffer as a path.
  It uses the split-stream helper, like the listing next to it.
* `verify_opened_path` failed open on a readlink error (`if let Ok(actual)`).
  A check that cannot see is not a check that saw nothing wrong; the macOS
  compile-time no-op is now spelled out too.
* A trailing slash on a write path resolved to the directory itself.

Nine new tests, all mutation-checked against the pre-fix behaviour. Two more
cases added to the ignored live-Docker test: a FIFO and a dangling symlink,
both timed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
This commit is contained in:
2026-08-23 15:40:05 -07:00
co-authored by Claude Opus 5
parent ed91423666
commit c6086b0ab3
2 changed files with 685 additions and 95 deletions
+38
View File
@@ -601,6 +601,44 @@ pub async fn exec_oneshot_as(
exec_oneshot_inner(container_id, user, cmd, env, MAX_ONESHOT_OUTPUT).await
}
/// [`exec_oneshot_as`] with a wall-clock ceiling on the whole call.
///
/// H8. Nothing in this module bounds how long a container command may take,
/// which is right for the callers that need it — a base-image migration replays
/// `apt-get` and takes minutes — and wrong for a short command that can be made
/// to block forever by a *file* the caller does not control. The upload
/// reservation is the one that bit: a shell redirect onto a FIFO blocks in
/// `open(2)` until a reader appears, so a single `mkfifo` in a project
/// directory left the Files pane on "Uploading…" for the rest of the session
/// with the rest of the batch abandoned.
///
/// So the ceiling is opt-in per call site rather than global. Note what it can
/// and cannot do: dropping the future closes our end of the stream, but Docker
/// has no "kill an exec" API, so a process that is genuinely wedged stays
/// wedged in the container's process table. That is why the primitive matters
/// more than the timeout — this turns "the app never comes back" into "that
/// upload failed", and it is the caller's job not to run something that blocks.
pub async fn exec_oneshot_as_within(
container_id: &str,
user: &str,
cmd: Vec<String>,
env: Vec<String>,
limit: std::time::Duration,
) -> Result<(String, i64), String> {
match tokio::time::timeout(
limit,
exec_oneshot_inner(container_id, user, cmd, env, MAX_ONESHOT_OUTPUT),
)
.await
{
Ok(result) => result,
Err(_) => Err(format!(
"The container did not answer within {}s — the command may still be running inside it.",
limit.as_secs()
)),
}
}
/// What a one-shot exec printed, with the two streams still tellable apart.
///
/// `combined` is stdout and stderr interleaved in arrival order — the shape