diff --git a/THIRD-PARTY.md b/THIRD-PARTY.md index 681c474f..cc2887e9 100644 --- a/THIRD-PARTY.md +++ b/THIRD-PARTY.md @@ -114,7 +114,7 @@ ghostty_vte: git: url: https://github.com/antgrid-ai/dart_terminal.git path: pkgs/vte/ghostty_vte - ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c + ref: 6cd393196ed301afa1d8ada7a996cc345b899b4a ``` `ghostty_vte_flutter` and `portable_pty` are pinned to the same repository and diff --git a/app/lib/util/external_url.dart b/app/lib/util/external_url.dart index ecec7932..82c2f525 100644 --- a/app/lib/util/external_url.dart +++ b/app/lib/util/external_url.dart @@ -1,4 +1,3 @@ -import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:url_launcher/url_launcher.dart'; @@ -79,8 +78,15 @@ Uri? openableTerminalHyperlink(String uri) { /// returns, so a rejection would reach `PlatformDispatcher.onError` as a fatal /// carrying no in-app frames. /// -/// On touch the destination is confirmed first — see -/// [showTerminalHyperlinkSheet] for why that is not merely a nag. +/// The destination is confirmed first unless it was already on screen, and +/// whenever the URI is shaped to be misread — see [showTerminalHyperlinkSheet] +/// for why that is not merely a nag, and [terminalHyperlinkLooksDeceptive] for +/// the shapes. +/// +/// [disclosed] is the caller saying it had this exact URI painted when the +/// activation landed — `TerminalHyperlinkPreview`, in practice. It defaults to +/// false because "we showed it" is a claim only the surface that showed it can +/// make, and a caller that has no readout must not inherit one by omission. /// /// [open] and [confirm] are injectable so tests can assert what would be /// launched instead of handing a URL to the real browser, matching @@ -88,6 +94,7 @@ Uri? openableTerminalHyperlink(String uri) { Future openTerminalHyperlink( BuildContext context, String uri, { + bool disclosed = false, Future Function(BuildContext, String) open = openExternalUrl, Future Function(BuildContext, Uri) confirm = showTerminalHyperlinkSheet, }) async { @@ -105,7 +112,7 @@ Future openTerminalHyperlink( } return; } - if (_browserRevealsDestination) { + if (disclosed && !terminalHyperlinkLooksDeceptive(target)) { await open(context, target.toString()); return; } @@ -130,18 +137,35 @@ Future openTerminalHyperlink( } } -/// Whether opening the link lands somewhere that reads the destination back. +/// Whether [target] is shaped like a link trying to pass as another one. +/// +/// Judged from the URI alone, which is all a terminal hyperlink hands over. +/// Three shapes qualify, and each is a host claiming to be a host it is not: /// -/// Desktop hands the URI to a browser whose address bar does, which is the same -/// disclosure every terminal leans on. The terminal's own hover affordance is -/// NOT it and never was: the view only swaps the cursor to a pointer, and -/// nothing there or here paints the URI. +/// * A userinfo prefix — `https://github.com@evil.example/` resolves to +/// `evil.example` while reading as GitHub. Dart parks the impostor in +/// [Uri.userInfo], where nothing in a URL bar's first glance looks at it. +/// * A punycoded label — `xn--pple-43d.com` renders as `apple.com` +/// with a Cyrillic first letter (U+0430). +/// * A percent-encoded host — Dart does NOT punycode a raw unicode host, it +/// percent-encodes it, so the same lie arrives spelled the other way and a +/// check for `xn--` alone misses half of it. /// -/// Mobile has no such guarantee. An `https:` host holding a verified App Link -/// opens that app instead of any browser (see [openableTerminalHyperlink]), so -/// the destination can be acted on without ever being shown. That is why the -/// mobile path asks first, and why the answer to a spoofed target on desktop is -/// a hover preview rather than the same sheet on both. -bool get _browserRevealsDestination => - defaultTargetPlatform != TargetPlatform.android && - defaultTargetPlatform != TargetPlatform.iOS; +/// Deliberately not a blocklist of hosts, and deliberately silent about the +/// lookalike it cannot see: `https://github.com.evil.example/` is an honest +/// subdomain of an honest domain, and only the link's visible TEXT contradicts +/// it — text that never reaches this app. A false negative here costs the user +/// the extra confirmation, not the disclosure: the sheet names the host either +/// way, and a link that was never on screen is confirmed regardless of shape. +bool terminalHyperlinkLooksDeceptive(Uri target) { + if (target.userInfo.isNotEmpty) { + return true; + } + final host = target.host; + if (host.contains('%')) { + return true; + } + // `Uri` lower-cases the host as it parses, but the loop is what a reader + // checks, not the parser's contract two files away. + return host.split('.').any((label) => label.toLowerCase().startsWith('xn--')); +} diff --git a/app/lib/widgets/terminal_hyperlink_preview.dart b/app/lib/widgets/terminal_hyperlink_preview.dart new file mode 100644 index 00000000..97e0554a --- /dev/null +++ b/app/lib/widgets/terminal_hyperlink_preview.dart @@ -0,0 +1,350 @@ +import 'dart:convert'; +import 'dart:math' as math; + +import 'package:flutter/widgets.dart'; + +import '../design/ab_colors.dart'; +import '../design/ab_icons.dart'; +import '../design/ab_tokens.dart'; +import '../design/widgets/ab_icon.dart'; + +/// Gap between the pointer and the card, in logical pixels. +/// +/// Roughly a line of terminal text — enough that the card clears the row it +/// describes rather than covering the link the user is reading. +const double _gap = 18; + +/// Distance kept from the panel's edges when the anchor is near one. +const double _margin = AbTokens.space8; + +/// Longest run of prefix, host or trailing text laid out. +/// +/// `maxLines: 1` and [TextOverflow.ellipsis] bound what is PAINTED, not what is +/// measured — the engine shapes the whole run before eliding it. A payload is +/// unbounded program-chosen text, so without this a single hover shapes all of +/// it on the UI thread. Same trap `_elide` guards in `external_url.dart`, which +/// documents it for the same data. +const int _maxPartChars = 128; + +/// Characters that can reorder or hide the rest of the readout. +/// +/// Bidi overrides and isolates re-order glyphs ACROSS span boundaries, so a +/// payload carrying U+202E paints a reading order that is not the URI's — on +/// the one surface whose whole job is letting exact characters be compared. +/// [terminalHyperlinkLooksDeceptive] cannot backstop it: the host itself is +/// clean, so such a link takes the desktop no-sheet path. +bool _isDisplayUnsafe(int rune) => + rune < 0x20 || + rune == 0x7F || + rune == 0x061C || + (rune >= 0x200E && rune <= 0x200F) || + (rune >= 0x202A && rune <= 0x202E) || + (rune >= 0x2066 && rune <= 0x2069); + +/// Renders [part] with the invisible characters spelled out. +/// +/// Percent-encoded rather than dropped, because that is what `Uri.toString()` +/// hands the launcher — so the card and the thing it describes agree on these +/// characters instead of diverging on exactly them. +String _display(String part) { + if (!part.runes.any(_isDisplayUnsafe)) return part; + final out = StringBuffer(); + for (final rune in part.runes) { + if (!_isDisplayUnsafe(rune)) { + out.writeCharCode(rune); + continue; + } + for (final byte in utf8.encode(String.fromCharCode(rune))) { + out.write('%${byte.toRadixString(16).toUpperCase().padLeft(2, '0')}'); + } + } + return out.toString(); +} + +/// Replaces the password half of a userinfo with a fixed mask. +/// +/// A hover is not consent to display a secret: `https://x:TOKEN@host/` is a +/// shape real tooling prints, and before this readout existed the view painted +/// only an underline. The NAME half survives — it is what an impostor prefix +/// uses to read as a familiar host, which is the thing this card exists to +/// expose. +String _maskPassword(String prefix) { + final at = prefix.lastIndexOf('@'); + if (at < 0) return prefix; + final schemeEnd = prefix.indexOf('://'); + if (schemeEnd < 0) return prefix; + final colon = prefix.indexOf(':', schemeEnd + 3); + if (colon < 0 || colon > at) return prefix; + return '${prefix.substring(0, colon + 1)}•••${prefix.substring(at)}'; +} + +/// Caps [part] to [_maxPartChars], keeping the end when [keepTail] is set. +/// +/// The prefix keeps its TAIL: what matters there is the `@` and the characters +/// immediately before the host, not the start of a padded userinfo. +String _cap(String part, {bool keepTail = false}) { + if (part.length <= _maxPartChars) return part; + // Back off a stranded surrogate half: `substring` cuts UTF-16 code units, and + // half a pair renders as a replacement glyph on exactly the characters a + // reader is trying to identify. + if (keepTail) { + var start = part.length - _maxPartChars; + if (_isLowSurrogate(part.codeUnitAt(start))) start += 1; + return '…${part.substring(start)}'; + } + var end = _maxPartChars; + if (_isHighSurrogate(part.codeUnitAt(end - 1))) end -= 1; + return '${part.substring(0, end)}…'; +} + +bool _isHighSurrogate(int unit) => unit >= 0xD800 && unit <= 0xDBFF; + +bool _isLowSurrogate(int unit) => unit >= 0xDC00 && unit <= 0xDFFF; + +/// The destination readout for the terminal link under the pointer. +/// +/// OSC 8 lets a link's visible text say one thing and its target say another, +/// and the terminal view paints only an underline — so without this the first +/// place the real URI appears is the browser it has already been opened in. +/// This is the desktop half of that disclosure; touch has no hover and gets the +/// confirm sheet instead (`showTerminalHyperlinkSheet`). +/// +/// Shaped as a twin of `TerminalUploadStrip` so the terminal's floating +/// overlays read as one family, and [IgnorePointer] for the same reason: it +/// floats over live terminal text, and the pointer it is describing is by +/// definition somewhere underneath it. +class TerminalHyperlinkPreview extends StatelessWidget { + const TerminalHyperlinkPreview({ + super.key, + required this.uri, + required this.anchor, + }); + + /// The raw OSC 8 payload, as the program wrote it apart from a masked + /// password and spelled-out control characters. + /// + /// Not a parsed [Uri]: this reports what the link SAYS, including a payload + /// no launcher would accept, and normalizing it would show the user a string + /// the terminal does not contain. + final String uri; + + /// Where the pointer was when this URI became the hovered one, in the + /// enclosing stack's coordinates. + final Offset anchor; + + /// The card itself, so a test can assert where the delegate put it. + static const Key cardKey = ValueKey('terminal.hyperlink.preview.card'); + + /// Widest the card may grow before its prefix and tail are elided. + /// + /// A URI is unbounded program-chosen text, so something has to stop it: the + /// cap is what keeps a long one from spanning the terminal it overlays. + static const double _maxWidth = 420; + + /// Widest the HOST span may grow. + /// + /// Held well under [_maxWidth] so the host is laid out before the elidable + /// spans get what is left — see [_HyperlinkText] for why it must never be the + /// part that gets cut. + static const double _maxHostWidth = 280; + + @override + Widget build(BuildContext context) { + final p = context.antgrid; + return IgnorePointer( + child: CustomSingleChildLayout( + delegate: _AnchoredNearPointer(anchor: anchor), + child: Container( + key: cardKey, + constraints: const BoxConstraints(maxWidth: _maxWidth), + padding: const EdgeInsets.symmetric( + horizontal: AbTokens.space10, + vertical: AbTokens.space6, + ), + decoration: BoxDecoration( + color: p.bgElevated, + borderRadius: AbTokens.borderRadius5, + border: Border.all(color: p.accent.withValues(alpha: 0.3)), + ), + child: Row( + mainAxisSize: MainAxisSize.min, + children: [ + AbIcon(AbIcons.link, size: 12, color: p.accent), + const SizedBox(width: AbTokens.space6), + Flexible(child: _HyperlinkText(uri: uri)), + ], + ), + ), + ), + ); + } +} + +/// The URI itself, split so the HOST is both the part that reads brightest and +/// the part that survives when the card runs out of room. +/// +/// The host is the only span that decides where a click actually lands, and it +/// is the span a spoofed URI works hardest to bury: a `github.com@` userinfo +/// prefix puts a familiar name where a glance stops reading, and the real +/// destination after it. +/// +/// Three separate [Text]s rather than one [Text.rich], because a single rich +/// run elides its TAIL — which behind a padded userinfo +/// (`https://github.com.login.oauth.…@evil.example/`) is the host itself, so +/// the readout would render as a clean GitHub URL and confirm the lie. Here the +/// host is the only non-flex child, so the elidable spans surrender their width +/// first and the host is cut last. +class _HyperlinkText extends StatelessWidget { + const _HyperlinkText({required this.uri}); + + final String uri; + + @override + Widget build(BuildContext context) { + final p = context.antgrid; + // Mono: this is a URL, and the whole point is that its exact characters can + // be compared. + final base = AbTokens.monoStyle( + fontSize: AbTokens.fontXs, + color: p.textSecondary, + ); + final range = _hostRange(uri); + // Nothing to emphasize, and a guess about which characters are the host + // would be exactly the wrong thing to be confident about. + if (range == null) { + return Text( + _display(_cap(uri)), + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: base, + ); + } + final (start, end) = range; + return Row( + mainAxisSize: MainAxisSize.min, + children: [ + Flexible( + child: Text( + _display( + _cap(_maskPassword(uri.substring(0, start)), keepTail: true), + ), + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: base.copyWith(color: p.textMuted), + ), + ), + ConstrainedBox( + constraints: const BoxConstraints( + maxWidth: TerminalHyperlinkPreview._maxHostWidth, + ), + child: Text( + _display(_cap(uri.substring(start, end))), + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: base.copyWith(color: p.textPrimary), + ), + ), + Flexible( + child: Text( + _display(_cap(uri.substring(end))), + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: base, + ), + ), + ], + ); + } +} + +/// The `[start, end)` span of the authority's host inside [uri], or null when +/// the payload names no host. +/// +/// Located POSITIONALLY in the original text, never by searching it for the +/// parsed host. Two reasons, and each is the difference between disclosing the +/// lie and repeating it: +/// +/// * `indexOf(host)` finds the FIRST occurrence, which an impostor puts in the +/// userinfo — `https://github.com.evil.tld@evil.tld/` would paint the +/// userinfo copy bright and leave the real authority reading as body text. +/// * `Uri.host` percent-encodes a raw unicode host, so it does not occur in +/// the original text at all and the search silently finds nothing, dropping +/// the emphasis on precisely the homoglyph it exists to expose. +/// +/// Rebuilding the string from the parse is not an option either: `Uri` +/// lower-cases and percent-encodes as it goes, and this exists to let the user +/// compare the characters the terminal actually printed. +(int, int)? _hostRange(String uri) { + final schemeEnd = uri.indexOf('://'); + if (schemeEnd < 0) return null; + final authorityStart = schemeEnd + 3; + var authorityEnd = uri.length; + for (var i = authorityStart; i < uri.length; i++) { + final c = uri[i]; + if (c == '/' || c == '?' || c == '#') { + authorityEnd = i; + break; + } + } + // LAST `@`: a userinfo may contain one, and the host is what follows the + // final separator — the same rule the parser applies. + final at = uri.substring(authorityStart, authorityEnd).lastIndexOf('@'); + final start = authorityStart + (at < 0 ? 0 : at + 1); + var end = authorityEnd; + final rest = uri.substring(start, authorityEnd); + if (rest.startsWith('[')) { + // IPv6 literal: its colons belong to the host, and only one after `]` + // starts a port. + final close = rest.indexOf(']'); + if (close >= 0) end = start + close + 1; + } else { + final colon = rest.indexOf(':'); + if (colon >= 0) end = start + colon; + } + return end > start ? (start, end) : null; +} + +/// Parks the card just below the pointer, flipping above it near the bottom +/// edge and sliding inward near the sides. +/// +/// Below by default because the terminal's live prompt is at the BOTTOM of the +/// panel: a fixed readout down there — the browser status-bar placement — would +/// sit on the one line the user is typing into. +class _AnchoredNearPointer extends SingleChildLayoutDelegate { + const _AnchoredNearPointer({required this.anchor}); + + final Offset anchor; + + @override + BoxConstraints getConstraintsForChild(BoxConstraints constraints) => + constraints.loosen().copyWith( + maxWidth: math.max(0, constraints.maxWidth - _margin * 2), + ); + + @override + Offset getPositionForChild(Size size, Size childSize) { + // `math.max` on the upper bound rather than a branch: `clamp` throws on an + // inverted range, so a panel narrower than the card has to resolve to the + // margin instead of to an assertion. Hoisted into `double` locals because + // `clamp` takes `num`: an inline `math.max` infers that from the parameter + // and hands back a `num` no `Offset` will take. + final double maxDx = math.max( + _margin, + size.width - childSize.width - _margin, + ); + final double maxDy = math.max( + _margin, + size.height - childSize.height - _margin, + ); + final dx = anchor.dx.clamp(_margin, maxDx); + var dy = anchor.dy + _gap; + if (dy + childSize.height > size.height - _margin) { + dy = anchor.dy - _gap - childSize.height; + } + return Offset(dx, dy.clamp(_margin, maxDy)); + } + + @override + bool shouldRelayout(_AnchoredNearPointer oldDelegate) => + oldDelegate.anchor != anchor; +} diff --git a/app/lib/widgets/terminal_hyperlink_sheet.dart b/app/lib/widgets/terminal_hyperlink_sheet.dart index ebe75339..5c607d89 100644 --- a/app/lib/widgets/terminal_hyperlink_sheet.dart +++ b/app/lib/widgets/terminal_hyperlink_sheet.dart @@ -10,9 +10,11 @@ import '../design/widgets/ab_dialog.dart'; /// /// OSC 8 lets a link's visible text disagree with its destination, so the text /// a user taps is not evidence of anything: `https://github.com@evil.example/` -/// reads as GitHub and resolves to `evil.example`. Desktop gets its disclosure -/// from the browser's address bar; a tap here may never reach a browser at all, -/// because a verified App Link opens its own app instead. +/// reads as GitHub and resolves to `evil.example`. This is what names the real +/// host wherever nothing else did — a pointer gets `TerminalHyperlinkPreview` +/// instead, and a finger gets no readout at all. Nor is a browser a backstop: +/// an activation may never reach one, because a verified App Link opens its own +/// app. /// /// Returns false when dismissed, so a stray tap outside the sheet cancels. Future showTerminalHyperlinkSheet( diff --git a/app/lib/widgets/terminal_upload_strip.dart b/app/lib/widgets/terminal_upload_strip.dart index 5d220668..39adcff0 100644 --- a/app/lib/widgets/terminal_upload_strip.dart +++ b/app/lib/widgets/terminal_upload_strip.dart @@ -8,7 +8,7 @@ import '../design/widgets/ab_progress_rule.dart'; import 'terminal_attachment_uploader.dart'; /// The in-flight attach readout, shaped as a twin of `SendToAgentButton` so the -/// terminal's two floating overlays read as one family. +/// terminal's floating overlays read as one family. /// /// [IgnorePointer] by construction: it floats over live terminal text, and a /// tap target there would compete with drag-selection for the same pixels. diff --git a/app/lib/widgets/terminal_view_wrapper.dart b/app/lib/widgets/terminal_view_wrapper.dart index 52f8d96a..6e30c1cb 100644 --- a/app/lib/widgets/terminal_view_wrapper.dart +++ b/app/lib/widgets/terminal_view_wrapper.dart @@ -26,6 +26,7 @@ import 'send_to_agent_button.dart'; import 'send_to_agent_comment.dart'; import 'terminal_attachment_uploader.dart'; import 'terminal_drop_target.dart'; +import 'terminal_hyperlink_preview.dart'; import 'terminal_quick_actions_bar.dart'; import 'terminal_upload_button.dart'; import 'terminal_upload_strip.dart'; @@ -68,6 +69,31 @@ class _TerminalViewWrapperState extends ConsumerState { /// overlay button only when the user has a non-empty selection. String? _selectedText; + /// The OSC 8 link under the pointer and where to park its readout, or null + /// when the pointer is over no link. + /// + /// Held here rather than read from the view because the view paints only an + /// underline: the destination itself is disclosed by + /// [TerminalHyperlinkPreview] or by nothing at all. A notifier rather than + /// `setState` because this is the terminal's primary surface and a hover + /// crossing a wall of links would otherwise relayout the whole panel — three + /// nested `LayoutBuilder`s and a fresh `GhosttyTerminalView` — per link, to + /// toggle one floating card. Same shape the sibling upload strip already uses. + final ValueNotifier<({String uri, Offset at})?> _hoveredLink = ValueNotifier( + null, + ); + + /// A hovered URI still waiting for the pointer position that produced it. + /// + /// The view reports the URI from a `MouseRegion` BELOW this widget's own + /// hover handler, and Flutter walks a hit-test path child-first — so at the + /// instant the URI arrives, the position for that same event has not reached + /// us yet, and on the first hover into the panel there is no earlier position + /// to fall back on. Parking the URI here until the enclosing handler supplies + /// the matching position is what keeps the card on the link it describes + /// rather than one hover behind it. + String? _pendingHoverUri; + /// Wraps the terminal subtree so we can detect when the user's primary /// focus is inside this view. Required for the paste interceptor, which /// must scope its effect to the focused terminal — `HardwareKeyboard` @@ -204,6 +230,7 @@ class _TerminalViewWrapperState extends ConsumerState { _focusScope.removeListener(_onFocusChange); _focusScope.dispose(); _uploader.dispose(); + _hoveredLink.dispose(); super.dispose(); } @@ -594,7 +621,18 @@ class _TerminalViewWrapperState extends ConsumerState { // uses the platform-default launch mode and reports nothing when it // fails. Route through the app's helper so a link opens externally and a // failure is visible, and so terminal-authored URIs are scheme-checked. - onOpenHyperlink: (uri) => openTerminalHyperlink(context, uri), + // `disclosed` is this widget answering for its own readout, not a guess + // from the platform: the card is up for THIS uri, so the destination was + // on screen when the activation landed. A desktop touchscreen, a Shift + // chord and a link that scrolled out from under a resting pointer all + // reach here with nothing shown, and all of them get the sheet — which a + // `defaultTargetPlatform` test silently exempted the first of. + onOpenHyperlink: (uri) => openTerminalHyperlink( + context, + uri, + disclosed: _hoveredLink.value?.uri == uri, + ), + onHyperlinkHover: _onHyperlinkHover, showHeader: false, showFocusRing: false, // Thin terminal-native scrollbar — thumb tracks @@ -652,47 +690,66 @@ class _TerminalViewWrapperState extends ConsumerState { node: _focusScope, child: Stack( children: [ - LayoutBuilder( - builder: (context, constraints) { - _maybeSendResize( - tab.terminalId, - constraints, - amDriver, - tab.driverClientId, - ); - - // Driver (or metrics not yet known) → fill the viewport. Pin - // the grid to the last settled width via `_TerminalGridFreeze` - // so transient resizes (e.g. dragging the agent/workspace - // divider) don't spam Ghostty grid resizes — - // `ghostty_vte_flutter` doesn't reflow soft-wrapped lines, and - // Ink-style TUI redraws (Claude Code) leak stale fragments - // when the grid changes underneath them. - final charWidth = _charWidth; - if (amDriver || charWidth == null || charWidth <= 0) { - return _TerminalGridFreeze( - onSettled: _onRenderSizeSettled, + // Wraps the terminal, not the whole subtree: this is the + // Stack's only non-positioned child, so it shares the Stack's + // origin — which is the space the preview's anchor is read in. + // Non-opaque so it joins the hit path above the view's own + // MouseRegion without taking anything from it. + MouseRegion( + opaque: false, + onHover: _onHoverPosition, + // The view's own exit report cannot be relied on alone: a + // MouseRegion unmounted while hovered never fires onExit, and + // the branches below swap widget types under the pointer + // (grid-freeze vs letterbox vs h-scroll), so a card can + // outlive the terminal that reported it. This is the one + // handler that survives those swaps. + onExit: (_) { + _pendingHoverUri = null; + _hoveredLink.value = null; + }, + child: LayoutBuilder( + builder: (context, constraints) { + _maybeSendResize( + tab.terminalId, + constraints, + amDriver, + tab.driverClientId, + ); + + // Driver (or metrics not yet known) → fill the viewport. Pin + // the grid to the last settled width via `_TerminalGridFreeze` + // so transient resizes (e.g. dragging the agent/workspace + // divider) don't spam Ghostty grid resizes — + // `ghostty_vte_flutter` doesn't reflow soft-wrapped lines, and + // Ink-style TUI redraws (Claude Code) leak stale fragments + // when the grid changes underneath them. + final charWidth = _charWidth; + if (amDriver || charWidth == null || charWidth <= 0) { + return _TerminalGridFreeze( + onSettled: _onRenderSizeSettled, + child: terminalView, + ); + } + + // Non-driver → size the grid to the driver's authoritative + // cols so wrapping matches exactly. Letterbox (center) when it + // fits; horizontal-scroll when the driver is wider than this + // viewport. No `_TerminalGridFreeze` here — a viewer must + // track the authoritative width, not pin a local one. + final authWidth = tab.cols * charWidth + _hPad; + final grid = SizedBox( + width: authWidth, child: terminalView, ); - } - - // Non-driver → size the grid to the driver's authoritative - // cols so wrapping matches exactly. Letterbox (center) when it - // fits; horizontal-scroll when the driver is wider than this - // viewport. No `_TerminalGridFreeze` here — a viewer must - // track the authoritative width, not pin a local one. - final authWidth = tab.cols * charWidth + _hPad; - final grid = SizedBox( - width: authWidth, - child: terminalView, - ); - return authWidth <= constraints.maxWidth - ? Align(alignment: Alignment.center, child: grid) - : SingleChildScrollView( - scrollDirection: Axis.horizontal, - child: grid, - ); - }, + return authWidth <= constraints.maxWidth + ? Align(alignment: Alignment.center, child: grid) + : SingleChildScrollView( + scrollDirection: Axis.horizontal, + child: grid, + ); + }, + ), ), // Top-LEFT, deliberately: `SendToAgentButton` owns top-right and // both can be live at once, while the bottom edge is where the @@ -729,6 +786,17 @@ class _TerminalViewWrapperState extends ConsumerState { ), if (showSendButton) SendToAgentButton(onPressed: _onSendToAgent), + Positioned.fill( + child: ValueListenableBuilder<({String uri, Offset at})?>( + valueListenable: _hoveredLink, + builder: (context, link, _) => link == null + ? const SizedBox.shrink() + : TerminalHyperlinkPreview( + uri: link.uri, + anchor: link.at, + ), + ), + ), ], ), ), @@ -737,6 +805,45 @@ class _TerminalViewWrapperState extends ConsumerState { ); } + /// Shows or hides the destination readout as the pointer enters and leaves + /// links. + /// + /// The view fires this only on a real change, so there is no same-value + /// rebuild to guard against here. A new link is only PENDING until + /// [_onHoverPosition] supplies the position it was hovered at — see + /// [_pendingHoverUri] for why the position cannot be read here. + void _onHyperlinkHover(String? uri) { + if (!mounted) return; + if (uri == null) { + _pendingHoverUri = null; + _hoveredLink.value = null; + return; + } + _pendingHoverUri = uri; + // Output can scroll a DIFFERENT link under a STATIONARY pointer, and no + // hover event follows to carry it — so repoint the card at once against + // the anchor it already has, rather than let it go on naming a + // destination the click would not open. The pending URI still re-anchors + // it on the next real move. + final shown = _hoveredLink.value; + if (shown != null && shown.uri != uri) { + _hoveredLink.value = (uri: uri, at: shown.at); + } + } + + /// Anchors a pending link to the pointer position of the event that produced + /// it. + /// + /// Sampled once, at the change, rather than tracked live: a card sliding + /// under the cursor it belongs to is unreadable, and re-laying it out every + /// hover frame would be work for nothing. + void _onHoverPosition(PointerHoverEvent event) { + final pending = _pendingHoverUri; + if (pending == null) return; + _pendingHoverUri = null; + _hoveredLink.value = (uri: pending, at: event.localPosition); + } + /// Sends a `terminal:resize` derived from the local viewport + cell metrics /// when this view just claimed focus, or when it is the driver and its /// native grid changed. The send is the SOLE resize source (the engine's diff --git a/app/pubspec.lock b/app/pubspec.lock index 915a3899..d9114c67 100644 --- a/app/pubspec.lock +++ b/app/pubspec.lock @@ -682,8 +682,8 @@ packages: dependency: "direct overridden" description: path: "pkgs/vte/ghostty_vte" - ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c - resolved-ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c + ref: "6cd393196ed301afa1d8ada7a996cc345b899b4a" + resolved-ref: "6cd393196ed301afa1d8ada7a996cc345b899b4a" url: "https://github.com/antgrid-ai/dart_terminal.git" source: git version: "0.1.4+antgrid.1" @@ -691,8 +691,8 @@ packages: dependency: "direct main" description: path: "pkgs/vte/ghostty_vte_flutter" - ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c - resolved-ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c + ref: "6cd393196ed301afa1d8ada7a996cc345b899b4a" + resolved-ref: "6cd393196ed301afa1d8ada7a996cc345b899b4a" url: "https://github.com/antgrid-ai/dart_terminal.git" source: git version: "0.1.4+antgrid.1" @@ -1204,8 +1204,8 @@ packages: dependency: "direct overridden" description: path: "pkgs/pty/portable_pty" - ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c - resolved-ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c + ref: "6cd393196ed301afa1d8ada7a996cc345b899b4a" + resolved-ref: "6cd393196ed301afa1d8ada7a996cc345b899b4a" url: "https://github.com/antgrid-ai/dart_terminal.git" source: git version: "0.0.6+antgrid.2" diff --git a/app/pubspec.yaml b/app/pubspec.yaml index 84515562..64c921cf 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -191,17 +191,17 @@ dependency_overrides: git: url: https://github.com/antgrid-ai/dart_terminal.git path: pkgs/vte/ghostty_vte_flutter - ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c + ref: 6cd393196ed301afa1d8ada7a996cc345b899b4a ghostty_vte: git: url: https://github.com/antgrid-ai/dart_terminal.git path: pkgs/vte/ghostty_vte - ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c + ref: 6cd393196ed301afa1d8ada7a996cc345b899b4a portable_pty: git: url: https://github.com/antgrid-ai/dart_terminal.git path: pkgs/pty/portable_pty - ref: f8e2e8201c7bef116ec04baa4cfc039acb53e29c + ref: 6cd393196ed301afa1d8ada7a996cc345b899b4a # Microsoft Store packaging (MSIX). The Store signs the package itself, so no # code-signing cert is used on this path (unlike the Inno Setup installer in diff --git a/app/test/terminal_hyperlink_preview_test.dart b/app/test/terminal_hyperlink_preview_test.dart new file mode 100644 index 00000000..c0d33859 --- /dev/null +++ b/app/test/terminal_hyperlink_preview_test.dart @@ -0,0 +1,235 @@ +import 'package:antgrid/design/theme_presets.dart'; +import 'package:antgrid/widgets/terminal_hyperlink_preview.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart'; +import 'package:flutter_test/flutter_test.dart'; + +const Size _panel = Size(600, 400); + +Future _pump(WidgetTester tester, String uri, Offset anchor) { + return tester.pumpWidget( + MaterialApp( + // The shipped palette, not `context.antgrid`'s tests-only fallback: + // without the extension both the widget and the assertions resolve to + // `_zincFallback`, so a colour assertion compares the fallback against + // itself and stays green however the emphasis is wired. + theme: ThemeData.dark().copyWith( + extensions: >[kDefaultPalette], + ), + home: Scaffold( + body: Center( + child: SizedBox( + width: _panel.width, + height: _panel.height, + child: Stack( + children: [ + Positioned.fill( + child: TerminalHyperlinkPreview(uri: uri, anchor: anchor), + ), + ], + ), + ), + ), + ), + ), + ); +} + +/// The card's rect relative to the panel it floats in. +/// +/// The widget itself is `Positioned.fill`, so its own rect IS the panel. +Rect _card(WidgetTester tester) { + final panel = tester.getTopLeft(find.byType(TerminalHyperlinkPreview)); + return tester + .getRect(find.byKey(TerminalHyperlinkPreview.cardKey)) + .shift(-panel); +} + +/// Every painted run in the readout, paired with the colour it is drawn in. +/// +/// The URI is three sibling [Text]s, not one rich run — see the widget for why +/// the host has to be laid out on its own. +List<(String, Color?)> _spans(WidgetTester tester) { + final riches = tester.widgetList( + find.descendant( + of: find.byType(TerminalHyperlinkPreview), + matching: find.byType(RichText), + ), + ); + final out = <(String, Color?)>[]; + for (final rich in riches) { + rich.text.visitChildren((span) { + if (span is TextSpan && (span.text ?? '').isNotEmpty) { + out.add((span.text!, span.style?.color)); + } + return true; + }); + } + return out; +} + +String _text(WidgetTester tester) => _spans(tester).map((s) => s.$1).join(); + +void main() { + group('TerminalHyperlinkPreview', () { + const link = 'https://github.com/antgrid-ai/antgrid/pull/13'; + + testWidgets('shows the payload verbatim', (tester) async { + await _pump(tester, link, const Offset(100, 100)); + + expect(_text(tester), link); + }); + + // A readout that normalized would show the user a string the terminal does + // not contain, on exactly the characters they are here to compare. + testWidgets('does not normalize the case it was handed', (tester) async { + await _pump(tester, 'https://GitHub.COM/OK', const Offset(100, 100)); + + expect(_text(tester), 'https://GitHub.COM/OK'); + }); + + // The point of the split: an impostor parked in userinfo reads dim and the + // host that actually resolves reads bright, which is the reverse of how the + // URI is written. + testWidgets('paints the host brightest, userinfo prefix dimmest', ( + tester, + ) async { + await _pump( + tester, + 'https://github.com@evil.example/antgrid/pull/13', + const Offset(100, 100), + ); + + final spans = _spans(tester); + expect(spans.first, ('https://github.com@', kDefaultPalette.textMuted)); + expect(spans[1], ('evil.example', kDefaultPalette.textPrimary)); + expect(spans.last.$1, '/antgrid/pull/13'); + }); + + // Searching the raw text for the parsed host finds the FIRST occurrence, + // which an attacker puts in the userinfo -- lighting up the impostor and + // leaving the real destination reading as body text. + testWidgets('a userinfo wearing the host does not steal the highlight', ( + tester, + ) async { + await _pump(tester, 'https://b.com.evil@b.com/x', const Offset(100, 100)); + + final spans = _spans(tester); + expect(spans.first, ('https://b.com.evil@', kDefaultPalette.textMuted)); + expect(spans[1], ('b.com', kDefaultPalette.textPrimary)); + expect(spans.last.$1, '/x'); + }); + + // `Uri.host` percent-encodes a raw unicode host, so it does not occur in + // the payload at all -- a search for it finds nothing and silently drops + // the emphasis on the one shape it exists to expose. + testWidgets('emphasizes a raw unicode host too', (tester) async { + await _pump(tester, 'https://\u0430pple.com/login', const Offset(10, 10)); + + final spans = _spans(tester); + expect(spans[1], ('\u0430pple.com', kDefaultPalette.textPrimary)); + }); + + // A single rich run elides its TAIL, and behind a padded userinfo the tail + // IS the host -- so the card would render as a clean GitHub URL and confirm + // the lie it exists to expose. + testWidgets('keeps the host when a padded userinfo overflows the card', ( + tester, + ) async { + const host = 'evil.example'; + await _pump( + tester, + 'https://github.com.${'padding.' * 40}@$host/pull/13', + const Offset(10, 10), + ); + + final paragraph = tester.renderObject(find.text(host)); + expect(paragraph.didExceedMaxLines, isFalse); + expect(paragraph.size.width, greaterThan(0)); + // The elidable prefix is what gave up its width instead. + expect(_card(tester).width, lessThanOrEqualTo(_panel.width)); + }); + + // Bidi overrides re-order glyphs across span boundaries, so a payload + // carrying one paints a reading order that is not the URI's -- and the + // deception check cannot see it, because the host itself is clean. + testWidgets('spells out a bidi control rather than obeying it', ( + tester, + ) async { + await _pump( + tester, + 'https://evil.example/\u202Egro.dirgtna', + const Offset(10, 10), + ); + + final text = _text(tester); + expect(text, contains('%E2%80%AE')); + expect(text, isNot(contains('\u202E'))); + }); + + // A hover is not consent to display a secret, and before this readout + // existed the view painted only an underline. + testWidgets('masks the password half of a userinfo', (tester) async { + await _pump( + tester, + 'https://oauth2:ghp_LIVE_TOKEN@github.com/org/repo', + const Offset(10, 10), + ); + + final text = _text(tester); + expect(text, isNot(contains('ghp_LIVE_TOKEN'))); + expect(text, startsWith('https://oauth2:')); + expect(_spans(tester)[1].$1, 'github.com'); + }); + + // Nothing to emphasize, and a guess about which characters are the host is + // exactly the wrong thing to be confident about. + testWidgets('renders an unparseable payload flat', (tester) async { + await _pump(tester, 'not a url at all', const Offset(100, 100)); + + final spans = _spans(tester); + expect(spans.length, 1); + expect(spans.single.$1, 'not a url at all'); + }); + + testWidgets('parks below the pointer, left edge on it', (tester) async { + await _pump(tester, link, const Offset(100, 100)); + + final card = _card(tester); + expect(card.left, 100); + expect(card.top, greaterThan(100)); + }); + + // The prompt the user is typing into is the bottom line of the panel, so + // the card has to leave it alone rather than settle over it. + testWidgets('flips above the pointer near the bottom edge', (tester) async { + await _pump(tester, link, Offset(100, _panel.height - 4)); + + final card = _card(tester); + expect(card.bottom, lessThan(_panel.height - 4)); + expect(card.top, greaterThanOrEqualTo(0)); + }); + + testWidgets('slides inward rather than off the right edge', (tester) async { + await _pump(tester, link, Offset(_panel.width - 4, 100)); + + final card = _card(tester); + expect(card.right, lessThanOrEqualTo(_panel.width)); + expect(card.left, lessThan(_panel.width - 4)); + }); + + // A URI is unbounded program-chosen text; without the cap one long enough + // would span the terminal it overlays. + testWidgets('caps its width on a very long URI', (tester) async { + await _pump( + tester, + 'https://example.com/${'segment/' * 200}', + const Offset(100, 100), + ); + + final card = _card(tester); + expect(card.width, lessThan(_panel.width * 0.75)); + expect(card.right, lessThanOrEqualTo(_panel.width)); + }); + }); +} diff --git a/app/test/terminal_hyperlink_test.dart b/app/test/terminal_hyperlink_test.dart index f942d3b3..8350e608 100644 --- a/app/test/terminal_hyperlink_test.dart +++ b/app/test/terminal_hyperlink_test.dart @@ -4,7 +4,6 @@ import 'dart:io'; import 'package:antgrid/util/ab_log.dart'; import 'package:antgrid/util/external_url.dart'; import 'package:antgrid/widgets/terminal_hyperlink_sheet.dart'; -import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -66,6 +65,72 @@ void main() { }); }); + group('terminalHyperlinkLooksDeceptive', () { + test('catches a host wearing another host as userinfo', () { + expect( + terminalHyperlinkLooksDeceptive( + Uri.parse('https://github.com@evil.example/antgrid/pull/13'), + ), + isTrue, + ); + expect( + terminalHyperlinkLooksDeceptive( + Uri.parse('https://user:pw@evil.example/'), + ), + isTrue, + ); + }); + + // Both spellings of the same lie: Dart punycodes nothing, so a raw unicode + // host arrives percent-encoded and an `xn--` check alone would pass it. + test('catches a homoglyph host, however it was spelled', () { + expect( + terminalHyperlinkLooksDeceptive( + Uri.parse('https://xn--pple-43d.com/login'), + ), + isTrue, + ); + final raw = Uri.parse('https://\u0430pple.com/login'); + expect(raw.host, contains('%')); + expect(terminalHyperlinkLooksDeceptive(raw), isTrue); + // A label merely containing the marker is not one starting with it. + expect( + terminalHyperlinkLooksDeceptive(Uri.parse('https://myxn--co.com/a')), + isFalse, + ); + }); + + // The cost of a false positive is a sheet on every ordinary link, which is + // how a confirmation stops being read at all. + test('passes ordinary links, ports and case included', () { + for (final ok in [ + 'https://github.com/antgrid-ai/antgrid/pull/13', + 'https://sub.github.com/ok', + 'https://github.com:443/ok', + 'https://gitHUB.com/OK', + 'http://192.168.1.9:8080/admin', + 'http://localhost:3000/', + ]) { + expect( + terminalHyperlinkLooksDeceptive(Uri.parse(ok)), + isFalse, + reason: ok, + ); + } + }); + + // Named so the gap is not mistaken for coverage: only the link's visible + // text contradicts this one, and that text never reaches the app. + test('does NOT catch a lookalike that is honestly its own host', () { + expect( + terminalHyperlinkLooksDeceptive( + Uri.parse('https://github.com.evil.example/antgrid'), + ), + isFalse, + ); + }); + }); + group('openTerminalHyperlink', () { late Directory tmp; late String logPath; @@ -167,10 +232,11 @@ void main() { expect(line['error'], contains('launcher exploded')); }); - // flutter_test reports android by default, so these run on the touch path - // unless they say otherwise. + // No platform override anywhere in this group any more: what decides is + // whether the caller says it had the destination on screen, and a caller + // that says nothing is a caller that showed nothing. testWidgets( - 'asks before opening on touch, and cancelling launches nothing', + 'asks when nothing disclosed the target, and cancelling launches nothing', (tester) async { final context = await pumpHost(tester); var launched = 0; @@ -218,13 +284,9 @@ void main() { expect(asked?.host, 'evil.example'); }); - testWidgets('desktop opens without asking -- hover already showed it', ( + testWidgets('opens an ordinary link that was on screen, without asking', ( tester, ) async { - // Cleared inside the body, not in addTearDown: the framework asserts - // the foundation debug vars are unset before teardown runs. - debugDefaultTargetPlatformOverride = TargetPlatform.macOS; - final context = await pumpHost(tester); final launched = []; var asked = 0; @@ -232,6 +294,7 @@ void main() { await openTerminalHyperlink( context, 'https://example.com/a', + disclosed: true, open: (_, url) async => launched.add(url), confirm: (_, _) async { asked++; @@ -240,11 +303,79 @@ void main() { ); await tester.pump(); - debugDefaultTargetPlatformOverride = null; - expect(asked, 0); expect(launched, ['https://example.com/a']); }); + + testWidgets('still asks when the URI is built to be misread', ( + tester, + ) async { + final context = await pumpHost(tester); + final launched = []; + Uri? asked; + + await openTerminalHyperlink( + context, + 'https://github.com@evil.example/antgrid/pull/13', + disclosed: true, + open: (_, url) async => launched.add(url), + confirm: (_, target) async { + asked = target; + return true; + }, + ); + await tester.pump(); + + // The hover readout would have shown this too -- and been read as + // GitHub, which is the whole reason the sheet names the host on its own + // line. + expect(asked?.host, 'evil.example'); + expect(launched, [ + 'https://github.com@evil.example/antgrid/pull/13', + ]); + }); + + testWidgets('cancelling a deceptive link launches nothing', (tester) async { + final context = await pumpHost(tester); + var launched = 0; + + await openTerminalHyperlink( + context, + 'https://xn--pple-43d.com/login', + disclosed: true, + open: (_, _) async => launched++, + confirm: (_, _) async => false, + ); + await tester.pump(); + + expect(launched, 0); + }); + + // The gap a `defaultTargetPlatform` test silently exempted: a finger tap on + // a desktop touchscreen took the desktop branch on the grounds that the + // hover readout had disclosed the destination, and there is no hover on + // touch. Asking the CALLER what it showed cannot answer that wrongly -- + // and it covers the Shift chord and a link scrolled out from under a + // resting pointer, which the platform test missed for the same reason. + testWidgets('asks on desktop when nothing was on screen', (tester) async { + final context = await pumpHost(tester); + var launched = 0; + var asked = 0; + + await openTerminalHyperlink( + context, + 'https://example.com/a', + open: (_, _) async => launched++, + confirm: (_, _) async { + asked++; + return false; + }, + ); + await tester.pump(); + + expect(asked, 1); + expect(launched, 0); + }, variant: TargetPlatformVariant.desktop()); }); group('showTerminalHyperlinkSheet', () {