feat(desktop): trim SandboxedFrame tests to invariants; document the allowlist contract

Nine change-detector tests become four invariants (one per behaviour, two
behaviours per module): every realm-escaping / unknown / mixed-case token is
dropped and an emptied set falls back to the default posture; allowlisted
tokens survive deduped; the rendered frame carries the posture; props cannot
re-open it.

Docs: TS signature, the exact allowlist and the strip list the ruling names
(allow-same-origin, allow-top-navigation*, allow-popups*, allow-modals,
allow-storage-access-by-user-activation), teardown, and the rss-reader
(#115972) migration off its stubbed /preview → openExternal chain.
This commit is contained in:
teknium1
2026-09-23 19:11:23 -07:00
committed by Teknium
parent d81db86d61
commit a48557baff
3 changed files with 60 additions and 49 deletions

View File

@@ -7,46 +7,32 @@ import { SANDBOXED_FRAME_DEFAULT_SANDBOX, SandboxedFrame, sanitizeFrameSandbox }
afterEach(cleanup)
describe('sanitizeFrameSandbox', () => {
it('defaults to the opaque-origin posture for an empty or missing sandbox', () => {
expect(sanitizeFrameSandbox(undefined)).toBe(SANDBOXED_FRAME_DEFAULT_SANDBOX)
expect(sanitizeFrameSandbox('')).toBe(SANDBOXED_FRAME_DEFAULT_SANDBOX)
expect(sanitizeFrameSandbox(' ')).toBe(SANDBOXED_FRAME_DEFAULT_SANDBOX)
})
it('keeps safe tokens and dedupes them', () => {
expect(sanitizeFrameSandbox('allow-scripts allow-forms')).toBe('allow-scripts allow-forms')
expect(sanitizeFrameSandbox('allow-scripts allow-scripts allow-forms')).toBe('allow-scripts allow-forms')
})
it('is case-insensitive, because the attribute is (a mixed-case escape used to pass)', () => {
// HTML: "an unordered set of unique space-separated tokens that are ASCII
// case-insensitive", and Chromium lower-cases each token before matching.
expect(sanitizeFrameSandbox('ALLOW-SAME-ORIGIN allow-scripts')).toBe('allow-scripts')
expect(sanitizeFrameSandbox('Allow-Top-Navigation allow-forms')).toBe('allow-forms')
expect(sanitizeFrameSandbox('ALLOW-SCRIPTS Allow-Forms')).toBe('allow-scripts allow-forms')
})
it('drops a token it does not know instead of forwarding it', () => {
// An allowlist, not a blocklist: a token nobody has heard of (or a future
// one) must not reach the attribute on the strength of being unlisted.
it('strips every realm-escaping or unknown token, case-insensitively, and never emits an empty attribute', () => {
// HTML: sandbox is "an unordered set of unique space-separated tokens that
// are ASCII case-insensitive"; Chromium lower-cases each token before
// matching, so `ALLOW-SAME-ORIGIN` once sailed past a case-sensitive check.
expect(sanitizeFrameSandbox('allow-scripts allow-same-origin')).toBe('allow-scripts')
expect(sanitizeFrameSandbox('ALLOW-SAME-ORIGIN Allow-Top-Navigation allow-scripts')).toBe('allow-scripts')
expect(sanitizeFrameSandbox('allow-scripts allow-invented-thing')).toBe('allow-scripts')
expect(sanitizeFrameSandbox('allow-invented-thing')).toBe(SANDBOXED_FRAME_DEFAULT_SANDBOX)
// A frame with NO sandbox attribute is fully privileged — an emptied set
// must fall back to the default posture, not to nothing.
for (const escape of [
undefined,
' ',
'allow-same-origin',
'allow-top-navigation allow-top-navigation-by-user-activation allow-popups allow-popups-to-escape-sandbox',
'allow-modals allow-storage-access-by-user-activation'
]) {
expect(sanitizeFrameSandbox(escape)).toBe(SANDBOXED_FRAME_DEFAULT_SANDBOX)
}
})
it('allows the safe tokens a real embed asks for', () => {
it('keeps the allowlisted tokens a real embed asks for, deduped', () => {
expect(sanitizeFrameSandbox('allow-scripts allow-scripts allow-forms')).toBe('allow-scripts allow-forms')
expect(sanitizeFrameSandbox('allow-downloads allow-forms allow-presentation')).toBe(
'allow-downloads allow-forms allow-presentation'
)
})
it('strips every realm-escaping token, falling back to the default when nothing is left', () => {
expect(sanitizeFrameSandbox('allow-scripts allow-same-origin')).toBe('allow-scripts')
expect(sanitizeFrameSandbox('allow-top-navigation allow-popups')).toBe(SANDBOXED_FRAME_DEFAULT_SANDBOX)
expect(sanitizeFrameSandbox('allow-same-origin')).toBe(SANDBOXED_FRAME_DEFAULT_SANDBOX)
expect(sanitizeFrameSandbox('allow-modals allow-storage-access-by-user-activation')).toBe(
SANDBOXED_FRAME_DEFAULT_SANDBOX
)
})
})
describe('SandboxedFrame', () => {
@@ -61,22 +47,20 @@ describe('SandboxedFrame', () => {
expect(frame.getAttribute('title')).toBe('Feed')
})
it('keeps its no-referrer / lazy posture when a caller tries to override it', () => {
it('keeps its posture when a caller tries to re-open it through props', () => {
const { container } = render(
<SandboxedFrame loading="eager" referrerPolicy="unsafe-url" src="https://example.com" title="Feed" />
<SandboxedFrame
loading="eager"
referrerPolicy="unsafe-url"
sandbox="allow-scripts allow-same-origin allow-popups"
src="https://example.com"
title="Feed"
/>
)
const frame = container.querySelector('iframe')!
expect(frame.getAttribute('sandbox')).toBe('allow-scripts')
expect(frame.getAttribute('referrerpolicy')).toBe('no-referrer')
expect(frame.getAttribute('loading')).toBe('lazy')
})
it('refuses an allow-same-origin / popup escape even when asked for one', () => {
const { container } = render(
<SandboxedFrame sandbox="allow-scripts allow-same-origin allow-popups" src="https://example.com" title="Feed" />
)
expect(container.querySelector('iframe')!.getAttribute('sandbox')).toBe('allow-scripts')
})
})

View File

@@ -46,7 +46,7 @@ export function sanitizeFrameSandbox(sandbox: string | undefined): string {
}
export interface SandboxedFrameProps extends Omit<React.ComponentProps<'iframe'>, 'src'> {
/** Absolute `http(s):` URL to embed. */
/** Absolute `http(s):` (or `data:`) URL to embed. */
src: string
/** Accessible title (required — an untitled frame is unlabelled in the a11y tree). */
title: string

View File

@@ -700,11 +700,38 @@ Migrations for the plugins that motivated this slot:
Use the SDK's `<SandboxedFrame src title />` for any external web content
(reader views, dashboards, docs). It renders a sandboxed iframe with the app's
guest-content posture: opaque origin, `allow-scripts` by default, `no-referrer`,
lazy loading. Realm-escaping tokens (`allow-same-origin`, top-navigation,
popups, modals) are stripped even if passed — the opaque origin IS the
containment. Never mount a raw Electron `<webview>`: it lands on the app's
lazy loading. Never mount a raw Electron `<webview>`: it lands on the app's
`persist:` preview partition, sharing the app's cookies and storage.
```ts
interface SandboxedFrameProps extends Omit<ComponentProps<'iframe'>, 'src'> {
src: string // http(s): or data: URL to embed
title: string // required — an untitled frame is unlabelled in the a11y tree
sandbox?: string // extra tokens; filtered through sanitizeFrameSandbox
}
SANDBOXED_FRAME_DEFAULT_SANDBOX = 'allow-scripts'
sanitizeFrameSandbox(sandbox?: string): string
```
*Arbitration (allowlist, not blocklist):* the only tokens a caller may add are
`allow-scripts`, `allow-forms`, `allow-downloads`, `allow-pointer-lock`,
`allow-orientation-lock`, `allow-presentation`. Everything else —
`allow-same-origin`, `allow-top-navigation*`, `allow-popups*`, `allow-modals`,
`allow-storage-access-by-user-activation`, and any token the primitive does not
know — is dropped case-insensitively even if passed; an emptied set falls back
to the default posture, because a frame with **no** `sandbox` attribute is
fully privileged. `loading="lazy"` and `referrerPolicy="no-referrer"` cannot be
overridden through props. The opaque origin IS the containment: guest content
cannot reach the app, its storage, or the preload bridge.
*Teardown:* it is a plain React element — unmounting your pane/page removes the
frame and its realm; nothing is registered app-side.
Migration for **rss-reader** (#115972): replace the stubbed `/preview` → 501 →
`host.openWorkspace('rss-browser')` → empty `RssBrowserFrame` → `ctx.os.openExternal`
chain with `<SandboxedFrame src={article.url} title={article.title} />` inside
the workspace page; drop the leftover `.rss-browser-frame-host webview` CSS.
### Transcript directives — inline components the model addresses
`TRANSCRIPT_DIRECTIVE_AREA` makes the transcript itself a contribution area.
@@ -1472,7 +1499,7 @@ pipeline as a trust boundary.
| Area payloads | `RouteContribution`, `SidebarNavContribution`, `StatusbarItem`, `TitlebarTool`, `PaletteContribution`, `KeybindContribution`, `ComposerMiddleware`, `ComposerAttachmentProvider`, `SessionRowSlotContribution`, `SidebarNavPrefsContribution` |
| React / state | `useValue`, `atom`, `computed`, `useQuery`, `useMutation`, `useQueryClient`, `queryClient`, `Contribute` |
| Theming | `useTheme`, `requestTheme`, `setAccentOverride`, `$accentOverride`, `retintTheme`, `themeHue`, `DesktopTheme`, `DesktopThemeColors`, plus OKLCH math (`hexToOklch`, `oklchToHex`, `oklchToSrgb255`, `mixOklab`, `maxChroma`, `hueDelta`, `normalizeHex`) and sRGB measures (`contrastRatio` — `number | null`, null for unparseable input — `readableOn`) |
| UI kit | `Button`, `Input`, `Textarea`, `Select*`, `Switch`, `Checkbox`, `SegmentedControl`, `Tabs*`, `Dialog*`, `ConfirmDialog`, `DropdownMenu*`, `ContextMenu*`, `Popover*`, `Tip`/`Tooltip*`, `Badge`, `Kbd`/`KbdGroup`, `SearchField`, `ScrollArea`, `Separator`, `Skeleton`, `GlyphSpinner`, `Loader`, `EmptyState`, `ErrorState`, `CopyButton`, `StatusDot`, `LogView`, `Codicon`, `DecodeText` |
| UI kit | `Button`, `Input`, `Textarea`, `Select*`, `Switch`, `Checkbox`, `SegmentedControl`, `Tabs*`, `Dialog*`, `ConfirmDialog`, `DropdownMenu*`, `ContextMenu*`, `Popover*`, `Tip`/`Tooltip*`, `Badge`, `Kbd`/`KbdGroup`, `SearchField`, `ScrollArea`, `Separator`, `Skeleton`, `GlyphSpinner`, `Loader`, `EmptyState`, `ErrorState`, `CopyButton`, `StatusDot`, `LogView`, `Codicon`, `DecodeText`, `SandboxedFrame` |
| Helpers | `cn`, `icons`, `haptic`, `useI18n`, `profileColor`, `profileColorSoft`, `relativeTime`, `fmtDateTime`, `fmtDayTime`, `coarseElapsed`, `evaluateRuntimeReadiness` |
The canonical, always-current export list is `apps/desktop/src/sdk/index.ts`.