Commit Graph
3 Commits
Author SHA1 Message Date
shadowdaoandClaude Opus 5.5 caaf70a66c fix(viewer): real errors, no leftover temp files, refuse read-only saves
Fix round 1 for Task 3, addressing task-3-review.md's I1-I3 (reproduced
under dash) plus M3 and M10 from the same review.

- I1: WRITE_SCRIPT read the target's hash through `sha256sum | cut … ||
  exit 1`. POSIX sh has no pipefail, so that `|| exit 1` tested only
  cut's exit status — an unreadable target (EACCES, EIO) left $actual
  empty, which never equals $expect, so the script silently reported
  exit 3 (conflict) instead of a real error. The user got a misleading
  "changed on disk" banner whose "Overwrite on save" could never
  succeed, since the next poll hit the same read error. Fixed by
  reading the hash from a plain command substitution
  (`actual=$(sha256sum -- "$target") || exit 1`) and splitting out the
  hash field in shell instead of piping into `cut`.

- I2 (+ M3): a failed `cp` into the staged file (ENOSPC, quota, EFBIG,
  EIO) left a partial `.<name>.triple-c-<pid>` behind in the user's
  own directory — the EXIT trap only ever removed $tmp. Fixed by
  creating the staged file with `mktemp` (M3: exclusive, unpredictable
  name, so it can't be planted or follow an existing symlink) and
  adding it to the trap as soon as it's assigned
  (`trap 'rm -f -- "$tmp" ${staged:+"$staged"}' EXIT`), so any later
  failure — cp, chmod, mv, or a signal — cleans it up too.

- I3 (controller ruling): the script only ever checked `[ -w "$dir" ]`,
  so a 0444 file (or one owned by another uid) was silently replaced
  via rename, defeating the file's own write protection even though
  spec §5 step 3 reads that way literally. Added `[ -w "$target" ]`
  before the branch; a non-writable target is refused with "The file
  is read-only for the container user." on stderr and a distinct exit
  code (5, `EXIT_READ_ONLY`) that `classify_write` maps to that same
  message rather than falling into the generic clipped-stderr arm.

- M10: added six `#[cfg(unix)]` tests that run WRITE_SCRIPT for real
  via `sh -c` against a temp directory on the host (not just needle
  matches against the script text) — clean save, stale-base conflict,
  gone target, unreadable target (I1), read-only target (I3), and a
  failed stage leaving no partial file behind (I2). The unreadable/
  read-only tests self-skip with a message if permission bits turn out
  not to block root, rather than false-failing under a root test
  runner.

Verified: `cargo test --offline file_viewer` — 19/19 passing, pristine
(up from 12; 6 new host-execution tests plus 1 for the new exit-5
classify_write arm). `cargo clippy --offline` (and `--tests`) — no
warnings in file_viewer::write; the 28 warnings clippy reports are
all pre-existing, in unrelated files.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 21:20:11 -07:00
shadowdaoandClaude Opus 5.5 bbcaee7797 feat(viewer): poll and save scripts run as the container user
poll.rs: one exec per tick that tests existence then hashes+stats the
file (sha256sum/stat), so the 2 s poll costs one exec instead of
re-downloading up to 1 MiB of archive per window per tick.

write.rs: sha256_hex/is_sha256_hex, MAX_WRITE_BYTES, and the save
script. Saving stages the payload in /tmp via the existing
write_file_to_container (owned by the container user, since the
Docker archive API writes as root), then an exec as `claude` checks
the base hash, swaps the file in with a same-directory rename when
the directory is writable (falling back to an in-place `cat >` when
it is not), and always cleans up the staged temp file via `trap`.
classify_write maps exit 0/3/4 to Saved/Conflict/Gone.

Applies three pre-flight rulings against the brief's literal text:
- P8: pulled the write script's argv shape and the size/hash checks
  into pure `write_command`/`check_write_input` helpers with their
  own unit tests, since both lived only inside the untested async
  `write_file` otherwise.
- P9: the brief's manual Docker smoke-test invocation
  (`sh w.sh save target tmp hash`) makes `$1` become "save" instead
  of the target, which is not what the script or the Rust caller
  expect. Verified in a throwaway container that invoking the file
  directly without the dummy "save" arg reproduces the Rust
  convention's `$1/$2/$3` correctly: exit 0 with the new hash and a
  removed payload on a clean save, exit 3 with the file untouched on
  a stale base hash, and exit 4 when the target is gone.
- P15: GNU sha256sum prefixes its output line with `\` when the
  path contains a backslash or newline. Without a fix that turns
  into a permanent false conflict (write.rs) and a blinded poll
  (poll.rs, hash: null forever). Both parsers now strip a leading
  `\`, and the script itself strips it from $actual before comparing
  to $expect. Verified against real sha256sum output in a container
  with a backslash-named file: the save no longer false-conflicts
  and the reported hash matches.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 21:11:19 -07:00
shadowdaoandClaude Opus 5.5 3c1d120305 feat(viewer): IPC types, wrappers and Rust module skeleton for the file viewer
Task 0 of the terminal file viewer plan: the shared interfaces that seven
later tasks build against in parallel. Adds ViewerLocation/ViewerTargetState/
ViewerState/ViewerFile/ViewerPoll to types.ts and their invoke() wrappers to
tauri-commands.ts, creates the file_viewer Rust module (mod.rs with
MAX_VIEWER_WINDOWS/VIEWER_LABEL_PREFIX/is_viewer_label, plus placeholder
registry/resolve/poll/write/window submodules), wires it into lib.rs, and
loosens visibility on the file_commands.rs helpers the viewer commands will
reuse (MAX_READ_BYTES, validate_container_path, validate_container_write_path,
FetchedFile, fetch_container_file, require_running, clip_container_text).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-22 20:56:33 -07:00