diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 667d2e9..9ea38fd 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -66,6 +66,15 @@ jobs: -e SC1090,SC2034,SC2155 \ "${shell_files[@]}" + behavior-tests: + name: paste-correctness behavioral tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Run behavioral regression suite (headless, mocked clipboard) + run: bash bin/flashpaste-selftest.sh + rust: name: rust fmt + clippy + unit tests runs-on: ubuntu-24.04 diff --git a/bin/flashpaste-doctor.sh b/bin/flashpaste-doctor.sh index 54625b7..7773026 100755 --- a/bin/flashpaste-doctor.sh +++ b/bin/flashpaste-doctor.sh @@ -332,6 +332,37 @@ for f in $(ls "$RDIR" | sort -n); do esac done +# ── Behavioral regression suite (paste-correctness guards) ──────────── +# Runs the headless bash test suite (wl-paste image-coexistence guard, +# kitty/tmux detector). A failure means a known paste bug could regress, +# so it counts as a doctor failure. +_selftest="$(dirname -- "$0")/flashpaste-selftest.sh" +if [ -x "$_selftest" ]; then + echo + hdr "paste-correctness self-test" + if "$_selftest" >/dev/null 2>&1; then + ok "self-test" "behavioral paste-correctness suite passed" + else + fail "self-test" "behavioral paste-correctness suite FAILED — run $_selftest" + fails=$((fails + 1)) + fi +fi + +# ── Keybinding drift check (kitty.conf vs tmux.conf vs canonical) ────── +# The double-paste bug was Ctrl+V drifting between the two configs. Warn +# (not fail) on drift: it degrades to the daemon's dedup, not a breakage. +_kbcheck="$(dirname -- "$0")/flashpaste-keybindings-check.sh" +if [ -x "$_kbcheck" ]; then + echo + hdr "keybinding drift" + if "$_kbcheck" >/dev/null 2>&1; then + ok "keybindings" "kitty + tmux Ctrl+V consistent with canonical source" + else + warn "keybindings" "Ctrl+V drift between kitty/tmux/canonical — run $_kbcheck" + warns=$((warns + 1)) + fi +fi + echo if [ "$fails" -eq 0 ] && [ "$warns" -eq 0 ]; then printf "${GREEN}All $core_checks core checks passed.${RESET} $optional_checks optional probe(s) also passed. flashpaste should work out of the box.\n" diff --git a/bin/flashpaste-keybindings-check.sh b/bin/flashpaste-keybindings-check.sh new file mode 100755 index 0000000..8a211a7 --- /dev/null +++ b/bin/flashpaste-keybindings-check.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# flashpaste keybinding drift check — read-only. +# +# Verifies the live kitty.conf and tmux.conf bind Ctrl+V the way the canonical +# source (config/keybindings.canonical) says they must. The double-paste bug +# was these two drifting apart; this catches the drift instead of silently +# pasting twice. Read-only: it never edits your dotfiles. +# +# Env overrides (for testing): KITTY_CONF, TMUX_CONF, CANONICAL. +# Exit 0 = consistent, 1 = drift/missing, 2 = canonical unreadable. +set -u + +ROOT="$(cd "$(dirname -- "$0")/.." && pwd)" +CANONICAL="${CANONICAL:-$ROOT/config/keybindings.canonical}" +KITTY_CONF="${KITTY_CONF:-$HOME/.config/kitty/kitty.conf}" +TMUX_CONF="${TMUX_CONF:-$HOME/.tmux.conf}" + +[ -r "$CANONICAL" ] || { echo "FATAL: canonical not readable: $CANONICAL"; exit 2; } + +drift=0 +while read -r surface key want; do + case "$surface" in ""|\#*) continue ;; esac + # Escape ERE metacharacters in the key (e.g. the '+' in ctrl+v). + key_re=$(printf '%s' "$key" | sed 's/[][\\.^$*+?(){}|]/\\&/g') + case "$surface" in + kitty) conf="$KITTY_CONF"; line=$(grep -E "^[[:space:]]*map[[:space:]]+$key_re[[:space:]]" "$conf" 2>/dev/null | tail -1) ;; + tmux) conf="$TMUX_CONF"; line=$(grep -E "^[[:space:]]*bind[[:space:]]+-n[[:space:]]+$key_re[[:space:]]" "$conf" 2>/dev/null | tail -1) ;; + *) echo " ? unknown surface '$surface' in canonical"; drift=1; continue ;; + esac + if [ -z "$line" ]; then + echo " ✗ $surface $key: no binding found in $conf" + drift=1 + elif printf '%s' "$line" | grep -Fq -- "$want"; then + echo " ✓ $surface $key -> $want" + else + echo " ✗ $surface $key: binding does not route through '$want'" + echo " live: $line" + drift=1 + fi +done < "$CANONICAL" + +if [ "$drift" = "0" ]; then + echo "keybindings: consistent with canonical source" +else + echo "keybindings: DRIFT detected — fix the live config(s) above" +fi +exit "$drift" diff --git a/bin/flashpaste-selftest.sh b/bin/flashpaste-selftest.sh new file mode 100755 index 0000000..fdc6e89 --- /dev/null +++ b/bin/flashpaste-selftest.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# flashpaste self-test — one command that proves the paste pipeline's +# correctness guards still hold. Runs the behavioral regression suite (the +# net under the image-bytes-as-text / blob-markup bug class) plus, with +# --rust, the daemon's unit tests. +# +# flashpaste-selftest.sh behavioral bash tests only (fast, no build) +# flashpaste-selftest.sh --rust also `cargo test -p flashpasted` +# +# Exits non-zero if any test fails. Headless-safe: the bash tests mock the +# clipboard, so this never touches the user's real selection and needs no +# display. Wired into CI (.github/workflows/lint.yml) and callable from +# flashpaste-doctor. +set -u + +ROOT="$(cd "$(dirname -- "$0")/.." && pwd)" +fail=0 + +echo "── flashpaste self-test ───────────────────────────────────" +for t in "$ROOT"/tests/*.test.sh; do + [ -f "$t" ] || continue + echo + echo "▶ $(basename "$t")" + if bash "$t"; then :; else fail=1; fi +done + +if [ "${1:-}" = "--rust" ]; then + echo + echo "▶ cargo test -p flashpasted" + if cargo test --manifest-path "$ROOT/rs/Cargo.toml" -p flashpasted 2>&1 | tail -3; then :; else fail=1; fi +fi + +echo +if [ "$fail" = "0" ]; then + echo "✓ flashpaste self-test: ALL PASSED" +else + echo "✗ flashpaste self-test: FAILURES (see above)" +fi +exit "$fail" diff --git a/bin/kitty-focused-is-tmux.py b/bin/kitty-focused-is-tmux.py new file mode 100755 index 0000000..2a9ddde --- /dev/null +++ b/bin/kitty-focused-is-tmux.py @@ -0,0 +1,54 @@ +#!/usr/bin/env python3 +"""Read `kitten @ ls --match state:focused` JSON on stdin; exit 0 iff the +focused kitty window's foreground process is tmux. + +Used by kitty-paste-router.sh to suppress kitty's redundant ctrl+v handler +when tmux's own `bind -n C-v` will handle the paste. Exit non-zero on ANY +uncertainty (no data, parse error, no tmux) so the caller falls through to +the normal paste path — a miss must degrade to "paste anyway", never to a +silent no-op. + +Why the foreground process: when tmux runs inside a kitty window, that +window's child IS the tmux client; the inner shells live under the tmux +server's process tree, not the kitty window. So a focused window whose +foreground process is `tmux` reliably means "this Ctrl+V is inside tmux". +""" +import json +import sys + + +def _is_tmux_cmdline(cmdline): + if not cmdline: + return False + exe = cmdline[0].rsplit("/", 1)[-1] + return exe == "tmux" or exe.startswith("tmux") + + +def main(): + try: + data = json.load(sys.stdin) + except Exception: + return 1 + if not isinstance(data, list): + return 1 + for os_window in data: + for tab in os_window.get("tabs", []): + for window in tab.get("windows", []): + # Only consider a window we can POSITIVELY confirm is focused. + # `--match state:focused` already filters to it (and sets + # is_focused=true), so requiring True here is exact on modern + # kitty. On older kitty that ignores the match and returns + # every window WITHOUT is_focused, no window qualifies -> we + # report "not tmux" -> the router falls through and the paste + # still happens. That is the fail-safe direction: never + # suppress a paste on a window we cannot prove is focused. + if window.get("is_focused") is not True: + continue + for proc in window.get("foreground_processes", []): + if _is_tmux_cmdline(proc.get("cmdline")): + return 0 + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/bin/kitty-paste-router.sh b/bin/kitty-paste-router.sh new file mode 100755 index 0000000..e9f89e0 --- /dev/null +++ b/bin/kitty-paste-router.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# ───────────────────────────────────────────────────────────────────── +# kitty ctrl+v router — eliminate the dual-handler double-paste at source. +# +# Problem: a single Ctrl+V inside kitty+tmux fires TWO handlers that both +# call flashpaste-trigger for the same pane — kitty's `map ctrl+v` AND +# tmux's `bind -n C-v`. They land hundreds of ms apart, so the daemon's +# (pane, content) dedup window has to race them. This router removes the +# redundant kitty fire entirely: if the FOCUSED kitty window is running +# tmux, tmux's own bind already handles the paste, so kitty does nothing. +# +# Safety: this is a BELT on top of the daemon's dedup SUSPENDERS. We only +# short-circuit when we can CONFIDENTLY detect tmux in the focused window +# (kitty remote control works AND its foreground process is tmux). On any +# uncertainty — remote control off, kitten missing, parse failure — we fall +# through to the original path, and the daemon's dedup still absorbs the +# duplicate. So a detection miss degrades to today's behaviour, never to a +# broken paste. +# +# Latency: the kitten round-trip (~30ms) only delays kitty's REDUNDANT +# handler. The user-visible paste already happened via tmux's fast bind, so +# this adds zero latency to the actual paste. When NOT in tmux, kitty's +# handler is the only one and ~30ms there is unnoticeable. +# ───────────────────────────────────────────────────────────────────── +set -u + +PASTE_IMAGE_FALLBACK="${FLASHPASTE_IMAGE_FALLBACK:-/home/deadpool/paste_image.sh}" +# Resolve kitty's remote-control socket. +# +# kitty appends "-" to the `listen_on` path, so `listen_on +# unix:.../kitty-main` actually opens `.../kitty-main-`. Crucially, +# KITTY_LISTEN_ON is NOT in the env this script inherits: kitty's `map +# ctrl+v launch --copy-env` copies kitty's OWN process env, which does not +# carry KITTY_LISTEN_ON (only child windows get it). So the old bare +# `kitty-main` fallback never matched the real `kitty-main-` socket — +# `kitten @ ls` failed every time, focused_window_is_tmux() always returned +# false, and this router NEVER suppressed kitty's redundant fire. Net effect: +# a single Ctrl+V pasted twice (tmux's bind immediately + this router ~1s +# later). Resolve robustly: trust KITTY_LISTEN_ON when present, else glob the +# real socket (newest wins if several kitty instances are running). +resolve_kitty_sock() { + if [ -n "${KITTY_LISTEN_ON:-}" ]; then + printf '%s\n' "$KITTY_LISTEN_ON" + return 0 + fi + local base s + base="/run/user/$(id -u)/kitty-main" + # Direct shell glob — no `ls`, no word-splitting (paths with spaces stay + # intact). kitty creates `kitty-main-`; with one instance there's + # exactly one match. With several this picks the first lexically — good + # enough for the common single-instance case, and a wrong guess only falls + # through to the normal paste path (the daemon's dedup is the backstop). If + # the glob matches nothing it stays literal and fails the `-S` test below. + for s in "$base"-* "$base"; do + [ -S "$s" ] && { printf 'unix:%s\n' "$s"; return 0; } + done + # Last-ditch: hand back the bare path; kitten will fail and we fall through + # to the normal paste path (the daemon's dedup is the remaining safety net). + printf 'unix:%s\n' "$base" +} +KITTY_SOCK="$(resolve_kitty_sock)" + +# Confidently true (exit 0) ONLY when the focused kitty window's foreground +# process is tmux. Any failure to determine that returns non-zero, so the +# caller falls through to the normal paste path. +focused_window_is_tmux() { + command -v kitten >/dev/null 2>&1 || return 1 + command -v python3 >/dev/null 2>&1 || return 1 + kitten @ --to "$KITTY_SOCK" ls --match state:focused 2>/dev/null \ + | python3 "$(dirname -- "$0")/kitty-focused-is-tmux.py" 2>/dev/null +} + +if focused_window_is_tmux; then + # tmux's `bind -n C-v` owns this paste. Do nothing. + exit 0 +fi + +# Not in tmux (or undetectable) — run the original kitty paste path. The +# daemon dedups if this turns out to be a duplicate of tmux's fire. +pane="$(tmux display-message -p "#{pane_id}" 2>/dev/null)" +# Only call the daemon trigger when we actually have a pane id. An empty +# pane (no tmux at all) can't be a paste target, so go straight to the +# image fallback instead of handing the daemon a blank pane. +if [ -n "$pane" ] && flashpaste-trigger "$pane" 2>/dev/null; then + exit 0 +fi +exec "$PASTE_IMAGE_FALLBACK" diff --git a/config/keybindings.canonical b/config/keybindings.canonical new file mode 100644 index 0000000..e0e2f02 --- /dev/null +++ b/config/keybindings.canonical @@ -0,0 +1,18 @@ +# flashpaste — canonical keybinding source of truth +# +# The double-paste bug was config DRIFT: kitty.conf and tmux.conf each bound +# Ctrl+V independently and got out of sync. This file is the single place that +# declares what each side MUST bind. bin/flashpaste-keybindings-check.sh reads +# it and verifies the live configs match, flagging drift (run by doctor). +# +# Format: one rule per non-comment line: +# The check passes when the live config's binding for contains the +# substring. Keep substrings stable (script basenames), not full command lines. + +# kitty's Ctrl+V must route through the tmux-aware router (suppresses kitty's +# handler inside tmux so only tmux's bind fires — see kitty-paste-router.sh). +kitty ctrl+v kitty-paste-router.sh + +# tmux's root Ctrl+V must hit the fast Rust trigger (daemon), falling back to +# the bash dispatcher when the daemon is down. +tmux C-v flashpaste-trigger diff --git a/docs/adr/0006-reduce-bash-surface-bounded-by-tier1-fallback.md b/docs/adr/0006-reduce-bash-surface-bounded-by-tier1-fallback.md new file mode 100644 index 0000000..e1547bd --- /dev/null +++ b/docs/adr/0006-reduce-bash-surface-bounded-by-tier1-fallback.md @@ -0,0 +1,66 @@ +# ADR 0006 — Reduce the bash surface, bounded by the Tier-1 fallback guarantee + +- **Status:** Proposed +- **Date:** 2026-06-10 +- **Deciders:** maintainers +- **Tags:** architecture, maintainability, scope + +## Context and problem statement + +flashpaste carries ~3.7k lines of bash across 17 tracked scripts alongside +~4k lines of Rust. Bash is hard to test and each script has been a breakage +source (the `wl-paste` shim, `clipboard-set.sh`, `tmux-paste-dispatch.sh`). +A tempting "improvement" is to fold the shell into the Rust daemon so the +logic is testable and typed. + +## The constraint that makes this an ocean + +Most of the bash is **load-bearing by design**, not accident: + +- `bin/tmux-paste-dispatch.sh` (657 lines) is the **Tier-1 canonical path** + per [ADR 0001](0001-three-progressive-tiers.md). It MUST run with zero + daemon, zero Rust toolchain, zero systemd. The daemon (Tier 3) execs it on + any failure. Folding it into the daemon would delete the very fallback that + makes "flashpasted is not running" a non-event. +- `bin/clipboard-set.sh` is invoked by tmux's `@clip` hook in the user's + shell, before any daemon round-trip — it has to be a script tmux can exec. +- `~/.local/bin/wl-paste` is a PATH shim that Claude Code reads through + directly; it only works because it IS a `wl-paste`-named executable. It + cannot become daemon-internal without changing how Claude reads the + clipboard. + +So "move the bash into Rust" is partly a **non-goal**: the bash surface is +the zero-dependency tier. This downgrades the ROI estimate the improvement +list gave #3 (friction +50% 🟠) — the genuinely reducible part is smaller. + +## What IS reducible (the real first slices) + +1. **Duplication between shims and daemon.** The daemon already has typed + probes (`read_clipboard_text_if_present`, `read_clipboard_image_if_present`, + `looks_like_text`) that re-implement logic also living in + `get-clipboard-text.sh` (169 lines) and parts of `tmux-paste-dispatch.sh`. + Where the daemon is up, those shells should call the daemon (one op) rather + than re-deriving. First slice: a `flashpaste-trigger --get-text` op that + `get-clipboard-text.sh` shells to when the socket exists, falling back to + its current logic otherwise. Removes ~100 lines of duplicated probe logic + without touching the Tier-1 guarantee. +2. **Behavioral test coverage for the bash that must stay.** Tier-1 is + permanent, so test it instead of deleting it. The `tests/*.test.sh` harness + (mocked clipboard, headless, in CI as of this session) is the pattern; grow + it to cover `tmux-paste-dispatch.sh`'s text-vs-image decision. + +## Decision + +**Proposed:** do NOT pursue a wholesale bash→Rust rewrite. Treat the Tier-1 +bash as a permanent, first-class artifact and invest in (a) deleting only the +*duplication* between shims and the daemon, slice by slice, and (b) behavioral +tests for the bash that must remain. Each slice ships independently and keeps +the no-daemon guarantee intact. + +## Consequences + +- The ~657-line Tier-1 dispatcher stays. That's correct, not debt. +- "Shrink the bash" becomes a bounded, test-first cleanup, not a multi-day + rewrite that risks the fallback path. +- Open question: if a future ADR retires the three-tier model (daemon becomes + mandatory), this constraint lifts and a fuller consolidation reopens. diff --git a/docs/adr/0007-wayland-data-control-on-non-mutter.md b/docs/adr/0007-wayland-data-control-on-non-mutter.md new file mode 100644 index 0000000..bd76588 --- /dev/null +++ b/docs/adr/0007-wayland-data-control-on-non-mutter.md @@ -0,0 +1,62 @@ +# ADR 0007 — Own the Wayland clipboard via data-control on non-Mutter compositors + +- **Status:** Proposed (deferred — untestable on the maintainer's Mutter box) +- **Date:** 2026-06-10 +- **Deciders:** maintainers +- **Tags:** architecture, wayland, correctness + +## Context and problem statement + +The whole shim layer exists because of one constraint: on GNOME 46 / Mutter, +the daemon **cannot own the Wayland clipboard**. Mutter ships neither +`ext-data-control` nor `wlr-data-control`, so a surfaceless client can't be a +selection owner. This is latched at `rs/flashpasted/src/wayland.rs` +(`WAYLAND_WEDGED`, ~line 41). Because the daemon can't own Wayland, when a +browser does "Copy image" the browser keeps Wayland ownership and advertises +`image/png` + a `text/html` ``; Claude reads both and +pastes the blob markup as text. We fixed that at the read chokepoint (the +`wl-paste` shim's image-coexistence guard) — a workaround, not a root fix. + +## The root-fix opportunity + +On compositors that DO implement a data-control protocol (wlroots: sway, +Hyprland, river; KDE Plasma 6 via `ext-data-control`), the daemon CAN take +Wayland ownership. There it could serve a clean clipboard directly — image +bytes only, no `text/html` — so the leak never reaches Claude and the shim +guard becomes belt-and-suspenders rather than the primary defense. That is a +source fix: it removes a whole class of read-side races. + +## Why this is deferred, not done + +The maintainer's box is Mutter, where `WAYLAND_WEDGED` latches true and this +code path never executes. Writing it blind and shipping it untested would +violate the project's "do not revert without a regression test" discipline +(see the shim header and ADR 0003). A correctness path that cannot be +exercised on the only available machine is a liability, not a feature. + +## Decision + +**Proposed:** add the data-control ownership path **guarded** behind the +existing `WAYLAND_WEDGED` detection, so it is a no-op on Mutter (today's +behavior, byte-for-byte) and only activates where data-control is actually +advertised. Do NOT enable or claim it works until it is tested on a real +wlroots/KDE session. + +Concrete shape when picked up: + +1. In `wayland.rs`, when `copy_multi` succeeds (compositor speaks + data-control), the daemon becomes the Wayland owner for staged images and + serves `image/png` (+ the real format) only — never `text/html`. +2. Mirror the X11 owner's policy already in `x11.rs`: text targets are not + served for an image selection. +3. Test matrix before flipping Status to Accepted: sway + Hyprland + KDE + Plasma 6, each running the `tests/` paste-correctness suite plus a live + "Copy image from a browser → paste into Claude" check. + +## Consequences + +- Zero change on Mutter (the gate stays wedged there). +- On supported compositors the shim guard becomes redundant defense-in-depth, + and flashpaste stops depending on a PATH shim for clipboard hygiene. +- Until tested, the ROI the improvement list gave #7 (correctness +30% 🟠, + "not on this box") stands: real, but unrealizable on current hardware. diff --git a/docs/adr/README.md b/docs/adr/README.md index c40ae6f..97d406c 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -21,6 +21,8 @@ The four hard-won facts in [`AGENTS.md`](../../AGENTS.md) are essentially mini-A | [0003](0003-kitty-send-text-not-tmux-send-keys.md) | Use `kitty @ send-text` for image-paste, not `tmux send-keys` | Accepted | 2026-05-19 | | [0004](0004-wayland-authoritative-has-image-policy.md) | Wayland-authoritative `has_image` policy | Accepted | 2026-05-19 | | [0005](0005-tmux-unbind-rebind-not-pass-through.md) | Unbind + detached-rebind C-v around send-text, not pass-through | Accepted | 2026-05-19 | +| [0006](0006-reduce-bash-surface-bounded-by-tier1-fallback.md) | Reduce the bash surface, bounded by the Tier-1 fallback guarantee | Proposed | 2026-06-10 | +| [0007](0007-wayland-data-control-on-non-mutter.md) | Own the Wayland clipboard via data-control on non-Mutter compositors | Proposed | 2026-06-10 | ## Writing a new ADR diff --git a/rs/flashpasted/src/ipc.rs b/rs/flashpasted/src/ipc.rs index c837902..a3976f4 100644 --- a/rs/flashpasted/src/ipc.rs +++ b/rs/flashpasted/src/ipc.rs @@ -39,13 +39,16 @@ use crate::state::{now_unix_ms, SharedState, StagedImage, StagedSelection, Stage const MAX_REQUEST_BYTES: u32 = 8 * 1024 * 1024; /// How long a paste with an identical `(pane, content)` signature is /// suppressed after one is accepted. Sized to swallow the dual-handler -/// spread (kitty's `map ctrl+v` and tmux's `bind -n C-v` both fire on one -/// keypress and land ~450–900 ms apart on this box — measured organic -/// double-pastes of 483 ms and 886 ms in the daemon journal). Because the -/// guard now keys on pane+content (not time alone), this wider window never -/// blocks a genuinely different paste — only a verbatim re-fire of the same -/// bytes into the same pane within ~1 s, which is exactly the bug. -const PASTE_DEDUP_WINDOW_MS: u64 = 1000; +/// spread: kitty's `map ctrl+v` launches the paste router as a BACKGROUND +/// process, so its redundant fire can land well after tmux's `bind -n C-v` +/// already pasted — a real measurement on this box showed the two pastes +/// **1.84 s apart** (12:16:31.417 then :33.253, identical 972-byte payload), +/// which sailed past the old 1 s window and produced a visible double. The +/// guard keys on pane+content (see `paste_signature`, which also trims +/// trailing newlines), so this wider window never blocks a genuinely +/// different paste — only a verbatim re-fire of the same bytes into the same +/// pane within ~2.5 s, which is exactly the bug. +const PASTE_DEDUP_WINDOW_MS: u64 = 2500; #[derive(Debug, Deserialize)] #[serde(tag = "op", rename_all = "snake_case")] @@ -188,6 +191,72 @@ async fn handle_paste(state: &Arc, pane: &str, started: Instant) -> // staged, read it now and stage it. The dir scan is ~1ms on a // typical screenshots folder; the file read is ~5-20ms for a 500KB // PNG. Cost is acceptable; correctness is critical. + eager_screenshot_pickup(state, pane).await; + eager_live_image_pickup(state, pane).await; + + // ─── Intent: text or image (most-recent staged wins) ────────────── + // The staged-selection slot is single-valued (set_staged_image + // replaces text and vice versa), so whatever's in the slot is the + // most-recent staged action. If the slot is empty / stale, fall back + // to scraping the live X11 clipboard for fresh text. Otherwise punt + // to bash. Dispatch itself still has a short duplicate-trigger guard + // so one physical paste gesture cannot insert the same payload twice. + let staged = resolve_paste_intent(state, pane).await; + + match staged { + Some(StagedSelection::Text(text)) => { + if !claim_paste_slot(state, paste_signature(pane, &text.bytes)) { + return deduped_response(pane, started); + } + let bytes = text.bytes.len(); + if let Err(e) = paste::dispatch_text_paste(state.clone(), pane.to_string(), text).await + { + error!(error = ?e, pane, "text paste dispatch failed"); + return json!({ "ok": false, "reason": "dispatch-failed", "fallback": "bash" }); + } + json!({ + "ok": true, + "kind": "text", + "bytes": bytes as u64, + "latency_ms": started.elapsed().as_millis() as u64, + }) + } + Some(StagedSelection::Image(img)) if img.is_fresh() => { + if !claim_paste_slot(state, paste_signature(pane, &img.bytes)) { + return deduped_response(pane, started); + } + if let Err(e) = paste::dispatch_image_paste(state.clone(), pane.to_string(), img).await + { + error!(error = ?e, pane, "image paste dispatch failed"); + return json!({ "ok": false, "reason": "dispatch-failed", "fallback": "bash" }); + } + json!({ + "ok": true, + "kind": "image", + "latency_ms": started.elapsed().as_millis() as u64, + }) + } + Some(StagedSelection::Image(_)) => { + warn!(pane, "paste: staged image too old; punting to bash"); + json!({ "ok": false, "reason": "stale-image", "fallback": "bash" }) + } + None => { + info!( + pane, + "paste: nothing staged and clipboard has no text; punting to bash" + ); + json!({ "ok": false, "reason": "no-content", "fallback": "bash" }) + } + } +} + +/// Eager fresh-screenshot pickup. GNOME's screenshot tool keeps the PNG fd +/// open for ~3-5s while rendering its "saved" toast, so inotify's CLOSE_WRITE +/// lags; a paste right after PrtScr would otherwise dispatch the stale staged +/// selection. On every paste we stat the screenshots dir for the freshest +/// image and, if it's newer than what's staged (and no fresh Wayland text +/// overrides it), stage it now. Extracted verbatim from handle_paste. +async fn eager_screenshot_pickup(state: &Arc, pane: &str) { if should_scan_screenshots(state) { if let Some((fresh_path, fresh_mtime)) = state .config @@ -235,7 +304,14 @@ async fn handle_paste(state: &Arc, pane: &str, started: Instant) -> } } } +} +/// Eager live-clipboard image pickup. Bridges "browser Copy Image" +/// (Firefox/Chrome write image bytes with no screenshot file), which the +/// inotify watcher and dir scan both miss. Probe the live clipboard; if it +/// advertises an image whose bytes differ from what's staged, stage it now. +/// Extracted verbatim from handle_paste. +async fn eager_live_image_pickup(state: &Arc, pane: &str) { // ─── Eager live-clipboard image pickup ──────────────────────────── // Bridges "browser Copy Image" — Firefox/Chrome under Wayland write // image bytes to the clipboard with no file under @@ -288,14 +364,15 @@ async fn handle_paste(state: &Arc, pane: &str, started: Instant) -> } } } +} - // ─── Intent: text or image (most-recent staged wins) ────────────── - // The staged-selection slot is single-valued (set_staged_image - // replaces text and vice versa), so whatever's in the slot is the - // most-recent staged action. If the slot is empty / stale, fall back - // to scraping the live X11 clipboard for fresh text. Otherwise punt - // to bash. Dispatch itself still has a short duplicate-trigger guard - // so one physical paste gesture cannot insert the same payload twice. +/// Resolve the paste intent: which selection (text or image) to dispatch. +/// Starts from the staged slot, then applies the live-clipboard override +/// cascade — scrape X11 text when nothing fresh is staged; let Wayland/X11 +/// text override a staged image once it's old enough; let live text override +/// stale staged text. Returns the selection to dispatch. Extracted verbatim +/// from handle_paste so the hot path reads as three named phases. +async fn resolve_paste_intent(state: &Arc, pane: &str) -> Option { let mut staged = state.staged_snapshot().await; if !matches!(&staged, Some(s) if s.is_fresh()) { let external_text = if should_probe_external_text(state) { @@ -420,51 +497,7 @@ async fn handle_paste(state: &Arc, pane: &str, started: Instant) -> } } - match staged { - Some(StagedSelection::Text(text)) => { - if !claim_paste_slot(state, paste_signature(pane, &text.bytes)) { - return deduped_response(pane, started); - } - let bytes = text.bytes.len(); - if let Err(e) = paste::dispatch_text_paste(state.clone(), pane.to_string(), text).await - { - error!(error = ?e, pane, "text paste dispatch failed"); - return json!({ "ok": false, "reason": "dispatch-failed", "fallback": "bash" }); - } - json!({ - "ok": true, - "kind": "text", - "bytes": bytes as u64, - "latency_ms": started.elapsed().as_millis() as u64, - }) - } - Some(StagedSelection::Image(img)) if img.is_fresh() => { - if !claim_paste_slot(state, paste_signature(pane, &img.bytes)) { - return deduped_response(pane, started); - } - if let Err(e) = paste::dispatch_image_paste(state.clone(), pane.to_string(), img).await - { - error!(error = ?e, pane, "image paste dispatch failed"); - return json!({ "ok": false, "reason": "dispatch-failed", "fallback": "bash" }); - } - json!({ - "ok": true, - "kind": "image", - "latency_ms": started.elapsed().as_millis() as u64, - }) - } - Some(StagedSelection::Image(_)) => { - warn!(pane, "paste: staged image too old; punting to bash"); - json!({ "ok": false, "reason": "stale-image", "fallback": "bash" }) - } - None => { - info!( - pane, - "paste: nothing staged and clipboard has no text; punting to bash" - ); - json!({ "ok": false, "reason": "no-content", "fallback": "bash" }) - } - } + staged } fn deduped_response(pane: &str, started: Instant) -> Value { @@ -483,7 +516,20 @@ fn paste_signature(pane: &str, content: &[u8]) -> u64 { use std::hash::{Hash, Hasher}; let mut h = std::collections::hash_map::DefaultHasher::new(); pane.hash(&mut h); - content.hash(&mut h); + // Normalize trailing line endings before hashing. The dual-handler + // delivers the *same* text via two paths whose trailing-newline handling + // differs — one appends a `\n`, so the two fires render as "+N lines" and + // "+N+1 lines" and, without this trim, hash to DIFFERENT signatures and + // dodge the dedup. Trimming trailing `\n`/`\r` makes the near-identical + // second fire collide on the same signature so it is absorbed. + let trimmed = { + let mut end = content.len(); + while end > 0 && matches!(content[end - 1], b'\n' | b'\r') { + end -= 1; + } + &content[..end] + }; + trimmed.hash(&mut h); h.finish() } @@ -670,9 +716,12 @@ async fn read_wayland_text_if_present() -> Option> { .iter() .find(|&&want| types.iter().any(|t| t == want))?; - let (mut pipe, _mime) = - get_contents(ClipboardType::Regular, Seat::Unspecified, MimeType::Specific(mime)) - .ok()?; + let (mut pipe, _mime) = get_contents( + ClipboardType::Regular, + Seat::Unspecified, + MimeType::Specific(mime), + ) + .ok()?; let mut bytes = Vec::new(); pipe.read_to_end(&mut bytes).ok()?; // Same guard as the X11 reader: reject image bytes leaking onto a @@ -691,6 +740,9 @@ async fn read_wayland_text_if_present() -> Option> { .flatten() } +// Only referenced by the unit test below; gated so a release-clippy build +// (which doesn't compile tests) doesn't flag it as dead code. +#[cfg(test)] fn is_text_target(target: &str) -> bool { matches!(target, "UTF8_STRING" | "STRING" | "TEXT") || target.starts_with("text/plain") } @@ -704,7 +756,7 @@ fn is_text_target(target: &str) -> bool { /// dumped verbatim into the prompt — a multi-KB wall of `\x89PNG…IEND` garbage. /// Real clipboard text is valid UTF-8 with no NUL bytes; every image payload /// fails one of those on its leading bytes, so this is both precise and cheap. -fn looks_like_text(bytes: &[u8]) -> bool { +pub(crate) fn looks_like_text(bytes: &[u8]) -> bool { if bytes.is_empty() { return false; } @@ -1003,7 +1055,9 @@ mod tests { #[test] fn looks_like_text_accepts_real_text() { assert!(looks_like_text(b"hello world")); - assert!(looks_like_text("https://lifted.sk/hu/products/gym".as_bytes())); + assert!(looks_like_text( + "https://lifted.sk/hu/products/gym".as_bytes() + )); assert!(looks_like_text("ÁÉÍ Slovak ďáčik · €4.40".as_bytes())); } @@ -1029,12 +1083,14 @@ mod tests { fn duplicate_paste_window_absorbs_only_recent_repeats() { // last_ms == 0 means "never pasted" — not a duplicate. assert!(!is_duplicate_paste(1_000, 0, PASTE_DEDUP_WINDOW_MS)); - // A repeat inside the window (covers the measured 483–886 ms - // dual-handler spread) is absorbed. + // A repeat inside the window is absorbed — including the measured + // 1.84 s kitty-router-vs-tmux-bind spread that the old 1 s window let + // through. assert!(is_duplicate_paste(1_900, 1_000, PASTE_DEDUP_WINDOW_MS)); + assert!(is_duplicate_paste(2_800, 1_000, PASTE_DEDUP_WINDOW_MS)); // 1.8s gap // A repeat at exactly the window edge or beyond is a fresh paste. - assert!(!is_duplicate_paste(2_000, 1_000, PASTE_DEDUP_WINDOW_MS)); - assert!(!is_duplicate_paste(2_400, 1_000, PASTE_DEDUP_WINDOW_MS)); + assert!(!is_duplicate_paste(3_500, 1_000, PASTE_DEDUP_WINDOW_MS)); + assert!(!is_duplicate_paste(3_501, 1_000, PASTE_DEDUP_WINDOW_MS)); } #[test] @@ -1046,6 +1102,14 @@ mod tests { assert_ne!(a, paste_signature("%5", b"hello")); // Different bytes => different signature (don't dedup distinct text). assert_ne!(a, paste_signature("%4", b"world")); + // Trailing-newline variants collide: the dual-handler delivers the + // same text with/without a trailing \n ("+N" vs "+N+1 lines"); both + // must dedup to the same signature. + assert_eq!(a, paste_signature("%4", b"hello\n")); + assert_eq!(a, paste_signature("%4", b"hello\r\n")); + assert_eq!(a, paste_signature("%4", b"hello\n\n")); + // But an interior newline is real content, not trailing — keep distinct. + assert_ne!(a, paste_signature("%4", b"hel\nlo")); } #[test] diff --git a/rs/flashpasted/src/paste.rs b/rs/flashpasted/src/paste.rs index 6cc620f..28e94ca 100644 --- a/rs/flashpasted/src/paste.rs +++ b/rs/flashpasted/src/paste.rs @@ -143,6 +143,25 @@ pub async fn dispatch_text_paste( let bytes_len = text.bytes.len(); let sanitized = sanitize_clipboard_text(&text.bytes); + // ─── Last-gate outcome guard ────────────────────────────────────── + // The daemon must NEVER write image/binary bytes into a pane as text. + // Every staging path *should* have rejected non-text already, but this + // is the single chokepoint all text pastes flow through, so guard here + // too: if the bytes we're about to paste aren't text, refuse and log a + // visible outcome instead of dumping a wall of `\x89PNG…` into the + // prompt. Cheap (a magic/NUL/UTF-8 check on the leading bytes). + if !crate::ipc::looks_like_text(sanitized.as_ref()) { + tracing::warn!( + pane, + kind = "text", + bytes = bytes_len, + outcome = "rejected-nontext", + "REFUSED text paste: payload is image/binary, not text (leak blocked at last gate)" + ); + anyhow::bail!("refusing to paste non-text payload as text"); + } + let html_sanitized = matches!(sanitized, std::borrow::Cow::Owned(_)); + // Single tmux fork: `load-buffer - ; paste-buffer` chained with tmux's // `;` command separator. tmux runs both commands in one process — the // load reads our text from stdin, then the paste writes the buffer into @@ -181,7 +200,15 @@ pub async fn dispatch_text_paste( anyhow::bail!("tmux load-buffer;paste-buffer non-zero: {:?}", status); } - info!(pane, kind = "text", bytes = bytes_len, "PASTED text"); + info!( + pane, + kind = "text", + bytes = bytes_len, + sent_bytes = sanitized.as_ref().len(), + html_sanitized, + outcome = "clean", + "PASTED text" + ); Ok(()) } @@ -253,9 +280,9 @@ fn html_to_plaintext(text: &str) -> String { match name { "br" => out.push('\n'), // Block-level elements: ensure a newline boundary. - "p" | "/p" | "div" | "/div" | "section" | "/section" | "article" - | "/article" | "li" | "tr" | "/tr" | "h1" | "h2" | "h3" | "h4" - | "h5" | "h6" | "/h1" | "/h2" | "/h3" | "/h4" | "/h5" | "/h6" => { + "p" | "/p" | "div" | "/div" | "section" | "/section" | "article" | "/article" + | "li" | "tr" | "/tr" | "h1" | "h2" | "h3" | "h4" | "h5" | "h6" | "/h1" | "/h2" + | "/h3" | "/h4" | "/h5" | "/h6" => { if !out.ends_with('\n') && !out.is_empty() { out.push('\n'); } @@ -339,7 +366,7 @@ fn extract_blob_domain(tag: &str) -> Option { .or_else(|| after_blob.strip_prefix("http://")) .unwrap_or(after_blob); let end = after_scheme - .find(|c: char| c == '/' || c == '"' || c == '\'' || c == ' ') + .find(['/', '"', '\'', ' ']) .unwrap_or(after_scheme.len()); let domain = &after_scheme[..end]; if domain.is_empty() { @@ -425,10 +452,7 @@ mod tests { fn block_elements_add_newlines() { let input = b"
first
second
"; let out = sanitize_clipboard_text(input); - assert_eq!( - std::str::from_utf8(out.as_ref()).unwrap(), - "first\nsecond" - ); + assert_eq!(std::str::from_utf8(out.as_ref()).unwrap(), "first\nsecond"); } #[test] @@ -443,6 +467,22 @@ mod tests { ); } + #[test] + fn text_path_rejects_image_bytes() { + // The last-gate guard: PNG bytes that somehow reached the text path + // must be refused, not pasted as a wall of garbage. + let png = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x01\x00"; + let sanitized = sanitize_clipboard_text(png); + assert!(!crate::ipc::looks_like_text(sanitized.as_ref())); + } + + #[test] + fn text_path_accepts_real_text() { + let t = b"see https://example.com/p?x=1 for details"; + let sanitized = sanitize_clipboard_text(t); + assert!(crate::ipc::looks_like_text(sanitized.as_ref())); + } + #[test] fn facebook_product_page_roundtrip() { // Realistic slice of what Facebook copies for a product listing. diff --git a/tests/keybindings-check.test.sh b/tests/keybindings-check.test.sh new file mode 100755 index 0000000..b43d972 --- /dev/null +++ b/tests/keybindings-check.test.sh @@ -0,0 +1,41 @@ +#!/usr/bin/env bash +# Fixture test for bin/flashpaste-keybindings-check.sh. Drives the checker +# against synthetic kitty/tmux configs via the KITTY_CONF/TMUX_CONF/CANONICAL +# env overrides — no dependence on the user's real dotfiles, so it runs in CI. +set -u + +HERE="$(cd "$(dirname -- "$0")" && pwd)" +CHECK="$HERE/../bin/flashpaste-keybindings-check.sh" +CANON="$HERE/../config/keybindings.canonical" +[ -x "$CHECK" ] || { echo "FATAL: checker not found"; exit 2; } + +T="$(mktemp -d)"; trap 'rm -rf "$T"' EXIT +pass=0; fail=0 +check() { # desc want_rc actual_rc + if [ "$2" = "$3" ]; then echo "ok - $1"; pass=$((pass+1)) + else echo "FAIL - $1 : want rc=$2 got rc=$3"; fail=$((fail+1)); fi +} + +# Consistent fixtures. +printf 'map ctrl+v launch -- /x/kitty-paste-router.sh\n' > "$T/kitty.ok" +printf 'bind -n C-v run-shell -b "flashpaste-trigger %%pane"\n' > "$T/tmux.ok" +CANONICAL="$CANON" KITTY_CONF="$T/kitty.ok" TMUX_CONF="$T/tmux.ok" bash "$CHECK" >/dev/null 2>&1 +check "consistent configs -> rc 0" 0 $? + +# Drift: kitty routes through the OLD inline path, not the router. +printf 'map ctrl+v launch -- sh -c flashpaste-trigger\n' > "$T/kitty.drift" +CANONICAL="$CANON" KITTY_CONF="$T/kitty.drift" TMUX_CONF="$T/tmux.ok" bash "$CHECK" >/dev/null 2>&1 +check "kitty drift (no router) -> rc 1" 1 $? + +# Missing tmux binding entirely. +printf '# no C-v here\n' > "$T/tmux.missing" +CANONICAL="$CANON" KITTY_CONF="$T/kitty.ok" TMUX_CONF="$T/tmux.missing" bash "$CHECK" >/dev/null 2>&1 +check "missing tmux binding -> rc 1" 1 $? + +# The literal '+' in ctrl+v must match literally, not as an ERE quantifier. +printf 'map ctrlv launch -- /x/kitty-paste-router.sh\n' > "$T/kitty.plusbug" +CANONICAL="$CANON" KITTY_CONF="$T/kitty.plusbug" TMUX_CONF="$T/tmux.ok" bash "$CHECK" >/dev/null 2>&1 +check "ctrl+v not matched by 'ctrlv' (literal +) -> rc 1" 1 $? + +echo "--- keybindings-check: PASS=$pass FAIL=$fail" +[ "$fail" = "0" ] diff --git a/tests/kitty-focused-is-tmux.test.sh b/tests/kitty-focused-is-tmux.test.sh new file mode 100755 index 0000000..5ecfcef --- /dev/null +++ b/tests/kitty-focused-is-tmux.test.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# Regression test for bin/kitty-focused-is-tmux.py — the detector that lets +# kitty-paste-router suppress kitty's redundant ctrl+v handler. Exit 0 must +# mean "the focused kitty window is running tmux"; ANY uncertainty must be +# non-zero so the router falls through to the normal paste path. +set -u + +HERE="$(cd "$(dirname -- "$0")" && pwd)" +DET="$HERE/../bin/kitty-focused-is-tmux.py" +command -v python3 >/dev/null 2>&1 || { echo "SKIP: python3 not available"; exit 0; } +[ -f "$DET" ] || { echo "FATAL: detector not found at $DET"; exit 2; } + +pass=0; fail=0 +check() { # desc json want_rc + local got + printf '%s' "$2" | python3 "$DET"; got=$? + if [ "$got" = "$3" ]; then echo "ok - $1"; pass=$((pass+1)) + else echo "FAIL - $1 : want rc=$3 got rc=$got"; fail=$((fail+1)); fi +} + +check "focused window foreground is tmux -> in tmux" \ + '[{"tabs":[{"windows":[{"is_focused":true,"foreground_processes":[{"cmdline":["/usr/bin/tmux","attach"]}]}]}]}]' 0 +check "focused window foreground is bash -> not tmux" \ + '[{"tabs":[{"windows":[{"is_focused":true,"foreground_processes":[{"cmdline":["/usr/bin/bash"]}]}]}]}]' 1 +check "empty input -> not tmux" "" 1 +check "garbage input -> not tmux" "not json" 1 +check "tmux only in a NON-focused window -> not tmux" \ + '[{"tabs":[{"windows":[{"is_focused":true,"foreground_processes":[{"cmdline":["nvim"]}]},{"is_focused":false,"foreground_processes":[{"cmdline":["tmux"]}]}]}]}]' 1 +# Old kitty that ignores --match and omits is_focused: no window is provably +# focused, so even with tmux present we must report "not tmux" (fail-safe: +# the router then lets the paste happen rather than suppressing it). +check "is_focused absent + tmux present -> not tmux (fail-safe)" \ + '[{"tabs":[{"windows":[{"foreground_processes":[{"cmdline":["/usr/bin/tmux","attach"]}]}]}]}]' 1 + +echo "--- kitty-focused-is-tmux: PASS=$pass FAIL=$fail" +[ "$fail" = "0" ] diff --git a/tests/wl-paste-guard.test.sh b/tests/wl-paste-guard.test.sh new file mode 100755 index 0000000..5a6ef7f --- /dev/null +++ b/tests/wl-paste-guard.test.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# Behavioral regression test for bin/wl-paste's image-coexistence guard. +# +# This is the net under the bug class that kept reaching the user: image +# bytes / blob- markup pasted as text. The guard says: when the +# clipboard carries an image, text-oriented reads return NOTHING (so Claude +# attaches the image and inserts no markup). +# +# Headless + non-intrusive: mocks `xclip` via PATH and forces the shim's +# wedge fast-path (private XDG_RUNTIME_DIR + cache file), so it never touches +# the real clipboard and needs no display. Safe to run in CI. +set -u + +HERE="$(cd "$(dirname -- "$0")" && pwd)" +SHIM="$HERE/../bin/wl-paste" +[ -x "$SHIM" ] || { echo "FATAL: shim not found at $SHIM"; exit 2; } + +T="$(mktemp -d)" +trap 'rm -rf "$T"' EXIT +mkdir -p "$T/bin" "$T/run" + +# Fake xclip: TARGETS from $FAKE_TARGETS; content as CONTENT:. +cat > "$T/bin/xclip" <<'EOF' +#!/usr/bin/env bash +tgt="" +while [ $# -gt 0 ]; do case "$1" in + -selection) shift 2;; + -t) tgt="$2"; shift 2;; + *) shift;; +esac; done +if [ "$tgt" = "TARGETS" ]; then printf '%s\n' $FAKE_TARGETS; exit 0; fi +if [ -n "$tgt" ]; then printf 'CONTENT:%s' "$tgt"; else printf 'CONTENT:default'; fi +EOF +chmod +x "$T/bin/xclip" + +export XDG_RUNTIME_DIR="$T/run" +: > "$T/run/clip-wayland-wedge" # force wedged → no real wl-paste call +export PATH="$T/bin:$PATH" + +pass=0; fail=0 +check() { # desc expected actual + if [ "$2" = "$3" ]; then echo "ok - $1"; pass=$((pass+1)) + else echo "FAIL - $1 : expected [$2] got [$3]"; fail=$((fail+1)); fi +} + +out=$(FAKE_TARGETS="TARGETS image/png text/html" bash "$SHIM" -t text/html); rc=$? +check "text/html suppressed when image present (empty)" "" "$out" +check "text/html suppressed -> rc=1" "1" "$rc" + +out=$(FAKE_TARGETS="TARGETS image/png text/html" bash "$SHIM" -t image/png) +check "image/png still returned when image present" "CONTENT:image/png" "$out" + +out=$(FAKE_TARGETS="TARGETS UTF8_STRING text/plain" bash "$SHIM" -t text/plain) +check "text/plain returned when no image" "CONTENT:text/plain" "$out" + +out=$(FAKE_TARGETS="TARGETS UTF8_STRING text/plain" bash "$SHIM") +check "default text returned when no image" "CONTENT:default" "$out" + +out=$(FAKE_TARGETS="TARGETS image/png text/html" bash "$SHIM" -l | tr '\n' ',') +check "list-types not suppressed" "TARGETS,image/png,text/html," "$out" + +out=$(FAKE_TARGETS="TARGETS image/png" bash "$SHIM") +check "default text suppressed when image present" "" "$out" + +echo "--- wl-paste-guard: PASS=$pass FAIL=$fail" +[ "$fail" = "0" ]