fix(client): re-validate OSC 8 activation and stop logging the URI #62

Merged

natsukium merged 24 commits from fix/osc8-activation-13-v2 into main 2026-09-04 12:03:37 +09:00

Conversation 0 Commits 24 Files changed 19 +1795 -129

natsukium commented 2026-09-04 11:33:15 +09:00

Owner

Copy link

Fixes #13.

Supersedes #60: that branch was behind main and could not be rebase-merged
(a CHANGELOG.md conflict), and force-pushing it was not an option, so the
same 24 commits were rebased onto current main here.

Summary

OSC 8 activation now crosses a typed boundary. felis-client-core gains
ActivationTarget, a validated newtype that re-checks the scheme allowlist and
rejects NUL, C0/C1 controls, and bidi controls at activation time, so a URI
stored by any future grid path cannot reach a platform launcher as a raw string.
The Linux (xdg-open), macOS (open), and Windows (ShellExecuteW) launchers
take only that type. Activation logging is reduced to outcome, scheme class, and
length — the URI itself is never logged, and ActivationTarget's Debug
redacts it too.

While the activation modifier is held, a control-safe and bidi-safe preview
overlay shows the target: the renderer sanitizes the text, marks a clipped
preview, and the bottom-bar precedence rules keep the preview from being
overwritten or silently shortened by a preedit or by producer ink (an image
placement no longer claims a row a chrome bar holds). CONTROL is kept held when
the launcher never starts or the activation is refused.

Activation is tied to the preview it promises: it arms only on the frame that
actually showed the preview, refuses to arm when the atlas cut that frame
short, and the hovered cell is re-derived whenever the grid moves under it, so
a scrolled-away link cannot be activated. The hover is re-read only on frames
that can move it.

Doc cascade

• docs/explanation/security-model.md — the activation boundary, the logging
  rule, and the rejected alternatives.
• docs/reference/security-audits.md, docs/reference/spec.md — the resulting
  facts and the requirement wording.
• docs/explanation/input.md, docs/reference/keybindings.md — the modifier-held
  preview and its interaction with preedit.
• docs/explanation/rendering/pipeline.md, plus the Kitty graphics
  reference/explanation twins — the chrome-row reservation and the clipping mark.
• CHANGELOG.md — user-affecting behavior.

Verified: just check green (fmt, clippy, nextest 3068 passed, deny), pi review PASS.

Fixes #13. Supersedes #60: that branch was behind `main` and could not be rebase-merged (a `CHANGELOG.md` conflict), and force-pushing it was not an option, so the same 24 commits were rebased onto current `main` here. ## Summary OSC 8 activation now crosses a typed boundary. `felis-client-core` gains `ActivationTarget`, a validated newtype that re-checks the scheme allowlist and rejects NUL, C0/C1 controls, and bidi controls at activation time, so a URI stored by any future grid path cannot reach a platform launcher as a raw string. The Linux (`xdg-open`), macOS (`open`), and Windows (`ShellExecuteW`) launchers take only that type. Activation logging is reduced to outcome, scheme class, and length — the URI itself is never logged, and `ActivationTarget`'s `Debug` redacts it too. While the activation modifier is held, a control-safe and bidi-safe preview overlay shows the target: the renderer sanitizes the text, marks a clipped preview, and the bottom-bar precedence rules keep the preview from being overwritten or silently shortened by a preedit or by producer ink (an image placement no longer claims a row a chrome bar holds). CONTROL is kept held when the launcher never starts or the activation is refused. Activation is tied to the preview it promises: it arms only on the frame that actually showed the preview, refuses to arm when the atlas cut that frame short, and the hovered cell is re-derived whenever the grid moves under it, so a scrolled-away link cannot be activated. The hover is re-read only on frames that can move it. ## Doc cascade - `docs/explanation/security-model.md` — the activation boundary, the logging rule, and the rejected alternatives. - `docs/reference/security-audits.md`, `docs/reference/spec.md` — the resulting facts and the requirement wording. - `docs/explanation/input.md`, `docs/reference/keybindings.md` — the modifier-held preview and its interaction with preedit. - `docs/explanation/rendering/pipeline.md`, plus the Kitty graphics reference/explanation twins — the chrome-row reservation and the clipping mark. - `CHANGELOG.md` — user-affecting behavior. Verified: just check green (fmt, clippy, nextest 3068 passed, deny), pi review PASS.

