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

Closed

natsukium wants to merge 16 commits from fix/osc8-activation-13 into main

pull from: fix/osc8-activation-13

merge into: natsukium:main

natsukium:main

natsukium:renovate/lock-file-maintenance

natsukium:ci/darwin-smoke

Conversation 1 Commits 16 Files changed 18 +1386 -109

natsukium commented 2026-09-04 02:21:11 +09:00

Owner

Copy link

Fixes #13.

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.

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, and the hover target is
re-read on every applied grid frame so a scrolled-away link cannot be activated.

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.
• CHANGELOG.md — user-affecting behavior.

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

Fixes #13. ## 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. 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, and the hover target is re-read on every applied grid frame so a scrolled-away link cannot be activated. ## 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. - `CHANGELOG.md` — user-affecting behavior. Verified: just check green (fmt, clippy, nextest 3047 passed, deny), pi review PASS.

natsukium added 15 commits 2026-09-04 02:21:11 +09:00

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

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 ... dae4c4c64c

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 ... c713798635

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 ... 3bc897b7ab

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 ... d98efa372a

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 ... 60e35dfc72

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 ... ae58553455

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 ... 2101f0229b

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 ... f2f900b4d7

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 ... 3d4e439697

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 ... 7b658ec178

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 ... 0956c53d2c

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 ... 44ea9011e6

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 ... 48b092594c

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 ... 0cb409cb5c

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

natsukium added 1 commit 2026-09-04 03:27:12 +09:00

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

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

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

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

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

Author

Owner

Copy link

Superseded by #62: this branch was behind main and a rebase merge conflicted on CHANGELOG.md, and force-pushing was not an option, so the same commits (plus 8 follow-up fixes) were rebased onto current main on fix/osc8-activation-13-v2.

Superseded by #62: this branch was behind main and a rebase merge conflicted on CHANGELOG.md, and force-pushing was not an option, so the same commits (plus 8 follow-up fixes) were rebased onto current main on fix/osc8-activation-13-v2.

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

Some checks are pending

Hide checks

build / build felis (aarch64-darwin) (pull_request) Successful in 1m35s

Details

bench / Criterion full-suite snapshot (pull_request) Has been skipped

Details

fuzz / cargo fuzz nightly long-run (pull_request) Has been skipped

Details

bench / Criterion regression gate (pull_request) Successful in 3m9s

Details

fuzz / cargo fuzz smoke (per target) (pull_request) Successful in 59s

Details

pr / cargo clippy (Windows cross) (pull_request) Successful in 29s

Required

Details

pr / frontend smoke (x86_64-linux) (pull_request) Successful in 2m18s

Required

Details

pr / publish felis (x86_64-linux) (pull_request) Has been skipped

Required

Details

pr / cargo check (MSRV) (pull_request) Successful in 26s

Required

Details

pr / nix flake check (pull_request) Successful in 16s

Required

Details

pr / cargo build / clippy / test / deny (pull_request) Successful in 2m20s

Required

Details

pr / generated code is current (pull_request) Successful in 2s

Required

Details

pr / cargo test (Windows) (pull_request) Successful in 6m41s

Required

Details

pr / frontend smoke (Windows) (pull_request) Successful in 4m40s

Required

Details

pr / package felis (x86_64-pc-windows-msvc) (pull_request) Has been skipped

Required

Details

windows /**

Required

darwin /**

Required

Pull request closed

This pull request cannot be reopened because the branch was deleted.

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!60

No description provided.