Commit Graph
249 Commits
Author SHA1 Message Date
shadowdao 31e8f9df5f Add the notes dock 2026-09-01 13:15:13 -07:00
shadowdao 3704064006 Add the Notes tab 2026-09-01 13:08:17 -07:00
shadowdao f79a44e0a8 Add the send-to-agent button 2026-09-01 13:02:22 -07:00
shadowdao 5a8e24ccbe Extract the Claude newline sequence and the session display name 2026-09-01 12:57:29 -07:00
shadowdao a1f4eee9a3 Fix critical cross-project data corruption bug in useNotes hook
When a save is in flight for project A and the user switches to project B before it resolves, the stale closure still has projectId=A. When A's save resolves, the post-save re-read of listNotes(projectId) runs with the stale closed-over projectId, and setNotes(reloaded) overwrites B's displayed notes with A's list—the same cross-project contamination class as Finding 2 but reintroduced through the fix itself.

Fix: Add a currentProjectId ref updated on every render, and guard both saveNote and deleteNote callbacks with a check before replacing/filtering the whole list. If the project changed while the async operation was in flight, bail out of the state update but still report success (the operation itself succeeded on the backend; only the stale list update is skipped).

Added test: a save in flight for one project, a switch to another, then the first save resolving—asserts the second project's notes are still displayed.
2026-09-01 12:50:23 -07:00
shadowdao b6ba6deb09 Fix critical data corruption and stale-data bugs in useNotes hook
- Finding 1 (saveNote): After a successful save, re-read the canonical list from the backend instead of patching in place. A successful save stamps a new updated_at, and the backend sorts by updated_at descending, so the record's position has changed and positional patching would disagree with what a reload would show. If the re-read fails, keep the save reported as successful and leave the existing list alone.

- Finding 2 (stale notes): Clear notes on projectId change (not only when empty) and on load failure. Previously, switching from project A to project B would leave A's notes on screen until B's fetch resolved, and if a user edited one, A's note would be written into B's notes file—cross-project data corruption. If a load fails, A's notes stay visible under B indefinitely.

- Added four new tests covering these scenarios: projectId change clears old notes, failed load leaves no stale notes, saving a new note ends with the backend's list, and saves re-read the list rather than patching.
2026-09-01 12:45:35 -07:00
shadowdao cd3160b1cd Add the notes hook and its IPC wrappers 2026-09-01 12:38:33 -07:00
shadowdao 60abff1717 Expose notes over IPC and drop them with the project 2026-09-01 12:34:35 -07:00
shadowdao cc767bd544 Add a per-project notes store 2026-09-01 12:28:09 -07:00
shadowdaoandClaude Opus 5 3a49a67c1f Fix terminal input reordering and Linux terminal rendering
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 4s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-linux (pull_request) Successful in 5m25s
Build App (Preview) / build-windows (pull_request) Successful in 5m32s
Build App (Preview) / prune-previews (pull_request) Successful in 8s
Two separate defects behind the same report: typing in a container terminal
is sluggish on Linux, and a backspace can land *after* the characters typed
behind it.

The web terminal was the control that separated them. It shares the Docker
exec, the PTY, `exec_manager`, the input channel and its serial writer task,
and xterm.js itself — and it does not exhibit either symptom. Only three
things differ, and each accounts for part of the report.

**Input ordering.** Every keystroke was its own `invoke("terminal_input")`.
That command is `async`, so Tauri spawns each one as an independent task, and
those tasks then race for the session mutex in `ExecSessionManager::send_input`
— nothing preserved the order the bytes were typed in. The serial writer
downstream cannot help, because the order is already lost before anything
reaches the channel. The web terminal gets ordering for free by awaiting
`send_input` inline in a single WebSocket reader loop.

`useTerminal` now holds a per-session queue: one write in flight at a time,
the next only after the previous resolves. Anything typed meanwhile coalesces
into the next chunk, which also collapses a burst of typing into a couple of
IPC round trips rather than one per key. The queue is module scope, not hook
scope, because `useTerminal()` is called from several components — a per-hook
queue would leave speech-to-text, image paste and typing racing each other.
Each caller's promise still settles only when its own bytes have gone, so
`await sendInput(...)` keeps its meaning.

**The DMA-BUF escape hatch did not exist.** `apply_webkit_wayland_workaround`
left any pre-set value alone, including `0`, on a stated assumption that
WebKitGTK reads the variable as a boolean. It reads presence, so
`WEBKIT_DISABLE_DMABUF_RENDERER=0` disabled DMA-BUF exactly like `=1`, and no
value a user could set got the accelerated path back. `0`/`false`/`no`/empty
now remove the variable, which is the only thing WebKitGTK reads as enabled.
The default is unchanged: unset still means disabled on Linux.

**WebGL does not degrade to canvas here.** The comment on that workaround
assumed `@xterm/addon-webgl` would fall back to the canvas renderer once
DMA-BUF was off. Its constructor throws only when WebGL is *absent*, and with
DMA-BUF disabled WebGL is still present — served by software rasterisation.
So the addon loads and every frame is rendered on the CPU, slower than the
canvas renderer it was assumed to fall back to. `AppSettings::terminal_gpu_
rendering` decides whether it loads at all: `None` is auto (on for macOS and
Windows, off on Linux), `Some(_)` forces it either way from Settings →
Terminal. `Option<bool>` rather than `bool` so the zero value means "we
choose" instead of pinning every existing settings file to one answer.

Verified: 643 frontend tests and 530 Rust tests pass, clippy clean, secret
scan clean. The Linux rendering half needs confirming on a real desktop —
neither symptom reproduces in a headless container.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ApLYH6ybHwQFkMCtKuHrrV
2026-08-28 12:51:18 -07:00
shadow-test 97e58db3c1 Close gateway-secret desync, TOCTOU, and undisclosed custom-image gaps
Secret Scan / scan (push) Successful in 6s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 5s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m53s
Build App (Preview) / build-linux (pull_request) Successful in 7m5s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Round 4 review findings:

- Disclose and warn on a custom Docker image the import would set (HIGH):
  it's the image every project container is created from, so an
  undisclosed change here was a sharper version of the redirected-base-URL
  problem round 3 already flagged for the model backends.
- Recreate a running gateway container when an import restores a new
  secret with the shape unchanged (MEDIUM): reconcile_gateway's shape
  comparison can't see a secret-only change, so the container would
  otherwise keep serving old key material indefinitely.
- Report keychain write failures back to the caller instead of only
  logging them (MEDIUM): apply_settings_import now returns
  SettingsImportOutcome with secret_restore_warnings so a partial restore
  can't read as unqualified success.
- Pin a hash of the previewed file's ciphertext and refuse to apply if it
  changed on disk (MEDIUM): closes a TOCTOU between preview and apply.
- Sanitize and cap every free-form string a preview surfaces, and move the
  warning boxes above the replace list in the UI (MEDIUM): an unbounded
  base URL or image name could otherwise push the security warnings below
  the scroll fold.
- Validate the Docker socket path on import the same as the SSH key and CA
  cert paths (LOW): it was the one mounted host path validate_settings_update
  didn't cover.
- Fix ExportedSecrets::is_empty() to treat whitespace-only as blank, like
  every other secret-presence check in this feature (LOW).
- Authenticate the file header as AEAD associated data (LOW, defense in
  depth) and correct two doc comments that overstated the password not
  being cached.
2026-08-27 14:24:06 -07:00
shadow-test a606e3ab20 Validate settings imports before writing secrets; disclose base URLs
Secret Scan / scan (push) Successful in 14s
Build App (Preview) / compute-version (pull_request) Successful in 7s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m43s
Build App (Preview) / build-windows (pull_request) Successful in 4m59s
Build App (Preview) / build-linux (pull_request) Successful in 7m29s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
A rejected import (bad env var name, disallowed host path) used to leave
keychain secrets already overwritten while the settings themselves stayed
unchanged. apply_settings_import now runs update_settings's validation
(extracted into validate_settings_update) before any secret write.

Also from this review round: sharpened two format-version tests that
previously passed against the pre-fix code too, added a direct test for
split_settings_and_secrets, warned on a dormant web terminal token even
when the terminal import leaves it off, matched the password-length check
to the frontend's unit of measure, zeroized the export plaintext buffer,
and surfaced non-blank Ollama/llama.cpp/OpenAI-compatible/gateway base
URLs in the import preview so a traffic redirect isn't silent.
2026-08-27 13:13:48 -07:00
shadow-testandClaude Sonnet 5 925e51e435 Fix a real credential-leak vector a review found, plus four smaller issues
Secret Scan / scan (push) Successful in 12s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m51s
Build App (Preview) / build-linux (pull_request) Successful in 5m12s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
The headline finding: WebTerminalSettings::access_token is a live bearer
credential for a server that binds every interface, stored as a plain
field on AppSettings — which this feature was exporting and importing
wholesale as if it were as inert as a port number. A crafted export file
could set web_terminal.enabled and access_token together, and importing
it (with no more warning than any other setting change) would silently
stand up a LAN-listening terminal server with an attacker-known token on
the victim's next launch.

Fixed by carving the token out into ExportedSecrets, same as the other
three global secrets, with the same "only overwrite what the import
actually has" treatment — except that has to be done by hand here, since
this one lives inside the AppSettings blob that gets replaced wholesale
rather than in the keychain. Added SettingsImportPreview::
enables_web_terminal so "this turns on a listening service" gets its own
visible warning in the confirmation modal rather than hiding inside a
generic "settings replaced" bullet list.

