security: close capability, CSP and auth-bridge holes
capabilities/default.json - Drop every `store:*` grant. `@tauri-apps/plugin-store` has no caller in `app/src`, and the plugin's `resolve_store_path` is a `PathBuf::push` against AppData — `push` discards the base for an absolute path, so the grant was an arbitrary host read/write from the webview. - Replace `opener:default` with a scoped `opener:allow-open-url` (http/https only). That drops `reveal_item_in_dir`, which the plugin does not scope-check and nothing here calls, and the unused mailto:/tel: scope. - Record the unscopable `drag:allow-start-drag` residual risk in `description`. tauri.conf.json - Add `form-action 'none'`, `base-uri 'none'`, `object-src 'none'`. `form-action` has no `default-src` fallback, so an injected auto-submitting form was unblocked even though `script-src 'self'` blocks XSS. - Remove the dead `asset:` / `https://asset.localhost` img-src and `data:` font-src grants; `blob:` stays (the file viewer uses it). auth_bridge - The reserved-port set covered only this project's mappings and the two browser-view ranges. It now also covers the gateway, STT and web-terminal host ports (configured value and shipped default, read off the settings models) and every other project's published host port. A container binding container-loopback 4000 / 9876 / 7681 while those services were stopped had that port mirrored onto the host, unauthenticated, within one poll. - Gate the host listener on fetch metadata: refuse a request that is a cross-site sub-resource, allow navigations (the OAuth redirect) and anything without `Sec-Fetch-*`. Non-HTTP connections are classified from their first line and forwarded verbatim. Residual risk is spelled out in the module docs. - Bound the forwards: max concurrent connections per port, a first-byte deadline enforced before any `docker exec` is created, and an idle timeout. browser_view/mod.rs - `pick_viewer_port` reads procfs with `/usr/bin/cat`, not a bare `cat` the container can shim via its writable PATH entry. - Treat port choice as check-then-bind: walk to the next free candidate when the viewer does not come up, instead of failing the start. BrowserTab.tsx - Sandbox the viewer iframe. Container-controlled content could `top.location` the app's webview away. `allow-top-navigation*` and `allow-popups-to-escape-sandbox` are deliberately absent. HelpDialog.tsx - Escape the quote characters in the entity pass and escape captured attribute values. `href="$2"` with `$2` = `[^)]+` let remote GitHub markdown close the attribute and open another, in a document rendered with `dangerouslySetInnerHTML`. web_terminal/terminal.html - SRI hashes plus `crossorigin` on the three jsdelivr bundles and the stylesheet, and a CSP for the page — it is served 0.0.0.0 behind a permissive CORS layer and nothing else gives it one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBq2rGum6GX7xXgsas1fDc
This commit is contained in:
@@ -0,0 +1,100 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { renderMarkdown } from "./HelpDialog";
|
||||
|
||||
/**
|
||||
* `renderMarkdown` builds HTML by regex substitution and the result is handed
|
||||
* to `dangerouslySetInnerHTML`. Its input is the help document, which is
|
||||
* fetched from GitHub at runtime — remote, versioned by someone else, and not
|
||||
* something the app gets to trust. These tests pin the escaping.
|
||||
*/
|
||||
/**
|
||||
* Parse rendered HTML and return its first anchor, asserting that *no* element
|
||||
* anywhere in the output grew an attribute outside the allowed set. A broken
|
||||
* attribute value is only interesting if it becomes an attribute, so the check
|
||||
* has to run through a real parser rather than over the string.
|
||||
*/
|
||||
const ALLOWED_ATTRS = new Set(["class", "href", "target", "rel", "id"]);
|
||||
|
||||
function onlyAnchor(html: string): HTMLAnchorElement {
|
||||
const doc = new DOMParser().parseFromString(html, "text/html");
|
||||
for (const el of Array.from(doc.body.querySelectorAll("*"))) {
|
||||
for (const name of attrNames(el)) {
|
||||
expect(ALLOWED_ATTRS.has(name), `unexpected attribute ${name}`).toBe(true);
|
||||
}
|
||||
}
|
||||
const anchors = doc.querySelectorAll("a");
|
||||
expect(anchors.length).toBeGreaterThan(0);
|
||||
return anchors[0] as HTMLAnchorElement;
|
||||
}
|
||||
|
||||
/** Attribute names the parser actually saw on an element. */
|
||||
function attrNames(el: Element): string[] {
|
||||
return Array.from(el.attributes).map((a) => a.name);
|
||||
}
|
||||
|
||||
describe("renderMarkdown escaping", () => {
|
||||
it("escapes the quote characters an attribute value is delimited by", () => {
|
||||
const html = renderMarkdown('He said "hi" and it\'s fine.');
|
||||
expect(html).not.toMatch(/said "hi"/);
|
||||
expect(html).toContain(""hi"");
|
||||
expect(html).toContain("it's");
|
||||
});
|
||||
|
||||
it("does not let a link target break out of href=\"…\"", () => {
|
||||
// The sink: the URL capture is `[^)]+`, which includes `"` and spaces, and
|
||||
// the value lands directly inside `href="…"`. Asserted through the DOM,
|
||||
// not by string matching — the payload text legitimately survives *inside*
|
||||
// the attribute value; what must not happen is it becoming an attribute.
|
||||
const a = onlyAnchor(
|
||||
renderMarkdown(
|
||||
'[click](https://example.com/" onmouseover="steal() formaction="https://evil.example)',
|
||||
),
|
||||
);
|
||||
expect(attrNames(a)).toEqual(["class", "href", "target", "rel"]);
|
||||
expect(a.getAttribute("href")).toContain('" onmouseover="');
|
||||
});
|
||||
|
||||
it("does not let an in-document anchor break out of href=\"#…\"", () => {
|
||||
const a = onlyAnchor(
|
||||
renderMarkdown('[jump](#top" onfocus="steal() autofocus="x)'),
|
||||
);
|
||||
expect(attrNames(a)).toEqual(["class", "href"]);
|
||||
});
|
||||
|
||||
it("does not let a bare URL break out of href=\"…\"", () => {
|
||||
const a = onlyAnchor(
|
||||
renderMarkdown('See https://example.com/a"onmouseover="steal()\n'),
|
||||
);
|
||||
expect(attrNames(a)).toEqual(["class", "href", "target", "rel"]);
|
||||
});
|
||||
|
||||
it("still renders ordinary links intact", () => {
|
||||
const html = renderMarkdown("[docs](https://example.com/a?x=1&y=2)");
|
||||
// `&` was entity-escaped by the first pass and must not be escaped twice.
|
||||
expect(html).toContain('href="https://example.com/a?x=1&y=2"');
|
||||
expect(html).not.toContain("&amp;");
|
||||
expect(html).toContain('target="_blank"');
|
||||
expect(html).toContain('rel="noopener noreferrer"');
|
||||
expect(html).toContain(">docs</a>");
|
||||
});
|
||||
|
||||
it("still renders an in-document anchor link intact", () => {
|
||||
const html = renderMarkdown("[jump](#getting-started)");
|
||||
expect(html).toContain('href="#getting-started"');
|
||||
});
|
||||
|
||||
it("keeps header slugs stable across the new quote escaping", () => {
|
||||
// The regression this guards: quotes now become entities *before*
|
||||
// `slugify` sees them, and an entity's letters would otherwise survive
|
||||
// into the id ("claude39s-setup"), silently breaking every
|
||||
// `[…](#claudes-setup)` in the document.
|
||||
expect(renderMarkdown("## Claude's setup")).toContain('id="claudes-setup"');
|
||||
expect(renderMarkdown('## The "safe" mode')).toContain('id="the-safe-mode"');
|
||||
});
|
||||
|
||||
it("still refuses to emit raw tags from the source document", () => {
|
||||
const html = renderMarkdown("<img src=x onerror=alert(1)>");
|
||||
expect(html).not.toContain("<img");
|
||||
expect(html).toContain("<img");
|
||||
});
|
||||
});
|
||||
@@ -12,21 +12,67 @@ function slugify(text: string): string {
|
||||
return text
|
||||
.toLowerCase()
|
||||
.replace(/<[^>]+>/g, "") // strip HTML tags (e.g. from inline code)
|
||||
// Quote characters are escaped to entities before this runs (see
|
||||
// `renderMarkdown`). Drop those two entities whole, so a header with an
|
||||
// apostrophe or a quote slugifies to what it did when the character was
|
||||
// simply stripped — otherwise every such anchor id silently changes and
|
||||
// the in-document links pointing at it stop resolving. `&`/`<`/
|
||||
// `>` are deliberately not in this list: they were already entities
|
||||
// before, so their existing (odd) slugs are the established ones.
|
||||
.replace(/"|'/g, "")
|
||||
.replace(/[^\w\s-]/g, "") // remove non-word chars except spaces/dashes
|
||||
.replace(/\s+/g, "-") // spaces to dashes
|
||||
.replace(/-+/g, "-") // collapse consecutive dashes
|
||||
.replace(/^-|-$/g, ""); // trim leading/trailing dashes
|
||||
}
|
||||
|
||||
/** Simple markdown-to-HTML converter for the help content. */
|
||||
function renderMarkdown(md: string): string {
|
||||
/**
|
||||
* Escape a captured markdown value that is about to be interpolated into an
|
||||
* HTML *attribute* value.
|
||||
*
|
||||
* `renderMarkdown` entity-escapes the whole document first, but that pass only
|
||||
* covered `&`, `<` and `>` — not the quote characters, which is all an
|
||||
* attribute value is delimited by. `[x](https://a" onload="…)` therefore closed
|
||||
* `href="` and started a new attribute, because the URL capture is `[^)]+` and
|
||||
* `"` is in `[^)]`. The document is remote GitHub markdown, so that capture is
|
||||
* not ours to trust.
|
||||
*
|
||||
* Only quotes are escaped here: `&`, `<` and `>` have already been converted by
|
||||
* the caller, and re-escaping the `&` would double-encode every `&` in a
|
||||
* query string.
|
||||
*/
|
||||
function attr(value: string): string {
|
||||
return value.replace(/"/g, """).replace(/'/g, "'");
|
||||
}
|
||||
|
||||
/**
|
||||
* Simple markdown-to-HTML converter for the help content.
|
||||
*
|
||||
* Exported for `HelpDialog.test.tsx`: the output goes to
|
||||
* `dangerouslySetInnerHTML`, so the escaping rules below are security rules and
|
||||
* need to be asserted rather than assumed.
|
||||
*/
|
||||
export function renderMarkdown(md: string): string {
|
||||
let html = md;
|
||||
|
||||
// Normalize line endings
|
||||
html = html.replace(/\r\n/g, "\n");
|
||||
|
||||
// Escape HTML entities (but we'll re-introduce tags below)
|
||||
html = html.replace(/&/g, "&").replace(/</g, "<").replace(/>/g, ">");
|
||||
// Escape HTML entities (but we'll re-introduce tags below).
|
||||
//
|
||||
// The quote characters are part of this on purpose. Everything below builds
|
||||
// HTML by regex substitution, and several of those substitutions drop a
|
||||
// capture straight into an attribute value (`href="$2"`). Leaving `"` and `'`
|
||||
// live meant a link target could close the attribute and open another one —
|
||||
// in a document fetched from GitHub at runtime and handed to
|
||||
// `dangerouslySetInnerHTML`. Escaping here closes every such sink at the
|
||||
// source; `attr()` below is the belt to this pair of braces.
|
||||
html = html
|
||||
.replace(/&/g, "&")
|
||||
.replace(/</g, "<")
|
||||
.replace(/>/g, ">")
|
||||
.replace(/"/g, """)
|
||||
.replace(/'/g, "'");
|
||||
|
||||
// Fenced code blocks (```...```)
|
||||
html = html.replace(/```(\w*)\n([\s\S]*?)```/g, (_m, _lang, code) => {
|
||||
@@ -84,13 +130,15 @@ function renderMarkdown(md: string): string {
|
||||
// Markdown-style anchor links [text](#anchor)
|
||||
html = html.replace(
|
||||
/\[([^\]]+)\]\(#([^)]+)\)/g,
|
||||
'<a class="help-link" href="#$2">$1</a>',
|
||||
(_m, text: string, anchor: string) =>
|
||||
`<a class="help-link" href="#${attr(anchor)}">${text}</a>`,
|
||||
);
|
||||
|
||||
// Markdown-style external links [text](url)
|
||||
html = html.replace(
|
||||
/\[([^\]]+)\]\((https?:\/\/[^)]+)\)/g,
|
||||
'<a class="help-link" href="$2" target="_blank" rel="noopener noreferrer">$1</a>',
|
||||
(_m, text: string, url: string) =>
|
||||
`<a class="help-link" href="${attr(url)}" target="_blank" rel="noopener noreferrer">${text}</a>`,
|
||||
);
|
||||
|
||||
// Unordered list items (- ...)
|
||||
@@ -117,7 +165,8 @@ function renderMarkdown(md: string): string {
|
||||
// Links - convert bare URLs to clickable links (skip already-wrapped URLs)
|
||||
html = html.replace(
|
||||
/(?<!="|'>)(https?:\/\/[^\s<)]+)/g,
|
||||
'<a class="help-link" href="$1" target="_blank" rel="noopener noreferrer">$1</a>',
|
||||
(_m, url: string) =>
|
||||
`<a class="help-link" href="${attr(url)}" target="_blank" rel="noopener noreferrer">${url}</a>`,
|
||||
);
|
||||
|
||||
// Wrap remaining loose text lines in paragraphs
|
||||
|
||||
@@ -457,6 +457,35 @@ export default function BrowserTab({ project, active }: Props) {
|
||||
// host-side gate checks before anything reaches the container.
|
||||
src={status.url ?? undefined}
|
||||
title={`Playwright browser view for ${project.name}`}
|
||||
// What is framed here is served by a process inside the container,
|
||||
// which is the untrusted side of this app. Unsandboxed, it could
|
||||
// simply set `top.location` and navigate the *app's* webview
|
||||
// somewhere of its choosing — the frame is cross-origin, so it cannot
|
||||
// read the app, but steering the whole window is not something a
|
||||
// viewer pane should be able to do.
|
||||
//
|
||||
// The allowances are what the Playwright dashboard actually needs and
|
||||
// no more:
|
||||
// allow-scripts — it is an application, not a document.
|
||||
// allow-same-origin — it must reach its own WebSocket and assets,
|
||||
// and the host-side gate recognises the pane's
|
||||
// own sub-resource requests by their
|
||||
// `Origin`/`Referer`; an opaque origin would
|
||||
// send `null` and be refused. This does not
|
||||
// grant access to *this* app: 127.0.0.1:4782x
|
||||
// is a different origin from the app's.
|
||||
// allow-forms/-modals/-downloads/-popups — dashboard UI affordances
|
||||
// (trace download, confirm dialogs, opening a
|
||||
// page in a new window).
|
||||
//
|
||||
// Deliberately absent, and the point of the attribute:
|
||||
// `allow-top-navigation`, `allow-top-navigation-by-user-activation`
|
||||
// and `allow-popups-to-escape-sandbox`. Do not add them.
|
||||
//
|
||||
// No `referrerPolicy` either: the gate in `browser_view/proxy.rs`
|
||||
// reads the token out of a same-origin `Referer`, so stripping it
|
||||
// would break the pane.
|
||||
sandbox="allow-scripts allow-same-origin allow-forms allow-modals allow-downloads allow-popups"
|
||||
className="flex-1 min-h-0 w-full border-0 bg-[var(--bg-primary)]"
|
||||
/>
|
||||
) : live ? (
|
||||
|
||||
Reference in New Issue
Block a user