natsukium added 24 commits 2026-09-04 11:33:15 +09:00

feat(client-core): add ActivationTarget for OSC 8 activation boundary ... c0a19d5909

The grid parser's C0/DEL filter is not the only path a URI can take
to the client: a non-felis daemon, or a future grid change, could
hand the client a GridMsg::Hyperlink the parser never saw, and
Unicode bidi controls are 0x80+ in UTF-8 so they pass that filter
untouched anyway. ActivationTarget::parse re-checks the scheme
allowlist independently and refuses an interior NUL, any other
control character, a bidi-reordering codepoint, or a URI past the
parser's own length cap, so a raw string can only reach a platform
launcher after crossing this typed boundary.

Bidi codepoints are refused rather than stripped: a URI carrying one
is hostile by construction (RFC 3986 URIs are ASCII after IRI
mapping), and stripping would activate a target the user never saw
rendered. log_fields() exists specifically to keep a log line down
to scheme class and byte length, never the URI itself.

Placed in felis-client-core rather than felis-client: it needs no
winit/wgpu dependency, is testable on every CI host, and any future
frontend reuses it without pulling in the GUI client.

Refs #13

Assisted-by: Claude Code

feat(render): add a Ctrl-hover preview overlay for OSC 8 hyperlinks ... c229f8b865

security-model.md already promised that hyperlinks display the
target on hover so the visible link text is never trusted to
represent the destination, but no renderer surface existed to keep
that promise. LinkPreviewOverlay reuses the confirm/search bars'
bottom-row text drawing rather than a new shader, and its bg is
SEARCH_BAR_BG rather than CONFIRM_BAR_BG: a hover preview is
informational, not a destructive prompt, so it must not read like
one.

The preview loses the bottom row to an active search or an open
confirmation, both more deliberate user actions than a Ctrl-hover;
without that precedence the preview's opaque background would paint
over whichever of those was drawn first into the same row.

Refs #13

Assisted-by: Claude Code

fix(client): re-validate OSC 8 activation and stop logging the URI ... d100885813

open_url took a raw &str with no scheme re-check of its own on
either platform path, and the Ctrl+Click handler logged the URI on
both the success and failure arm. Routing activation through
ActivationTarget::parse closes both gaps: javascript:/data: schemes
are refused even if a future grid path stored them, and the log
lines now carry only the scheme class and byte length on success, or
which ActivationRejection fired on refusal.

launch_argv is factored out of the Unix open_url so the argv shape
(program name plus the target as one whole argument) is pinned in a
unit test the way ShellExecuteArgs already pins the Windows shape;
ShellExecuteArgs::for_url now takes the validated target directly,
with wide_z's own interior-NUL check kept as defense in depth.

hover_target tracks the same Ctrl+hover condition the Pointer cursor
icon already keys on, updated in the one place
(update_mouse_cursor_icon) both need to stay in lockstep, and feeds
the preview overlay built each redraw.

update_mouse_cursor_icon set hover_target but never told the redraw
scheduler: the app runs on ControlFlow::Wait, so nothing repainted the
wgpu preview bar on a Ctrl press/release over a stationary pointer, on
CursorLeft, or after Ctrl+Click activation cleared the modifier, even
though the OS-level Pointer cursor updated immediately (set_cursor
takes effect without a redraw). It now requests a redraw exactly when
hover_target changes, via a hover_target_changed helper kept next to
pointer_icon_for_cell so the decision is unit-testable without
constructing a full App.

Refs #13

Assisted-by: Claude Code

docs: record the OSC 8 activation-boundary re-validation ... 25f227dcf6