Also fixed:

- read_and_decrypt checked format_version only after attempting to parse
  the full payload, so a future version bump that isn't
  deserialize-compatible would fail on the shape mismatch before the
  version check ever ran — and serde's type-mismatch errors quote the
  offending value inline, which is a real leak path since the plaintext
  here can hold a live credential. Now probes just the version field
  first, and neither error path interpolates the underlying serde message
  into what the user sees.
- apply_settings_import cleared the pending-import path before it could
  fail, so a rejected import (an invalid host path, anything
  update_settings validates) dead-ended the modal with no way back except
  cancelling and reopening the file picker. The path is now only cleared
  on success.
- Secrets are restored before the settings replace runs, not after —
  replacing settings is what triggers reconcile_gateway, and restoring
  secrets afterward left a real window where a gateway recreation
  happened against the destination's stale keys.
- The 8-character password minimum was frontend-only; export_settings now
  enforces it too, since that's the actual boundary a weak password has
  to cross. The derived key and decrypted plaintext are wrapped in
  zeroize::Zeroizing (already in the tree via aes-gcm).

Added test coverage the review named as missing: format-version
ordering, the generic-error-message guarantee, non_blank's blank-vs-
absent handling, and the new web-terminal preview/warning behavior on
both sides of the IPC boundary.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 12:16:43 -07:00
shadow-testandClaude Sonnet 5 722d9aeff1 Add password-encrypted settings export/import
Secret Scan / scan (push) Successful in 8s
Build App (Preview) / compute-version (pull_request) Successful in 6s
Secret Scan / scan (pull_request) Successful in 9s
Build App (Preview) / create-release (pull_request) Successful in 5s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m59s
Build App (Preview) / build-linux (pull_request) Successful in 6m29s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Closes #35. Exports the host environment — global AppSettings (already
the non-secret shape persisted to settings.json) plus the global secrets
that live in the OS keychain instead (the shared Claude Code OAuth login,
the model gateway's provider API key and master key) — to one
password-encrypted file, and restores it on another machine.
Per-project settings, per-project secrets, and Docker volumes are
deliberately out of scope; this is not a project backup.

Designed with the user in issue #35's comments: global settings only, no
docker volumes, the password is the lock/key, and the export is portable
as one file.

Crypto (storage/settings_crypto.rs): Argon2id derives a 256-bit key from
the password (memory-hard, meaningfully resistant to GPU/ASIC
brute-forcing in a way PBKDF2 at any reasonable iteration count is not),
AES-256-GCM does the actual encryption. A wrong password fails GCM's
authentication tag rather than producing silent garbage. Salt and nonce
are random per export and stored in the clear in the file header — their
job is uniqueness, not secrecy.

The save/open dialogs are opened from Rust, matching the boundary
file_commands.rs's pick_save_path/pick_files_to_upload already establish:
a frontend-driven dialog handing Rust a host path is the exact shape of
bug that produced this app's past criticals. preview_settings_import
resolves the chosen import path itself and remembers it
(AppState::pending_settings_import) so apply_settings_import re-reads the
same file without a path crossing back over IPC. The password is
re-entered rather than cached between preview and apply, so nothing here
holds decrypted plaintext in memory for longer than one command's
execution; the preview returned to the frontend carries counts and
presence flags only, never a secret value.

Import replaces settings wholesale (an import is "restore this
environment"), but only writes secrets actually present in the file — an
absent secret means "the source machine never had this configured," not
"delete this on import."

Added storage::secure::store_gateway_master_key and get_gateway_master_key
(read-only, unlike get_or_create_gateway_master_key which mints one as a
side effect) since neither existed and import needs to restore an exact
captured value rather than mint a new random one.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 11:57:16 -07:00
shadow-testandClaude Sonnet 5 8f62949902 Correct two overclaims in the Wayland workaround's comment
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m38s
Build App (Preview) / build-windows (pull_request) Successful in 4m48s
Build App (Preview) / build-linux (pull_request) Successful in 6m49s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Review found: "nothing this app's UI depends on" is backwards — the
terminal's @xterm/addon-webgl renderer is exactly the GPU compositing path
this setting disables, it just degrades gracefully (the addon's own
construction already handles WebGL being unavailable) rather than
crashing. And the "not simply Wayland vs X11" justification for going
unconditional doesn't hold up: WAYLAND_DISPLAY is exported into an
XWayland client's environment too, so gating on it would have caught that
case as well — the real reason to go unconditional is that there's no
reliable heuristic for the thing that actually matters (which
Mesa/driver/compositor combination is affected), not that the naive gate
misses XWayland specifically.

Also noted, not changed: the env var leaks to whatever the app spawns
afterwards (a cold-launched default browser via xdg-open), and the "=0
re-enables it" parenthetical isn't verified against WebKitGTK's own
source, so softened to say what's actually guaranteed (an already-set
value is left alone) rather than assume presence-vs-boolean parsing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 10:57:45 -07:00
shadow-testandClaude Sonnet 5 6354cb42b2 Work around WebKitGTK's EGL crash on Wayland (triple-c#34)
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m39s
Build App (Preview) / build-windows (pull_request) Successful in 4m45s
Build App (Preview) / build-linux (pull_request) Successful in 5m10s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
Reported on CachyOS/Arch with Wayland: the app aborts immediately with
"Could not create default EGL display: EGL_BAD_PARAMETER. Aborting."
printed straight to stderr by WebKitGTK's own C code, before Triple-C's
own logging even gets a chance to say anything useful about it.

This is WebKitGTK's DMA-BUF renderer (its default accelerated-compositing
path since 2.42) failing on some Mesa/driver/compositor combinations. Set
WEBKIT_DISABLE_DMABUF_RENDERER=1 unconditionally on Linux before the Tauri
builder runs, which is where GTK/WebKitGTK actually read it — there's no
reliable way to detect the affected combination ahead of time (reports of
this exact failure exist under XWayland too, not just pure Wayland
sessions), and WebKitGTK's fallback compositing path costs some rendering
performance this app's UI doesn't need. Left alone if a user has already
set the variable themselves.

Does not address the other two things filed under the same issue (links
not opening on the host, and a request for a native Arch/CachyOS package)
— those need more information / are a separate scope, respectively.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 10:45:24 -07:00
shadow-testandClaude Sonnet 5 049232099b Dedupe the preview-build predicate, fix two comment inaccuracies
Secret Scan / scan (push) Successful in 24s
Build App (Preview) / compute-version (pull_request) Successful in 6s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m39s
Build App (Preview) / build-windows (pull_request) Successful in 4m44s
Build App (Preview) / build-linux (pull_request) Successful in 6m17s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Final review pass gave this a clean bill of health overall but named
three small things:

- get_app_version() and check_for_updates() each read
  option_env!("TRIPLE_C_BUILD_SUFFIX") independently with slightly
  different idioms — if one were ever edited alone, the About panel and
  the update check could silently disagree about whether this is a
  preview build. Extracted preview_build_suffix() as the single place
  that reads and classifies it.
- pick_update's doc comment described the unparseable-tag case as a
  `-preview.<sha>` suffix; the actual tag build-app-preview.yml creates is
  `preview-<sha>` (no version, no dot) — already correct in the
  neighboring GitHubRelease::prerelease comment, just not here.
- That same prerelease comment claimed defence against a preview release
  leaking through backfill-releases.yml, but a preview's tag already fails
  semver parsing on its own — this field's actual job is the case parsing
  can't catch: a normally-tagged release someone flags prerelease on
  Gitea (a hotfix candidate, an RC) that a backfill would otherwise mirror
  as-is.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 10:34:56 -07:00
shadow-testandClaude Sonnet 5 945883bb9d Actually offer a preview the release it precedes, and fix two more gaps
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m39s
Build App (Preview) / build-windows (pull_request) Successful in 4m50s
Build App (Preview) / build-linux (pull_request) Successful in 7m25s
Build App (Preview) / prune-previews (pull_request) Successful in 6s
An Opus review of the previous commit found its headline claim didn't
hold: a preview and the release it precedes compute to the identical
numeric version by construction, but check_for_updates compared with a
strict `>` against the bare CARGO_PKG_VERSION (never the suffixed display
string), so `(0,4,13) > (0,4,13)` is false and the release was never
offered. Plain semver ordering doesn't make a `-preview.<sha>` suffix sort
below the same numeric release on its own here, since the comparison
never sees the suffix at all.

pick_update now takes is_preview_build, derived from whether
TRIPLE_C_BUILD_SUFFIX was baked in, and relaxes that one comparison to
`>=` — so "a release exists at my own number" reads as an update. A
production build still requires strictly newer.

