Compare commits

..
10 Commits
Author SHA1 Message Date
jknapp 3537b234d8 Make links in Claude's output clickable (#59)
Build App / compute-version (push) Successful in 6s
Secret Scan / scan (push) Successful in 6s
Build App / build-macos (push) Successful in 2m44s
Build App / build-windows (push) Successful in 4m58s
Build App / build-linux (push) Successful in 5m51s
Build App / sync-to-github (push) Successful in 8s
Build App / create-tag (push) Successful in 9s
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.
2026-09-19 03:20:15 +00:00
shadowdaoandClaude Opus 5 83c9c24951 test: give two synthesised clicks the detail a real click carries
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 9s
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 2m43s
Build App (Preview) / build-linux (pull_request) Successful in 7m58s
Build App (Preview) / build-windows (pull_request) Successful in 4m54s
Build App (Preview) / prune-previews (pull_request) Successful in 1s
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>
2026-09-18 20:08:36 -07:00
shadowdaoandClaude Opus 5 c6f9c1d43f fix: tighten the click-count check and stop three comments overstating
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 5s
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 2m44s
Build App (Preview) / build-linux (pull_request) Successful in 6m6s
Build App (Preview) / build-windows (pull_request) Successful in 5m0s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
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>
2026-09-18 20:07:59 -07:00
shadowdaoandClaude Opus 5 593b8168eb fix: a selection is not a request to leave the app
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 7s
Build App (Preview) / create-release (pull_request) Successful in 4s
Build App (Preview) / build-macos (pull_request) Successful in 2m42s
Build App (Preview) / build-linux (pull_request) Successful in 6m39s
Build App (Preview) / build-windows (pull_request) Successful in 5m0s
Build App (Preview) / prune-previews (pull_request) Successful in 2s
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>
2026-09-18 20:00:42 -07:00
jknapp d647b56b43 Do not read an unreachable Docker daemon as an absent container (#58)
Secret Scan / scan (push) Successful in 5s
Build App / compute-version (push) Successful in 17s
Build App / build-macos (push) Successful in 2m49s
Build App / build-windows (push) Successful in 5m3s
Build App / build-linux (push) Successful in 8m4s
Build App / create-tag (push) Successful in 4s
Build App / sync-to-github (push) Successful in 9s
Closes #56.

Reviewed twice; the second round's findings on the first fix are addressed in f662ed0 and a3840f7.
2026-09-19 02:59:20 +00:00
shadowdaoandClaude Opus 5 a3840f7263 fix: say which check failed, and stop claiming an order we do not use
Secret Scan / scan (push) Successful in 5s
Build App (Preview) / compute-version (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Secret Scan / scan (pull_request) Successful in 4s
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 4m58s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
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>
2026-09-18 19:53:52 -07:00
shadowdaoandClaude Opus 5 ac50c38891 fix: gate OSC 8 link activation instead of merely hinting at it
Secret Scan / scan (push) Successful in 4s
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 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m42s
Build App (Preview) / build-windows (pull_request) Successful in 5m3s
Build App (Preview) / build-linux (pull_request) Successful in 7m13s
Build App (Preview) / prune-previews (pull_request) Successful in 3s
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>
2026-09-18 19:45:11 -07:00
shadowdaoandClaude Opus 5 f662ed04ce fix: a reading nobody consults must not destroy the report
Secret Scan / scan (push) Successful in 11s
Build App (Preview) / compute-version (pull_request) Successful in 11s
Secret Scan / scan (pull_request) Successful in 8s
Build App (Preview) / create-release (pull_request) Successful in 8s
Build App (Preview) / build-macos (pull_request) Successful in 2m44s
Build App (Preview) / build-windows (pull_request) Successful in 5m35s
Build App (Preview) / build-linux (pull_request) Successful in 7m17s
Build App (Preview) / prune-previews (pull_request) Successful in 4s
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>
2026-09-18 19:38:31 -07:00
shadowdaoandClaude Opus 5 f311ca1990 feat: make links in Claude's output clickable
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 2m47s
Build App (Preview) / build-linux (pull_request) Successful in 8m8s
Build App (Preview) / build-windows (pull_request) Failing after 13m30s
Build App (Preview) / prune-previews (pull_request) Skipped
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>
2026-09-18 19:23:18 -07:00
shadowdaoandClaude Opus 5 84a5757c74 fix: do not read an unreachable Docker daemon as an absent container (#56)
Secret Scan / scan (push) Successful in 5s
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 4m59s
Build App (Preview) / build-windows (pull_request) Successful in 4m56s
Build App (Preview) / prune-previews (pull_request) Successful in 6s
`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>
2026-09-18 06:04:41 -07:00
3 changed files with 1789 additions and 70 deletions
+576 -49
View File
@@ -101,25 +101,56 @@ fn pick_recorded_lineage(
/// snapshot to fall back on. /// snapshot to fall back on.
const NOTHING_TO_PROBE: &str = "This project has no container or snapshot image yet, so there is nothing to compare against the base image."; const NOTHING_TO_PROBE: &str = "This project has no container or snapshot image yet, so there is nothing to compare against the base image.";
/// The project's container, as the daemon reported it.
///
/// `running` lives *inside* `Present` because it is only ever read about a
/// container that was found: `is_container_running` needs an id. Keeping the
/// two in one variant makes "running, but no container" unrepresentable rather
/// than merely unreached, which is what [`pick_probe_source`] relies on when it
/// hands a container id to the container probe arms.
#[derive(Debug, PartialEq, Eq)]
enum ContainerState {
/// The project genuinely has no container — an answer, not a failure to
/// look.
Absent,
Present {
id: String,
running: bool,
},
}
impl ContainerState {
fn id(&self) -> Option<&str> {
match self {
ContainerState::Absent => None,
ContainerState::Present { id, .. } => Some(id),
}
}
}
/// Where [`get_container_staleness`] reads the project's *current* filesystem /// Where [`get_container_staleness`] reads the project's *current* filesystem
/// from, in descending order of how current the answer is. /// from, in descending order of how current the answer is.
///
/// The container variants carry the id they will be probed with, so that
/// "there is a container to read" and "here is which one" cannot come apart
/// downstream.
#[derive(Debug, PartialEq, Eq)] #[derive(Debug, PartialEq, Eq)]
enum ProbeSource { enum ProbeSource<'a> {
/// `docker exec` into the live container. The only source that includes /// `docker exec` into the live container. The only source that includes
/// everything installed since the last commit *in this session*. /// everything installed since the last commit *in this session*.
RunningContainer, RunningContainer(&'a str),
/// Commit the stopped container's writable layer to a throwaway image and /// Commit the stopped container's writable layer to a throwaway image and
/// probe that. Exactly as current as the container, which is what makes it /// probe that. Exactly as current as the container, which is what makes it
/// preferable to the snapshot — see below. /// preferable to the snapshot — see below.
StoppedContainer, StoppedContainer(&'a str),
/// A throwaway container from `triple-c-snapshot-<id>:latest`. /// A throwaway container from `triple-c-snapshot-<id>:latest`.
Snapshot, Snapshot,
/// Nothing to read: no container, no snapshot. /// Nothing to read: no container, no snapshot.
Nothing, Nothing,
} }
/// Pick the probe source. `container_running` is `None` when the project has no /// Pick the probe source, or report the one reading this decision needed and
/// container at all, `Some(false)` when it has a stopped one. /// did not get.
/// ///
/// **A stopped container outranks the snapshot.** The snapshot image is not a /// **A stopped container outranks the snapshot.** The snapshot image is not a
/// checkpoint — `commit_container_snapshot` runs only before a removal (a /// checkpoint — `commit_container_snapshot` runs only before a removal (a
@@ -133,12 +164,30 @@ enum ProbeSource {
/// Getting this wrong is what made a stopped, never-recreated project report /// Getting this wrong is what made a stopped, never-recreated project report
/// "no container or snapshot image yet" — with its container sitting right /// "no container or snapshot image yet" — with its container sitting right
/// there — and left Update disabled on the projects that most needed it. /// there — and left Update disabled on the projects that most needed it.
fn pick_probe_source(container_running: Option<bool>, snapshot_exists: bool) -> ProbeSource { ///
match (container_running, snapshot_exists) { /// **`snapshot_exists` is consulted only where it decides something.** When a
(Some(true), _) => ProbeSource::RunningContainer, /// container answered, the snapshot is not part of this decision at all, so a
(Some(false), _) => ProbeSource::StoppedContainer, /// failed `image_exists` is passed over rather than surfaced: destroying a
(None, true) => ProbeSource::Snapshot, /// report the running container could have supplied in full would be the same
(None, false) => ProbeSource::Nothing, /// mistake, in the other direction, as reading an unreachable daemon as an
/// absent container. It is load-bearing only with no container at all, and
/// there its failure *is* the answer this function cannot give.
fn pick_probe_source<'a>(
container: &'a ContainerState,
snapshot_exists: &Result<bool, String>,
) -> Result<ProbeSource<'a>, String> {
match container {
ContainerState::Present { id, running: true } => Ok(ProbeSource::RunningContainer(id)),
// The stopped path may still want the snapshot, but only as a fallback
// it can do without — see `stopped_probe_policy` and the commit-failure
// arm in `get_container_staleness`, which each handle an unreadable
// snapshot themselves.
ContainerState::Present { id, running: false } => Ok(ProbeSource::StoppedContainer(id)),
ContainerState::Absent => match snapshot_exists {
Ok(true) => Ok(ProbeSource::Snapshot),
Ok(false) => Ok(ProbeSource::Nothing),
Err(e) => Err(probe_failed(e)),
},
} }
} }
@@ -159,8 +208,8 @@ enum StoppedProbe {
/// touches nothing, which is what makes it the right answer while another /// touches nothing, which is what makes it the right answer while another
/// operation owns the container. /// operation owns the container.
SnapshotInstead, SnapshotInstead,
/// Report rather than guess. /// Report rather than guess, with the message to report.
Defer, Defer(String),
} }
/// Pick what to do about a stopped container. /// Pick what to do about a stopped container.
@@ -173,14 +222,190 @@ enum StoppedProbe {
/// container with a hard `?`, so a non-404 from a remove that raced this commit /// container with a hard `?`, so a non-404 from a remove that raced this commit
/// fails the whole Start with an opaque "Failed to remove container". Reading /// fails the whole Start with an opaque "Failed to remove container". Reading
/// the claim costs nothing and takes that failure off the table. /// the claim costs nothing and takes that failure off the table.
fn stopped_probe_policy(project_is_busy: bool, snapshot_exists: bool) -> StoppedProbe { ///
/// `snapshot_exists` matters only once the project is busy, because that is the
/// only state in which the snapshot is the alternative to committing. An
/// unreadable snapshot there leaves nothing to fall back *to*, so its error is
/// what gets reported: "try again once it finishes" alone would be a claim that
/// waiting is all that stands in the way, which a failed `image_exists` has not
/// established.
fn stopped_probe_policy(
project_is_busy: bool,
snapshot_exists: &Result<bool, String>,
) -> StoppedProbe {
match (project_is_busy, snapshot_exists) { match (project_is_busy, snapshot_exists) {
(false, _) => StoppedProbe::Commit, (false, _) => StoppedProbe::Commit,
(true, true) => StoppedProbe::SnapshotInstead, (true, Ok(true)) => StoppedProbe::SnapshotInstead,
(true, false) => StoppedProbe::Defer, (true, Ok(false)) => StoppedProbe::Defer(PROJECT_BUSY.to_string()),
(true, Err(e)) => StoppedProbe::Defer(probe_failed(e)),
} }
} }
/// Reported as `probe_error` when a probe input could not be read at all.
///
/// Deliberately distinct from [`NOTHING_TO_PROBE`]: a failed reading is not
/// evidence that the project has no container, and saying "no container or
/// snapshot image yet" on a transient fault was confidently wrong about a
/// project that may well have both.
///
/// Deliberately *neutral about the cause*, too. Only one of the four readings
/// implies an unreachable daemon: `mig::image_id` maps a 404 to `Ok(None)` and
/// returns `Err` for any other status, and `find_existing_container` /
/// `image_exists` wrap every list failure the same way — all of which a daemon
/// that answered perfectly well can produce. The base image name comes from
/// user settings, so a malformed reference alone reaches here, and telling that
/// user to go fix a running daemon would be the same unestablished claim about
/// a cause that this whole probe path exists to stop making.
///
/// The underlying error is carried through verbatim, because "Docker is not
/// running" and "permission denied on /var/run/docker.sock" call for different
/// fixes from the user.
///
/// The sentence names *the check*, not the container, because only two of the
/// four readings are about the container at all — the other two are the base
/// image and the snapshot image. Saying "this project's container could not be
/// inspected" for a malformed base image name in settings would point the user
/// at the wrong object, which is the same mistake one size down.
fn probe_failed(e: &str) -> String {
format!("This project could not be checked against its base image: {}", e)
}
/// The four daemon readings [`get_container_staleness`] takes before it can
/// choose a probe source, each still carrying whether it is an answer.
///
/// **Absence and unreachability are different answers, and only one of them is
/// an answer.** All four callees already draw that line — `image_id` maps a 404
/// to `Ok(None)`, `find_existing_container` and `image_exists` return `Ok` with
/// an empty filtered list — so a call site that writes `.unwrap_or(None)` /
/// `.unwrap_or(false)` is not defaulting, it is *discarding a distinction the
/// callee went to the trouble of making*. That is what let an unreachable
/// daemon reach [`pick_probe_source`] as "no container, no snapshot" and report
/// [`NOTHING_TO_PROBE`] — a confident claim about a project nothing had
/// actually looked at.
///
/// The `Result` fields are the guard against that returning: the call site
/// hands over what the daemon said, unmodified, and an `.unwrap_or` there no
/// longer type-checks.
#[derive(Debug)]
struct ProbeReadings {
/// `docker::find_existing_container`.
container_id: Result<Option<String>, String>,
/// `docker::is_container_running`, and `None` when there was no container
/// to ask about — not a swallowed error.
container_running: Option<Result<bool, String>>,
/// `mig::image_id` for the configured base image.
base_image_id: Result<Option<String>, String>,
/// `docker::image_exists` for the project's snapshot image.
snapshot_exists: Result<bool, String>,
}
/// The readings [`get_container_staleness`] carries past the point where a
/// missing one would have stopped it.
#[derive(Debug)]
struct ProbeInputs {
/// The current base image's ID, or `None` when it is not pulled locally —
/// which [`mig::image_id`] reports as `Ok(None)`, not an error.
current_base_image_id: Option<String>,
container: ContainerState,
/// Still a `Result`, because whether it is load-bearing depends on the
/// container: see [`pick_probe_source`].
snapshot_exists: Result<bool, String>,
}
/// What [`get_container_staleness`] does next, once the readings are in.
#[derive(Debug)]
enum ProbeStart {
/// Go ahead, with these inputs.
Inputs(Box<ProbeInputs>),
/// Stop, and hand the user this report.
///
/// **Reported, not returned.** The hook's `catch` sets `staleness` to
/// `null`, and `ContainerMigrationBanner` renders nothing at all for a null
/// staleness — so an `Err` out of the command would make the banner vanish
/// at exactly the moment it has something to say. A `probe_error` on an
/// otherwise-default report keeps it on screen, reading "Container base
/// could not be checked". Carrying a `ContainerStaleness` rather than an
/// error string is what keeps that decision here, where it is tested,
/// instead of in the `?` someone adds at the call site later.
Report(Box<ContainerStaleness>),
}
/// Decide whether the collected readings are enough to probe with.
///
/// Only the readings this decision actually rests on can stop it:
///
/// * `container_id` selects the probe source outright, so a failure to read it
/// leaves nothing to choose between. Fatal.
/// * `base_image_id` is fatal too, and deliberately so: it is the right-hand
/// side of the staleness comparison, where `None` ("not pulled locally", an
/// answer) and `Err` ("could not ask") both otherwise collapse into
/// `stale: false`. Reporting a project as up to date because the base image
/// could not be read is exactly the #56 mistake, one field over.
/// * `container_running` is asked only about a container that was found, and
/// decides between two live probe sources. Fatal when present.
/// * `snapshot_exists` is *not* fatal here, because it is load-bearing in only
/// two of the downstream states — no container at all, and a stopped
/// container on a busy project. It travels as a `Result` so each of those can
/// surface it, and the states that never consult it are not punished for it.
///
/// The first error wins, because when the daemon is unreachable they fail
/// together and the user needs the reason once, not three times. The order is
/// `container_id`, then `base_image_id`, then `container_running` — chosen
/// priority, deliberately *not* the order the daemon was called in, so that the
/// reported error is most often the one that stopped the probe rather than
/// whichever reading happened to run first. (It is at most three, not four:
/// `container_running` is only attempted when `container_id` answered with a
/// container.)
///
/// **A caveat this cannot fix here.** `docker::is_container_running` swallows
/// `inspect_container` failures into `Ok(false)` itself and errors only when the
/// client cannot be built, so a daemon that dies between the list and the
/// inspect still reads as "stopped" rather than as an error. That is a fix
/// inside that function, not at this call site; threading its `Result` through
/// at least stops *this* layer from adding a second swallow on top.
fn start_probe(readings: ProbeReadings) -> ProbeStart {
match collect_probe_inputs(readings) {
Ok(inputs) => ProbeStart::Inputs(Box::new(inputs)),
Err(e) => ProbeStart::Report(Box::new(ContainerStaleness {
probe_error: Some(e),
..Default::default()
})),
}
}
fn collect_probe_inputs(readings: ProbeReadings) -> Result<ProbeInputs, String> {
let ProbeReadings {
container_id,
container_running,
base_image_id,
snapshot_exists,
} = readings;
let container_id = container_id.map_err(|e| probe_failed(&e))?;
let current_base_image_id = base_image_id.map_err(|e| probe_failed(&e))?;
let container_running = container_running
.transpose()
.map_err(|e| probe_failed(&e))?;
let container = match (container_id, container_running) {
(Some(id), Some(running)) => ContainerState::Present { id, running },
// No container: whatever `container_running` says is about nothing, and
// the caller only produces `None` here anyway.
(None, _) => ContainerState::Absent,
// A container was found but nobody asked whether it was running. The
// caller cannot produce this, and guessing "stopped" would cost a
// running project the only probe source that sees this session's
// installs — so say what happened instead.
(Some(_), None) => return Err(probe_failed("the container's state was not read")),
};
Ok(ProbeInputs {
current_base_image_id,
container,
snapshot_exists,
})
}
/// Runs two filesystem probes (~3 s each) and is therefore meant to be called /// Runs two filesystem probes (~3 s each) and is therefore meant to be called
/// on demand, not polled. /// on demand, not polled.
/// ///
@@ -214,7 +439,35 @@ pub async fn get_container_staleness(
let snapshot_image = docker::get_snapshot_image_name(&project); let snapshot_image = docker::get_snapshot_image_name(&project);
let mut out = ContainerStaleness::default(); let mut out = ContainerStaleness::default();
out.current_base_image_id = mig::image_id(&base_image).await.unwrap_or(None);
// Every reading the daemon owes us, taken up front and handed on exactly as
// it came back, so that "could not ask" stays distinguishable from "asked,
// and the answer is no". [`start_probe`] is where that distinction is acted
// on; nothing between here and there may collapse one into the other, and
// the `Result` fields of [`ProbeReadings`] are what stop it being possible.
let container_id_result = docker::find_existing_container(&project).await;
let container_running_result = match &container_id_result {
Ok(Some(id)) => Some(docker::is_container_running(id).await),
// No container, or no usable reading of one: nothing to inspect, and
// the container lookup's own error is what gets reported.
_ => None,
};
let readings = ProbeReadings {
container_id: container_id_result,
container_running: container_running_result,
base_image_id: mig::image_id(&base_image).await,
snapshot_exists: docker::image_exists(&snapshot_image).await,
};
let inputs = match start_probe(readings) {
ProbeStart::Inputs(inputs) => *inputs,
// Reported, not returned — see [`ProbeStart::Report`].
ProbeStart::Report(report) => return Ok(*report),
};
let container = inputs.container;
let container_id = container.id();
out.current_base_image_id = inputs.current_base_image_id;
out.snapshot_created_at = mig::image_created(&snapshot_image).await; out.snapshot_created_at = mig::image_created(&snapshot_image).await;
// Lineage, most authoritative source first: the live container's label, // Lineage, most authoritative source first: the live container's label,
@@ -228,8 +481,7 @@ pub async fn get_container_staleness(
// as an answer and skip the snapshot entirely, so a snapshot that *did* // as an answer and skip the snapshot entirely, so a snapshot that *did*
// record a lineage was never consulted and the project reported "unknown" // record a lineage was never consulted and the project reported "unknown"
// with the information sitting one lookup away. // with the information sitting one lookup away.
let container_id = docker::find_existing_container(&project).await.unwrap_or(None); let from_container = match container_id {
let from_container = match &container_id {
Some(id) => container_label(id, mig::LABEL_BASE_IMAGE_ID).await, Some(id) => container_label(id, mig::LABEL_BASE_IMAGE_ID).await,
None => None, None => None,
}; };
@@ -248,17 +500,19 @@ pub async fn get_container_staleness(
}; };
// ── Probes ─────────────────────────────────────────────────────────── // ── Probes ───────────────────────────────────────────────────────────
let container_running = match &container_id { let snapshot_exists = &inputs.snapshot_exists;
Some(id) => Some(docker::is_container_running(id).await.unwrap_or(false)), let source = match pick_probe_source(&container, snapshot_exists) {
None => None, Ok(source) => source,
// The only reading this decision needed and did not get — see
// [`ProbeStart::Report`] for why this is a report and not an `Err`.
Err(e) => {
out.probe_error = Some(e);
return Ok(out);
}
}; };
let snapshot_exists = docker::image_exists(&snapshot_image).await.unwrap_or(false); let from_manifest = match source {
let from_manifest = match ( ProbeSource::RunningContainer(id) => mig::manifest_from_container(id).await,
pick_probe_source(container_running, snapshot_exists), ProbeSource::StoppedContainer(id) => {
&container_id,
) {
(ProbeSource::RunningContainer, Some(id)) => mig::manifest_from_container(id).await,
(ProbeSource::StoppedContainer, Some(id)) => {
let busy = crate::project_lock::held(&project_id).is_some(); let busy = crate::project_lock::held(&project_id).is_some();
match stopped_probe_policy(busy, snapshot_exists) { match stopped_probe_policy(busy, snapshot_exists) {
StoppedProbe::Commit => { StoppedProbe::Commit => {
@@ -275,7 +529,12 @@ pub async fn get_container_staleness(
// layer; the snapshot probe allocates nothing) and a // layer; the snapshot probe allocates nothing) and a
// 409 from an operation that claimed the project after // 409 from an operation that claimed the project after
// the check above. // the check above.
Err(e) if snapshot_exists => { // `Ok(true)` specifically: an `image_exists` that
// failed has not established that there is anything to
// fall back to, and probing a snapshot that may not
// exist would replace the commit's real error with a
// confusing one.
Err(e) if matches!(snapshot_exists, Ok(true)) => {
log::warn!( log::warn!(
"Probing the stopped container for project {} failed ({}) — \ "Probing the stopped container for project {} failed ({}) — \
falling back to its snapshot image, which may lag it", falling back to its snapshot image, which may lag it",
@@ -295,15 +554,16 @@ pub async fn get_container_staleness(
); );
mig::manifest_from_image(&snapshot_image).await mig::manifest_from_image(&snapshot_image).await
} }
StoppedProbe::Defer => Err(PROJECT_BUSY.to_string()), StoppedProbe::Defer(message) => Err(message),
} }
} }
(ProbeSource::Snapshot, _) => mig::manifest_from_image(&snapshot_image).await, ProbeSource::Snapshot => mig::manifest_from_image(&snapshot_image).await,
// `container_running` is `Some` exactly when `container_id` is, so the // Reached only when there is genuinely neither a container nor a
// two arms above are the only ones those variants can reach. This arm // snapshot: `ProbeSource` carries the container id in its container
// is `ProbeSource::Nothing` — and now *only* that: it used to also // variants, so a container that exists can no longer fall through to
// swallow every stopped container, which is the bug. // here — which is the bug this arm used to hide, swallowing every
(_, _) => Err(NOTHING_TO_PROBE.to_string()), // stopped container.
ProbeSource::Nothing => Err(NOTHING_TO_PROBE.to_string()),
}; };
let (from_manifest, base_manifest) = match from_manifest { let (from_manifest, base_manifest) = match from_manifest {
@@ -2113,13 +2373,39 @@ mod tests {
assert_eq!(pick_recorded_lineage(some(""), None), None); assert_eq!(pick_recorded_lineage(some(""), None), None);
} }
/// The readings as the daemon answered them, all four healthy: no
/// container, nothing pulled, no snapshot. Tests override the one reading
/// they are about, which keeps it obvious which reading each case is
/// actually exercising.
fn readings() -> ProbeReadings {
ProbeReadings {
container_id: Ok(None),
container_running: None,
base_image_id: Ok(None),
snapshot_exists: Ok(false),
}
}
fn present(running: bool) -> ContainerState {
ContainerState::Present {
id: "c1".to_string(),
running,
}
}
/// What every one of the four readings looks like when the socket is gone:
/// generic over what it was going to return.
fn daemon<T>() -> Result<T, String> {
Err("Failed to list containers: connection refused".to_string())
}
#[test] #[test]
fn a_stopped_container_is_probed_rather_than_reported_missing() { fn a_stopped_container_is_probed_rather_than_reported_missing() {
// The regression: a container that exists but is stopped, with no // The regression: a container that exists but is stopped, with no
// snapshot ever taken, read as "nothing to compare against". // snapshot ever taken, read as "nothing to compare against".
assert_eq!( assert_eq!(
pick_probe_source(Some(false), false), pick_probe_source(&present(false), &Ok(false)),
ProbeSource::StoppedContainer Ok(ProbeSource::StoppedContainer("c1"))
); );
} }
@@ -2128,42 +2414,283 @@ mod tests {
// The snapshot lags the container by everything installed since the // The snapshot lags the container by everything installed since the
// last commit, in both states. // last commit, in both states.
assert_eq!( assert_eq!(
pick_probe_source(Some(true), true), pick_probe_source(&present(true), &Ok(true)),
ProbeSource::RunningContainer Ok(ProbeSource::RunningContainer("c1"))
); );
assert_eq!( assert_eq!(
pick_probe_source(Some(false), true), pick_probe_source(&present(false), &Ok(true)),
ProbeSource::StoppedContainer Ok(ProbeSource::StoppedContainer("c1"))
); );
} }
#[test] #[test]
fn the_snapshot_is_the_fallback_only_once_the_container_is_gone() { fn the_snapshot_is_the_fallback_only_once_the_container_is_gone() {
assert_eq!(pick_probe_source(None, true), ProbeSource::Snapshot); assert_eq!(
pick_probe_source(&ContainerState::Absent, &Ok(true)),
Ok(ProbeSource::Snapshot)
);
} }
#[test] #[test]
fn nothing_to_probe_is_reserved_for_no_container_and_no_snapshot() { fn nothing_to_probe_is_reserved_for_no_container_and_no_snapshot() {
// The one case the "no container or snapshot image yet" message may // The one case the "no container or snapshot image yet" message may
// still describe. // still describe.
assert_eq!(pick_probe_source(None, false), ProbeSource::Nothing); assert_eq!(
pick_probe_source(&ContainerState::Absent, &Ok(false)),
Ok(ProbeSource::Nothing)
);
}
#[test]
fn an_unreadable_snapshot_only_costs_the_report_where_the_snapshot_is_the_answer() {
// A container answered, so `image_exists` decides nothing: its failure
// must not cost a report the container can supply in full. Treating it
// as fatal turned "running container, one flaky `image_exists`" into a
// bare probe_error with Update disabled.
assert_eq!(
pick_probe_source(&present(true), &daemon()),
Ok(ProbeSource::RunningContainer("c1"))
);
assert_eq!(
pick_probe_source(&present(false), &daemon()),
Ok(ProbeSource::StoppedContainer("c1"))
);
// With no container, the snapshot is the whole decision, so its failure
// is reported — and never as "no container or snapshot image yet",
// which nothing has established.
let e = pick_probe_source(&ContainerState::Absent, &daemon()).unwrap_err();
assert!(e.contains("connection refused"), "{}", e);
assert_ne!(e, NOTHING_TO_PROBE);
} }
#[test] #[test]
fn a_stopped_container_is_committed_only_when_nothing_else_owns_the_project() { fn a_stopped_container_is_committed_only_when_nothing_else_owns_the_project() {
assert_eq!(stopped_probe_policy(false, false), StoppedProbe::Commit); assert_eq!(
assert_eq!(stopped_probe_policy(false, true), StoppedProbe::Commit); stopped_probe_policy(false, &Ok(false)),
StoppedProbe::Commit
);
assert_eq!(stopped_probe_policy(false, &Ok(true)), StoppedProbe::Commit);
// Not the snapshot's business either way when the project is free: an
// unreadable `image_exists` does not stop the commit that would not
// have consulted it.
assert_eq!(stopped_probe_policy(false, &daemon()), StoppedProbe::Commit);
} }
#[test] #[test]
fn a_busy_project_falls_back_rather_than_racing_a_recreate() { fn a_busy_project_falls_back_rather_than_racing_a_recreate() {
// The snapshot lags, but a stale answer beats failing someone's Start. // The snapshot lags, but a stale answer beats failing someone's Start.
assert_eq!( assert_eq!(
stopped_probe_policy(true, true), stopped_probe_policy(true, &Ok(true)),
StoppedProbe::SnapshotInstead StoppedProbe::SnapshotInstead
); );
// Nothing to fall back to: say so instead of committing anyway. // Nothing to fall back to: say so instead of committing anyway.
assert_eq!(stopped_probe_policy(true, false), StoppedProbe::Defer); assert_eq!(
stopped_probe_policy(true, &Ok(false)),
StoppedProbe::Defer(PROJECT_BUSY.to_string())
);
// Busy *and* the fallback could not be read: "try again once it
// finishes" would promise that waiting is all that stands in the way,
// which the failed reading has not established. Report what happened.
match stopped_probe_policy(true, &daemon()) {
StoppedProbe::Defer(message) => {
assert!(message.contains("connection refused"), "{}", message);
assert_ne!(message, PROJECT_BUSY);
}
other => panic!("expected Defer, got {:?}", other),
}
}
#[test]
fn an_unreachable_daemon_is_never_read_as_an_absent_container() {
// The bug: every one of these used to be flattened to "no" by an
// `unwrap_or`, which reached `pick_probe_source` as "no container, no
// snapshot" and reported "no container or snapshot image yet" about a
// project nobody had managed to look at.
let e = collect_probe_inputs(ProbeReadings {
container_id: daemon(),
..readings()
})
.unwrap_err();
assert!(e.contains("connection refused"), "{}", e);
assert_ne!(e, NOTHING_TO_PROBE);
let e = collect_probe_inputs(ProbeReadings {
base_image_id: daemon(),
..readings()
})
.unwrap_err();
assert!(e.contains("connection refused"), "{}", e);
let e = collect_probe_inputs(ProbeReadings {
container_id: Ok(Some("c1".into())),
container_running: Some(daemon()),
..readings()
})
.unwrap_err();
assert!(e.contains("connection refused"), "{}", e);
// The fourth reading is not fatal here — see
// `an_unreadable_snapshot_only_costs_the_report_where_the_snapshot_is_the_answer`
// — but it must still arrive as an error rather than as "no snapshot".
let inputs = collect_probe_inputs(ProbeReadings {
snapshot_exists: daemon(),
..readings()
})
.unwrap();
assert!(inputs.snapshot_exists.is_err());
let e = pick_probe_source(&inputs.container, &inputs.snapshot_exists).unwrap_err();
assert_ne!(e, NOTHING_TO_PROBE);
}
#[test]
fn a_base_image_that_could_not_be_read_is_never_reported_as_up_to_date() {
// `image_id` answers `Ok(None)` for "not pulled locally", which is a
// legitimate `stale: false`. An `Err` is not: it is the right-hand side
// of the comparison missing, and letting it through as `None` would
// report the project up to date on the strength of a reading nobody
// got. This is #56 one field over, so it is fatal on purpose.
let e = collect_probe_inputs(ProbeReadings {
base_image_id: Err("invalid reference format".into()),
container_id: Ok(Some("c1".into())),
container_running: Some(Ok(true)),
snapshot_exists: Ok(true),
})
.unwrap_err();
assert!(e.contains("invalid reference format"), "{}", e);
}
#[test]
fn a_failed_reading_is_not_blamed_on_a_daemon_that_answered() {
// Three of the four readings return `Err` from a daemon that replied
// perfectly well: `image_id` maps only a 404 to `Ok(None)`, and the two
// list-based readings wrap any failure. The base image name is
// user-supplied, so a typo in settings lands here — and used to be
// reported as "Docker could not be reached", sending the user to fix a
// daemon that was running.
// The payload is the shape bollard really produces for this case, and
// it contains the word "Docker" itself — so asserting the *message*
// lacks that word would pass here only because a synthetic payload was
// chosen. What must be true is that nothing *we* add claims the daemon
// was unreachable, or names the container when the reading was about
// the base image.
let raw = "Docker responded with status code 400: invalid reference format";
let e = collect_probe_inputs(ProbeReadings {
base_image_id: Err(raw.into()),
..readings()
})
.unwrap_err();
assert!(!e.contains("could not be reached"), "{}", e);
assert!(!e.contains("container"), "{}", e);
// The cause still comes through verbatim: "Docker isn't running" and
// "permission denied on the socket" need different fixes and must stay
// distinguishable.
assert!(e.contains(raw), "{}", e);
}
#[test]
fn the_first_daemon_error_is_the_one_reported() {
// When the daemon is down these fail together, and the user needs the
// reason once rather than three times. Call order wins, and
// `container_id` leads because it is what selects the probe source.
//
// At most three fail, not four: `container_running` is only attempted
// when `container_id` answered with a container, so the caller cannot
// produce an `Err` container id alongside a `Some(..)` running reading.
let e = collect_probe_inputs(ProbeReadings {
container_id: Err("first".into()),
container_running: None,
base_image_id: Err("second".into()),
snapshot_exists: Err("third".into()),
})
.unwrap_err();
assert!(e.ends_with("first"), "{}", e);
let e = collect_probe_inputs(ProbeReadings {
container_id: Ok(Some("c1".into())),
container_running: Some(Err("third".into())),
base_image_id: Err("second".into()),
snapshot_exists: Err("fourth".into()),
})
.unwrap_err();
assert!(e.ends_with("second"), "{}", e);
}
#[test]
fn a_container_id_cannot_arrive_without_a_reading_of_its_state() {
// `ContainerState` makes "running, but no container" unrepresentable;
// this is the other half — a container found, but never asked about.
// The caller cannot produce it, and guessing "stopped" would cost a
// running project the only probe source that sees this session's
// installs.
let e = collect_probe_inputs(ProbeReadings {
container_id: Ok(Some("c1".into())),
container_running: None,
..readings()
})
.unwrap_err();
assert!(e.contains("state was not read"), "{}", e);
}
#[test]
fn a_daemon_that_answers_no_is_an_answer_and_passes_through() {
// No container, no snapshot, base image not pulled: all the readings
// are `Ok`, and the "nothing to probe" path downstream is then
// genuinely earned.
let inputs = collect_probe_inputs(readings()).unwrap();
assert_eq!(inputs.current_base_image_id, None);
assert_eq!(inputs.container, ContainerState::Absent);
assert_eq!(inputs.snapshot_exists, Ok(false));
assert_eq!(
pick_probe_source(&inputs.container, &inputs.snapshot_exists),
Ok(ProbeSource::Nothing)
);
// And the fully populated reading survives intact.
let inputs = collect_probe_inputs(ProbeReadings {
container_id: Ok(Some("c1".into())),
container_running: Some(Ok(true)),
base_image_id: Ok(Some("sha256:base".into())),
snapshot_exists: Ok(true),
})
.unwrap();
assert_eq!(inputs.current_base_image_id.as_deref(), Some("sha256:base"));
assert_eq!(inputs.container, present(true));
assert_eq!(inputs.snapshot_exists, Ok(true));
}
#[test]
fn a_failed_reading_keeps_the_banner_on_screen_instead_of_erroring() {
// The load-bearing design decision of this path: a failed reading is a
// report with `probe_error` set, never an `Err` out of the command. An
// `Err` reaches the hook's `catch`, which nulls `staleness`, and
// `ContainerMigrationBanner` renders nothing at all for a null one — so
// the banner would vanish at exactly the moment it has something to say.
match start_probe(ProbeReadings {
container_id: daemon(),
..readings()
}) {
ProbeStart::Report(report) => {
let message = report.probe_error.clone().expect("probe_error");
assert!(message.contains("connection refused"), "{}", message);
// Everything else at its default: a field being empty means
// "nothing found", and nothing was found because nothing was
// read. `stale: false` here is the absence of a claim, which is
// only honest because `probe_error` is carrying the reason.
assert_eq!(
*report,
ContainerStaleness {
probe_error: Some(message),
..Default::default()
}
);
}
ProbeStart::Inputs(_) => panic!("a failed reading must not be probed on"),
}
// And a healthy set of readings still goes on to probe.
assert!(matches!(start_probe(readings()), ProbeStart::Inputs(_)));
} }
#[test] #[test]
@@ -1,6 +1,10 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { render, fireEvent, cleanup, act } from "@testing-library/react"; import { render, fireEvent, cleanup, act } from "@testing-library/react";
import TerminalView, { supersedes } from "./TerminalView"; import TerminalView, {
OSC8_HOVER_CLASS,
createOsc8LinkHandler,
supersedes,
} from "./TerminalView";
import { useAppState } from "../../store/appState"; import { useAppState } from "../../store/appState";
import { import {
uploadHostFileToTerminal, uploadHostFileToTerminal,
@@ -41,6 +45,57 @@ const ptyOutput = vi.hoisted(() => ({
listeners: new Map<string, (e: { payload: number[] }) => void>(), listeners: new Map<string, (e: { payload: number[] }) => void>(),
})); }));
/**
* What `TerminalView` actually handed the `Terminal` constructor, and the
* instances it built.
*
* The real xterm is kept — these tests depend on its parser, its modes and its
* DOM — and only the constructor is wrapped, because the wiring of
* `linkHandler` is otherwise unobservable from outside: xterm decides when to
* call it from cell geometry that jsdom has no layout for, so deleting the
* `linkHandler:` line changed nothing any assertion could see.
*/
const xterm = vi.hoisted(() => ({
options: null as Record<string, unknown> | null,
instances: [] as unknown[],
}));
/**
* The click handler `TerminalView` hands `WebLinksAddon`.
*
* Captured for the same reason the `Terminal` constructor is: xterm decides
* when to call it from cell geometry jsdom has no layout for, so the only way
* to ask "does the plain-text-URL path apply the same gate as the OSC 8 one?"
* is to hold the function and call it.
*/
const webLinks = vi.hoisted(() => ({
handler: null as null | ((event: MouseEvent, uri: string) => void),
}));
vi.mock("@xterm/xterm", async (importOriginal) => {
const actual = await importOriginal<typeof import("@xterm/xterm")>();
class SpyTerminal extends actual.Terminal {
constructor(options?: ConstructorParameters<typeof actual.Terminal>[0]) {
super(options);
xterm.options = (options ?? null) as Record<string, unknown> | null;
xterm.instances.push(this);
}
}
return { ...actual, Terminal: SpyTerminal };
});
vi.mock("@xterm/addon-web-links", async (importOriginal) => {
const actual = await importOriginal<typeof import("@xterm/addon-web-links")>();
type Args = ConstructorParameters<typeof actual.WebLinksAddon>;
class SpyWebLinksAddon extends actual.WebLinksAddon {
constructor(...args: Args) {
super(...args);
webLinks.handler = (args[0] ?? null) as typeof webLinks.handler;
}
}
return { ...actual, WebLinksAddon: SpyWebLinksAddon };
});
/** /**
* Shift+Enter has to reach the container as ESC+CR. * Shift+Enter has to reach the container as ESC+CR.
* *
@@ -157,6 +212,9 @@ beforeEach(() => {
useAppState.setState({ toasts: [] }); useAppState.setState({ toasts: [] });
document.body.innerHTML = ""; document.body.innerHTML = "";
useAppState.setState({ sessions: [] }); useAppState.setState({ sessions: [] });
xterm.options = null;
xterm.instances.length = 0;
webLinks.handler = null;
}); });
afterEach(() => { afterEach(() => {
@@ -1074,3 +1132,653 @@ describe("TerminalView — releasing a captured mouse", () => {
expect(terminalInput).not.toHaveBeenCalled(); expect(terminalInput).not.toHaveBeenCalled();
}); });
}); });
describe("the hover hint names the key that actually works", () => {
const platform = (value: string) =>
Object.defineProperty(navigator, "platform", { value, configurable: true });
const original = navigator.platform;
afterEach(() => platform(original));
const hoverHint = (
tracking: boolean,
macOptionClickForcesSelection = true,
): string => {
const host = document.createElement("div");
createOsc8LinkHandler(() => host, () => ({
mouseTracking: tracking,
hasSelection: false,
macOptionClickForcesSelection,
})).hover?.(new MouseEvent("mousemove"), "https://example.com/x", {
start: { x: 1, y: 1 },
end: { x: 1, y: 1 },
});
return host.textContent ?? "";
};
// The hint and the gate read one predicate; these pin that they cannot
// drift, because a hint naming a key the gate does not accept is the bug
// that was already fixed once on this branch.
it("says Option on a Mac, because that is xterm's force-selection modifier there", () => {
platform("MacIntel");
expect(hoverHint(true)).toContain("Option+click");
expect(hoverHint(true)).not.toContain("Shift+click");
});
it("says Shift everywhere else", () => {
platform("Linux x86_64");
expect(hoverHint(true)).toContain("Shift+click");
});
// No program holds the mouse, so no modifier is needed — and naming one
// would tell the user to press a key the gate ignores.
it("names no modifier at all while nothing is tracking the mouse", () => {
platform("Linux x86_64");
const hint = hoverHint(false);
expect(hint).toContain("Click to open");
expect(hint).not.toContain("Shift+click");
platform("MacIntel");
expect(hoverHint(false)).not.toContain("Option+click");
});
// `macOptionClickForcesSelection` defaults to false in xterm and this view
// sets it true, so the Mac branch is only live because of that line. If it
// ever goes, Option stops being the force-selection modifier and the gate
// can never pass while a program holds the mouse — so the card must not go
// on naming a key that does nothing.
it("does not promise Option+click when the option behind it is off", () => {
platform("MacIntel");
const hint = hoverHint(true, false);
expect(hint).not.toContain("Option+click");
expect(hint).not.toContain("Shift+click");
});
});
describe("createOsc8LinkHandler — clicking a link Claude Code printed", () => {
/**
* The handler is exercised directly rather than through a rendered terminal.
*
* xterm decides *when* to call it from cell geometry, and jsdom gives every
* element a zero-sized box — so a test driving the mouse over the pane would
* be asserting that jsdom's layout engine exists, not that this app validates
* what it opens. What xterm hands over is the OSC 8 parameter verbatim, which
* is exactly what these arguments are.
*/
const range = {
start: { x: 1, y: 1 },
end: { x: 80, y: 1 },
} as unknown as Parameters<
NonNullable<ReturnType<typeof createOsc8LinkHandler>["hover"]>
>[2];
let host: HTMLDivElement;
let handler: ReturnType<typeof createOsc8LinkHandler>;
/**
* What the terminal answers about itself when the gate asks, per test.
*
* Mutable rather than fixed at construction because both of the first two
* change *under* the handler: the container sets the mouse mode with a
* DECSET, and the selection is whatever the gesture that ended in this
* mouseup left behind.
*/
let state: {
mouseTracking: boolean;
hasSelection: boolean;
macOptionClickForcesSelection: boolean;
};
beforeEach(() => {
host = document.createElement("div");
document.body.appendChild(host);
state = {
mouseTracking: false,
hasSelection: false,
// What `TerminalView` sets on the real terminal.
macOptionClickForcesSelection: true,
};
handler = createOsc8LinkHandler(() => host, () => state);
});
afterEach(() => host.remove());
function hoverCard(): HTMLElement | null {
return host.querySelector<HTMLElement>(`.${OSC8_HOVER_CLASS}`);
}
/** A real single click: one press, one release, `detail` 1. */
const click = (init: MouseEventInit = {}) =>
new MouseEvent("click", { button: 0, detail: 1, ...init });
it("refuses a target that fails validation, without reaching the opener", () => {
// The visible text can be anything; the parameter is what gets opened, and
// a container is free to put a scheme in it that the host must never hand
// to an OS-level opener.
handler.activate(click(), "javascript:alert(1)", range);
handler.activate(click(), "file:///etc/passwd", range);
handler.activate(click(), "https://claude.ai@evil.tld/authorize", range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("opens a valid target through the one sink", async () => {
const url =
"https://claude.ai/oauth/authorize?code=true&client_id=abc123&scope=user%3Ainference";
await act(async () => {
handler.activate(click(), url, range);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(url);
});
describe("the gate on activation", () => {
const URL = "https://example.com/x";
/**
* The attack this gate exists for.
*
* xterm's mouse-reporting mousedown does *not* cancel anything —
* `cancelEvents` defaults to false — and the Linkifier is a descendant of
* the element those listeners are bound to, so the link layer sees every
* click first and `_handleMouseUp` activates with no modifier, button or
* mode check of its own. A TUI widget the user is meant to click can
* therefore be wrapped in an OSC 8 pointing anywhere, and a plain click
* opens the host browser on it while the mouse report still reaches the
* program, so nothing looks wrong. The modifier is the only thing that
* separates "I clicked the menu item" from "I asked to leave the app".
*/
it("refuses a plain click while a program is tracking the mouse", () => {
state.mouseTracking = true;
handler.activate(click(), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("opens on the force-selection modifier while tracking", async () => {
state.mouseTracking = true;
await act(async () => {
handler.activate(click({ shiftKey: true }), URL, range);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
it("opens on a plain click when nothing holds the mouse", async () => {
// A normal shell. This is what `WebLinksAddon` does for the plain-text
// URLs in the same buffer, and asking for a modifier here would read as
// a broken link.
state.mouseTracking = false;
await act(async () => {
handler.activate(click(), URL, range);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
it("ignores every button but the primary one", () => {
// Right-click is the context menu this pane already binds; middle-click
// is paste. Neither is a request to leave the app.
state.mouseTracking = false;
handler.activate(click({ button: 2 }), URL, range);
handler.activate(click({ button: 1 }), URL, range);
handler.activate(click({ button: 2, shiftKey: true }), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
/**
* Selecting text is not asking to leave the app.
*
* `Linkifier._handleMouseUp` has no `detail` check, no drag threshold and
* no timestamp — it activates whenever the mouseup lands on the same link
* the mousedown did. `SelectionService` is bound on the *document* and the
* Linkifier on `screenElement`, so the selection gesture and the link
* activation both run, the link layer first. Every gesture below is one a
* user makes to *copy* a string, and none of them may open a browser.
*/
describe("a selection gesture is not a click", () => {
it("refuses a double-click, which selects the word under it", () => {
// xterm selects the word on the *mousedown* of the second click, so
// by this mouseup the selection is already there.
state.hasSelection = true;
handler.activate(click({ detail: 2 }), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("refuses a triple-click, which selects the whole row", () => {
state.hasSelection = true;
handler.activate(click({ detail: 3 }), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
// The one the click count cannot see: a drag is a single press and a
// single release, so `detail` is 1 throughout. Only the selection it
// left behind distinguishes it from a click.
it("refuses a drag that selected characters, at click count 1", () => {
state.hasSelection = true;
handler.activate(click(), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
/**
* The worst version, and the reason the modifier alone is not a gate.
*
* While a program holds the mouse, Shift/Option+drag is the *only* way
* to select text at all — so "the deliberate request to leave the app"
* and "I am copying this line" are byte-identical gestures. A container
* that wraps each of its output rows in an OSC 8 turns every legitimate
* copy into a browser open.
*/
it("refuses a force-selection drag while a program holds the mouse", () => {
state.mouseTracking = true;
state.hasSelection = true;
handler.activate(click({ shiftKey: true }), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
// Belt to the selection check's braces: independent of whether xterm
// managed to select anything (a double-click on trailing whitespace
// selects nothing), a second click is not a first one.
it("refuses a repeat click even when nothing ended up selected", () => {
state.hasSelection = false;
handler.activate(click({ detail: 2 }), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
});
/**
* The card and the gate must not disagree about what the user has to do.
*
* The hint is computed once, when the pointer arrives; the mode it was
* computed from is the container's to change, and `?1002l` takes effect
* synchronously with the write. So a card reading "Shift+click to open"
* can be on screen while the live mode says a bare click is enough —
* which is also the shape of the flicker attack in FINDING 2. The gate
* therefore honours the *stricter* of what was promised and what is true
* now: a modifier the card asked for is still required when the click
* lands.
*/
describe("what the card promised still binds when the click lands", () => {
it("keeps demanding the modifier after the container drops tracking", () => {
state.mouseTracking = true;
handler.hover?.(new MouseEvent("mousemove"), URL, range);
expect(host.textContent).toContain("+click to open");
// `?1002l`, mid-hover.
state.mouseTracking = false;
handler.activate(click(), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("still opens on the modifier the card named", async () => {
state.mouseTracking = true;
handler.hover?.(new MouseEvent("mousemove"), URL, range);
state.mouseTracking = false;
await act(async () => {
handler.activate(click({ shiftKey: true }), URL, range);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
it("does not hold a stale demand against the next link", async () => {
state.mouseTracking = true;
handler.hover?.(new MouseEvent("mousemove"), URL, range);
handler.leave?.(new MouseEvent("mouseout"), URL, range);
// A plain shell now, and a fresh card that says so.
state.mouseTracking = false;
handler.hover?.(new MouseEvent("mousemove"), URL, range);
expect(host.textContent).toContain("Click to open");
await act(async () => {
handler.activate(click(), URL, range);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
});
/**
* FINDING 6: the modifier is xterm's, including the option it hangs on.
*
* xterm's rule is `isMac ? altKey && macOptionClickForcesSelection :
* shiftKey`. Hardcoding `altKey` agrees with the app only for as long as
* the app keeps setting that option, and nothing tells you when it stops.
*/
describe("the Mac modifier follows the terminal's own option", () => {
const platform = (value: string) =>
Object.defineProperty(navigator, "platform", {
value,
configurable: true,
});
const original = navigator.platform;
afterEach(() => platform(original));
it("opens on Option+click while the option is on", async () => {
platform("MacIntel");
state.mouseTracking = true;
state.macOptionClickForcesSelection = true;
await act(async () => {
handler.activate(click({ altKey: true }), URL, range);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
it("refuses Option+click when the terminal does not treat it as force-select", () => {
platform("MacIntel");
state.mouseTracking = true;
state.macOptionClickForcesSelection = false;
handler.activate(click({ altKey: true }), URL, range);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("ignores the option off a Mac, where Shift is the modifier", async () => {
platform("Linux x86_64");
state.mouseTracking = true;
state.macOptionClickForcesSelection = false;
await act(async () => {
handler.activate(click({ shiftKey: true }), URL, range);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
});
});
it("shows the real origin on hover, not the text on screen", () => {
// The point of the affordance. OSC 8 decouples label from target: the row
// can read `https://claude.ai` while the parameter points anywhere.
handler.hover?.(
new MouseEvent("mousemove"),
"https://evil.example.com/claude.ai/oauth/authorize?code=true",
range,
);
const card = hoverCard();
expect(card).not.toBeNull();
const origin = card!.querySelector('[data-testid="osc8-hover-origin"]');
expect(origin?.textContent).toBe("https://evil.example.com");
expect(card!.textContent).not.toContain("https://claude.ai");
handler.leave?.(new MouseEvent("mouseout"), "https://evil.example.com/", range);
expect(hoverCard()).toBeNull();
});
it("keeps a very long origin whole, and gives way in the remainder instead", () => {
// The attacker picks the origin's length. `https://claude.ai.<300 a's>
// .evil.tld/` parses, passes every `sanitizeRelayUrl` rule, and under a
// non-shrinking flex item runs off the right edge of the pane — which
// hides the registrable domain just as effectively as an ellipsis would.
const origin = `https://claude.ai.${"a".repeat(300)}.${"b".repeat(200)}.evil.tld`;
handler.hover?.(new MouseEvent("mousemove"), `${origin}/oauth?code=1`, range);
const originEl = hoverCard()!.querySelector<HTMLElement>(
'[data-testid="osc8-hover-origin"]',
)!;
// Whole origin or nothing: every character is in the DOM...
expect(originEl.textContent).toBe(origin);
// ...and it is allowed to wrap rather than be clipped or pushed off-pane.
expect(originEl.style.flexShrink).not.toBe("0");
expect(originEl.style.whiteSpace).not.toBe("nowrap");
expect(originEl.style.overflowWrap).toBe("anywhere");
// The truncatable half is the remainder, and only the remainder.
const restEl = hoverCard()!.querySelector<HTMLElement>(
'[data-testid="osc8-hover-rest"]',
)!;
expect(restEl.style.textOverflow).toBe("ellipsis");
expect(restEl.style.whiteSpace).toBe("nowrap");
});
it("cannot take the pointer away from the link that summoned it", () => {
// The card is appended to `Terminal.element`, a *sibling* of the
// `screenElement` the Linkifier listens on, so `xterm-hover` buys nothing
// here: a card under the pointer means `mouseleave` on screenElement, the
// card is torn down, and the mouseup that would activate the link lands on
// the card instead of the terminal.
handler.hover?.(new MouseEvent("mousemove"), "https://example.com/x", range);
expect(hoverCard()!.style.pointerEvents).toBe("none");
});
it("says so on hover when the target would be refused", () => {
handler.hover?.(new MouseEvent("mousemove"), "javascript:alert(1)", range);
const card = hoverCard();
expect(card).not.toBeNull();
expect(card!.querySelector('[data-testid="osc8-hover-origin"]')).toBeNull();
// Never echo the rejected target: it is untrusted text on its way to a DOM
// node, and the only thing worth saying is that clicking does nothing.
expect(card!.textContent).not.toContain("javascript:");
});
it("does not call a refused web address something other than a web address", () => {
// `https://claude.ai@evil.tld/` is a perfectly good URL; it is refused
// because the userinfo makes the visible host a lie. Telling the user it
// "is not a web address" is false, and a false explanation teaches them to
// distrust the card.
handler.hover?.(
new MouseEvent("mousemove"),
"https://claude.ai@evil.tld/authorize",
range,
);
expect(hoverCard()!.textContent).not.toContain("not a web address");
});
it("drops a stale card when the pane is no longer on screen", () => {
// `leave` only ever arrives from the Linkifier's `_clearCurrentLink`, and
// switching tabs from the keyboard moves no pointer: without this, the
// card is still sitting there when the user comes back.
handler.hover?.(new MouseEvent("mousemove"), "https://example.com/x", range);
expect(hoverCard()).not.toBeNull();
handler.dismiss();
expect(hoverCard()).toBeNull();
});
it("pushes the shared toast when the host opener fails", async () => {
vi.mocked(openUrlExternal).mockRejectedValueOnce(new Error("no opener"));
await act(async () => {
handler.activate(click(), "https://example.com/x", range);
await Promise.resolve();
});
const toasts = useAppState.getState().toasts;
expect(toasts).toHaveLength(1);
expect(toasts[0].kind).toBe("error");
expect(toasts[0].detail).toContain("no opener");
// Same card as every other dead-opener report in this view.
expect(toasts[0].dedupeKey).toBe("host-open-failed");
});
});
describe("the link handler is wired into the terminal, and reads its live mode", () => {
const range = {
start: { x: 1, y: 1 },
end: { x: 80, y: 1 },
} as unknown as Parameters<
NonNullable<ReturnType<typeof createOsc8LinkHandler>["hover"]>
>[2];
/** What the mounted view passed as `linkHandler`. */
function wiredHandler() {
const handler = xterm.options?.linkHandler as
| ReturnType<typeof createOsc8LinkHandler>
| undefined;
if (!handler) throw new Error("no linkHandler was passed to Terminal");
return handler;
}
/** Feed the terminal a DECSET the way the container would. */
async function write(data: string) {
const term = xterm.instances.at(-1) as { write(d: string, cb: () => void): void };
await act(
() => new Promise<void>((resolve) => term.write(data, resolve)),
);
}
it("passes one at all — without it OSC 8 links are inert", () => {
mountSession("claude");
const handler = wiredHandler();
expect(typeof handler.activate).toBe("function");
expect(typeof handler.hover).toBe("function");
});
// The gate has to ask the terminal, not a boolean captured at construction:
// the mode changes whenever the container prints a DECSET, which is several
// times a second in Claude Code.
it("refuses a plain click once the container turns mouse tracking on", async () => {
mountSession("claude");
await write("\x1b[?1002h");
wiredHandler().activate(
new MouseEvent("click", { button: 0, detail: 1 }),
"https://example.com/x",
range,
);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("opens again once the container gives the mouse back", async () => {
mountSession("claude");
await write("\x1b[?1002h");
await write("\x1b[?1002l");
await act(async () => {
wiredHandler().activate(
new MouseEvent("click", { button: 0, detail: 1 }),
"https://example.com/x",
range,
);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith("https://example.com/x");
});
});
/**
* FINDING 4: the sibling path opens the same browser.
*
* `WebLinksAddon` matches rendered text and activates through the same
* `Linkifier._handleMouseUp`, with the same absence of any check. It also
* picks up links the OSC 8 handler never sees: `OscLinkProvider` drops a
* non-http(s) hyperlink target before `linkHandler` is reached, which leaves
* the addon free to match the *label* — so an OSC 8 with a `javascript:`
* target and an `https://evil.tld/x` label arrives here and nowhere else.
* Both routes end at `openUrlExternal`, so both ask the same question first.
*/
describe("the plain-text URL path is gated the same way", () => {
function webLinksHandler() {
if (!webLinks.handler) throw new Error("no handler was passed to WebLinksAddon");
return webLinks.handler;
}
function term() {
return xterm.instances.at(-1) as unknown as {
write(d: string, cb: () => void): void;
select(column: number, row: number, length: number): void;
};
}
async function write(data: string) {
await act(() => new Promise<void>((resolve) => term().write(data, resolve)));
}
const click = (init: MouseEventInit = {}) =>
new MouseEvent("click", { button: 0, detail: 1, ...init });
const URL = "https://example.com/x";
it("opens on a plain click in an ordinary shell", async () => {
mountSession("bash");
await act(async () => {
webLinksHandler()(click(), URL);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
it("refuses a plain click while a program holds the mouse", async () => {
mountSession("claude");
await write("\x1b[?1002h");
webLinksHandler()(click(), URL);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("opens on the force-selection modifier while tracking", async () => {
mountSession("claude");
await write("\x1b[?1002h");
await act(async () => {
webLinksHandler()(click({ shiftKey: true }), URL);
await Promise.resolve();
});
expect(openUrlExternal).toHaveBeenCalledWith(URL);
});
it("refuses the mouseup that ended a selection", async () => {
mountSession("bash");
await write("https://example.com/x");
await act(async () => {
term().select(0, 0, 5);
});
webLinksHandler()(click(), URL);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("refuses a repeat click", async () => {
mountSession("bash");
webLinksHandler()(click({ detail: 2 }), URL);
expect(openUrlExternal).not.toHaveBeenCalled();
});
it("still refuses a target that fails validation", async () => {
mountSession("bash");
webLinksHandler()(click(), "https://claude.ai@evil.tld/authorize");
expect(openUrlExternal).not.toHaveBeenCalled();
});
});
+504 -20
View File
@@ -1,5 +1,5 @@
import { useCallback, useEffect, useRef, useState } from "react"; import { useCallback, useEffect, useRef, useState } from "react";
import { Terminal } from "@xterm/xterm"; import { Terminal, type ILinkHandler } from "@xterm/xterm";
import { FitAddon } from "@xterm/addon-fit"; import { FitAddon } from "@xterm/addon-fit";
import { WebglAddon } from "@xterm/addon-webgl"; import { WebglAddon } from "@xterm/addon-webgl";
import { WebLinksAddon } from "@xterm/addon-web-links"; import { WebLinksAddon } from "@xterm/addon-web-links";
@@ -21,6 +21,7 @@ import {
extendsUrl, extendsUrl,
parseUrlRelayOsc, parseUrlRelayOsc,
sanitizeRelayUrl, sanitizeRelayUrl,
urlOrigin,
} from "../../lib/urlRelay"; } from "../../lib/urlRelay";
import { classifyDrop, DROP_BLOCKED_TOAST } from "../../lib/dropTarget"; import { classifyDrop, DROP_BLOCKED_TOAST } from "../../lib/dropTarget";
import { useSignInOpenTarget } from "../../hooks/useSignInOpenTarget"; import { useSignInOpenTarget } from "../../hooks/useSignInOpenTarget";
@@ -106,12 +107,460 @@ export function supersedes(
return extendsUrl(next.url, current.url); return extendsUrl(next.url, current.url);
} }
/**
* Marks the hover card, for xterm's stylesheet and for the tests.
*
* It does *not* make xterm route pointer events around the card. xterm only
* consults this class inside `Linkifier._handleMouseMove`, which is registered
* on `screenElement`; the card is appended to `Terminal.element`, a *sibling*
* of that node, so the check never sees it. What keeps the card out of the way
* is `pointerEvents: "none"` on the card itself — see `hover` below for what
* goes wrong without it.
*/
export const OSC8_HOVER_CLASS = "xterm-hover";
/**
* Report a failed handoff to the host's browser.
*
* One sink, one card. See the long note on `handleOpenUrl` for what this catch
* does *not* catch on Linux; a click that appears to do nothing is the
* complaint either way, so every route that opens a URL says the same thing in
* the same place.
*/
function reportOpenFailure(e: unknown) {
useAppState.getState().pushToast({
kind: "error",
message: "Could not open that link in your browser",
detail: String(e),
// A dead opener fails for every link in the buffer. One card.
dedupeKey: "host-open-failed",
});
}
/**
* `ILinkHandler`, plus the one thing xterm never asks for.
*
* `leave` is only ever reached through `Linkifier._clearCurrentLink`, i.e. a
* pointer that moved. Switching tabs from the keyboard moves no pointer and
* the Linkifier's dispose path does not clear either, so the card outlives the
* pane and is still there when the user comes back. {@link dismiss} is how the
* view says "this pane is gone" without pretending to be a mouse event.
*/
export type Osc8LinkHandler = ILinkHandler & { dismiss(): void };
/**
* Is a program holding the mouse?
*
* One expression, two readers that must never disagree: the status-bar badge
* (`syncMouseCapture`) and the gate on opening a link ({@link opensOnClick}).
* A gate that thought tracking was off while the badge said it was on would be
* the whole security hole back again.
*/
function terminalTracksMouse(term: Terminal): boolean {
return term.modes.mouseTrackingMode !== "none";
}
/**
* Everything the gate asks the terminal, sampled at the moment of the click.
*
* A struct rather than three getters because the three are read together and
* must describe one instant: `hasSelection` is only meaningful against the
* `mouseTracking` that decided which gestures could have produced it.
*/
export interface ClickContext {
/** {@link terminalTracksMouse} — the container's to change, at any time. */
mouseTracking: boolean;
/** Does the terminal hold a selection *right now*? See {@link opensOnClick}. */
hasSelection: boolean;
/** xterm's `macOptionClickForcesSelection`, read rather than assumed. */
macOptionClickForcesSelection: boolean;
}
function readClickContext(term: Terminal): ClickContext {
return {
mouseTracking: terminalTracksMouse(term),
hasSelection: term.hasSelection(),
macOptionClickForcesSelection:
term.options.macOptionClickForcesSelection ?? false,
};
}
/** xterm's `isMac` verbatim (`common/Platform.ts`), so we split where it does. */
function isMacPlatform(): boolean {
const platform = typeof navigator === "undefined" ? "" : navigator.platform;
return ["Macintosh", "MacIntel", "MacPPC", "Mac68K"].includes(platform);
}
/**
* xterm's `SelectionService.shouldForceSelection`, mirrored.
*
* The modifier is not our choice and it must not drift: while a program holds
* the mouse, this is the one gesture the user already has for "this click is
* for the terminal, not for the program", so it is the gesture that may open a
* link. xterm's rule is
* `isMac ? e.altKey && rawOptions.macOptionClickForcesSelection : e.shiftKey`,
* and the option is read from the terminal rather than assumed: this view sets
* it true today, so the two agreed, but xterm's default is false and nothing
* would have reported the day that line went. A hardcoded `altKey` would then
* accept a modifier xterm no longer treats as force-select.
*
* The gate and the hint below both call this. A hint that names a key the gate
* does not accept is worse than no hint — the user concludes the link is
* broken — and that is a bug this branch has already shipped once, so the two
* are not allowed separate answers.
*/
function forcesSelection(
event: { altKey: boolean; shiftKey: boolean },
macOptionClickForcesSelection: boolean,
): boolean {
return isMacPlatform()
? event.altKey && macOptionClickForcesSelection
: event.shiftKey;
}
/**
* What the card tells the user to do, for the state the terminal is in *now*.
*
* Conditional because the gesture is: with no program tracking the mouse a
* plain click opens the link, and naming a modifier then would send the user
* hunting for a key that changes nothing. The last branch is the same rule
* once more: on a Mac with `macOptionClickForcesSelection` off there *is* no
* force-selection modifier, so {@link opensOnClick} can never pass while a
* program holds the mouse, and naming Option would be naming a dead key.
*/
function openHintLabel(ctx: ClickContext): string {
if (!ctx.mouseTracking) return "Click to open";
if (!isMacPlatform()) return "Shift+click to open";
if (!ctx.macOptionClickForcesSelection) {
return "Not clickable while a program holds the mouse";
}
return "Option+click to open";
}
/**
* Whether this mouseup is a request to leave the app for the host browser.
*
* xterm asks none of this. `Linkifier._handleMouseUp` activates whenever the
* mouseup lands on the same link the mousedown did — no button check, no mode
* check, no `detail`, no drag threshold, no timestamp (`SelectionService` has
* a `_mouseDownTimeStamp`; the Linkifier has nothing). Four refusals, for four
* different mistakes:
*
* - **Anything but the primary button.** Without this a *right*-click
* activates the link as well as opening this pane's context menu, and a
* middle-click paste opens it too.
* - **A mouseup that ended a selection.** This is the load-bearing one, and
* the reason is that a drag is a single press and a single release, so its
* click count is 1 and nothing else distinguishes it from a click. Both
* gestures a user makes to *copy* a string end here: drag across a few
* characters, or double-click a word (xterm selects it on the second
* mousedown, so the selection is already in the model by the time this
* runs). Worse, while a program holds the mouse Shift/Option+drag is the
* *only* way to select at all — byte-identical to the modifier below — so
* without this check a container that wraps each output row in an OSC 8
* turns every legitimate copy into a browser open. The selection check is
* also cheap to be wrong about in the safe direction: xterm's
* `_handleSingleClick` clears the model on the mousedown of a plain click,
* so an old selection elsewhere in the buffer is already gone by the time a
* real click on a link arrives here.
*
* The limit of this check, stated because the paragraph above reads
* absolute: it sees a drag only once the drag has spanned a *cell*. A press
* and release inside one character cell, or a drag walked back to where it
* started, leaves `finalSelectionEnd === finalSelectionStart`, so
* `hasSelection()` is false and the link opens. Nothing reached the
* clipboard in that case and the card showed the real origin first, so the
* cost is small — but it is the gap a mousedown/mouseup distance check
* would have closed, and it is the price of not keeping that second source
* of truth.
* - **A repeat click**, `detail > 1`. Belt to the above's braces: it holds
* even when the selection came out empty (a double-click on trailing
* whitespace selects nothing) and it does not depend on xterm having
* updated the selection model before the Linkifier's listener runs. It is
* `!== 1`, not `> 1`: a mouseup derived from a real click always carries
* `detail >= 1`, so `> 1` would have waved through anything synthesised
* with `detail` 0. Nothing in the container can dispatch a DOM event, so
* that is hardening rather than a hole being closed.
* Comparing mousedown and mouseup *coordinates* would be a third signal,
* but xterm hands this handler only the mouseup — the mousedown is not
* ours to see without binding our own listener to the host element, which
* is a second source of truth about the same gesture.
* - **A plain click while a modifier is required.** 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 the widget also responds, so nothing looks wrong. Requiring the
* force-selection modifier there makes the two intents distinguishable.
*
* `modifierPromised` is that last requirement made sticky, and it is about the
* card rather than the click: the hint is rendered once, at hover, from a mode
* the container may change before the user's finger comes down. The gate
* honours the stricter of what the card promised and what is true now, so a
* card reading "Shift+click to open" cannot be on screen while a bare click
* opens the link.
*
* **What this does not close.** `mouseTracking` is a permission the attacker
* grants itself — see `activate`.
*
* With nothing tracking the mouse and nothing promised, a bare click is
* correct and expected: it is what `WebLinksAddon` does for the plain-text
* URLs in the same buffer, which is why that handler applies this same gate.
*/
function opensOnClick(
event: MouseEvent,
ctx: ClickContext,
modifierPromised = false,
): boolean {
if (event.button !== 0) return false;
if (event.detail !== 1) return false;
if (ctx.hasSelection) return false;
if (!ctx.mouseTracking && !modifierPromised) return true;
return forcesSelection(event, ctx.macOptionClickForcesSelection);
}
/**
* Makes OSC 8 hyperlinks clickable, and shows where they actually go.
*
* ## Why xterm's own link matching is not enough
*
* `WebLinksAddon` matches *rendered text*, row by row. Claude Code prints its
* links as OSC 8 hyperlinks whose visible text is hard-wrapped into
* terminal-width pieces — measured against 2.1.226, a 346-character sign-in
* URL arrives as five emissions, each carrying the whole URL in its OSC 8
* parameter and about 80 characters of it on screen (see `lib/urlDetector.ts`,
* which had to grow the same second branch). So the addon matches a fragment
* or nothing at all, which is the entire reason the URL toast exists. xterm
* hands `linkHandler` the complete parameter instead, however the label was
* sliced, so this covers exactly the case the addon cannot — and the addon
* stays, because it covers the plain-text URLs in ordinary shell output that
* carry no OSC 8 at all.
*
* ## xterm applies no gate of its own, so this one does
*
* There is a tempting story in which xterm's mouse-reporting mousedown cancels
* the event before the link layer sees it, leaving only the force-selection
* modifier a way through. It is false in both halves. That branch calls
* `cancel(e)`, which is a no-op unless `cancelEvents` is set and it defaults to
* false; and the mouse-reporting listeners are bound on `Terminal.element`
* while the Linkifier is bound on `screenElement`, a descendant, so bubbling
* reaches the link first no matter what. `Linkifier._handleMouseUp` then
* activates the link with no check on the button, the modifier or the mouse
* mode.
*
* So the gate is {@link opensOnClick}, applied in `activate`, and everything it
* asks about is read from the terminal at the moment of the click rather than
* captured — the container changes the mouse mode whenever it likes, and the
* selection is whatever the gesture that ended in this mouseup left behind.
*
* ## What the mouse mode is worth, honestly
*
* Reading it fresh makes it *current*; it does not make it *trustworthy*. The
* mode is set by the container, with a DECSET, and `?1002l` takes effect as
* soon as xterm *parses* it (on its queued write task, not synchronously with
* the container's output) — so a hostile container can drop tracking for
* a few hundred milliseconds at a time and a plain click that lands in one of
* those windows passes the mode half of the gate. It cannot time the user's
* click, but it does not need to: a fraction of clicks is enough, and the only
* tell is the status-bar badge flickering. This is a **known residual**, not
* something this gate closes, and the freshness of the read must not be read
* as an answer to it.
*
* Two things narrow it, neither of which depends on the mode. The selection
* and click-count checks hold in either tracking state, so the gestures a user
* makes to copy text are refused whatever the container has the mode set to —
* which removes the "wrap every row in an OSC 8 and harvest the shift-drags"
* version entirely. And the hover card's promise is sticky (see
* `modifierPromised`): the flicker now has to cover the *hover* as well as the
* click, because a card drawn while tracking was on goes on demanding the
* modifier after the container drops it. What remains is a container that
* drops tracking before the pointer arrives and holds it off until the click —
* at which point the card also says "Click to open", so the user is at least
* not being told one thing and given another. The real fix is a signal the
* container cannot write, and there is none in this pane today.
*
* ## The hover card is the security half, not a nicety
*
* OSC 8 fully decouples the visible text from the target: the container can
* print `https://claude.ai` and link it anywhere. That is strictly worse than
* the userinfo spoofing `sanitizeRelayUrl` already rejects, because here
* nothing in the painted row is even *derived* from the destination. So the
* origin of the real target is shown before the user commits, the same way the
* URL toast shows it and for the same reason ({@link urlOrigin}'s note): the
* origin decides where the user's credentials end up, so it is rendered in
* full and the *remainder* is the only part an ellipsis may eat.
*
* The card sits at the bottom of the pane rather than beside the pointer —
* where a browser puts it, and never underneath the cursor, so it cannot
* flicker the link out from under the hover that summoned it.
*
* @param getHost returns `Terminal.element`, which does not exist until
* `term.open()` has run — hence a getter rather than the element.
* @param readState samples {@link ClickContext} — a getter for the same
* reason, and the *only* reason: every one of those answers changes
* under us, between the hover and the click that follows it.
*/
export function createOsc8LinkHandler(
getHost: () => HTMLElement | null,
readState: () => ClickContext,
): Osc8LinkHandler {
let card: HTMLDivElement | null = null;
/**
* Did the card the user is looking at name a modifier?
*
* Written whenever a card is drawn, and cleared with it — `hover()` clears
* and returns early when there is no host element, which leaves this false,
* the stricter of the two directions. xterm only activates a link
* it is currently hovering (`Linkifier._currentLink`), so there is always a
* fresh hover behind a click — which is what makes this the promise the user
* actually read, rather than a stale one. See `opensOnClick`.
*/
let modifierPromised = false;
const clear = () => {
card?.remove();
card = null;
modifierPromised = false;
};
const span = (text: string, style: Partial<CSSStyleDeclaration>) => {
const el = document.createElement("span");
el.textContent = text;
Object.assign(el.style, style);
return el;
};
return {
activate(event, text) {
// Opening the host browser is the one thing in this pane the container
// may not provoke on its own *and* the one thing no selection gesture
// may provoke by accident. See `opensOnClick` — including the residual
// it does not close.
if (!opensOnClick(event, readState(), modifierPromised)) return;
// Same sink and same rule as the WebLinksAddon branch: this came off the
// container's output, so it is validated before it reaches the OS
// opener. One implementation — `sanitizeRelayUrl` — on purpose.
const safe = sanitizeRelayUrl(text);
if (!safe) {
console.warn("Refusing to open a link that failed validation");
return;
}
openUrlExternal(safe).catch(reportOpenFailure);
},
hover(_event, text) {
clear();
const host = getHost();
if (!host) return;
const ctx = readState();
// Sampled here and held, because this is what the card is about to tell
// the user — and the gate has to honour it even if the container has
// moved on by the time they click.
modifierPromised = ctx.mouseTracking;
card = document.createElement("div");
card.className = OSC8_HOVER_CLASS;
card.dataset.testid = "osc8-hover";
Object.assign(card.style, {
position: "absolute",
left: "8px",
bottom: "8px",
maxWidth: "calc(100% - 16px)",
boxSizing: "border-box",
zIndex: "30",
// The card lands under the pointer for a link in the bottom rows, and
// it is not a sibling the Linkifier hit-tests around (see
// `OSC8_HOVER_CLASS`). Without this, `screenElement` gets `mouseleave`
// the moment the card appears — card removed, pointer back on the
// link, card back: a flicker loop — and worse, the `mouseup` that
// activates the link lands on the card, so the link cannot be opened
// at all. Nothing here is interactive, so nothing is lost.
pointerEvents: "none",
display: "flex",
alignItems: "baseline",
gap: "6px",
padding: "3px 8px",
fontSize: "12px",
fontFamily: "monospace",
background: "var(--bg-secondary)",
border: "1px solid var(--border-color)",
// The origin wraps, so the card grows downward rather than sideways;
// this is the backstop for anything that still cannot fit.
overflow: "hidden",
borderRadius: "6px",
boxShadow: "var(--shadow-overlay)",
color: "var(--text-primary)",
} as Partial<CSSStyleDeclaration>);
const safe = sanitizeRelayUrl(text);
const origin = safe && urlOrigin(safe);
if (!safe || !origin) {
// Nothing of the rejected target is echoed into the DOM — it is
// untrusted text, and the only useful thing to say is that the click
// will not do anything. Deliberately not "it is not a web address":
// `https://claude.ai@evil.tld/` and an over-length URL both are one,
// and a card that explains a refusal wrongly teaches the user to
// distrust the card.
card.appendChild(
span("This link will not be opened — it failed the URL safety check", {
color: "var(--text-secondary)",
}),
);
} else {
const rest = safe.startsWith(origin) ? safe.slice(origin.length) : safe;
const originEl = span(origin, {
fontWeight: "700",
// The part that decides where the credentials go, so all of it is
// shown: truncating it *is* the spoof, and so is pushing its tail
// off the right edge of the pane. The attacker picks the length —
// `https://claude.ai.<300 chars>.evil.tld` parses and passes every
// `sanitizeRelayUrl` rule — so "do not shrink" is not enough:
// `flex-shrink: 0` pins a flex item at its max-content width and the
// text never wraps, it just overflows. It wraps instead, onto as
// many lines as it needs, and the truncatable remainder below is the
// thing that gives way.
flexShrink: "1",
minWidth: "0",
overflowWrap: "anywhere",
whiteSpace: "normal",
});
originEl.dataset.testid = "osc8-hover-origin";
card.appendChild(originEl);
const restEl = span(rest, {
color: "var(--text-secondary)",
overflow: "hidden",
textOverflow: "ellipsis",
whiteSpace: "nowrap",
minWidth: "0",
});
restEl.dataset.testid = "osc8-hover-rest";
card.appendChild(restEl);
const hint = span(openHintLabel(ctx), {
color: "var(--text-secondary)",
flexShrink: "0",
marginLeft: "4px",
});
card.appendChild(hint);
}
host.appendChild(card);
},
leave: clear,
dismiss: clear,
};
}
export default function TerminalView({ sessionId, active }: Props) { export default function TerminalView({ sessionId, active }: Props) {
const containerRef = useRef<HTMLDivElement>(null); const containerRef = useRef<HTMLDivElement>(null);
const terminalContainerRef = useRef<HTMLDivElement>(null); const terminalContainerRef = useRef<HTMLDivElement>(null);
const termRef = useRef<Terminal | null>(null); const termRef = useRef<Terminal | null>(null);
const fitRef = useRef<FitAddon | null>(null); const fitRef = useRef<FitAddon | null>(null);
const webglRef = useRef<WebglAddon | null>(null); const webglRef = useRef<WebglAddon | null>(null);
// Held only so the hover card can be taken down when this pane leaves the
// screen — see `Osc8LinkHandler.dismiss`.
const osc8LinkHandlerRef = useRef<Osc8LinkHandler | null>(null);
const detectorRef = useRef<UrlDetector | null>(null); const detectorRef = useRef<UrlDetector | null>(null);
const { sendInput, pasteImage, resize, onOutput, onExit } = useTerminal(); const { sendInput, pasteImage, resize, onOutput, onExit } = useTerminal();
const gpuRenderingSetting = useAppState(s => s.appSettings?.terminal_gpu_rendering ?? null); const gpuRenderingSetting = useAppState(s => s.appSettings?.terminal_gpu_rendering ?? null);
@@ -378,7 +827,7 @@ export default function TerminalView({ sessionId, active }: Props) {
const syncMouseCapture = useCallback(() => { const syncMouseCapture = useCallback(() => {
const term = termRef.current; const term = termRef.current;
if (!term) return; if (!term) return;
const captured = term.modes.mouseTrackingMode !== "none"; const captured = terminalTracksMouse(term);
if (captured === mouseCapturedRef.current) return; if (captured === mouseCapturedRef.current) return;
mouseCapturedRef.current = captured; mouseCapturedRef.current = captured;
setMouseCaptured(captured); setMouseCaptured(captured);
@@ -410,7 +859,10 @@ export default function TerminalView({ sessionId, active }: Props) {
useEffect(() => { useEffect(() => {
if (!containerRef.current) return; if (!containerRef.current) return;
const term = new Terminal({ // Annotated because `linkHandler` below refers to `term` (for the element
// it must anchor its hover card to, which does not exist until
// `term.open()`), and TypeScript cannot infer a type it is already using.
const term: Terminal = new Terminal({
cursorBlink: true, cursorBlink: true,
fontSize: 14, fontSize: 14,
// Let the user select text even while a program holds the mouse. // Let the user select text even while a program holds the mouse.
@@ -420,6 +872,17 @@ export default function TerminalView({ sessionId, active }: Props) {
// the only way to copy from a mouse-driven TUI is to take the mouse back // the only way to copy from a mouse-driven TUI is to take the mouse back
// first. `SelectionService.shouldForceSelection`. // first. `SelectionService.shouldForceSelection`.
macOptionClickForcesSelection: true, macOptionClickForcesSelection: true,
// OSC 8 hyperlinks — the form Claude Code prints its links in, and the
// one `WebLinksAddon` structurally cannot match. See
// `createOsc8LinkHandler`, including why opening one while a program
// holds the mouse needs the same Shift/Option the line above is about.
// Both arguments are getters because neither answer exists yet: the
// element arrives with `term.open()`, and the mouse mode changes
// whenever the container prints a DECSET.
linkHandler: (osc8LinkHandlerRef.current = createOsc8LinkHandler(
() => term.element ?? null,
() => readClickContext(term),
)),
fontFamily: "'JetBrains Mono', 'Fira Code', 'Cascadia Code', Menlo, Monaco, monospace", fontFamily: "'JetBrains Mono', 'Fira Code', 'Cascadia Code', Menlo, Monaco, monospace",
theme: { theme: {
background: "#0d1117", background: "#0d1117",
@@ -453,28 +916,39 @@ export default function TerminalView({ sessionId, active }: Props) {
// misses OAuth URLs that end mid-line). // misses OAuth URLs that end mid-line).
// eslint-disable-next-line no-control-regex // eslint-disable-next-line no-control-regex
const urlRegex = /https?:\/\/[^\s'"`<>\x00-\x20\x7f]+/; const urlRegex = /https?:\/\/[^\s'"`<>\x00-\x20\x7f]+/;
const webLinksAddon = new WebLinksAddon((_event, uri) => { const webLinksAddon = new WebLinksAddon((event, uri) => {
// Same sink, same rule: what xterm matched came off the container's // Same gate and same sink as `createOsc8LinkHandler`, because this
// output, so it is validated before it reaches the OS opener. A click // reaches the same `openUrlExternal` through the same
// here is a deliberate act on visible text, but "visible" is exactly // `Linkifier._handleMouseUp`, which checks nothing here either. Without
// what a userinfo-spoofed URL subverts. // it a container that prints a plausible-looking `https://` row in a TUI
// got a browser open on a plain click while it held the mouse, and a
// double-click that merely selected a URL opened it.
//
// It is also the only gate on a real bypass of the OSC 8 one:
// `OscLinkProvider` drops a hyperlink whose target is not http(s)
// *before* `linkHandler` sees it (`allowNonHttpProtocols` is unset), so
// an OSC 8 carrying a `javascript:` target and an `https://evil.tld/x`
// label leaves the addon free to match the label. That click arrives
// here and nowhere else.
//
// No `modifierPromised`: this path paints an underline rather than a
// card, so it promises the user nothing to be held to.
//
// This branch is the one where the click really is an act on visible
// text — the match *is* the painted characters — so the spoof it has to
// survive is a userinfo-spoofed URL, which `sanitizeRelayUrl` rejects.
// An OSC 8 link is not like that at all: its label and its target are
// unrelated strings, which is why that handler shows the target's origin
// on hover before a click can happen. Neither branch replaces the other:
// this one covers plain-text URLs in ordinary shell output, which carry
// no hyperlink parameter for xterm to hand over.
if (!opensOnClick(event, readClickContext(term))) return;
const safe = sanitizeRelayUrl(uri); const safe = sanitizeRelayUrl(uri);
if (!safe) { if (!safe) {
console.warn("Refusing to open a link that failed validation"); console.warn("Refusing to open a link that failed validation");
return; return;
} }
// Same failure reporting as the toast's Open button — see the long note openUrlExternal(safe).catch(reportOpenFailure);
// on `handleOpenUrl`, including what this catch does *not* catch on
// Linux. A click that appears to do nothing is the complaint either way.
openUrlExternal(safe).catch((e) =>
useAppState.getState().pushToast({
kind: "error",
message: "Could not open that link in your browser",
detail: String(e),
// A dead opener fails for every link in the buffer. One card.
dedupeKey: "host-open-failed",
}),
);
}, { urlRegex }); }, { urlRegex });
term.loadAddon(webLinksAddon); term.loadAddon(webLinksAddon);
@@ -763,9 +1237,19 @@ export default function TerminalView({ sessionId, active }: Props) {
webglRef.current = null; webglRef.current = null;
term.dispose(); term.dispose();
termRef.current = null; termRef.current = null;
osc8LinkHandlerRef.current = null;
}; };
}, [sessionId]); // eslint-disable-line react-hooks/exhaustive-deps }, [sessionId]); // eslint-disable-line react-hooks/exhaustive-deps
// A hover card only ever clears on a *pointer* leaving the link, so switching
// tabs from the keyboard leaves one hanging over a pane nobody is looking at,
// to be found still there on the way back. Hiding the wrapper does not fire
// `mouseleave`, so nothing else would.
useEffect(() => {
if (active) return;
osc8LinkHandlerRef.current?.dismiss();
}, [active]);
// Manage WebGL lifecycle and re-fit when tab becomes active. // Manage WebGL lifecycle and re-fit when tab becomes active.
// Only the active terminal holds a WebGL context to avoid exhausting // Only the active terminal holds a WebGL context to avoid exhausting
// the browser's limited pool (~8-16 contexts). // the browser's limited pool (~8-16 contexts).