Relative specifiers now follow Vite 6's tryCleanFsResolve order (exact
file, js->ts twin, .mjs/.js/.mts/.ts/.jsx/.tsx/.json, then index), so a
dotted name like ./evil.impl and a .mjs shadowing a .ts resolve to the
file Vite loads. The @tauri-apps/api(/core) boundary covers every code
file under src/, tests included; the main-window count includes
.js/.mjs/.mts/.jsx sources. tauri-commands.ts must call invoke inside a
wrapper's function body and may not load modules dynamically.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Replace the regex/hand-rolled lexer with ts.createSourceFile + AST walking:
module edges from import/export-from/literal import(); aliases, query
suffixes, computed import(), require, import.meta.glob and any non
member-access use of a tauri-commands namespace alias throw; wrapper to
command extraction is read from the AST too.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fix round 3: re-review found stripCommentsAndStrings collapsed whole
backtick template literals, including ${...} substitutions, before the
namespace-alias occurrence scan ever saw them. A tagged template hands
each substitution's value to the tag function by reference,
unstringified, so tag`${X}` smuggled the tauri-commands.ts namespace
object past the check exactly like fn(X) does, and neither threw.
Replaced the regex-based comment/string stripper with a small
hand-rolled scanner (skipQuoted/scanSubstitution/maskTemplate) that
drops literal template text but keeps a substitution's source intact,
recursively re-stripped for its own comments/strings/nested templates,
so an alias referenced only inside ${...} stays visible to (and, when
used via member access, correctly counted by) the occurrence scan.
Unterminated strings/comments/templates/substitutions now throw
(fail-closed) rather than running off the end of the text.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fix round 2: re-review found the resolve-based namespace-import fix
from round 1 unsound for non-dot-access uses. fn(X), const y = X, and
X["name"]/X[expr] all hand the whole tauri-commands.ts namespace
object somewhere the member-access regex can't follow, and none of
them failed closed. In any viewer-closure file that namespace-imports
tauri-commands, every occurrence of the alias after its import line
(comments and strings stripped, best-effort) must now be a plain
alias.identifier member access or the test throws, naming the file
and telling the author to use named imports instead. Also fails
closed on `export * as ns from ".../tauri-commands"`, which the
member-access scan can't audit either. Both checks are scoped to the
viewer side of the ACL boundary (wrapperNamesImportedBy's new strict
parameter) since that's where a missed case is a real escape; the
main-window count stays permissive as before.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fixes four plan-mandated Important findings from review round 1:
namespace imports of tauri-commands.ts are resolved into the wrapper
set (not dropped), export{...}from/export*from re-exports are
followed by the closure walk and counted as wrapper usage, relative
dynamic import() is followed by the closure walk and also checked at
the @tauri-apps/api/core boundary, and an unresolvable non-relative
specifier now throws (fail-closed) instead of silently exiting the
closure. Also narrows the asset-extension filter to an allowlist of
.ts/.tsx/.js/.jsx as source, everything else treated as a non-source
asset.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Task 2's fix round (008c8c0 on feat/app-manifest-lockdown) taught build.rs
to refuse any capability source command_census.rs can't see — non-top-level
capabilities/ entries, .toml/.json5, webviews/remote keys, inline
app.security.capabilities in any tauri config or TAURI_CONFIG — to name
which check failed, and to skip OS/editor junk. Documents those checks in
both capability descriptions and CLAUDE.md's Key Conventions, plus the
known gap that a new tauri.<platform>.conf.json only takes effect on a
clean or CI build.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Task 2 already closed the risk (build.rs declares a Tauri AppManifest and
gates every app command per window); this task brings the docs in line so
the threat model of record no longer claims app commands are ungated or
that any local window can call any app command.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
tauri-build loads capabilities/**/*.{json,toml,json5} plus inline
app.security.capabilities from any tauri config or TAURI_CONFIG, but the
census read only top-level capabilities/*.json, so a .toml, a
subdirectory or an inline capability could over-grant a window with a
green build. build.rs now fails on any of those, on a JSON5/TOML tauri
config it cannot read, and on a `webviews` or `remote` key in a
capability file. OS/editor junk (.DS_Store, swap files) that tauri never
loads is skipped in capabilities/ and permissions/. Each failure is
headed by the check that failed rather than always "capabilities do not
match generate_handler!".
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
build.rs now derives an AppManifest from generate_handler!, which makes
tauri 2.11 apply the ACL to app commands (it skips them entirely without
one). default.json grants the 110 main-window commands, file-viewer.json
the five viewer_* commands, and build.rs refuses to build on a missing,
misspelled, duplicated, misfiled or deny-* grant, or on a hand-written
permission file. Stale autogenerated permissions are pruned per build.
Closes the residual risk recorded by the terminal file viewer: a
compromised viewer window could invoke any app command.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review round 1 (Minor promoted to required fix): the parser applied
rsplit("::").next() once per line, so `a::x, b::y,` on a single line
collapsed to one item and silently dropped a::x — a denied command at
runtime with nothing flagging it. Strip `//` comments per line first (a
whole-line comment strips to nothing, a trailing one leaves the code before
it), then split the cleaned text on `,` so every grant is its own item
regardless of how many share a line.
Adds two_commands_on_one_line_are_both_kept (the regression case) and
a_fixture_shaped_like_the_real_handler_list_parses_every_command (section
comments plus 1-, 2- and 3-segment paths, mirroring lib.rs's real shape).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Moves the generate_handler! parser out of the lib.rs test into
src/command_census.rs and adds the capability rules (one allow-* grant per
command, in the file its name says, no deny-*) with unit tests. No
behaviour change yet: build.rs does not use it until the next commit.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- write.rs: a save's new base is sha256 of the bytes written; the script's
post-mv hash comes back as disk_hash, and a mismatch (another writer landed
after us) shows "Changed on disk" instead of being adopted (ledger M2).
- write.rs: conflict:/gone:/read-only strings are constants with a pure
saved_file() mapping and tests; app/src/viewer/ipcMessages.ts is the one TS
copy and a cargo test checks it against the Rust originals.
- write.rs: the comment now says the in-place `cat >` fallback follows a
planted symlink, and why that is accepted (runs as claude).
- poll.rs: a file deleted between `test -f` and `sha256sum` reads as gone.
- viewerState/EditorPane: poll_failed carries its message; only the
"Start the project before" refusal reads as Container not running, anything
else gets its own banner and leaves Save enabled.
- EditorPane: a failed first read shows Retry and is retried by the poll.
- spec §1: refused OSC 8 targets keep the refusal card (Task 9 ruling).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- CLAUDE.md: document the viewer.html fallback trap (missing/broken Vite entry
silently serves index.html into the viewer window) and point at the Rust test
that pins it.
- Give the manual verification checklist a durable, tracked home: append it as
a markdown task list to the terminal-file-viewer design spec, including the
file-path hover key-hint wording check and the CRLF/BOM round-trip save check.
- default.json: state the app-command residual risk and the pending AppManifest
lockdown directly in the capability file's own description, not only in
CLAUDE.md, since this file is the reviewed threat model of record.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add CLAUDE.md structure notes for the viewer/ frontend and file_viewer/
backend, plus a Key Conventions reminder that a new local window needs its
own capability file and lib.rs's on_window_event guard. Extend default.json's
threat-model census to name file-viewer.json and its allow-destroy grant.
Tighten the capability test in file_viewer/mod.rs from a substring check to
a parsed serde_json assertion of the exact windows list and permission set
for both capability files, per Task 12's controller ruling.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- Keep CRLF (or CR) line endings and a UTF-8 BOM through the editor:
textFormat.ts records the dominant separator and the BOM on load and
restores both on save, so a save changes only the user's edits.
- A clean document whose reload failed retries on the next poll.
- A poll that overlaps a save, or was issued before one settled, is
ignored instead of reading the pre-save hash as a change.
- A conflict whose follow-up poll has no hash shows an error with a
Reload button rather than an Overwrite that could only conflict again.
- Match write.rs's exact read-only message; show the read-only reason as
visible text; error banners are role="alert".
- vite/client types move to src/vite-env.d.ts.
- Tests: CRLF and BOM saves, reload retry, poll/save race, null-hash
conflict, Save and close success and failure, and CodeEditor.setDoc
keeping cursor and scroll.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A second click while the first window was still being built removed its
registry entry, giving a broken window, a duplicate and a cap bypass. The
registry now records when a window is built; `reserve` dedupes, prunes only
built entries whose window is gone (any state, so a leak cannot hold a cap
slot), and enforces the cap in one critical section. Choosing a file already
open elsewhere focuses that window and closes the chooser instead of
resolving a second entry. The not-running sentence names the real action.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
EditorPane loads the resolved file, polls it every 2 s while visible,
reloads a clean buffer silently and shows the "Changed on disk" banner
for a dirty one, saves against the loaded hash, and intercepts closing
with unsaved edits. ViewerApp routes to the editor, the not-found list
or the choose list.
Preflight rulings carried: one reload helper that passes the truncated
flag and polled hash (P3/P14), a poll right after a save conflict so
Overwrite on save adopts the current hash (P4), a chunked base64
encoder (P5), StatusIndicator for the badge (P11), banner-only test
queries (P2), and a Range geometry stub for jsdom (P17). A save the
container user may not write is reported as read-only and keeps the
buffer.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Fix round 1 for Task 3, addressing task-3-review.md's I1-I3 (reproduced
under dash) plus M3 and M10 from the same review.
- I1: WRITE_SCRIPT read the target's hash through `sha256sum | cut … ||
exit 1`. POSIX sh has no pipefail, so that `|| exit 1` tested only
cut's exit status — an unreadable target (EACCES, EIO) left $actual
empty, which never equals $expect, so the script silently reported
exit 3 (conflict) instead of a real error. The user got a misleading
"changed on disk" banner whose "Overwrite on save" could never
succeed, since the next poll hit the same read error. Fixed by
reading the hash from a plain command substitution
(`actual=$(sha256sum -- "$target") || exit 1`) and splitting out the
hash field in shell instead of piping into `cut`.
- I2 (+ M3): a failed `cp` into the staged file (ENOSPC, quota, EFBIG,
EIO) left a partial `.<name>.triple-c-<pid>` behind in the user's
own directory — the EXIT trap only ever removed $tmp. Fixed by
creating the staged file with `mktemp` (M3: exclusive, unpredictable
name, so it can't be planted or follow an existing symlink) and
adding it to the trap as soon as it's assigned
(`trap 'rm -f -- "$tmp" ${staged:+"$staged"}' EXIT`), so any later
failure — cp, chmod, mv, or a signal — cleans it up too.
- I3 (controller ruling): the script only ever checked `[ -w "$dir" ]`,
so a 0444 file (or one owned by another uid) was silently replaced
via rename, defeating the file's own write protection even though
spec §5 step 3 reads that way literally. Added `[ -w "$target" ]`
before the branch; a non-writable target is refused with "The file
is read-only for the container user." on stderr and a distinct exit
code (5, `EXIT_READ_ONLY`) that `classify_write` maps to that same
message rather than falling into the generic clipped-stderr arm.
- M10: added six `#[cfg(unix)]` tests that run WRITE_SCRIPT for real
via `sh -c` against a temp directory on the host (not just needle
matches against the script text) — clean save, stale-base conflict,
gone target, unreadable target (I1), read-only target (I3), and a
failed stage leaving no partial file behind (I2). The unreadable/
read-only tests self-skip with a message if permission bits turn out
not to block root, rather than false-failing under a root test
runner.
Verified: `cargo test --offline file_viewer` — 19/19 passing, pristine
(up from 12; 6 new host-execution tests plus 1 for the new exit-5
classify_write arm). `cargo clippy --offline` (and `--tests`) — no
warnings in file_viewer::write; the 28 warnings clippy reports are
all pre-existing, in unrelated files.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
showFileCard now records modifierPromised like the OSC 8 hover, and the
file-path provider's gate goes through the handler's new opensFileLink,
so a "Shift+click to open" card cannot be answered by a bare click after
the container drops mouse tracking. A click before the session's project
is known now toasts instead of doing nothing. Refusal-card tests assert
the card is present; a misplaced test comment is back on its test.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Registers a file-path ILinkProvider (after WebLinksAddon) that opens the
file viewer at the matched line, and turns on allowNonHttpProtocols so
OSC 8 file: targets reach createOsc8LinkHandler, which now parses every
target and refuses anything but file: and http(s):. The hover card gains
an "Open in viewer" variant, exposed as showFileCard(rawPath) so relative
paths are shown as printed (preflight P6).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Implements Task 10 of the terminal file viewer plan: highlightLine.ts
(line-range StateField + setHighlight effect), viewerTheme.ts (EditorView
theme + syntax HighlightStyle), and CodeEditor.tsx (the React wrapper with
getDoc/setDoc/goTo/focus).
Also implements languages.ts + languages.test.ts, moved here from Task 7
per preflight ruling P1 (they need @codemirror/* packages, which only
Task 6 installs).
Preflight rulings applied:
- P1: languages.ts/.test.ts live here, not in Task 7.
- P12: syntax colours are `--syntax-*` custom properties in index.css,
not hard-coded hex.
- P13: languageFor tests assert `.not.toBeNull()` for mapped extensions
and move README to the "returns null" case, instead of the vacuous
`.resolves.toBeDefined()`.
- P14(c): readOnly extension array factored into readOnlyExt(readOnly)
instead of being duplicated in CodeEditor.
- P16: no custom Mod-g binding; searchKeymap's Mod-Alt-g covers "go to
line" already.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
poll.rs: one exec per tick that tests existence then hashes+stats the
file (sha256sum/stat), so the 2 s poll costs one exec instead of
re-downloading up to 1 MiB of archive per window per tick.
write.rs: sha256_hex/is_sha256_hex, MAX_WRITE_BYTES, and the save
script. Saving stages the payload in /tmp via the existing
write_file_to_container (owned by the container user, since the
Docker archive API writes as root), then an exec as `claude` checks
the base hash, swaps the file in with a same-directory rename when
the directory is writable (falling back to an in-place `cat >` when
it is not), and always cleans up the staged temp file via `trap`.
classify_write maps exit 0/3/4 to Saved/Conflict/Gone.
Applies three pre-flight rulings against the brief's literal text:
- P8: pulled the write script's argv shape and the size/hash checks
into pure `write_command`/`check_write_input` helpers with their
own unit tests, since both lived only inside the untested async
`write_file` otherwise.
- P9: the brief's manual Docker smoke-test invocation
(`sh w.sh save target tmp hash`) makes `$1` become "save" instead
of the target, which is not what the script or the Rust caller
expect. Verified in a throwaway container that invoking the file
directly without the dummy "save" arg reproduces the Rust
convention's `$1/$2/$3` correctly: exit 0 with the new hash and a
removed payload on a clean save, exit 3 with the file untouched on
a stale base hash, and exit 4 when the target is gone.
- P15: GNU sha256sum prefixes its output line with `\` when the
path contains a backslash or newline. Without a fix that turns
into a permanent false conflict (write.rs) and a blinded poll
(poll.rs, hash: null forever). Both parsers now strip a leading
`\`, and the script itself strips it from $actual before comparing
to $expect. Verified against real sha256sum output in a container
with a backslash-named file: the save no longer false-conflicts
and the reported hash matches.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Task 6 of the terminal file viewer plan: app/viewer.html plus
src/viewer/{main,ViewerApp}.tsx (placeholder ViewerApp for Task 11 to
replace), registers viewer.html as a second Rollup input in
vite.config.ts, adds the file-viewer capability restricted to
file-viewer-* windows (allow-listen/unlisten for the goto event,
allow-destroy for the close-button/prevent_close interaction,
allow-internal-toggle-devtools to match default.json's dev
convenience), installs the CodeMirror packages Task 10 builds
languages.ts on top of (P1), and pins the viewer entry/capability
with a Rust fallback-trap test.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- viewerState.ts: pure reducer for the clean/dirty, same/changed/gone,
container-down and overwrite-on-save states (spec §5), plus the
pollEffect/canSave helpers EditorPane will drive off.
- editability.ts: classifies a fetched file as text/image/binary and
decides whether it is editable, deferring to Rust's readonly_reason
when it refuses.
Per preflight P1, languages.ts/.test.ts move to Task 10 (needs the
CodeMirror packages Task 6 installs; out of scope for this task's
worktree). Per P3, the "reloaded" action now carries `truncated` and
`polledHash` so a poll-driven reload of a truncated (prefix-hash-only)
file adopts the polled full-file hash instead of re-triggering a
reload on every subsequent poll -- with a reducer test covering it.
Per P13, tightened the poll_failed/canSave test to start from a dirty
doc so it actually exercises containerDown rather than passing only
because the doc was clean.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds findFilePathLinks, a pure matcher that finds file paths (with
optional :line, :line:col, :start-end and #L42/#L40-L50 suffixes) in
a line of terminal text, skipping anything inside a URL and stripping
markdown/quote wrapping from the matched span.
Per pre-flight ruling P7, a slash-having token still requires the
basename to carry a real extension (any extension) or be a known
extensionless basename -- a bare and/or or src/components is no
longer treated as a path.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Implements resolve.rs: candidate_paths() turns a raw path Claude printed
into an ordered, capped, deduped list of container paths (absolute paths
pass through as-is; relative paths probe /workspace then each project
mount, in order), PROBE_SCRIPT + parse_probe_output() run one exec that
prints realpath -e of every candidate that is a regular file, and
probe_candidates() wires the two together over exec_oneshot_streams_as.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Task 2 of the terminal file viewer plan: ViewerRegistry tracks which
file-viewer-<n> window is looking at which container path. reserve()
takes the cap check and label allocation atomically under one lock so
two concurrent open requests cannot both slip past the 20-window cap;
find_open() only matches windows in the Resolved state, so a window
still choosing a candidate or reporting not-found is never treated as
"open on" a path.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds joinWrappedRows/offsetToCell for reassembling a wrapped xterm
line into its full text plus offset->cell mapping, so link matching
can find paths that xterm has soft-wrapped across rows.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Task 0 of the terminal file viewer plan: the shared interfaces that seven
later tasks build against in parallel. Adds ViewerLocation/ViewerTargetState/
ViewerState/ViewerFile/ViewerPoll to types.ts and their invoke() wrappers to
tauri-commands.ts, creates the file_viewer Rust module (mod.rs with
MAX_VIEWER_WINDOWS/VIEWER_LABEL_PREFIX/is_viewer_label, plus placeholder
registry/resolve/poll/write/window submodules), wires it into lib.rs, and
loosens visibility on the file_commands.rs helpers the viewer commands will
reuse (MAX_READ_BYTES, validate_container_path, validate_container_write_path,
FetchedFile, fetch_container_file, require_running, clip_container_text).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reviewed three times. Rounds 1 and 2 each found a real hole in the gate -- a plain click opened links, then a selection gesture did -- both addressed. The attacker-controlled mouse mode is recorded as a known residual rather than claimed closed.
Still unverified on a real desktop: double-click and drag-select across a link in both tracking states.
The previous commit tightened the gate's click-count check from `> 1` to
`!== 1`, which two tests in the wiring block did not survive: they built
`new MouseEvent("click", { button: 0 })` directly rather than through the
`click()` helper, so `detail` defaulted to 0 and the gate refused them.
The gate is right and the tests were wrong -- a mouseup derived from a real
click always carries `detail >= 1`, and 0 is exactly the synthetic-event
shape the tightening was for. Both now pass `detail: 1`.
I pushed the previous commit without noticing this, having read a truncated
test summary that hid the failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Third-round review polish; no behaviour change beyond the first item.
`detail > 1` was justified in a comment by noting a synthesised event
carries `detail` 0 -- which is an argument for letting untrusted synthetic
events through the click-count half of the gate. A mouseup derived from a
real click always carries `detail >= 1`, so the check is now `!== 1`.
Nothing in the container can dispatch a DOM event, so this is hardening
rather than a hole; the comment now says that instead of the reverse.
Three comments claimed more than they hold. The selection check's
paragraph read as though it caught every copy gesture: it sees a drag only
once the drag has spanned a cell, so a press and release inside one
character cell -- or a drag walked back to its start -- still opens the
link. That is the gap the rejected mousedown/mouseup distance check would
have closed, and it is now recorded beside the reason for rejecting it.
`?1002l` was described as taking effect synchronously with the write; it
takes effect when xterm parses it, on its queued write task. And
`modifierPromised` was described as written on every hover, when `hover()`
clears and returns early with no host element -- which leaves it false, the
stricter direction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re-review found the gate did not cover the gesture users actually make.
xterm's `Linkifier._handleMouseUp` has no click-count check, no distance
threshold and no timestamp, so it activates on the mouseup that *ends a
selection* as readily as on a click. Double-clicking a word or dragging
across a few characters inside an OSC 8 link therefore opened the browser.
Worse with a program holding the mouse: the only way to select text there
is Shift/Option+drag, which is byte-identical to the gesture the gate
accepted as a deliberate request to open. A container wrapping each output
row in a link would have harvested every legitimate copy.
`term.hasSelection()` is the load-bearing check: a drag is one press and
one release, so its click count is 1 and `detail` cannot see it. `detail >
1` is belt-and-braces for the case where the selection came out empty, and
for not depending on the selection model being written before the
Linkifier's listener runs -- it is, but the check costs nothing. Drag
distance was rejected rather than forgotten: xterm hands `activate` only
the mouseup, so measuring it means binding our own listener and keeping a
second source of truth about one gesture.
The hover card's promise is now sticky. The hint was computed once at hover
while the gate re-read the mode at mouseup, so a card reading "Shift+click
to open" could be on screen while a bare click opened the link. The gate
now requires the modifier if either the card asked for it or the live mode
does.
The same gate is applied to the WebLinksAddon branch, which had none. That
also closes a real bypass: `OscLinkProvider` drops non-http(s) OSC 8
targets before `linkHandler` sees them, so a `javascript:` target with an
`https://evil.tld` label fell through to WebLinks and opened ungated.
What is not closed, and is now recorded rather than papered over: the mouse
mode is a permission the container grants itself. It can drop tracking
before the pointer arrives and hold it off through the click. The selection
and click-count checks hold either way, so the mass-harvest variant is
gone, but the real fix needs a signal the container cannot write and this
pane does not have one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two accuracy defects from re-review, both the same class as the bug this
branch exists to fix.
`probe_failed` rendered every failure as "This project's container could
not be inspected", but only two of the four readings are about the
container -- the others are the base image and the snapshot. A malformed
base image name in settings therefore pointed the user at the wrong object.
The sentence now names the check rather than the container.
The doc claimed "the first error wins, in call order". It does not: the
checks run container_id, base_image_id, container_running, while the daemon
is called in a different order entirely. The priority is deliberate -- it
puts the reading that stopped the probe first -- so the comment now says
that, instead of describing an order the code does not use.
The test guarding the first point asserted the message does not contain
"Docker", using a synthetic payload. The real bollard error for that case
is "Docker responded with status code 400: invalid reference format", so
the assertion passed only because the payload was invented. It now uses the
real shape and asserts what actually matters: that nothing we add claims
the daemon was unreachable or names the container.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of this branch found its central premise was false. The claim was
that xterm cancels a mousedown before the link layer while a program holds
the mouse, so only a Shift+click could reach a link. None of that holds:
`cancel()` is `if (this.options.cancelEvents || force)` and `cancelEvents`
defaults to false and is never set here, so it does nothing; the mouse
reporting listeners bind to `.xterm` while the Linkifier is constructed on
`screenElement`, a descendant, so the link layer sees the event first
regardless; and `_handleMouseUp` checks neither the modifier nor the
button before calling `activate`.
So a plain click opened the link, and so did a right-click. That is not a
missing convenience. OSC 8 lets the container wrap any clickable TUI widget
-- a menu row, a "1. Yes", a file chip -- in a link to anywhere, and
because the mouse report still reaches the program afterwards the widget
responds too and nothing looks wrong. The hover card was the only
mitigation, and it assumes a user deliberately reaching for a link.
`opensOnClick` is now a real gate: primary button only, and while a program
tracks the mouse the force-selection modifier is required -- the gesture
the user already has for "this click is for the terminal, not the program".
With nothing tracking, a bare click opens, which is what WebLinksAddon
already does for plain-text URLs in the same buffer. The mode is read per
click through a getter rather than captured, and `syncMouseCapture` and the
gate share one expression, because a gate that disagreed with the badge
would be the hole again.
The gate and the hint also share one modifier predicate, and the hint is
conditional on tracking, so it can never name a key that does nothing.
Three more from the same review. The origin span had `flexShrink: 0`, which
beats `overflowWrap` under flexbox, so an attacker-controlled 600-character
origin ran off the pane and hid the registrable domain -- the same spoof as
an ellipsis, without one; it now wraps and the remainder is what gives way.
The card had no `pointerEvents: none`, and `xterm-hover` is inert at this
placement, so a card under the pointer took `mouseleave` from screenElement
and made bottom-row links flicker and refuse to activate at all. And the
design doc comment had come adrift from its function.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of this branch found the first cut made every probe error fatal,
including one that is usually irrelevant. `snapshot_exists` is consulted
only when there is no container, or when a stopped container coincides with
a busy project -- `pick_probe_source` discards it outright for a running
one. So a daemon hiccup between the four sequential readings turned a full
report into a bare "could not be checked" with Update disabled, in a change
whose whole purpose is handling exactly that hiccup better.
It is now carried as a `Result` to the points that consult it and surfaced
only there. `stopped_probe_policy` carries its own message, because
"try again once it finishes" claims waiting is the only obstacle, which a
failed `image_exists` has not established.
`base_image_id` stays fatal, deliberately: it is the right-hand side of the
comparison, and `image_id` already distinguishes "not pulled locally"
(`Ok(None)`, a legitimate not-stale) from "could not ask". Letting an `Err`
through as `None` would report a project up to date on a reading nobody
got -- #56 one field over.
The message no longer blames the daemon. Three of the four callees can
`Err` from a daemon that answered perfectly: `image_id` maps only 404 to
`Ok(None)`, and the base image name is user-supplied, so a malformed
reference told the user to go fix a daemon that was running fine. That is
the same category of error as #56 itself.
`ContainerState` makes "running is known but no container was found"
unrepresentable rather than merely unreached, so the downstream match has
no impossible arm and the invariant is enforced where it is established.
Finally, the tests covered the new function but not the line the bug was
on: a partial revert to `.unwrap_or(None)` kept them all green. The
readings now travel as a named struct of `Result`s, so that revert is a
compile error -- verified by performing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude Code prints links as OSC 8 hyperlinks whose visible text is
hard-wrapped into terminal-width pieces -- urlDetector's header records a
346-character sign-in URL arriving as five emissions, each carrying the
whole URL in its parameter and about 80 characters on screen. WebLinksAddon
regex-matches the painted characters row by row, so against Claude it
matches a fragment or nothing, which is why the URL toast exists.
xterm 5.5 hands over the exact parameter through `linkHandler`, so the
slicing stops mattering. WebLinksAddon stays for plain-text URLs in
ordinary shell output; the two cover different cases and neither replaces
the other. Both now share one failure reporter and one validator.
No new key handling was needed. xterm's mousedown handler is
`if (areMouseEventsActive && !shouldForceSelection(e)) return cancel(e)`,
so holding the force-selection modifier lets the event reach the link
layer while Claude still holds the mouse -- Shift+click, or Option+click on
macOS, which this terminal already enables for text selection.
The hover card is the security half rather than decoration. OSC 8
decouples the label from the target completely: a container can print
`https://claude.ai` and link it anywhere, which is strictly worse than the
userinfo spoofing already guarded against and which invalidated the
justification for opening a click without confirmation ("a deliberate act
on visible text"). Hovering now shows the real origin, in full and never
truncated, because truncating it is the spoof. A target that fails
validation says so and deliberately echoes nothing of itself.
The hint names the modifier for the platform, from xterm's own `isMac`
list, so it cannot tell a Mac user to press a key that does nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`get_container_staleness` collected four probes through `unwrap_or`, so a
transient daemon fault landed on the same arm as a genuine absence and the
banner said, confidently and wrongly, that the project has no container or
snapshot image to compare against.
The four readings are now taken as `Result`s and funnelled through a pure
`collect_probe_inputs`, following `pick_probe_source` and
`stopped_probe_policy` in the same file, so the rule is unit-testable
without touching Docker. The first error in call order wins and becomes
`probe_error`; the command still returns `Ok`, because the hook's `catch`
sets `staleness` to null and the banner returns early on null -- an `Err`
here would hide the fault instead of reporting it.
One of the issue's premises did not hold. `is_container_running` does not
distinguish absent from unreachable: its body flattens every
`inspect_container` failure to `Ok(false)`, so only a `get_docker` failure
can surface as `Err`. Its `Result` is threaded through anyway, since that
one case is a real daemon-unreachable signal and this layer no longer adds
a second swallow on top, and the remaining gap is documented where the
decision is made rather than patched in `docker/container.rs`, which the
issue puts out of scope and whose doc comment says the swallow is
deliberate. In practice `find_existing_container` runs immediately before
and would already have errored if the daemon were down.
No frontend change: `probeUnavailable` in ContainerMigrationBanner already
routes a set `probe_error` to "Some checks did not complete".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of this branch found that `update_project` restored
`browser_view_enabled` from the store but took `auth_bridge_enabled` from
the IPC payload, on a comment claiming the Config tab edits it through that
save. The comment was wrong. `AuthBridgeRow` is the only writer, it calls
`set_auth_bridge_enabled` out of band precisely so the switch works while a
login is hanging, and it never writes the value back into frontend state --
so a payload's copy of that flag is always a stale snapshot.
The consequence was not cosmetic: turn the bridge off, then close a renamed
terminal tab, and `useTerminal` round-trips the stale `true` and the
reconcile block restarts a bridge whose own UI warns that a bridged port is
unauthenticated and reachable by any local process. Defaulting the flag to
true earlier in this branch made it worse, since the stale value is now
true for every pre-existing project.
Both flags are now restored from the store by `restore_store_owned_fields`,
and the reconcile block is gone rather than corrected: with the value
always restored it could only re-assert what was already true, and every
writer already owns its own side effect -- the setter starts and stops
synchronously, container start arms the bridge, launch reconcile re-arms
it, and the poller re-reads the flag each tick and self-terminates.
Re-adding a start path to the one function that no longer owns the flag is
what caused this.
Turning the browser view off also stopped tearing the session down when the
project record had vanished, because the persist used `?` and returned
early -- the supervisor's own `store.get()` check exists because records do
vanish mid-session. Teardown is now unconditional and the write error still
surfaces afterwards, since the stored flag saying "enabled" means the view
returns on next launch and that is worth reporting.
Finally, the opener no longer falls through to `gio` on any non-zero exit.
xdg-open's 1, 2 and 3 assert no handler ran; 4 also covers a handler that
was launched and then failed, which would have opened the link twice --
two authorize requests for one click in an OAuth flow. Reasoned from
documented exit codes rather than an observed double-open, and the cost is
stated: a genuine code-4 failure no longer reaches gio.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two findings from review of this branch.
Awaiting the open instead of dismissing up front bought a window: on Linux
it is at least OPENER_GRACE, doubled when xdg-open fails and gio is tried.
If the container relays a second URL inside that window, the first open's
resolution blanked the second prompt -- losing a link that exists only in
the container's transcript, which is the failure "dismiss on success only"
was made to prevent. The slot already carried a `seq` for exactly this
reason; dismissal is now conditional on it.
`urlPromptRef` is written eagerly by the two functions that change the slot
rather than synced by an effect. That is load-bearing: an effect-synced
mirror lags state by a commit, and a promise microtask can resolve between
`setUrlPrompt` and React flushing passive effects -- so it answers "did a
newer prompt land?" wrong in precisely the window the guard exists for.
Dropping the functional updater also fixes `promptSeqRef.current += 1`
being mutated inside a state updater React is free to invoke twice.
The guard is a sibling function rather than an optional argument on
`dismissUrlPrompt`, because that function is passed by reference as
UrlToast's `onDismiss` and React would hand it a MouseEvent as its first
argument -- the seq check would fail and the close button would silently
stop working, with the types still assignable.
Separately, the sign-in hint was binary on which button leads, but "host
leads" covers both a live bridge and a fallback where nothing is set up to
catch the callback at all. In the second case the toast promised the bridge
would carry it and the login hung to its timeout. The target is now
three-state, the hint tells the truth in the fallback case and names the
control that fixes it, and the hook starts at `host-fallback` rather than
assuming a bridge it has not confirmed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tracked build output; regenerated by the Tauri build from
capabilities/default.json.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
default.json carried this grant with an explicit accepted residual risk:
a compromised webview could make the OS open an attacker-chosen http(s)
URL. It was accepted because it could not be narrowed -- WebLinksAddon
opens links Claude printed inside the container, which are arbitrary by
construction, so a host allowlist would have deleted the feature.
Now that every host-browser open routes through `open_url_external`, the
webview has no reason to reach the plugin directly, and the risk closes
rather than stays recorded. The plugin remains a dependency: macOS and
Windows still use it, through `OpenerExt::open_url`, whose desktop
implementation calls `crate::open::open` directly and is not gated by
capabilities at all (tauri-plugin-opener-2.5.3/src/lib.rs:60) -- verified
rather than assumed, since the whole point is that the Rust path keeps
working. What is removed is the webview's ability to reach the opener
without passing the Rust-side validation.
The census note in default.json is rewritten to match, and lib.rs's
grant-list test is updated deliberately, as its own assertion message
demands.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Rust command existed but nothing called it. All four frontend call
sites still used `openUrl` from `@tauri-apps/plugin-opener`, so the
environment fix was inert and the three dialogs carried the same Linux bug
as the terminal: DockerInstallDialog's docs link, ClaudeAuthModal's sign-in
link and UpdateDialog's release link would all have reported success while
launching nothing.
`openUrlExternal` in tauri-commands.ts is now the single sink. There is no
platform branch: Linux gets the sanitized spawn, macOS and Windows reach
the same plugin as before but from Rust, and every platform picks up the
Rust-side re-validation, which matters because these URLs originate in an
untrusted container.
Comments in urlRelay.ts and urlDetector.ts that named `openUrl` as the sink
they guard are updated to match, and the two test files that mocked
`@tauri-apps/plugin-opener` now mock the command instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
On Linux the app ships as a single AppImage, and the AppImage environment
leaks into everything it spawns. linuxdeploy's AppRun, linuxdeploy-plugin-gtk
and our own wayland fallback hook all export LD_LIBRARY_PATH, GTK_PATH,
GIO_MODULE_DIR and friends pointing inside $APPDIR, and main.rs sets
WEBKIT_DISABLE_DMABUF_RENDERER process-wide for the webview. A browser that
is already running shrugs this off, because xdg-open just hands the URL to
the existing process. A cold-launched one inherits the lot and dies before
painting -- with xdg-open still exiting 0, which is why this looked like the
button doing nothing at all.
`url_open` captures a pristine snapshot of the environment in main() before
any mutation runs, then hands children a repaired copy: a saved original is
restored where one exists, otherwise the process-start value is restored
where we changed it, otherwise only the colon-separated entries that live
under $APPDIR are dropped and the user's own are kept. Outside an AppImage
it is a no-op.
The command re-validates the URL in Rust rather than trusting the frontend,
because the URL originates in an untrusted container: http/https only, no
embedded credentials, no control characters or whitespace, length capped,
ASCII asserted before it reaches execvp, and error messages never echo the
input. Spawning is Command with explicit args and never a shell, trying
xdg-open then gio open.
No portal. org.freedesktop.portal.OpenURI would pull in a D-Bus client stack
for one call on the one platform where we ship self-contained, and it only
helps where a portal is running -- the same case where xdg-open already
works once the environment is clean. `gio open` as a second candidate
recovers most of the missing-MIME-association case for free.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`isAnthropicSignInUrl` made the container the default action for every
Anthropic sign-in link, justified by "the host has nothing to catch it
with". That was wrong in both directions. The host does have something --
the auth bridge -- and the container side is not a general browser at all
but Playwright's dashboard, whose packages and chromium are deliberately
not baked into the image. So the default pointed at the one path that is
uninstalled on a fresh project, on every platform, while the path that
works sat behind a switch.
The decision now lives in `useSignInOpenTarget`: a live auth bridge picks
the host, otherwise a container that can actually launch a browser picks
the container, otherwise the host. It resolves at mount rather than when a
URL arrives, so the buttons do not swap under a moving mouse, and it
re-decides on `auth-bridge-changed` so flipping the switch during a
hanging login takes effect. A bridge with port conflicts reads as not
live; an empty `active_ports` does not, since there is nothing to bridge
until the CLI binds its listener and that races the URL.
Both buttons still render either way -- this changes which one leads.
`sanitizeRelayUrl` is byte-for-byte unchanged, so the embedded copy in
web_terminal/terminal.html needs no matching edit.
The host "Open" path also failed silently: `dismissUrlPrompt()` ran before
`openUrl`, so the toast vanished and a rejected promise reached only the
devtools console. Dismissal now happens on success only, leaving "In
container" one click away after a failure, and the error surfaces through
the same toast the container path already used. On Linux this catch will
not fire for the common case -- `xdg-open` routinely exits 0 having done
nothing -- so it complements the AppImage environment fix rather than
replacing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A CLI running `claude login` inside the container binds a random ephemeral
loopback port and hands the provider a redirect pointing at it. The browser
is on the host, so the callback lands on a closed host port and the login
hangs with no diagnostic. The auth bridge is the thing that fixes this --
it mirrors container loopback listeners onto the same host port -- so
having it default to off made a hang the out-of-the-box experience.
`auth_bridge_enabled` now defaults to true through a
`default_auth_bridge_enabled()` serde helper, matching the shape already
used by `use_shared_auth_token`. Because the default is applied at
deserialisation, projects stored before the bridge existed pick it up too;
`migrate_from_value` writes neither flag, so nothing defeats it, and a
regression test pins that.
Separately, `BrowserViewManager.enabled` was in-memory only and the durable
`browser_view_enabled` field on the project record was never implemented.
Rather than sync the two, the cache is removed and the record becomes the
single home for the flag, mirroring how `AuthBridgeManager` already works.
`stop()` deliberately does not clear it, since container teardown and
migration reach that path and neither is the user changing their mind.
Durable does not mean auto-started: a restarted app reports enabled with
the viewer off.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
