fix(security): isolate previews from native APIs

[yyhhyyyyyy]

Summary

Harden preview isolation and native IPC access following the retirement of legacy HTML artifacts. This PR addresses only the HTML preview security issue.

• Restrict document preview frames to workspace-preview: URLs and keep Chromium web security enabled in floating windows.
• Expose broad preload APIs only to isolated, top-level main and settings documents.
• Validate the live sender frame and document before native route dispatch or clipboard access.
• Preserve the splash language lookup and registered plugin settings' existing scoped routes.

Compatibility

HTML previews retain scripts, ES modules, and relative resource loading on their own origin. No layout changes, dependencies, settings, or data migrations are introduced.

Custom previews that access the parent app's native APIs, or rely on disabled same-origin/CORS checks in floating windows, will no longer work. MCP approval semantics and unrelated security issues are unchanged.

Preview behavior

BEFORE: Preview URL -> iframe without scheme validation
AFTER:  workspace-preview: -> isolated iframe
        Other URLs        -> existing fallback rendering

Validation

• Format, i18n, lint, typecheck, and full build
• 174 main-process tests and 36 renderer tests
• 12 Electron compatibility/security tests using built pages
• 4 Electron tests using the Vite development server
• Regression checks failed on the old implementation and passed after the fix

Summary by CodeRabbit

• Security
  • Executable workspace previews are isolated from the app and cannot access its bridge.
  • App capabilities and system requests are restricted to authorized app documents.
  • Floating windows retain browser security protections.
• Bug Fixes
  • Invalid or untrusted preview URLs now display as literal source instead of loading as previews.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (2)

AGENTS.md — auto-discovered
CLAUDE.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: e808f9fd-21a3-4823-8c57-4a81b77db3c9

📥 Commits

Reviewing files that changed from the base of the PR and between d983d4f and 42af69c.

📒 Files selected for processing (14)

• AGENTS.md
• docs/issues/html-preview-isolation/spec.md
• src/main/app/clipboardIpc.ts
• src/main/app/composition.ts
• src/main/app/rendererIpcSecurity.ts
• src/main/desktop/floatingButton/FloatingButtonWindow.ts
• src/main/desktop/window/FloatingChatWindow.ts
• src/main/routes/index.ts
• src/preload/index.ts
• src/renderer/src/components/sidepanel/viewer/WorkspacePreviewPane.vue
• src/shared/rendererDocument.ts
• test/e2e/specs/43-preview-isolation.smoke.spec.ts
• test/main/app/rendererIpcSecurity.test.ts
• test/renderer/components/WorkspacePreviewPane.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The change validates workspace preview URLs, gates preload APIs by renderer document, and authorizes native IPC senders before route dispatch or clipboard access. It also enables web security for the floating button and chat windows.

Changes

Renderer Isolation

Layer / File(s)	Summary
Document and preview boundaries src/shared/rendererDocument.ts, src/preload/index.ts, src/renderer/src/components/sidepanel/viewer/WorkspacePreviewPane.vue, test/renderer/components/WorkspacePreviewPane.test.ts, test/e2e/specs/43-preview-isolation.smoke.spec.ts	The document matcher identifies main, settings, and splash entries. Preload APIs are exposed only in main and settings documents. Preview URLs are normalized and accepted only when they use workspace-preview:, have a hostname, and contain no credentials or port. Tests cover invalid preview sources, preload exposure after navigation, and preview frame isolation.
Native IPC authorization src/main/app/rendererIpcSecurity.ts, src/main/app/composition.ts, src/main/routes/index.ts, src/main/app/clipboardIpc.ts, test/main/app/rendererIpcSecurity.test.ts, test/e2e/specs/43-preview-isolation.smoke.spec.ts	The authorizer checks sender state, frame identity, current document, and route permissions. Route and clipboard handlers invoke it before dispatch or clipboard operations. The application composition supplies the authorizer to both registrations. Tests cover allowed and rejected documents and routes, and verify that rejected requests cause no dispatch or clipboard calls.
Window security settings and isolation guidance src/main/desktop/floatingButton/FloatingButtonWindow.ts, src/main/desktop/window/FloatingChatWindow.ts, AGENTS.md, docs/issues/html-preview-isolation/spec.md	The floating button and chat windows now enable webSecurity. The guidance and specification describe preview isolation constraints, preload exposure, IPC checks, scope, and validation criteria.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Renderer
  participant DeepchatRoutes
  participant RendererIpcAuthorizer
  participant RouteDispatcher
  Renderer->>DeepchatRoutes: Invoke route
  DeepchatRoutes->>RendererIpcAuthorizer: Authorize event and route
  RendererIpcAuthorizer-->>DeepchatRoutes: Allow or reject
  DeepchatRoutes->>RouteDispatcher: Dispatch authorized route

Loading

Suggested reviewers: zerob13, zhangmo8

Merge Risk: ⚪ Minimal · up to 42af6

The preview and native-API isolation changes are mergeable after normal checks; no actionable issue remains established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 42af6