Also: ported build-app.yml's `git tag --points-at HEAD` guard into the
preview version computation. Without it, workflow_dispatch (which this
workflow allows on main, not just PR builds) run on a commit a release
was already cut from would compute one past that release — reintroducing
"preview outranks production" through the manual-dispatch door. And
corrected two comments that claimed the prerelease filter was currently a
no-op: backfill-releases.yml mirrors every Gitea release to GitHub
unfiltered, prerelease flag included, so it's real defence-in-depth
against a dispatched backfill leaking a preview release, not a no-op.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 10:24:24 -07:00
shadow-testandClaude Sonnet 5 b71e15c2c0 Make preview versions monotonic and distinguishable from production
Secret Scan / scan (push) Successful in 6s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 3s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m51s
Build App (Preview) / build-linux (pull_request) Successful in 6m26s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
build-app-preview.yml computed its patch number as
`git rev-list --count <latest tag>..HEAD` — the exact formula build-app.yml
itself documents as broken and replaced (#26): a distance from whichever
tag sorts highest, not a counter, so it resets to zero on every release and
previews went backwards (0.4.62 -> 0.4.0) the moment one landed. Ported the
same "one past the highest patch already used" computation build-app.yml
uses for real releases, reading the same tags (including -mac/-win
suffixes), so a preview built right before a release now computes the
exact number that release is about to take — semver already orders
`0.4.12-preview.<sha> < 0.4.12`, so a preview user is offered the release
the moment it ships instead of being silently pinned forever.

The installed preview's reported version was also indistinguishable from
production: the bundle's own version field strips the `-preview.<sha>`
suffix before touching tauri.conf.json/Cargo.toml/package.json, since the
Windows MSI's ProductVersion has no room for one. Rather than risk that
(unverifiable without an actual Windows build), preview builds now bake
the suffix into the binary separately via a TRIPLE_C_BUILD_SUFFIX
build-time env var, and get_app_version() appends it when present — a
production build sets nothing, so this is a no-op there.

Also: added `prerelease` to `GitHubRelease` and filter on it in
check_for_updates (currently a no-op against real data — nothing mirrored
to GitHub is ever prerelease:true — but the updater is no longer
structurally incapable of enforcing a channel split if one is ever made
explicit). And deleted sync-release.yml: workflow_dispatch-only, reading
gitea.event.release.* fields a manual dispatch never populates, so it
could never have actually run; build-app.yml's inline mirror already does
the same job.

Refactored check_for_updates' filtering into a pure, testable pick_update
helper (this file had no tests before), and added tests for it and the
new get_app_version suffix handling.

Fixes #32.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 10:11:35 -07:00
shadow-testandClaude Sonnet 5 61bdbc4a5b Close the crash-window gap and exec-session leak a third review found
Secret Scan / scan (push) Successful in 16s
Build App (Preview) / compute-version (pull_request) Successful in 6s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 3s
Build App (Preview) / build-macos (pull_request) Successful in 2m37s
Build App (Preview) / build-windows (pull_request) Successful in 4m52s
Build App (Preview) / build-linux (pull_request) Successful in 6m17s
Build App (Preview) / prune-previews (pull_request) Successful in 2s
A third Opus review pass confirmed round 2's fixes hold up, then found:

- The pending-cleanup record `remove_project` writes is fully durable
  (fsync'd); the projects_store.remove() that follows it is a plain
  fs::write with no fsync. A crash or power loss in that window — or that
  store write failing outright, beyond what the previous round's in-process
  rollback catches — leaves a record on disk naming a project
  projects.json still lists as present. The very next startup retry would
  then delete that project's container, snapshot image, and both volumes
  (including the one holding the OAuth credential and every session
  transcript) out from under a project the user still sees in the sidebar.
  retry_pending_cleanup_logged now takes the ProjectsStore and refuses to
  touch — clearing instead — any record whose project id still exists.
  Also stopped swallowing the round-2 rollback's own failure.
- Resolving the container through find_existing_container instead of
  project.container_id (round 2's stale-id fix) changed what drove
  close_sessions_for_container in remove_project and rebuild_project_
  container: sessions are now leaked when Docker is unreachable (nothing
  resolves, so nothing closes, and the project record is gone a moment
  later) and in the stale-id race itself (sessions were opened against the
  container that actually exists, not the id find_existing_container
  bypasses). Both functions now close sessions for the stored id
  unconditionally, and again for the resolved id if it differs.
- A pronoun-agreement bug in the no-retry removal toast ("remove them
  manually" for a single leftover) that was fixed one line above for verb
  agreement but not for the pronoun.

Also closed the test gaps the review named: the pending-cleanup
corrupt-record aside-move had no test, the Reset toast's leftover copy
was inline and untested (extracted to lib/resetOutcome.ts, mirroring
components/projects/home/removalReport.ts, with unit tests), and nothing
asserted rebuild()'s success path maps outcome.project into the list
rather than the whole outcome.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 09:47:45 -07:00
shadow-testandClaude Sonnet 5 439ef16f07 Fix two new bugs a second review found: stale container id, orphaned record
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 5s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m40s
Build App (Preview) / build-windows (pull_request) Successful in 4m59s
Build App (Preview) / build-linux (pull_request) Successful in 6m28s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
A second Opus review of commit 2 found it had introduced real problems of
its own rather than just polish gaps:

- remove_project's "None or stale" container-id fallback only handled
  None. A stale id (the documented start-failure race in
  start_project_container_locked, where the old container is removed and
  the new one's id isn't persisted until after start_container succeeds)
  still 404'd on removal — now treated as success by commit 1's own fix —
  while the real container survived to block every volume removal with a
  409 forever, with nothing in the pending-cleanup record ever naming it.
  Both remove_project and rebuild_project_container now resolve the
  container via find_existing_container() unconditionally, matching every
  other container-destroying path in the codebase, and remove_project
  fails closed (records a leftover rather than silently skipping) if
  Docker itself can't be reached to check.
- remove_project could leave a pending-cleanup record for a project still
  live in projects.json: if the store's own save failed after the record
  was written, startup housekeeping would delete that project's container
  and volumes out from under it on the next launch. The record is now
  rolled back when the store write fails.
- rebuild_project_container (Reset) only surfaced a leftover volume, not a
  leftover snapshot image — the more serious failure, since the next
  container is built from that image whenever it exists, silently
  reviving the exact system layer Reset was asked to discard.
  ProjectResetOutcome now carries leftover_image too, and the toast's
  "run docker volume rm" advice is corrected: the new container has
  already remounted the volume by the time the toast renders, so that
  command would just hit the same conflict Reset did.

Also from the same pass: reworded a couple of log/toast lines that still
asserted resources were "still present" when the daemon-unreachable case
covered by the same code path can't actually confirm that; fixed a
singular/verb mismatch in the leftover toast text; moved an unparseable
pending-cleanup record aside instead of re-warning about it forever; and
added a debug log when a record's recorded_at can't be parsed, so aging
never silently no-ops.

Pulled describeLeftovers/leftoverVerb out of ProjectHome.tsx into their
own module with unit tests, and added tests for the recorded_at staleness
check — the previous commit's equivalent logic had none.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 09:09:46 -07:00
shadow-testandClaude Sonnet 5 d8bb5ab262 Address review findings: durability, stale container ids, honest toasts
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 6s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m38s
Build App (Preview) / build-windows (pull_request) Successful in 6m18s
Build App (Preview) / build-linux (pull_request) Successful in 7m48s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
An Opus review of the previous commit found several real gaps:

- pending_cleanup::save used plain write-temp-then-rename, unlike
  migration_store's fsync'd write it claimed to mirror — a crash in that
  window left a truncated record that list() would skip forever, silently
  reproducing the exact bug this module exists to fix. Now matches
  migration_store's File::create/write_all/sync_all/rename/sync_dir shape,
  and the tests exercise the real save/list/clear functions against a temp
  dir instead of re-implementing their bodies inline.
- remove_project and rebuild_project_container only ever looked at
  project.container_id, unlike every other container-destroying path in the
  codebase, which falls back to find_existing_container for exactly this
  race (a crash between creating a container and persisting its id). A miss
  here left a container that then blocked every subsequent volume removal
  with a 409, forever. Both now resolve the same way the rest of the
  codebase does, and record the container by its deterministic name rather
  than its id so a retry still has something that resolves.
- remove_project's toast promised an automatic retry unconditionally, even
  when writing the pending-cleanup record itself failed (the one case
  where nothing will actually retry). ProjectRemovalReport now carries
  retry_scheduled, and the UI is honest about which case it's in.
- remove_volumes_by_name now retries once after a short delay on a 409,
  since Docker releasing a volume's mount reference right after its
  container is removed is not always instantaneous, and this is exactly
  the sequence remove_project runs.
- rebuild_project_container (Reset) returns ProjectResetOutcome so the UI
  can warn when Reset could not fully clear a project's volumes, instead
  of only logging it — the new container silently reuses old data
  otherwise, which is what Reset promises not to do.
- retry_pending_cleanup_logged escalates a record's log level after it has
  failed for a week, since recorded_at was otherwise write-only.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 08:36:27 -07:00
shadow-testandClaude Sonnet 5 4827170715 Report and retry Docker resources remove_project could not delete
Secret Scan / scan (push) Successful in 10s
Build App (Preview) / compute-version (pull_request) Successful in 7s
Secret Scan / scan (pull_request) Successful in 8s
Build App (Preview) / create-release (pull_request) Successful in 5s
Build App (Preview) / build-linux (pull_request) Successful in 6m5s
Build App (Preview) / build-macos (pull_request) Successful in 2m45s
Build App (Preview) / build-windows (pull_request) Successful in 5m42s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
remove_project_volumes always returned Ok(()) regardless of what actually
happened, making the `if let Err(e)` guarding it at every call site dead
code. remove_project then dropped the project record unconditionally, so a
volume, image or container that failed to delete became permanently
unreachable — confirmed against a real orphaned volume pair found in the
wild (fixes #31).

remove_project_volumes/remove_snapshot_image/remove_container now report
what they could not remove (treating "already gone" as success rather than
a leftover), remove_project surfaces this to the user via a toast, and
before dropping the project record it writes a pending-cleanup record that
startup housekeeping retries automatically on the next launch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
2026-08-27 08:18:41 -07:00
shadow-testandClaude Opus 5 68b73a9102 Refuse a commit that adds something shaped like a credential
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 4s
Secret Scan / scan (pull_request) Successful in 5s
Build App (Preview) / create-release (pull_request) Successful in 2s
Build App (Preview) / build-macos (pull_request) Successful in 2m34s
Build App (Preview) / prune-previews (pull_request) Canceled after 0s
Build App (Preview) / build-linux (pull_request) Canceled after 3m23s
Build App (Preview) / build-windows (pull_request) Canceled after 3m26s
The companion to the fixture removal. A site-admin token sat in a test file for
92 commits and fourteen days on a public mirror, past five audit rounds and two
independent reviews, because all of them read the code under change and this was
not under change. A grep would have caught it the first day.

`scripts/scan-secrets.sh` is that grep, in three rules:

  * vendor-prefixed credentials — `ghp_`, `github_pat_`, `glpat-`, `xox*-`,
    `sk-`, `AKIA`/`ASIA`, `ya29.`, `AIza`, `npm_`, `dckr_pat_`. Shape alone
    identifies these, so there is no context to get wrong.
  * `BEGIN … PRIVATE KEY` blocks.
  * an opaque literal assigned to a secret-shaped name — the rule that would
    have caught this one.

The third rule needs **both** halves, and that is what makes it usable rather
than another disabled check. Measured before writing it: an entropy-only rule
flags 317 literals in this tree, and name-proximity alone flags four, three of
which are `secure::get_project_secret(&id, "aws-secret-access-key")` — a
keychain *key name* sitting next to the word `secret`. Requiring the literal
itself to be hex or base64 with no word structure is what excludes those.

Validated rather than asserted:

  * **0 false positives** across every tracked file.
  * **Catches the real incident** — `--range 9b2f4fe~1..9b2f4fe` is refused.
  * Twelve shaped cases pass and fail as intended, including a sha256 in an
    `assert_eq!`, a git sha in a comment and the new dummy fixture, none of
    which trip it.
  * The hook was proved to block an actual `git commit`, not just to exist.

Two halves, because each covers the other's gap:

  * `.githooks/pre-commit`, enabled per clone by `npm run hooks`. Git will not
    let a repository set its own hooks path — cloning would then be enough to
    run its code — so this is opt-in everywhere and `--no-verify` skips it.
  * `Secret Scan`, which nobody can bypass. It carries **no `paths:` filter** on
    purpose: the leak lived in `app/**` and `build.yml` only runs for
    `container/**`, so a path-filtered scan would have missed the very thing it
    exists for. It scans the whole tracked tree rather than a range, because a
    wrong range fails *open* and the full pass takes 0.5s.

Also fixed while here: `core.hooksPath` in this clone pointed at
`/workspace/.git/hooks`, a directory that does not exist — so git hooks were
disabled outright and anything dropped in `.git/hooks` would have been ignored
in silence. A hook that never runs is worse than no hook, because the checklist
says it is there.

`--tracked` skips binaries. Feeding a blob to grep gets "binary file matches"
instead of the line, so a genuine finding inside one would arrive as a sentence
nobody can act on.

A line ending `pragma: allowlist secret` is skipped — wordy on purpose, so it
reads as a claim and leaves something greppable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHL9ty7arp8FHwvE77ne7y
2026-08-25 11:51:04 -07:00
shadow-testandClaude Opus 5 d09e2a2743 Stop using a live credential as a test fixture
`the_custom_env_fingerprint_never_carries_the_value` asserted that the custom-env
fingerprint does not leak a secret — using the maintainer's real Gitea token as
the secret. It was committed on 2026-08-11 in 9b2f4fe, reached 92 commits, and
was readable for fourteen days in the public GitHub mirror at
`shadowdao/triple-c`, confirmed by fetching the raw file.

The token was a **site-admin** token (`is_admin: true`, user id 1) with admin
rights on every repository the account can see, not a repo-scoped one. It has
been revoked; the API now answers 401.

Release bundles were never affected — the literal is inside `#[cfg(test)]`, and
a search of the shipped 0.4.62 AppImage finds nothing.

The fixture is now an obviously fake string, and the test is unchanged
otherwise. Mutation-checked against the replacement: a fingerprint that returns
the raw value, one that returns the key name, and one that ignores its input are
all still caught, so nothing about the test's power depended on the value being
real — which was true the whole time.

History is deliberately **not** rewritten. The value was public for two weeks, so
rotation is the fix and the old value is now worthless; rewriting 92 commits of
published history would break every clone to hide something already seen.

Worth noting how this survived: five audit rounds and two independent reviews
all pointed at new code, and this sat in a test file that none of them had reason
to open. A high-entropy-literal check in CI would have caught it on the day.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHL9ty7arp8FHwvE77ne7y
2026-08-25 11:29:16 -07:00
shadow-testandClaude Opus 5 eead748222 Close what two reviews found in the Files tab transfers
Build App (Preview) / compute-version (pull_request) Successful in 5s
Build Container / build-container (pull_request) Successful in 37s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m40s
Build App (Preview) / build-windows (pull_request) Successful in 4m56s
Build App (Preview) / build-linux (pull_request) Successful in 5m11s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Two independent reviews of 2c9482a, one for correctness and one against the
threat model. Between them they found two ways to lose a file, one way for a
container to choose a Windows save destination, and a rule that made every
dotfile unsavable. Every finding below was demonstrated against a real
container before being fixed, and the fixes are demonstrated the same way.

## The download was accepting truncated reads and refusing whole ones

The `[ -f ]` bracket around the read was wrong in both directions.

It missed the case that loses data. Truncation *in place* — `> file`, log
rotation, `tar -x`, most build tools — leaves a regular file behind, so `dd`
stopped at the new EOF and exited 0. Measured: a 600 MB source truncated
mid-read delivered 34 MB, which was then renamed over the user's own earlier
copy and toasted as `Saved (34.1 MB)`. Truncate-to-zero did the same and needs
no adversary at all.

And it failed *good* downloads. `dd` already holds the fd, and neither `rm` nor
`mv` can touch an open one — the bytes are complete. But `rm` makes `[ -f ]`
false, so a finished 600 MB transfer of a file a bundler happened to unlink was
deleted, reporting "nothing was saved". The comment claiming the bracket caught
a "mix of two files" was simply wrong; an open fd cannot be a mix.

So the script now measures the file before reading it and puts the answer on
stderr, and Rust checks that at least that many bytes arrived. One rule,
subsuming everything the bracket was for. Verified against a real container:
truncation mid-read and the FIFO race are refused; deletion, rename-over, a
growing file, an empty file and an untouched 600 MB read all pass.

## On Windows the container, not the user, was naming the save destination

`suggested_save_name` split on `/` only, and it feeds the save dialog's
pre-filled name. Backslash is a legal Linux filename character and
`validate_container_path` has no reason to object — `..\..\Users\…` is one
POSIX segment. The Windows common file dialog parses its name box as a path on
Save, so a container-created file called
`..\..\..\Users\vic\AppData\Roaming\Microsoft\Word\STARTUP\x.dotm` put its own
bytes in an auto-loading Office directory on one un-read click. `resolve_host_path`
did not stop it: Word's `STARTUP` and Excel's `XLSTART` are not in the autorun
denylist, which this file's own docs already concede is "losing by
construction" and was never meant to be the boundary here.

The name is now sanitized of every separator, the drive colon and the rest of
what NTFS refuses, so it cannot be a path on any platform this ships to.

## No dotfile could be saved, and the app pre-filled the name that guaranteed it

The write policy judged the leaf for hiddenness, so `/workspace/.env` was
refused *after* the modal and the overwrite prompt — quoting the name the app
itself had suggested. `.gitignore`, `.dockerignore`, `.eslintrc.json`, `.nvmrc`:
all unsavable, while uploading them worked, so a dotfile could go in and never
come out.

`HostPathUse` gains a third mode. `WriteChosenName` drops the leaf check and
keeps every directory rule, and only `download_container_file` uses it — the
dialog is a real boundary for that caller and only that caller.
`download_container_backup` still takes its path over IPC as a string and keeps
the strict rule.

## Smaller, all found by the reviews

  * A download had no ceiling. `dd` resolves through the container's `PATH`,
    which its agent owns with passwordless sudo; a replacement writing forever
    was measured at ~6 GB/s, so one click on a file listed as 2 KB filled the
    host disk with no progress shown and no cancel. Bounded now by what the
    file measured, with slack that is absolute for small files and
    proportional for large ones, so an honest growing log is unaffected.
  * "Framed rather than verbatim" did not stop container text reaching the
    toast headline: `readableRefusal` matches with `includes`, and it has to,
    because the app's own refusals carry those markers mid-sentence. Anchoring
    would break them. The fix is at the injection point — container text is
    clipped to one 200-character line with control characters stripped — plus a
    `max-h-40` on the toast message, which the `detail` block always had and
    this half did not. 8 KB of prose in a `z-[60]` card pushed its own dismiss
    button off-screen.
  * `savingPath` was a scalar while the design deliberately allows concurrent
    saves. Starting a second freed the first's row mid-transfer, and whichever
    finished first cleared both; dismissing the second dialog was enough. It is
    a `Set` now.
  * `setUploading(false)` fired when the command settled, not when the refresh
    finished, so a second click landed mid-relisting.
  * `upload_files_to_container` checked the container directory before the
    picker and then used the unresolved path. A modal has no time limit. It
    re-checks after, which is what `docker::exec`'s doc comment already claimed.
  * `wait_for_exec_exit` flattened a missing exit code to `Some(0)`, which made
    the new `!= Some(0)` check unreachable by construction.
  * `normalize_host_path` did not collapse repeated separators, so the lexical
    system-root rule was silently absent for `C:\\Windows\…`. Not exploitable —
    the resolved pass catches it — but a documented layer that does nothing is
    a trap for the next caller.
  * README still carried the "no host path crosses IPC in either direction"
    claim the previous commit narrowed everywhere else, and HOW-TO-USE
    described the hidden rule without saying it applies to folders only.

480 Rust tests, 602 frontend, no new clippy warnings. Eleven mutations against
the new tests, all killed — two of the first round survived and were rewritten:
one because `split_whitespace` already handled the case I thought I was
testing, one because I had deleted a comment rather than the behaviour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHL9ty7arp8FHwvE77ne7y
2026-08-25 10:42:26 -07:00
shadow-testandClaude Opus 5 2c9482a67d Give the Files tab back its uploads and downloads
Build App (Preview) / compute-version (pull_request) Successful in 4s
Build Container / build-container (pull_request) Successful in 1m35s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m38s
Build App (Preview) / build-windows (pull_request) Successful in 5m51s
Build App (Preview) / build-linux (pull_request) Successful in 6m50s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
`upload_file_to_container` and `download_container_file` existed on main before
any of this work started. "Ship the Files tab container-side only" removed them
and called it narrowing scope; from a user's side it was a regression they
upgraded into. This restores the feature.

The reason for the removal was real — four consecutive audits found their
criticals in host paths crossing IPC — so the feature comes back only in the
shape that removes the class rather than patching it a fifth time. The dialogs
are opened by **Rust** (`pick_save_path`, `pick_files_to_upload`), not by the
webview. A frontend `open()`/`save()` handing the backend a path string is
exactly what failed, and the backend cannot tell such a string from one a
compromised webview invented. Now the webview can ask for a picker and that is
the whole of its influence: it cannot name a host path as an input. That is the
shape the previous round's own notes named as the honest one if this ever
returned.

None of the machinery the audits condemned returns. No `link(2)` destination
reservation, no placeholder rollback, no collision marker: the OS save dialog
already asks about overwriting and Docker's extractor overwrites on upload the
way `cp` does, so there was nothing left for it to do. Download reuses the
sequence `download_container_backup` has been using unchanged — resolve, stream
into a partial file beside the destination, rename last — so a failed transfer
never touches the file that was already there. Upload reuses the terminal
drop's hardened uploader, with the container's uid/gid resolved once per
selection rather than once per file.

Against a container that is actively hostile rather than merely surprising:

  * the read is `dd iflag=nonblock`, not `cat`. `[ -f ]` and the `open` after it
    are two syscalls and the container owns the filesystem in between; a loop
    swapping the file for a FIFO wins that race, and `cat` then blocks forever
    with no writer and no timeout anywhere on the path — the `invoke` never
    settles and a partial is left in the user's directory for good. Verified in
    a real container that `cat` hangs, that `iflag=nonblock` returns, and that
    it is byte-identical on a regular file.
  * the read is bracketed by a second `[ -f ]`, because non-blocking turns that
    hang into an empty file that would otherwise be renamed over the
    destination and reported as a successful save.
  * an *undeterminable* exit code is a failure. Backup catches this class with
    its `total == 0` check, which download cannot have because an empty file is
    a legitimate save; without a replacement, a project restarted mid-download
    renames a truncated partial over the user's file and reports the byte count
    as if it were whole.
  * container stderr is capped. Every other reader of container output in the
    tree is capped for this reason; the two streaming commands were the
    exception, and stdout was bounded by disk while stderr was bounded by
    nothing.
  * the script's refusals are framed rather than used verbatim, so a directory
    named to look like one of our own sentences cannot become the toast
    headline through `readableRefusal`.
  * the partial name is capped at NAME_MAX. A bundler's 230-character content
    hash is a name that fits its directory and produces a partial name that
    does not.

Also: a non-UTF-8 dialog path is refused by name rather than silently mangled
into a different path by U+FFFD substitution; both actions carry in-flight
state, so a second click cannot open a second dialog and a slow save is not
indistinguishable from a dead button; and the upload's completion message names
the directory, since the picker is modal and the user can browse elsewhere
while it is open.

Not restored: drag-and-drop, in either direction. `drag:allow-start-drag` stays
ungranted and `hold/disk-and-dragout` still holds that work.

Two bugs the new tests caught while being written: a double-click on "Save to
host…" opened the file viewer on top of the save dialog, and an N-file upload
made N redundant execs to re-ask `id -u`.

Docs that asserted this feature did not and must not exist are corrected —
CLAUDE.md, README, HOW-TO-USE, TECHNICAL and the capability threat model. The
"no host path crosses IPC" claim is deliberately narrowed to the inbound
direction: paths do still travel outward inside error text, canonical ones
included, and the reviewed record should not overstate.

600 frontend tests, 473 Rust, no new clippy warnings. Every new test was
mutation-checked; four that survived their first mutation were rewritten,
including two whose mutations turned out to be unfaithful and one that was
blind to a dismissal leaving a row stuck on "Saving…".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LHL9ty7arp8FHwvE77ne7y
2026-08-25 10:00:21 -07:00
shadow-testandClaude Opus 5 88ffb4744a Cancel the keydown on Shift+Enter, or xterm submits anyway
Build App (Preview) / compute-version (pull_request) Successful in 4s
Build Container / build-container (pull_request) Successful in 36s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m56s
Build App (Preview) / build-linux (pull_request) Successful in 5m8s
Build App (Preview) / build-windows (pull_request) Successful in 5m38s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
The handler returned `false` from xterm's custom key handler and a
comment claimed that was enough to stop the bare CR. It is not.
`_keyDown` returns the instant the handler says `false` — before it sets
`_keyDownHandled` and before it cancels the event — and `_keyPress` then
checks that same flag, finds it false, and emits a bare CR for Enter's
charCode 13.

So the headline feature of this branch did the wrong thing in a real
browser: Shift+Enter inserted the newline and then submitted the
half-written prompt, now with a stray blank line in it. Arguably worse
than before the fix. Reproduced in Chromium and confirmed against the
bundled xterm 5.5.0 source.

`preventDefault()` is what stops the browser firing keypress at all.
Applied at both call sites — the desktop terminal and the web terminal's
copy.

The test could not have caught this. jsdom never synthesizes the
follow-up keypress, so `expect(sent()).not.toContain("\r")` was asserting
a property the environment cannot falsify — a test named for a behaviour
it could not exercise. It now asserts `defaultPrevented`, which is the
mechanism that actually suppresses the keypress and which jsdom can
observe. Mutation-checked: removing the `preventDefault()` fails it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 20:47:01 -07:00
shadow-testandClaude Opus 5 016de8f641 Close the blockers from the fifth audit
Build App (Preview) / compute-version (pull_request) Successful in 3s
Build Container / build-container (pull_request) Successful in 10m5s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 4m31s
Build App (Preview) / build-linux (pull_request) Successful in 5m21s
Build App (Preview) / build-windows (pull_request) Successful in 19m1s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
Docs and disclosure. HOW-TO-USE.md's settings table still described the
pre-fix behaviour — and help_commands.rs fetches that file from GitHub
main at runtime, ahead of the embedded copy, so it would have reached
every user's Help dialog the moment this merged. The Config tab named
three settings that need a base-image update; there are four, and the
omitted one (Session recap) is the one that fails *without* the "won't
switch off" symptom the warning teaches. Both now also state the cost
nobody had written down: changing any of these recreates the container,
which commits a layer.

Two stale comments that told a reviewer the code was safe when it was
not. compute_claude_code_settings_fingerprint still claimed the
historical fingerprint is preserved so an upgrade cannot churn every
container — carried over from before the widening, false since the
format string changed. And capabilities/default.json, which is the
reviewed threat model of record, described a "Save to host…" action this
branch deletes.

Security and correctness. update_settings validated env vars and nothing
else, so the *global* default_ssh_key_path — the fallback for every
project without an override — took `/` and read-only bind-mounted the
host, which entrypoint.sh then copies into the home volume. classify_
mount_source ran canonicalize on the raw string, which resolves a
relative path against Triple-C's own cwd, so `.` and `..` were accepted
or refused depending on where the app was launched; the daemon then
refuses the mount and the project can never start. Its test passed only
because its examples did not exist under app/src-tauri.

bind_mount_exclusions still derived a path from every row while
project_path_mounts had learned to skip unmountable ones, so a legacy
row made /workspace/<name> ordinary container content that a migration
would then exclude from staging and destroy. The skip is also logged now
rather than silently dropping a folder.

The terminal's file-in path checked is_dir() but not file type, so a
dropped FIFO blocked forever with no timeout — and it is the only route
in now. The web terminal labelled sessions from a global set at request
time, so two quick opens swapped them; harmless until Shift+Enter became
type-dependent, at which point a mislabelled Claude session submitted a
half-written prompt. Opened now carries the type.

Every ~/.claude.json write goes through one atomic helper. The
awsAuthRefresh branches still truncated in place — the same corruption
the Shift+Enter block was fixed for twenty lines later, and its own
comment said so. Demonstrated: a failed write now leaves the original
byte-identical.

And the registration test I added yesterday could pass while the
property was false: an audit got five real unregistered commands past its
exact-string attribute match, and "exactly once" was in its name but not
its body. Mutation-checked against all six shapes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 18:45:11 -07:00
shadow-testandClaude Opus 5 4d1a5a2417 Remove list_sibling_containers, which nothing called
It listed every container on the daemon — including the user's unrelated
postgres, mysql and other work — and handed the summaries to the webview.
It had a registration, a command, a docker-layer helper, a typed frontend
wrapper and a `SiblingContainer` type, and zero call sites.

An audit named it as step one of an escalation chain: enumerate the
daemon's containers, then point `update_project`'s unvalidated
`container_id` at one and read its files through the file-command
surface. The second half of that chain is closed now, but a command that
exposes the user's unrelated containers and serves no feature is surface
with no upside.

Found by the registration test added in the previous commit, which is the
answer to "why test something the compiler already checks": the compiler
is perfectly happy with a command nobody calls.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 17:15:38 -07:00
shadow-test a323047964 Merge branch 'r4/scrub' into ship/core 2026-08-23 17:12:34 -07:00
shadow-testandClaude Opus 5 11216c45e3 Assert every command is registered, and every registration exists
This is the shape of the bug behind the original OAuth-callback report:
`set_auth_bridge_enabled` existed, worked, and had a typed frontend
wrapper — with zero call sites. The switch the docs told users to flip
was wired to nothing, so every login callback was refused. Both halves
compiled, so nothing noticed.

The reverse direction is the sharper one: a command that is registered
but reachable from nowhere is still IPC surface a compromised webview
can call. `list_sibling_containers`, which returns every container on the
daemon including the user's unrelated work, sits in exactly that state.

Mutation-checked both ways: removing a registration fails the test,
restoring it passes. The first parser I wrote split the list on commas,
which glued each `// Docker` style comment to the command after it and
then dropped that command as a comment — silently, once per group, 17 in
total. Line-based now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 17:12:34 -07:00
shadow-testandClaude Opus 5 913aa85805 Stop the pre-commit scrub running with its guards silently absent
Two fail-open gaps, both latent on the shipped image and both live on an
ImageSource::Custom one.

`--one-file-system` is the only bound on the scrub's `rm -rf`, and it was
probed into `$rmopt` with `rm --one-file-system --help`. BusyBox answers
that with `unrecognized option` and exits 1, so on any busybox- or
toybox-derived image the variable was empty and the delete ran as root
across mount boundaries — reported as a completed `Reclaimed(n)`.
Measured in `busybox:latest` with a named volume one level below the
match at /tmp/claude-x/inner: emptied, `###TRIPLE-C-SCRUBBED 102423`. It
is a prerequisite now, probed by `rm --one-file-system -f -- ''` — the
code path that matters, with the one operand no filesystem can name — and
an image without it prints `###TRIPLE-C-SCRUB-UNAVAILABLE` and deletes
nothing. That costs Alpine and busybox images their scrub, which is the
cheaper half of the trade: declining costs disk, proceeding costs the
mount.

The second is that resetting `PATH` was never the whole of command
lookup. `bash` builds functions out of `BASH_FUNC_<name>%%` environment
variables, function lookup precedes `PATH` entirely, and `command -v`
reports a function as found — so a planted `stat` passed the prerequisite
probe and then answered the containment checks. Every in-shell answer is
itself importable: `unset`, `command`, and — the point that settles it —
`[`, `pwd` and `cd`, which are checks 0 to 2 rather than merely the
tools. So the script is no longer run by the shell that read the
environment. The exec's argv is a bootstrap using only reserved words,
parameter expansions and command words containing a `/` (which `bash`
refuses to import a function for), and it hands the script as `$1` to a
second `/bin/sh` started by `env -i`. Measured on `/bin/sh -> bash` with
`BASH_FUNC_stat%%` set on the container and a volume mounted at the
match: the old invocation emptied it and printed 306565, the bootstrap
left it intact and printed 65536. `/usr/bin/env` then `/bin/env`, because
H3's lesson about hardcoded coreutils locations applies to `env` too.

The comment claiming the `PATH` reset "defeats it just as completely as
spelling /usr/bin/stat out" is corrected, as is the one claiming a
sudo-written /usr/bin/stat does not survive a container restart — it
lands in the writable layer, which is exactly what the commit this runs
in front of captures.

Also here: a stored project path row with an empty host_path or
mount_name no longer becomes a mount. The first sends `field Source must
not be empty` back for the whole create, so the project cannot start at
all until the row is gone, and no amount of save-time validation reaches
a record already on disk; the second mounts over /workspace itself and
the daemon then creates the other rows' mount points inside the user's
real folder.

And in migration_commands, the deferred-reconcile claim is RAII rather
than a trailing statement — a panic inside `reconcile_migration_now`
stranded it for the rest of the process — and `await_release` looks
before it sleeps, so a project released a moment later no longer costs a
full twenty seconds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 17:09:47 -07:00
shadow-test e9902f0564 Merge branch 'r4/narrow' into ship/core 2026-08-23 17:08:04 -07:00
shadow-test 7488fc5b70 Merge branch 'r4/host' into ship/core 2026-08-23 17:08:04 -07:00
shadow-testandClaude Opus 5 06ccb4d818 Ship the Files tab container-side only
Four successive audits found the same thing: host filesystem paths crossing
IPC is where the criticals in this work live. The most recent one found the
`link(2)` upload reservation returning success against a *directory* (linking
into it, leaving permanent stray files, and via a symlink-to-directory writing
outside the validated write root), failing every upload permanently on any
filesystem without hard links, and the post-resolution credential check
weakened from a general rule to an eleven-name denylist.

Rather than fix that a fifth time, the Files tab ships as what it is good at:
a browser, viewer and renamer that never touches the host.

Removed: `upload_file_to_container`, `download_container_file`, and everything
that existed only for them — the whole reservation (`UPLOAD_RESERVATION_SCRIPT`,
`reserve_upload_destination`, the placeholder rollback, `exec_oneshot_as_within`
which had no other caller), `stream_container_file_to_host`, `ChannelReader`,
`save_to_host`, the download ceiling, and the collision marker with its
frontend contract. On the frontend: the upload button, the pane's
`onDragDropEvent` handler, both "Save to host…" affordances, `uploadPaths` /
`downloadFile` / the overwrite prompt, and `OverwriteConfirmModal`.
`lib/uploadErrors.ts` is now `lib/refusalText.ts` and keeps only the half that
turns any backend refusal into the sentence a person reads.

Kept, and not weakened: `upload_host_file_to_terminal` and
`download_container_backup`. They predate this work, their hardening is a real
improvement over main, and they are now the whole answer to "how do I get a
file in or out" — drop it on the Terminal, or Back up container. The drop gate
(`lib/dropTarget.ts`, `PaneVisibility`) is untouched.

`resolve_host_path` gets the general hidden-component rule back. Round 3
replaced it with `HOST_CREDENTIAL_DIRS`, which is allow-by-omission for the
rest of `$HOME`: `~/.local/bin` (write there and you own the user's next shell
command), `~/.password-store`, browser profiles and `~/.pki/nssdb` were all
reachable through a planted symlink with a visible name — verified against a
real home directory, and all five refused now. It over-catches `.pnpm` and
`~/.cache`; for two occasional callers that is the cheaper mistake, and the
refusal says which folder it resolved through.

Two defects fixed while in here:

  * A symlinked directory listed as empty. `find` defaults to `-P`, which does
    not follow a symlink even as the starting point, so `-mindepth 1` discarded
    the only match and a real directory rendered as "Empty directory" — a
    first-order defect now that browsing *is* the feature. `-H` follows the
    starting point and nothing else, so a loop is `ELOOP` rather than a walk
    that does not end; verified against a live container for a symlinked
    directory, a broken link and a loop. `find`'s errno for the loop case is
    now a sentence.
  * `finish_download`'s replace path fired on *any* rename failure with a
    destination present — a vanished partial, a permission error, a directory
    at the destination — and deleted the user's file to complete a move that
    could not complete. It is now fenced to Windows (where a rename onto an
    existing path genuinely fails) and to a partial that still exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 17:05:56 -07:00
shadow-testandClaude Opus 5 9472cb3c4c Resolve host paths before mounting them, and stop reading stored data as choice
Four fixes that share a shape: a value already on disk, or one spelled
around a check, being taken at face value.

`/..` bind-mounted the entire host filesystem read-write. `is_filesystem_root`
was purely lexical — trim trailing separators, refuse what was left only if it
was empty or a bare `C:` — and nothing in the file ever called `canonicalize`,
so `/..`, `/./`, `/home/..`, `/etc/../` and `C:\..` all passed. The daemon
resolves them: `docker run -v /..:/mnt/probe` mounts the host root, and the app
mounts read-*write* into a container whose agent has passwordless sudo. It is
the escalation `check_mount_name_stays_under_workspace` exists to close,
reached through the host-path half of the mount instead of the mount-name half.

`classify_mount_source` replaces it and asks the OS: `canonicalize` applies
`..`, follows symlinks, and resolves 8.3 aliases and UNC spellings on Windows.
A path that cannot be resolved — `projects.json` synced from another machine,
a folder not created yet — falls back to a lexical collapse rather than being
refused, because refusing would make such a project unsavable; the gap is
bounded, since what resolution adds is a property of paths that exist. A path
that names no location at all (`C:x`, a relative path) is refused rather than
guessed at. Same check now guards `ssh_key_path` and `ca_cert_path`, whose
read-only mounts were whole-host disclosure at /tmp/.host-ssh.

Custom env var names had no charset check anywhere, so `BASH_FUNC_stat%%` —
bash's wire format for an exported shell function, body in the value — reached
the container environment verbatim. Latent today because the image's /bin/sh is
dash, but the pre-commit scrub runs `/bin/sh -c` as root and nothing pins that.
Keys are now shell identifiers, on the project and the global list both, with
the same grandfathering the folder rows get: a stored key is admitted, a new or
edited one is not.

The blank workspace row was persisted. The comment said it was dropped on save;
the code computed the filtered list and then saved the unfiltered one, so
"+ Add folder" plus a blur stored `{"Target": "/workspace/", "Source": ""}` and
the project could never be started or recreated again. Every save in the
section now goes through one filter, and a blur that changed nothing saves
nothing.

Widening the five `ClaudeCodeSettings` booleans to `Option<bool>` reinterpreted
every stored record. They were plain `bool`s that always serialised, so every
project ever saved carries an explicit `"env_scrub": false` that nobody chose —
and under the new merge that `Some(false)` beats a global `Some(true)`, where
the old rule let the global win. Upgrading silently turned five settings off,
"strip credentials from subprocess environments" among them. Deserialisation
now goes through a shim that dates the record by the presence of the
pre-widening `enable_session_recap` key and reads its `false`s as unset. The
fields skip serialising when unset, so an older binary can still parse
`projects.json` after a downgrade — a `null` would fail to parse and take the
whole list down, since `ProjectsStore` parses all-or-nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 17:02:03 -07:00
shadow-testandClaude Opus 5 dd23a52b41 Fix four upgrade-path defects the coherence audit found
The ~/.claude.json write was `printf ... > "$CLAUDE_JSON"`, which
truncates before it writes. A write that fails part-way — a full home
volume, which is the exact condition half this release exists to prevent
— leaves the file unparseable, and it never self-heals: the next start's
jq fails on the corrupt file, MERGED is empty, and the guard skips the
write that would have repaired it. That file holds the OAuth account, so
the failure mode is a permanently lost login, in service of a cosmetic
flag that suppresses a tip. Demonstrated: old pattern loses the
credential, new tmp+rename leaves the original intact. The correct
pattern was already in triple-c-task-runner.

The web terminal scoped its xterm key handler to Claude sessions but not
its mobile input bar or its dedicated newline button, so both sent ESC+CR
into `bash -l`, where readline has no binding for it. Silent no-op, and
worse from a button that stays on screen looking live. Both now consult
the active session's type, and the button is disabled with a reason on a
shell tab.

The Config tab claimed "Off overrides a global On" without qualification.
True for the env-var-driven settings, false for TUI mode, Effort level
and Focus mode, whose off state is *removing* a key — an older base
image's entrypoint ignores the instruction to remove it. The copy now
says so and points at the base-image update.

HOW-TO-USE.md said there is no add-task form; AutomationTab renders a
"New task" button. That file is fetched from GitHub at runtime by
help_commands.rs, so the error was live in every user's Help dialog.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 16:45:20 -07:00
shadow-testandClaude Opus 5 168b61d632 Close two cross-file handoffs from the round-3 fixes
The terminal's drop target derived the tar entry name from the host path
*after* symlink resolution, so dropping ~/Downloads/latest.log — where
latest.log is a symlink — landed the file in the container under the
target's name. Nothing errored; the user got a name they never typed. The
Files pane had the identical bug and was fixed with `host_upload_name`;
the terminal now calls the same helper, so the two drop targets cannot
drift. It also drops the "dropped-file" fallback, which silently renamed
anything the old `file_name()` could not parse.

`migration_store::load` told the user a backup existed when it had
deliberately not written one. `load` runs on every reconcile, survey and
reaper pass, so a persistently corrupt record reaches MAX_CORRUPT_BACKUPS
within seconds; from then on the copy was skipped while the log still
read "(a copy was kept at <path>)". Three outcomes are now distinct, and
the "enough already" case says so rather than naming a file that is not
there — that being the message someone reads immediately before going to
look for their data.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 15:52:49 -07:00
shadow-test 47960e46df Merge branch 'r3/drop' into ship/core 2026-08-23 15:48:19 -07:00
shadow-test 73dfaf5785 Merge branch 'r3/auth' into ship/core 2026-08-23 15:48:19 -07:00
shadow-test 01fd38bc4b Merge branch 'r3/files' into ship/core 2026-08-23 15:48:19 -07:00
shadow-testandClaude Opus 5 00128f9b1a Make the scrub work off /usr/bin, and stop a log level deciding whether it runs
H3 — the pre-commit scrub was a silent no-op on any base image whose coreutils
are not under /usr/bin. Hardening against a PATH-planted `stat` shim by naming
every tool absolutely bought nothing the `PATH` reset on the first line had not
already bought — the shim lives in the persisted home volume, and uid 1000
cannot write /usr/bin or /bin — and it cost the whole feature on Alpine, which
Settings -> Docker -> Custom accepts. Measured on one seeded tree in a real
container: the absolute-path script printed `###TRIPLE-C-SCRUBBED 0` and left
every planted file in place; the PATH-resolved one reclaims 77824 bytes. The
default image is unchanged at 521038.

It was silent three times over, and all three are fixed:

* The script now probes for all six things it needs (`command -v` for the five
  tools, plus the root device id reading back as a number) and, if any is
  missing, prints `###TRIPLE-C-SCRUB-UNAVAILABLE <what>` and no total at all.
* `scrub_writable_layer` reads that marker first and returns a new
  `ScrubOutcome::Unavailable`, warning with what the image is missing; a
  genuine `Reclaimed(0)` now leaves a debug line rather than nothing.
* `commit_log_suffix` renders `Reclaimed(0)` as "ran and found nothing to drop"
  rather than "0.00 MB dropped", which is what a scrub that could not run used
  to look like.

Verified in real containers: the mount-at-the-match defence still holds with a
home-volume `stat` shim first on PATH (the volume survives; deleting the PATH
reset from the same script empties it, so the harness can tell the difference).

H2 — the pre-migration scrub had been folded into `log::info!`'s argument list
to satisfy `#[must_use]`. `log::info!` expands to `if Info <= max_level() { … }`,
so the awaited scrub lived inside the level check, and `logging::init`
tolerates `dispatch.apply()` failing — which returns before `set_max_level` and
leaves the process at `Off`. In that state the scrub never ran and the layer
was committed into the longest-lived snapshot the app takes. The outcome is
bound first now, `logging::init` restores the level on failure and says so on
stderr, and a test scans all four files for an `.await` inside any `log::*!`
argument list.

Also:

* `reconcile_migration` deferred a held project instead of dropping it. Its
  only caller fires once per "Docker became available", so a project held at
  that instant was never revisited for the session — phase un-normalised, no
  resume or rollback offered, pin left `Claimed`. It now waits for the holder
  to let go (20s x 90, one waiter per project) and reconciles then.
* The scrub's byte total counts what a partly failed `rm` removed, by
  re-measuring rather than dropping the whole subtree on a non-zero exit —
  which was exactly the `--one-file-system` case.
* The scrub exec blanks `LD_PRELOAD`, `LD_AUDIT` and `LD_LIBRARY_PATH`.
  `LD_PRELOAD` is in none of the reserved env families, so a project's custom
  env var reached a root exec and injected code into every tool the scrub runs,
  `PATH` reset or not. Verified against Engine 29.7 that `docker exec -e` wins.
* The device test is described honestly: it is a mount test under `overlay2`
  and not under `vfs`, where checks 1 and 2 are what still hold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 15:47:36 -07:00
shadow-testandClaude Opus 5 39934299f9 Stop the snapshot retry being an unconfirmed revoke, and unbrick legacy configs
Three things, all reachable from a single credential-handling round.

**The "Retry snapshot cleanup" button deleted the token.** `clear_claude_token`
called `secure::delete_claude_oauth_token()` unconditionally; the `mode`
argument only changed the toast wording, so there was no sweep-only path on the
wire. The leftover panel rendered on `leftover !== null` alone and
`onAuthenticated` never cleared it, so the sequence revoke -> snapshot skipped
-> re-authenticate from the button directly above -> press the retry the panel
is still offering threw away the token acquired seconds earlier, announced by a
message about images. The deliberate Revoke needs a confirmation modal; this
needed nothing.

`sweep_claude_token_snapshots` is the honest primitive: it rewrites the images
and never touches the keychain. The images are the durable record, so the retry
re-derives its work from Docker and needs no stored token. Re-authenticating
now clears the panel, and the Authenticate button is disabled while a cleanup
runs — a sweep is a per-image inspect/create/commit/rmi over the Docker socket
and takes minutes.

**Sweep-first left the token live for that whole window.** The keychain delete
sat behind `list_images` plus the per-image loop, at bollard's 120s-per-request
default, while the UI said "Revoking...". A quit or crash in there and nothing
was revoked at all; worse, `has_claude_token` stayed true and
`shared_claude_auth` reads the keychain at container-*create* time, so a
project whose `SecretScrub` guard had already released could be started later
in the same sweep and be handed a fresh copy of the credential in its env.
Keychain-first now, and the comment that claimed "no window in which a scrubbed
image is re-poisoned" — true of images, silent about containers — is corrected.
The reorder's original justification (crash-mid-sweep recoverability) is what
the sweep-only command covers.

A keychain refusal no longer discards a scrub report, because nothing has been
swept yet, and both remedies stay on screen: Revoke, and the image sweep, which
is now offered in every state rather than only when nothing is stored.

**`update_project` validating every folder list bricked existing projects.** It
validated nothing until recently while `WorkspaceSection` saved `{paths}` on
every blur, so `projects.json` can hold a half-filled row, a mount name with a
space, a duplicate, or `/` as a host path. Any such project became entirely
unsavable — every Config toggle, every permission-mode change and
`useTerminal.ts`'s tab rename came back with a message about folders — and
refusing the save did not unmount anything. `validate_project_paths_update`
admits a row carried over verbatim from what is stored and holds a new or
edited row to every rule, which keeps the escalation closed: introducing a bad
value through this command is exactly what a non-carried-over row is. The
`/workspace/../tmp/claude-x` chain is the one exception and runs on every row
regardless, because a stored one is live data loss rather than untidy data.

`ssh_key_path` and `ca_cert_path` had no check at all; a filesystem root there
read-only bind-mounts the whole host at /tmp/.host-ssh. Refused on change, with
the same grandfathering.

Tests are mutation-checked: reverting to sweep-first fails all four new Rust
ordering tests, pointing the retry back at `clear_claude_token` fails five
frontend tests, and validating an update in isolation fails the legacy-data
tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 15:42:20 -07:00
shadow-testandClaude Opus 5 c6086b0ab3 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
2026-08-23 15:40:05 -07:00
shadowdaoandClaude Opus 5 f7db4323be Make the drop gate a state question, not a geometry one
The gate that decides whether a native file drop is accepted has been wrong
twice in opposite directions, both times because it tried to be precise about
*which points* a dialog covers:

- Round 1 asked `el.contains(elementFromPoint(x, y))` and was handed the inner
  xterm host while the overlays are siblings, so the always-rendered
  Following/Paused button made the terminal's top-right corner permanently
  refuse drops.
- Round 2 replaced that with "is a blocking overlay painted here?" and deleted
  the document-wide gate. `elementFromPoint` returns the *topmost* element, and
  ToastHost is z-[60] against the Modal backdrop's z-50 in the same stacking
  context — so a refused drop pushed a toast, the toast covered the dialog, and
  the next drop released on it was reported clear and landed in the directory
  the dialog was covering. The gate armed its own hole.

Split the two questions instead of merging them:

- Geometry answers *whose* drop it is (rect hit test, unchanged), so exactly
  one listener speaks for a drop and a hidden pane's zero-size rect still keeps
  TerminalView and FilesTab from both firing.
- `dropIsBlocked` answers whether the app should take a drop at all —
  document-wide, no z-index in it. While a modal or blocking overlay is on
  screen anywhere, every drop is refused.

There is no `elementFromPoint` call left, so no future overlay can become a
drop hole by being painted high enough and no chrome can become a dead zone by
being painted at all. The cost is over-refusal while a dialog is open, in a
state the user entered deliberately, announced, writing nothing.

Also:
- `[aria-hidden="true"]` no longer disqualifies a blocker. It is not a
  visibility statement (it sits on visible decorative content), so a blocker
  nested in such a wrapper would have silently stopped blocking.
- Modal drops `data-blocks-drop` when its pane hides, and moves focus out of
  itself rather than leaving it inside a `display:none` panel.
- The refusal notice stays `kind: "info"` (an expected refusal is not an
  error, and an error card never auto-dismisses) and carries a `dedupeKey`, so
  repeated refusals replace rather than stack.

Tests: mutation-checked against the previous implementation — four in
dropTarget.test.ts, two in each of TerminalView/FilesTab, two in Modal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 15:31:13 -07:00
shadow-testandClaude Opus 5 ed91423666 Hold back the Disk panel and OS drag-out from the ship branch
This is a scope reduction, not an abandonment. Both subsystems are
preserved in full on `hold/disk-and-dragout` and are intended to come
back once they have been hardened separately. Nothing here is a
judgement that the features are unwanted — three successive
audit-and-fix cycles each closed a critical defect in these two areas
and each opened a new one, so the rest of the round ships now and these
two get their own cycle rather than holding it up.

Removed: the Disk settings panel and its whole reclaim / destroy /
compaction surface — `DiskSettings`, `DiskProjectTable`, `useDiskUsage`,
`docker/disk.rs`, `disk_tests.rs`, the disk commands in
`docker_commands.rs`, and their `generate_handler!` entries. Dropping
the IPC entries is the point: a UI-only removal would have left five
commands callable by a compromised webview, one of them a verified
arbitrary-DELETE primitive. `sweep_orphaned_snapshots`'s *command* goes
with them (the panel was its only caller); the sweep itself stays.

Removed: OS drag-out from the Files tab — `stage_container_file_for_drag`
and its host staging lifecycle, the pointer gesture and `dragPreview`,
`stageForDrag` / `isStagedHostPath`, the `tauri-plugin-drag` and
`@crabnebula/tauri-plugin-drag` dependencies, and the
`drag:allow-start-drag` capability grant, which could not be scoped.
The capability test's expected list is updated; its `*:default` and
`store:*` assertions are untouched.

Kept, deliberately: drag-and-drop *into* the app (Files pane and
terminal) and "Save to host…", which is now the only route out of a
container. The prevention work is untouched — the pre-commit scrub and
`SNAPSHOT_SCRUB_PATHS`, capped container logs, the `triple-c.base` /
`triple-c.managed` labels, `sweep_orphaned_snapshots` and the startup
housekeeping, the migration pin/probe reapers, scheduler log pruning,
`formatBytes.ts`, and `project_lock.rs` in full with every acquisition
site outside `disk.rs`.

Entanglements, resolved rather than deleted blind:
* `container.rs`'s `a_compaction_runs_this_module_s_scrub_script_byte_for_byte`
  pinned the compaction Dockerfile against `snapshot_scrub_script()`.
  Dropped — it existed only for compaction. `snapshot_scrub_script` and
  its containment tests are untouched.
* `lib.rs`'s startup reap of `:compacting` tags and `triple-c-compact-*`
  containers is dropped: nothing on this branch creates them.
* `project_lock`'s `Compaction` / `CacheClear` variants and
  `any_held_excluding`, `migration_commands::is_migrating`, and
  `formatBytes{Delta,Ceiling}` lose their last production caller but are
  kept and still tested, annotated with why.
* `projects_store::corrupt_since` and `migration_store::peek_ownerless_since`
  were read only by the disk survey and are removed. The corrupt-load
  marker and `.bak` are still written.

Verified: `npm run test` 611 passing, `npx tsc --noEmit` clean,
`npm run build` green; `cargo test` 419 passed / 2 ignored,
`cargo build` 0 warnings. Every test removed belongs to a removed
feature — no kept-behaviour test was weakened or deleted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 15:20:22 -07:00
shadow-testandClaude Opus 5 6a8972980d Stop an untouched secret field from deleting the stored credential
Making `null` actually clear a secret — which it had to, since a blanked
token was previously never revoked — turned the config editors into a
credential shredder. Secrets are `#[serde(skip_serializing)]`, so the
inputs always render empty whether or not one is stored, and the blur
handlers sent `value || null` unconditionally. Focusing the Git token
field and tabbing away deleted it, with nothing shown and no undo.

`useSecretField` encodes the rule: only a field the user typed in may
speak about a secret. Untouched, `patch()` contributes no key at all, and
Rust already distinguishes an absent key from an explicit null.

`withoutUntouchedSecrets` covers the structural half. `saveBedrock`
spreads `{ ...bedrock, ...patch }`, and when that falls back to
DEFAULT_BEDROCK_CONFIG the literal spells every secret out as `null` — so
editing the AWS region would have wiped the credentials as a side effect.

Also here: `WorkspaceSection` no longer saves a half-filled folder row,
which `update_project`'s new validation would refuse on every keystroke
between the two inputs; `snapshots_skipped` is declared on the wire type
rather than widened locally; and `#[must_use]` on `ProjectGuard` and
`ScrubOutcome` — which immediately caught the migration path discarding
its scrub outcome, the one scrub whose silence is expensive because the
layer it declined to clean is about to be committed.

The capability test reads the real file and is mutation-verified: adding
`core:default` back makes it fail. That grant pulls in an unscoped
`std::fs::read` of any host path and went unnoticed for months, because
nothing in the suite read the file at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
2026-08-23 13:23:00 -07:00
shadow-test 7bbb699e4e Merge branch 'r2/front' into integration/round-1 2026-08-23 13:15:02 -07:00
shadow-test 5df3e7996d Merge branch 'r2/files' into integration/round-1 2026-08-23 13:15:02 -07:00