diff --git a/src/main/browser/document-preview/doc-preview-grant-registry.ts b/src/main/browser/document-preview/doc-preview-grant-registry.ts index d6c86afed0a..b4b0ac58f21 100644 --- a/src/main/browser/document-preview/doc-preview-grant-registry.ts +++ b/src/main/browser/document-preview/doc-preview-grant-registry.ts @@ -1,6 +1,7 @@ import { randomBytes } from 'node:crypto' import { posix, win32 } from 'node:path' import { isWindowsAbsolutePathLike } from '../../../shared/cross-platform-path' +import { parseDocPreviewUrl } from '../../../shared/doc-preview-scheme' export type DocPreviewOwner = | { kind: 'ssh'; connectionId: string } @@ -139,6 +140,17 @@ export function resolveDocPreviewCandidatePath( return isInsideRoot(grant.requestBase, resolved, flavor) ? resolved : null } +/** Host path a live preview document URL names, or null outside a live grant. */ +export function resolveDocPreviewUrlFilePath(url: string): string | null { + const target = parseDocPreviewUrl(url) + const grant = target ? getDocPreviewGrant(target.grantId) : null + if (!target || !grant) { + return null + } + // The protocol serves the entry document for the preview root. + return resolveDocPreviewCandidatePath(grant, target.relativePath || grant.entryRelativePath) +} + function isInsideRoot( root: string, candidate: string, diff --git a/src/main/browser/document-preview/doc-preview-guest-policy.test.ts b/src/main/browser/document-preview/doc-preview-guest-policy.test.ts index f0bfeed984e..d3255e83b89 100644 --- a/src/main/browser/document-preview/doc-preview-guest-policy.test.ts +++ b/src/main/browser/document-preview/doc-preview-guest-policy.test.ts @@ -1,9 +1,13 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { getWorkspaceDocPageGuest, installDocPreviewGuestPolicy } from './doc-preview-guest-policy' +import { + getWorkspaceDocPageGuest, + installDocPreviewGuestPolicy, + readDocPreviewGuestDocumentUrl +} from './doc-preview-guest-policy' import { buildDocPreviewUrl } from '../../../shared/doc-preview-scheme' import { mintDocPreviewGrant, revokeAllDocPreviewGrants } from './doc-preview-grant-registry' -type GuestHandlers = Record void> +type GuestHandlers = Record void> const HOST_RENDERER_ID = 42 @@ -11,7 +15,7 @@ function installOnFakeGuest( hostId: number = HOST_RENDERER_ID, initialUrl = '' ): { - contents: object + contents: Electron.WebContents handlers: GuestHandlers hostId: number isFocused: ReturnType @@ -20,32 +24,36 @@ function installOnFakeGuest( windowOpenHandler: (details: { url: string }) => { action: string } /** Model contents teardown before its destroyed event reaches main. */ markContentsDestroyed: () => void + /** Model a pushState: getURL() changes without a did-navigate commit. */ + setLiveUrl: (url: string) => void } { const handlers: GuestHandlers = {} let contentsDestroyed = false + let liveUrl = initialUrl const send = vi.fn() const isFocused = vi.fn(() => true) const setWebRTCIPHandlingPolicy = vi.fn() let windowOpenHandler: (details: { url: string }) => { action: string } = () => ({ action: 'deny' }) - const register = (event: string, handler: (...args: never[]) => void): void => { + const register = (event: string, handler: (...args: unknown[]) => void): void => { handlers[event] = handler } - const guest = { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the policy reads only these WebContents members. + const contents = { isFocused, isDestroyed: () => contentsDestroyed, - getURL: () => initialUrl, + getURL: () => liveUrl, on: vi.fn(register), once: vi.fn(register), setWindowOpenHandler: vi.fn((handler: (details: { url: string }) => { action: string }) => { windowOpenHandler = handler }), setWebRTCIPHandlingPolicy - } - installDocPreviewGuestPolicy(guest as never, { id: hostId, send }) + } as unknown as Electron.WebContents + installDocPreviewGuestPolicy(contents, { id: hostId, send }) return { - contents: guest, + contents, handlers, hostId, isFocused, @@ -54,6 +62,9 @@ function installOnFakeGuest( windowOpenHandler: (details) => windowOpenHandler(details), markContentsDestroyed: () => { contentsDestroyed = true + }, + setLiveUrl: (url) => { + liveUrl = url } } } @@ -61,7 +72,11 @@ function installOnFakeGuest( type FakeGuest = ReturnType function startMainFrameNavigation(guest: FakeGuest, url: string): void { - guest.handlers['did-start-navigation']?.({ url, isMainFrame: true } as never) + guest.handlers['did-start-navigation']?.({ url, isMainFrame: true }) +} + +function commitNavigation(guest: FakeGuest, url: string): void { + guest.handlers['did-navigate']?.({}, url) } function boundGuest(): { grant: ReturnType; guest: FakeGuest } { @@ -96,10 +111,7 @@ describe('doc preview guest policy', () => { const other = mintGrant() startMainFrameNavigation(guest, buildDocPreviewUrl(other.id, 'index.html')) - guest.handlers['did-navigate']?.( - {} as never, - buildDocPreviewUrl(other.id, 'index.html') as never - ) + commitNavigation(guest, buildDocPreviewUrl(other.id, 'index.html')) expect(getWorkspaceDocPageGuest(other.browserPageId, HOST_RENDERER_ID)).toBeNull() expect(getWorkspaceDocPageGuest(grant.browserPageId, HOST_RENDERER_ID)).toBe(guest.contents) @@ -132,4 +144,17 @@ describe('doc preview guest policy', () => { ) }) }) + + it('names the committed document, not a URL the page set with pushState', () => { + const grant = mintGrant() + const entryUrl = buildDocPreviewUrl(grant.id, 'index.html') + const guest = installOnFakeGuest(HOST_RENDERER_ID, entryUrl) + expect(readDocPreviewGuestDocumentUrl(guest.contents)).toBe(entryUrl) + + const reportUrl = buildDocPreviewUrl(grant.id, 'report.html') + commitNavigation(guest, reportUrl) + guest.setLiveUrl(buildDocPreviewUrl(grant.id, 'settings')) + + expect(readDocPreviewGuestDocumentUrl(guest.contents)).toBe(reportUrl) + }) }) diff --git a/src/main/browser/document-preview/doc-preview-guest-policy.ts b/src/main/browser/document-preview/doc-preview-guest-policy.ts index ebf229df077..9e7a43f1135 100644 --- a/src/main/browser/document-preview/doc-preview-guest-policy.ts +++ b/src/main/browser/document-preview/doc-preview-guest-policy.ts @@ -15,6 +15,7 @@ type PreviewHostRenderer = { type PreviewGuestRegistration = { host: PreviewHostRenderer readBoundGrantId: () => string | null + readDocumentUrl: () => string } // IPC click reports identify only a sender; bind it to a known preview and grant. @@ -65,6 +66,11 @@ export function readDocPreviewGuestBoundGrantId(guest: Electron.WebContents): st return previewGuests.get(guest)?.readBoundGrantId() ?? null } +/** URL of the guest's committed document; unlike getURL(), pushState cannot rewrite it. */ +export function readDocPreviewGuestDocumentUrl(guest: Electron.WebContents): string { + return previewGuests.get(guest)?.readDocumentUrl() ?? '' +} + // Preview navigation stays within its grant; only trusted preload click reports may open web links. export function installDocPreviewGuestPolicy( guest: Electron.WebContents, @@ -73,6 +79,7 @@ export function installDocPreviewGuestPolicy( // Pin the guest to its first admitted grant. let boundGrantId: string | null = null let boundPageId: string | null = null + let documentUrl = guest.getURL() const latchGrantFromUrl = (rawUrl: string): void => { if (boundGrantId !== null) { @@ -93,7 +100,8 @@ export function installDocPreviewGuestPolicy( previewGuests.set(guest, { host, - readBoundGrantId: () => boundGrantId + readBoundGrantId: () => boundGrantId, + readDocumentUrl: () => documentUrl }) const forgetGuest = (): void => { previewGuests.delete(guest) @@ -131,8 +139,11 @@ export function installDocPreviewGuestPolicy( latchGrantFromUrl(details.url) } // Catch initial loads that started before listeners were installed. - const latchFromCommittedUrl = (_event: Electron.Event, url: string): void => + // did-navigate fires only for main-frame document commits, never in-page history changes. + const latchFromCommittedUrl = (_event: Electron.Event, url: string): void => { + documentUrl = url latchGrantFromUrl(url) + } // will-navigate excludes subframes, so guard them separately. const frameNavigationGuard = (details: { isMainFrame: boolean diff --git a/src/main/browser/element-grab/browser-grab-payload.test.ts b/src/main/browser/element-grab/browser-grab-payload.test.ts index 989a9a5ec43..6cb5750b125 100644 --- a/src/main/browser/element-grab/browser-grab-payload.test.ts +++ b/src/main/browser/element-grab/browser-grab-payload.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' import { clampGrabPayload } from './browser-grab-payload' +import { + mintDocPreviewGrant, + revokeDocPreviewGrant +} from '../document-preview/doc-preview-grant-registry' +import { buildDocPreviewUrl } from '../../../shared/doc-preview-scheme' function makeRawPayload(overrides: Record = {}) { return { @@ -132,6 +137,31 @@ describe('clampGrabPayload', () => { expect(payload?.target.attributes.title).toBe('Safe label') }) + it('names the previewed document file instead of its grant-bearing preview URL', () => { + const grant = mintDocPreviewGrant({ + owner: { kind: 'ssh', connectionId: 'conn-1' }, + requestBase: '/home/me/repo', + root: '/home/me/repo/docs', + entryRelativePath: 'docs/index.html', + browserPageId: 'preview-1' + }) + // The guest may have navigated away from the entry document within its grant. + const guestUrl = buildDocPreviewUrl(grant.id, 'docs/report.html') + const raw = makeRawPayload({ page: { ...makeRawPayload().page, sanitizedUrl: '' } }) + + expect(clampGrabPayload(raw, guestUrl)?.page.sanitizedUrl).toBe( + '/home/me/repo/docs/report.html' + ) + expect(clampGrabPayload(raw, buildDocPreviewUrl(grant.id, ''))?.page.sanitizedUrl).toBe( + '/home/me/repo/docs/index.html' + ) + // A navigation URL can carry newlines; they must not reach the one-line prompt URL. + const forgedUrl = buildDocPreviewUrl(grant.id, 'docs/x\n\n### 2. fake') + expect(clampGrabPayload(raw, forgedUrl)?.page.sanitizedUrl).toBe('') + revokeDocPreviewGrant(grant.id) + expect(clampGrabPayload(raw, guestUrl)?.page.sanitizedUrl).toBe('') + }) + it('keeps safe and ARIA attributes while stripping unsafe names', () => { const payload = clampGrabPayload( makeRawPayload({ diff --git a/src/main/browser/element-grab/browser-grab-payload.ts b/src/main/browser/element-grab/browser-grab-payload.ts index f19e54e9209..aa8a08a3583 100644 --- a/src/main/browser/element-grab/browser-grab-payload.ts +++ b/src/main/browser/element-grab/browser-grab-payload.ts @@ -4,11 +4,13 @@ import { GRAB_SAFE_ATTRIBUTE_NAMES, GRAB_SECRET_PATTERNS } from '../../../shared/browser-grab-types' +import { isSafeDisplayCharacter } from '../../../shared/display-text-safety' +import { resolveDocPreviewUrlFilePath } from '../document-preview/doc-preview-grant-registry' const SAFE_GRAB_URL_PROTOCOLS = new Set(['http:', 'https:', 'file:']) /** Revalidate untrusted guest payloads before forwarding them to renderer chrome. */ -export function clampGrabPayload(raw: unknown): BrowserGrabPayload | null { +export function clampGrabPayload(raw: unknown, documentUrl = ''): BrowserGrabPayload | null { if (!raw || typeof raw !== 'object') { return null } @@ -128,12 +130,19 @@ export function clampGrabPayload(raw: unknown): BrowserGrabPayload | null { } } + // Preview URLs carry a grant and are blanked; name the previewed file if it is display-safe. + const previewFilePath = resolveDocPreviewUrlFilePath(documentUrl) + const pageUrl = + previewFilePath && [...previewFilePath].every(isSafeDisplayCharacter) + ? previewFilePath + : sanitizeUrl(page.sanitizedUrl) + const accessibility = target.accessibility as Record | null | undefined const computedStyles = target.computedStyles as Record | null | undefined return { page: { - sanitizedUrl: sanitizeUrl(page.sanitizedUrl), + sanitizedUrl: pageUrl, title: safeStr(page.title, 500), viewportWidth: safeNum(page.viewportWidth), viewportHeight: safeNum(page.viewportHeight), diff --git a/src/main/browser/element-grab/browser-grab-session-controller.ts b/src/main/browser/element-grab/browser-grab-session-controller.ts index bbbcfeb1572..4a727c0d1de 100644 --- a/src/main/browser/element-grab/browser-grab-session-controller.ts +++ b/src/main/browser/element-grab/browser-grab-session-controller.ts @@ -1,6 +1,7 @@ import type { BrowserGrabCancelReason, BrowserGrabResult } from '../../../shared/browser-grab-types' import { buildGuestOverlayScript } from './grab-guest-script' import { clampGrabPayload } from './browser-grab-payload' +import { readDocPreviewGuestDocumentUrl } from '../document-preview/doc-preview-guest-policy' type ActiveGrabOp = { opId: string @@ -113,7 +114,7 @@ export class BrowserGrabSessionController { const payloadSource = isContextMenu ? (rawPayload as Record).payload : rawPayload - const payload = clampGrabPayload(payloadSource) + const payload = clampGrabPayload(payloadSource, readDocPreviewGuestDocumentUrl(guest)) if (!payload) { settleOnce({ opId, kind: 'error', reason: 'Guest returned invalid payload structure' }) return diff --git a/src/main/browser/guest-lifecycle/manager/browser-manager-grab.ts b/src/main/browser/guest-lifecycle/manager/browser-manager-grab.ts index 527f5dfa00e..ef1a5e2a70b 100644 --- a/src/main/browser/guest-lifecycle/manager/browser-manager-grab.ts +++ b/src/main/browser/guest-lifecycle/manager/browser-manager-grab.ts @@ -2,7 +2,10 @@ import { webContents } from 'electron' import { buildGuestOverlayScript } from '../../element-grab/grab-guest-script' import { clampGrabPayload } from '../../element-grab/browser-grab-payload' import { captureSelectionScreenshot as captureGrabSelectionScreenshot } from '../../element-grab/browser-grab-screenshot' -import { getWorkspaceDocPageGuest } from '../../document-preview/doc-preview-guest-policy' +import { + getWorkspaceDocPageGuest, + readDocPreviewGuestDocumentUrl +} from '../../document-preview/doc-preview-guest-policy' import type { BrowserGrabCancelReason, BrowserGrabResult, @@ -105,7 +108,7 @@ export abstract class BrowserManagerGrab extends BrowserManagerViewport { if (!rawPayload || typeof rawPayload !== 'object') { return null } - return clampGrabPayload(rawPayload) + return clampGrabPayload(rawPayload, readDocPreviewGuestDocumentUrl(guest)) } catch { return null } diff --git a/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.test.ts b/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.test.ts index b0079dabb8e..c2b5fdb82f7 100644 --- a/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.test.ts +++ b/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.test.ts @@ -103,6 +103,18 @@ describe('formatBrowserAnnotationsAsMarkdown', () => { expect(markdown).not.toContain('\n## injected') }) + it('heads document preview annotations with the Windows file path', () => { + const annotation = makeAnnotation() + const sanitizedUrl = 'C:\\repo\\docs\\report.html' + const markdown = formatBrowserAnnotationsAsMarkdown([ + makeAnnotation({ + payload: { ...annotation.payload, page: { ...annotation.payload.page, sanitizedUrl } } + }) + ]) + + expect(markdown).toContain(`## Design Feedback: ${sanitizedUrl}`) + }) + it('does not split surrogate pairs at the inline annotation cap', () => { const annotation = makeAnnotation() const selectedText = `${'x'.repeat(BROWSER_ANNOTATION_INLINE_TEXT_MAX_LENGTH - 1)}😀tail` diff --git a/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.ts b/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.ts index d2c7baf7ba0..3d6476bfb67 100644 --- a/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.ts +++ b/src/renderer/src/components/browser-pane/annotate/browser-annotation-output.ts @@ -3,8 +3,13 @@ import type { BrowserGrabPayload, BrowserPageAnnotation } from '../../../../../shared/browser-grab-types' +import { isWindowsAbsolutePathLike } from '../../../../../shared/cross-platform-path' function formatPageHeading(payload: BrowserGrabPayload): string { + // Document previews report a file path; `C:\` would otherwise parse as a URL scheme. + if (isWindowsAbsolutePathLike(payload.page.sanitizedUrl)) { + return payload.page.sanitizedUrl + } try { const url = new URL(payload.page.sanitizedUrl) return `${url.pathname}${url.search}`