The changes tighten preview isolation and native API access. No introduced privilege bypass was identified in the reviewed paths, but incomplete downstream coverage and unverified recovery behavior leave limited residual uncertainty.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The assessed boundary protects native route dispatch and the user's OS clipboard from preview-controlled or unrelated documents. Trusted main/settings documents still receive broad native authority. Workspace preview resource access remains scoped by the existing registered-file/root resolver; downstream native privileges were not exhaustively inventoried.

Security Findings and Attack Paths

• observed — Regression test source asserts that preview modules and relative fetch remain functional while parent bridge access fails, navigation to an unrelated file loses preload APIs, and direct IPC from an unrelated document is rejected. Unit tests also assert rejection before route dispatch, clipboard access, or image decoding. These are inspected assertions, not independently verified execution results.

Trust Boundaries and Controls

• observed — The plugin exception is constrained by registered WebContents identity and four allowed routes. Existing window controls deny new windows and navigation away from the plugin entry file; route handlers reject requests for a different registered owner. These controls counter the hypothesis that an unrelated preview can inherit plugin settings authority.

Resilience and Maintainability Implications

• observed — IPC registration cleanup is not added: route registration replaces its handler, while clipboard registration retains its existing non-idempotent form. This behavior predates the PR. The existing route lifecycle fence rejects dispatch while stopping or stopped, and the new sender gate rejects destroyed windows. Same-process reconstruction was not demonstrated and is not established as a newly introduced exposure.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 11 files. (3 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: isolating previews from native APIs through security restrictions.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 11 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[zerob13]

Review: fix(security): isolate previews from native APIs

Verdict: Approve. This is a well-constructed security hardening with genuine defense in depth — the preload gates exposure at the renderer side, the main process independently re-validates the live sender on every route dispatch and clipboard call, and the preview surface is restricted at the URL level. The one breaking change (custom previews that used native APIs or relied on disabled web security) is honestly declared in the PR body's compatibility section. Verified on this head: 26/26 rendererIpcSecurity tests, 14/14 WorkspacePreviewPane tests, typecheck clean in PR-touched files (the single residual error is the known pre-existing acp-extension-core drift in an untouched file).

The security model, verified layer by layer

• Preload fails closed (src/preload/index.ts): contextBridge.exposeInMainWorld now runs only when context isolation is on, the frame is the main frame, AND the document is exactly the main or settings entry document. The old non-isolated fallback (window.api = api) is deleted outright — without context isolation you now get no API rather than an unprotected one.
• Main-side re-validation on every call (src/main/app/rendererIpcSecurity.ts): sender not destroyed, senderFrame === sender.mainFrame (child iframes can't invoke), senderFrame.url === sender.getURL() (a mid-flight navigation race can't carry a stale document's privileges), and the document must exactly match one of the three entry URLs — exact href matching, not prefix, so arbitrary HTML files in the renderer directory (i.e., preview artifacts on the same origin) never qualify. Splash gets exactly one route (config.getLanguage); plugin settings windows get the four plugin routes, which the route layer already guards with per-plugin ownership checks.
• Document matching is precise (src/shared/rendererDocument.ts): entries resolved from the renderer directory URL, dev-server URLs validated (http:/https: only, credentials rejected), hash/search stripped for normalization. Same matcher is shared between preload and main process — one definition of "app document", no drift.
• Preview frames are origin-locked (WorkspacePreviewPane.vue): the iframe URL must be workspace-preview: with a hostname and no credentials or port; anything else renders no frame at all, with an inline comment warning against ever substituting srcdoc/blob URLs (which would re-couple the preview to the app origin).
• Floating windows restore Chromium web security: both FloatingButtonWindow and FloatingChatWindow flip webSecurity: false → true, so previews there no longer run with same-origin/CORS checks disabled.
• Splash language lookup survives: the splash screen has its own minimal preload (splash-preload.ts → window.deepchatSplash), so it never needed the broad bridge — the main-side whitelist covers its single config.getLanguage invocation.

Test coverage

• 26 unit tests on the authorizer cover the full matrix: top-level vs child frame, live vs navigated URL, each document kind, splash's single allowed route, plugin windows' route set, and the rejection paths.
• 14 renderer tests cover the preview URL validator (protocol, hostname, credentials, port, malformed input → no frame).
• The new e2e spec (3 scenarios) does real end-to-end verification — a workspace HTML file with ES modules and relative fetches keeps working, while the app bridge stays inaccessible from the preview origin. Not runnable in this review environment (needs a build); the code reads correct.

Minor (non-blocking)

• The compatibility note about custom previews breaking is accurate but terse — if any users had built previews that call window.deepchat, a one-line migration hint in the release notes ("previews are sandboxed to their own origin; use the documented preview APIs") would preempt support questions.
• documentUrl strips hash and search before matching — correct for normalization; just noting that index.html?anything intentionally still counts as the app document (it is the app document).

Detailed references

• src/main/app/rendererIpcSecurity.ts — the live-sender authorizer.
• src/shared/rendererDocument.ts — the shared exact-entry matcher.
• src/preload/index.ts:88-104 — the fail-closed exposure gate.
• src/renderer/src/components/sidepanel/viewer/WorkspacePreviewPane.vue:129-145 — the workspace-preview: URL validation.
• docs/issues/html-preview-isolation/spec.md — the issue record.