Summary

Describe the problem and fix in 2–5 bullets:

• Problem: chat image clicks were hardened in one place, but URL safety and opener hardening were still ad hoc and easy to bypass with future raw window.open(...) calls.
• Why it matters: this is a security-sensitive UI path (tabnabbing + unsafe URL schemes), and drift/regression risk is high without a shared guardrail.
• What changed:
  • Added shared safe external-open utility: ui/src/ui/open-external-url.ts (resolveSafeExternalUrl, openExternalUrlSafe).
  • Migrated chat image click open path to use shared helper with allowDataImage: true.
  • Added focused tests for URL allowlist behavior and browser click-path behavior.
  • Added a guard script (scripts/check-no-raw-window-open.mjs) and wired it into UI checks (test:ui) to block raw runtime window.open(...) usage outside the safe helper.
• What did NOT change (scope boundary): no changes to gateway/channel/runtime message routing; no new network calls; no protocol/schema changes.

Change Type (select all)

• Bug fix
• Feature
• Refactor
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes #
• Related fix(ui): prevent tabnabbing in chat images #18685

User-visible / Behavior Changes

• Chat image links now open only when they resolve to allowed schemes (http, https, blob, and data:image/* when explicitly allowed by the caller).
• Unsafe schemes (for example javascript:, file:, non-image data:) are ignored for image click opens.

Security Impact (required)

• New permissions/capabilities? (No)
• Secrets/tokens handling changed? (No)
• New/changed network calls? (No)
• Command/tool execution surface changed? (No)
• Data access scope changed? (No)
• If any Yes, explain risk + mitigation:

Repro + Verification

Environment

• OS: macOS (local dev)
• Runtime/container: Node + pnpm
• Model/provider: N/A
• Integration/channel (if any): Control UI chat
• Relevant config (redacted): N/A

Steps

1. In Control UI chat, render a message with image URL content.
2. Click an image with safe URL (https://... or data:image/...).
3. Click an image with unsafe URL (javascript:... or data:text/html,...).

Expected

• Safe URL: opens a new tab with opener isolation (noopener,noreferrer, opener=null).
• Unsafe URL: no new tab is opened.

Actual

• Matches expected behavior.

Evidence

Attach at least one:

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

• Verified scenarios:
  • pnpm lint
  • pnpm lint:ui:no-raw-window-open
  • pnpm --dir ui exec vitest run --config vitest.config.ts --browser.enabled=false src/ui/open-external-url.test.ts src/ui/chat/message-extract.test.ts src/ui/chat/message-normalizer.test.ts src/ui/chat/tool-helpers.test.ts
  • pnpm --dir ui build
• Edge cases checked:
  • Allowed data:image/* while rejecting non-image data: payloads.
  • Relative URLs resolve against the current origin and then pass protocol allowlist checks.
• What you did not verify:
  • Browser-mode Vitest (*.browser.test.ts) execution in this environment, because Playwright binaries are not installed locally.

Compatibility / Migration

• Backward compatible? (Yes)
• Config/env changes? (No)
• Migration needed? (No)
• If yes, exact upgrade steps:

Failure Recovery (if this breaks)

• How to disable/revert this change quickly: revert this PR.
• Files/config to restore:
  • ui/src/ui/chat/grouped-render.ts
  • ui/src/ui/open-external-url.ts
  • scripts/check-no-raw-window-open.mjs
  • package.json (UI check hook)
• Known bad symptoms reviewers should watch for:
  • Legitimate image URLs unexpectedly not opening from chat.

Risks and Mitigations

• Risk: over-restrictive URL policy could block valid image opens in edge cases.
  • Mitigation: explicit allowlist includes current valid paths (http/https/blob + opt-in data:image/*), with targeted unit coverage.
• Risk: future contributors could bypass helper and reintroduce raw window.open usage.
  • Mitigation: dedicated guard script enforced in UI check path.

Greptile Summary

Centralizes external URL opening for chat images by introducing a shared safe-open utility (ui/src/ui/open-external-url.ts) with protocol allowlisting (http/https/blob + opt-in data:image/*) and tabnabbing prevention (noopener,noreferrer + opener=null). Adds guard script (scripts/check-no-raw-window-open.mjs) enforced via test:ui to block future raw window.open(...) calls outside the safe helper. Migration consolidates ad-hoc hardening from #18685 into a reusable, testable pattern.

Key improvements:

• Protocol allowlist blocks unsafe schemes (javascript:, file:, non-image data: URLs)
• Guard script prevents regression by enforcing use of centralized helper
• Comprehensive test coverage validates URL filtering and browser click behavior
• Clean migration path with no breaking changes

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• Security-focused refactor with defense-in-depth approach (protocol allowlist + guard script), comprehensive test coverage for both URL validation logic and browser integration, clean backward-compatible migration, and effective regression prevention mechanism
• No files require special attention

_{Last reviewed commit: 6faa2ca}