security-model.md and REQ-910 described only the grid-side scheme
filter; they now state the independent client-side re-check, the
rejected classes (interior NUL, other control chars, bidi
codepoints), the Ctrl-hover preview that keeps the standing
"hyperlinks display the target on hover" promise, and the
scheme-class-and-length-only log contract. security-audits.md's OSC 8
entry follows the same typed boundary through both platform hand-offs.

keybindings.md's Mouse section previously had no entry at all for
Ctrl+Click hyperlink activation even though the gesture already
shipped; it gains one alongside the new preview, since that page is
where every other mouse gesture is already documented.

The "Confusable URLs" bullet under "Text rendering" still promised the
old, unbuilt shape ("on hover or in a status surface") instead of
pointing at what shipped, leaving two bullets in the same doc making
overlapping but inconsistent claims; it now points at the "OSC 8
hyperlinks and OSC 7 CWD" section for the actual guarantee.

The precedence the preview loses to search/confirm, and the choice of
SEARCH_BAR_BG over CONFIRM_BAR_BG, existed only as code comments; both
are design decisions per CLAUDE.md's "record inline in the owning
explanation doc" rule, so input.md gains a "Link preview" section
alongside "Confirmation bar", the other decision about the same shared
chrome row.

Also thins the em dashes doc-prose flags as an LLM tell (the "not X —
Y" contrast punch) out of the new security-model.md, security-audits.md,
and spec.md prose.

Refs #13

Assisted-by: Claude Code

fix(client): keep CONTROL held when an OSC 8 activation is refused ... 29df392eff

Ctrl+Click on a link whose stored URI fails `ActivationTarget::parse`
launches nothing and leaves the window focused, so the focus-steal that
justifies dropping the CONTROL bit never happens. Clearing it anyway
desynced `self.modifiers` from the physically held key until the next
`ModifiersChanged`: a following Ctrl+Click on a valid link was ignored,
and `input::encode` sent a bare `c` where the program expected ^C.

Refs #13

Assisted-by: Claude Code

fix(client): re-read the hover target on every applied grid frame ... 5df6cc46a8

Content under a stationary pointer changes without any pointer event: a
RowDelta, a scroll, a Hyperlink table update or a rehydrate burst can put
a link where there was none, or move one away. Refreshing only on
PointerShape left the Ctrl-hover preview and the Pointer icon showing a
target Ctrl+Click no longer resolves, which for a security surface is the
one disagreement it must not have. PointerShape needs no arm of its own
any more: it is one of those frames.

Refs #13

Assisted-by: Claude Code

test(client): pin the entry the Unix launcher test skips ... ef9d41506f

The loop over HOSTILE_URLS silently skipped anything ActivationTarget
refuses, so tightening the activation boundary (rejecting `%` or `&` in
a path, say) would empty the loop and leave the test asserting nothing
about the argv shape while still passing. Its Windows twin already pins
the skipped entry; both now also fail if the pinned count drops.

Refs #13

Assisted-by: Claude Code

refactor(render): make the bottom-bar precedence testable ... fb28b63343

The rule that a hover preview loses the bottom chrome row to an open
search or confirmation bar lived inline in build_cell_instances, which
needs a live wgpu device and so runs in no test: deleting the gate kept
every test green while the preview's opaque background painted over an
open prompt. bottom_bar_claim is the same decision as a pure function
beside the extend_* helpers, where the row's other invariants are
already pinned.

Refs #13

Assisted-by: Claude Code

docs: complete the link-preview decision record ... e275818481

The section stated only the chosen rationale for the bar's precedence and
its color, while every other decision in the explanation quadrant carries
its rejected alternatives and a Revisit-if trigger inline (CLAUDE.md,
"Recording design decisions"); the sibling "Confirmation bar" section it
cross-references is the model. Names the two live alternatives — the
preview winning the row, and sharing it — and what would reopen each
choice.

Refs #13

Assisted-by: Claude Code

fix(client): keep CONTROL held when the launcher never starts ... 1146f0dd82

A spawn failure (`xdg-open` missing, fork refused) leaves the window
focused, so the focus-steal that justifies dropping the CONTROL bit
never happens. Clearing it anyway desynced `self.modifiers` from the
physically held key until the next `ModifiersChanged`: the preview bar
vanished, a following Ctrl+Click on a valid link was ignored, and
`input::encode` sent a bare `c` where the program expected ^C — the
same desync a refused target already avoids.

Refs #13

Assisted-by: Claude Code

fix(render): mark a clipped preview and yield the row to a preedit ... d6c195cc98

The bottom bar stopped painting at the last column, so on any window
narrower than the target the preview ended mid-URI with nothing saying
it had been cut: a host truncated away reads as the whole host, which is
the misreading the preview exists to prevent. Fitting is now measured in
display cells and ends in the same `…` `ActivationTarget::preview` uses,
which also keeps a two-cell glyph out of a one-cell remainder, where its
span was clamped while its quad still painted past the bar.

An IME composition on the bottom row collided with the preview the same
way: preedit glyphs are emitted first but every foreground quad draws
after every background one, so neither hid the other and the target came
out interleaved. Composing is deliberate and the hover is incidental, so
the preview yields the row, as it already does to search and confirm.

Refs #13

Assisted-by: Claude Code

docs: record the link preview's clipping mark and preedit rank ... cacc27e6d2

The bottom-row order and the preview's safety bullet named the bars the
preview competes with but not the IME composition it also collides with,
and said the preview is clipped without saying that a clip is marked —
the property that keeps a shortened target from reading as a whole one.
Both are decisions a later reader would otherwise re-litigate.

Refs #13

Assisted-by: Claude Code

fix(render): let nothing overwrite or silently shorten a bottom bar ... 2603988bcb

The link preview is only worth drawing if what it shows is the whole
target and nothing can paint something else on top of it. Three ways it
was neither:

The ellipsis a narrow window makes the bar synthesize was never handed
to the glyph atlas — only the overlay's own text is populated — and a
glyph the atlas has not seen is skipped without a trace, so a clipped
target painted as if it ended where it was cut.

Images with `z >= 0` draw after the cells, so a placement anchored on
the bottom row covered the bar; a producer could hide the real target or
paint a picture of a different one over it. Image quads are now clipped
at the top edge of a row the chrome holds, which leaves their order
against the terminal's own content untouched.

A chord can arm a confirmation while a search composes, so both bars
could be live at once; since every foreground quad draws after every
background one, their glyphs interleaved instead of one hiding the
other. Only the bar that owns the row draws it, and a search that loses
it keeps its hit highlights.

Refs #13

Assisted-by: Claude Code

docs: record that client chrome rows take no image and one bar ... 2a84e4955e

The Kitty z-order says images with `z >= 0` sit above the cells, and the
preview bullet promised a target the producer cannot restyle; nothing
said which wins where they meet, so a reader had no way to know the
chrome row is excluded from that order — the property the promise rests
on. The input doc also claimed the search and confirmation bars are
mutually exclusive, which a chord arming a confirmation mid-search
disproves; the ranking that actually holds is recorded instead.

Refs #13

Assisted-by: Claude Code

fix(render): keep producer ink out of the row a chrome bar holds ... 42d9a5fab0

Skipping the reserved row in the grid walk left two ways into it. A
glyph anchored on the row above can reach down — a tall bitmap, or an
OSC 66 run scaled across rows and bottom-aligned in its block — and it
rides the same foreground pass as the bar's own text, so nothing hides
it: a producer could write beside or through the link preview's target.
Cell, decoration and image quads are now all cut at the row's top edge
before the bar's instances are appended.

The bars' own glyphs could also go missing for a frame. They were put in
the atlas when the overlay was set, but the grid walk that opens the
frame recycles the whole sheet when it fills, dropping slots handed out
earlier, and its retry re-populates grid glyphs only. Populating the
overlay glyphs after that walk keeps a label — or the mark that says the
preview's target is clipped — from vanishing exactly when the screen is
busiest.

Refs #13

Assisted-by: Claude Code

fix(render): re-walk the grid when overlay text resets the atlas ... 7213d268c3

The chrome-bar and preview glyphs are primed after the grid walk so a
walk that recycled the sheet cannot swallow the bar's label. That
ordering leaves the overlay as the one primer that can overflow the
sheet once the grid already holds its slots: the reset drops them, and
because rendering is damage-driven those cells would paint blank and
stay blank until unrelated damage arrived. Reading the reported reset
again after priming reuses the existing bounded second walk.

Assisted-by: Claude Code

docs: thin the em dashes out of the link-preview prose ... 95875dd12e

The doc-prose norm defaults to none and calls the "not X — Y" contrast
punch an LLM tell, and the sweep earlier on this branch only covered the
pages that existed then. The link-preview section of input.md, the
chrome-row sections of kitty-graphics.md and pipeline.md, and the two
dashes the preview bullet in security-model.md picked up afterwards were
all written after it.

Punctuation only: every claim is unchanged.

Refs #13

Assisted-by: Claude Code

fix(client): tie OSC 8 activation to the preview it promises ... f97b1ddafc

Two ways the gesture and its affordance disagreed. A stored URI that
fails `ActivationTarget::parse` still swallowed the Ctrl+Left press,
and the release behind it, while the cursor over those same cells was
already the plain arrow: a producer that blankets the screen with
bidi-poisoned OSC 8 links silently ate every Ctrl+Left a `set mouse=a`
vim or lazygit user made, opening nothing and telling the program
nothing. And the preview bar yields its row to a search bar, a
confirmation, or an IME composition, but activation did not yield with
it, so REQ-910's "a control-safe preview is shown while the activation
modifier is held" was off in exactly the states the user's attention is
on the bar: Ctrl+Click opened a target that was never displayed, which
is the spoofing the preview exists to stop.

`activation_target` is now the single answer the Pointer icon, the
preview bar, and the click handler all read. It says no for a refused
URI and for a lost preview row alike, and `None` means "not an
activation": the press falls through to the ordinary routing rather
than being eaten by a link the cursor never offered. The row question
is resolved through the renderer's own `bottom_bar_claim` rather than a
second copy of the precedence.

The alternative for the second half was to narrow the REQ and the
security-model text to "unless a more deliberate bar holds the row".
Rejected: the preview is the whole anti-spoofing argument for opening
producer-controlled URIs at all, so a state that drops it must drop the
open too.

Refs #13

Assisted-by: Claude Code

perf(client): re-read the OSC 8 hover only when a frame can move it ... 295b8f6551

Every applied `GridMsg` ran the hover recompute and an unconditional
`Window::set_cursor`, and `main.rs` posts one event per Grid-kind frame:
a RowDelta, a Bell, a palette entry, a notification, all of them. winit
0.30's Wayland backend dedupes nothing, so each call resolved the icon
against the cursor theme and issued a `wl_pointer.set_cursor` with a
surface attach/damage/commit. A `cat` of a large file therefore put
hundreds of cursor-theme lookups and pointer requests a second on the
wire, plus a `ActivationTarget::parse` allocation per frame while Ctrl
rested over a link, where the old code issued none.

Both halves are now conditional: the frame handler asks
`grid_msg_moves_hover_target` first, since only a message that can move
a cell, the link table, the viewport, or the program's pointer shape can
change what is under a stationary pointer, and `set_cursor` runs only
when the icon actually differs from the one this window already has.

Refs #13

Assisted-by: Claude Code

test(client-core): make the preview sweep generate targets that parse ... 1d9003ddc7

`preview_of_any_parsed_string_never_forbidden` drew its input from
`".*"`, so reaching the assertion needed an arbitrary Unicode string to
begin with one of four scheme prefixes. Across the default 256 cases
that never happened: the `prop_assert!` never ran, and the test passed
just as green with a `preview` that prepended U+202E to every target.

The generator now joins an allowlisted scheme to an arbitrary tail, so
cases actually reach `preview`, and the count of accepted cases is
asserted afterwards. That second half is the same guard commit ae585534
added to the `HOSTILE_URLS` loop: a future tightening of `parse` must
fail here rather than quietly empty the sweep again.

Refs #13

Assisted-by: Claude Code

fix(client-core): redact the URI from ActivationTarget's Debug ... 04891d12a5

The activation boundary exists to keep producer-controlled URIs out of
diagnostics, and the call sites carry only the scheme class and the byte
length. A derived `Debug` left the whole URI one `?target` field away —
in a log line, a panic message, or an `assert_eq!` failure — so the
guarantee held only as long as nobody reached for the obvious tracing
idiom. Printing exactly what `log_fields` permits makes the type itself
the enforcement instead of a convention over its users.

Refs #13

Assisted-by: Claude Code

fix(client): arm OSC 8 activation on the frame that showed the preview ... f6f7b87141

"No preview, no activation" was decided from the current state rather
than from what was on screen. The bottom row changes owner with no event
of its own behind it — a search bar dismissed with Esc, a composition
committed — and the hover target was re-read only on pointer, modifier,
and grid events, so between the state change and its repaint a Ctrl+Left
opened a target no bar had ever shown. The anti-spoofing argument rests
on a preview the user could read, so the press now has to match the
target the last frame that reached the compositor actually painted, and
the hover is settled inside that frame rather than only from events that
happen to precede it.

The cost is that the first click after a bar closes can land before the
preview repaints and falls through as an ordinary press. Rejected:
re-arming from the state alone, which is what left the gap; and hooking
every search, confirmation, and IME transition, which spreads the rule
across every site that can touch the row and leaves the next one to
remember it.

The docs also promised a plain arrow while the gesture is disarmed; the
cursor reverts to the program's own `OSC 22` shape, which a program may
have set to `pointer`.

Refs #13

Assisted-by: Claude Code

fix(client): re-derive the hovered cell when the grid moves under it ... 57f8438dd0

The pointer cell was cached from the last `CursorMoved` and never
recomputed, but a font zoom, a DPI change, or a letterbox re-origin
moves the grid under a motionless pointer and winit reports no motion
for it. Everything keyed on that cell then names a glyph that is no
longer there, and the OSC 8 gesture inherits it: the preview shows, and
Ctrl+Left opens, the link that used to sit under the pointer. Reflow is
where every metric change already converges, so the cell and the
`?1016` pixel are re-derived there from the pointer position winit last
reported.

Refs #13

Assisted-by: Claude Code

fix(client): refuse to arm OSC 8 on a frame the atlas cut short ... f15b0a83b4

Two ways a frame could arm an activation the user had not really been
shown. A glyph atlas that fills during a frame drops slots handed out
earlier, and the bar's text is skipped glyph by glyph without a trace,
so a preview missing its leading characters reads as a different host —
which a producer can provoke by flooding the atlas. Arming now requires
a frame that drew everything it was asked for; the retry frame the
renderer already requests arms it a moment later.

The hovered cell is also re-derived inside the frame settle, because
dimensions change from under a stationary pointer through an
authoritative `GridMsg::Size` as well as through a local reflow, and
that path carries no window event to re-read the pointer on.

Refs #13

Assisted-by: Claude Code

natsukium referenced this pull request 2026-09-04 11:33:25 +09:00

fix(client): re-validate OSC 8 activation and stop logging the URI #60

natsukium merged commit 734756d102 into main 2026-09-04 12:03:37 +09:00

natsukium deleted branch fix/osc8-activation-13-v2 2026-09-04 12:03:37 +09:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

priority/P0

Must be resolved before the v0.1.0 tag

  

priority/P1

Must be settled before the first public contract freezes

  

priority/P2

Explicitly deferred until after v0.1.0

  

release/v0.1.0

Work tracked for the first tagged release

  

status/blocked

Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  

status/planned

Triaged with a source-grounded plan and a passed pi review; ready to implement

  

type/bug

Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  

type/design

An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  

type/test-gap

Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  

type/tracker

Parent issue that tracks child work

No labels

priority/P0

priority/P1

priority/P2

release/v0.1.0

status/blocked

status/planned

type/bug

type/design

type/test-gap

type/tracker

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

natsukium/felis!62

No description provided.