Craft Pages: locally-served, agent-authored web pages (behind two flags) - #1017
Draft
anukin wants to merge 44 commits into
Draft
Craft Pages: locally-served, agent-authored web pages (behind two flags)#1017anukin wants to merge 44 commits into
anukin wants to merge 44 commits into
Conversation
Design doc for a local, agent-authored web page capability that reuses existing connectors, credentials, and packaging. Key architectural decisions, after two security review passes: - Pages are served by a dedicated PagesServer with its own node:http listener. They must NOT share the RPC/WS port: WsRpcServer attaches its WebSocketServer to the same http server (transport/server.ts:293) and performs no Origin check on upgrade, so same-origin page JS could upgrade a socket carrying the session cookie and obtain full RPC access. - Agent-authored HTML is untrusted (prompt-injectable via connector data) and is sandboxed via a CSP *response header*, not an iframe attribute, so the sandbox also applies when /p/* is opened directly rather than framed. A trusted wrapper owns all connector access. - Live-data pages are in-app only. No sandbox flag restricts a document navigating its own frame, and CSP's navigate-to was removed from the spec and never shipped, so self-navigation exfiltration is unblockable in a third-party browser. Electron's webRequest can default-deny egress on a dedicated partition; nothing equivalent exists in Chrome. Static pages remain openable anywhere. - Page content is session-scoped; a workspace-level catalog exists only so pageId resolution survives restart. Updates write immutable revision directories and swap a pointer, since a directory cannot be atomically renamed over a non-empty directory on any platform.
`bun run lint` and `bun run validate:ci` both failed on a missing file, and .github/workflows/validate.yml runs validate:ci — so CI could not go green. package.json referenced six scripts that exist only on branches never merged to main: scripts/check-raw-sends.sh scripts/lint-i18n-staged.sh scripts/check-task-tool-checks.sh scripts/lint-i18n-strings.sh scripts/check-i18n-coverage.ts scripts/typecheck-staged.sh All six are restored from history rather than stubbed. A stub that exits 0 is worse than a missing file: the missing file fails loudly, the stub reports green while checking nothing. check-raw-sends.sh is restored with two changes: - Its exclusion list dropped menu.ts and ipc/workspace.ts. ipc/workspace.ts no longer exists and menu.ts no longer contains raw sends, so both globs were stale — and a stale exclusion is a silent blind spot. The only remaining raw webContents.send calls are window-manager.ts:99 (the typed wrapper's own pre-handshake fallback) and browser-pane-manager.ts's toolbar sends (separate preload context, outside BroadcastEventMap). - The original ran rg with stderr suppressed and no `set -e`, so a missing rg produced an empty result and printed "OK". It now requires rg or grep and exits 2 if neither is present. Each guard was negative-tested by injecting a violation and confirming a non-zero exit, not merely observed to pass: check-raw-sends.sh 1750 refs / 885 files scanned; bait caught check-task-tool-checks.sh bait caught check-i18n-coverage.ts 1750 references, 1640 en keys, all resolve lint-i18n-strings.sh 363 .tsx files scanned; bait caught Seven other scripts referenced by package.json are still absent (release.ts, oss-sync.ts, build.ts, check-version.ts, fresh-start.ts, sync-secrets.sh, electron-dev.sh). These are release/dev tooling stripped from the OSS mirror and are not reachable from lint or validate:ci, so they do not block CI.
Two further breaks found once bun was available to actually run the pipeline. Both fail BEFORE the previously-restored lint scripts ever run, so neither was visible from static inspection. 1. bun install --frozen-lockfile failed .github/workflows/validate.yml runs `bun install --frozen-lockfile` before `validate:ci`, so CI aborted at the install step and never reached a single test. bun.lock carried workspace entries for three directories that do not exist in the OSS mirror: apps/marketing packages/craft-agents-commands packages/craft-cli Regenerating the lockfile removes them. The diff is 86 deletions and ZERO insertions — no dependency version moved, this is purely dropping phantom workspaces. 2. tsconfig.base.json was missing Four packages extend ../../tsconfig.base.json: packages/session-tools-core packages/pi-agent-server (x2 configs) packages/session-mcp-server The file was absent, so tsc silently fell back to default compiler options (ES3/ES5 target, no skipLibCheck) and emitted a cascade of errors that looked like real type bugs but were config artifacts: TS1501 regex flag requires es6+ -> needs target ES2022 TS2802 Set iteration needs es2015+ -> needs target ES2022 TS2339 on a discriminated union -> narrowing under old target 12 errors inside @types/cacheable-request -> needs skipLibCheck Restored from history (0e84b1c); like the lint scripts, it lives only on a branch never merged to main. Adding it back clears all of the above with no source changes. Verified on bun 1.3.10 (matching setup-bun in validate.yml): bun install --frozen-lockfile clean, lockfile stable bun run validate:ci exit 0 typecheck:all 8 packages, clean test:shared:all 109 pass / 0 fail test:doc-tools 19 pass i18n parity 6 locales x 1640 keys i18n sorted clean i18n coverage 1750 refs resolve against 1640 keys Note: `bun run lint` still fails, separately and pre-existing — 9 craft-styles/no-nonstandard-shadows errors in renderer components untouched by this branch. `lint` is not part of validate:ci and is not run by CI, so it does not gate the pipeline; worth a separate fix.
…e plan
Throwaway prototype (spike/ws0-pages-security/, delete after the ADR lands)
that runs the WS0 exit criteria against a real engine instead of reasoning
about the spec. Chromium via Electron 39.2.7, macOS arm64.
Four results contradict or extend plan r3. Full evidence in
spike/ws0-pages-security/FINDINGS.md.
1. CSP 'self' WORKS — plan r2/r3 was wrong
r2 claimed 'self' matches nothing for a sandboxed document and that the
policy must name the origin explicitly. Measured: external stylesheet
applied (rgb(0,128,0)), classic script executed. A header-delivered
policy takes its self-origin from the response URL at parse time. The
explicit-origin workaround is dropped.
2. Framed self-navigation is blocked by the WRAPPER'S frame-src
r3 said Electron webRequest was the only control against self-navigation
exfiltration. It is not. Measured:
framed in wrapper BLOCKED ERR_BLOCKED_BY_CSP (wrapper frame-src 'self')
top-level document SUCCEEDED, reached the network
top-level + webRequest BLOCKED ERR_BLOCKED_BY_CLIENT
The embedding document's frame-src governs where a child frame may
navigate — browser-native, no Electron needed. webRequest is demoted to a
second layer for top-level loads.
The top-level result confirms the §2.5 capability split: the page
navigated itself to 127.0.0.1:9999/collect?d=SECRET_PAYLOAD and genuinely
attempted the connection (ERR_CONNECTION_REFUSED). Absence of
allow-top-navigation does NOT stop a top-level document navigating itself.
Consequence stronger than r3 stated: a live-data page must ALWAYS be
viewed framed in the wrapper, never top-level, including in-app. An
"open in browser pane" action forfeits the frame-src protection.
3. ES modules do not execute — not anticipated
<script type="module"> silently failed with NO CSP violation reported.
Module scripts fetch in CORS mode, which is cross-origin from an opaque
origin. Verified by adding Access-Control-Allow-Origin: * — modules then
ran. Decision: classic scripts only, no ACAO header (it would let any
site that learns the port and pageId read page content). The skill must
say so; a model reaches for modules by default and the failure is silent.
4. style-src 'self' — open decision closed by measurement
<style> block blocked (style-src-elem)
style="..." attribute blocked (style-src-attr)
setAttribute('style', …) blocked (style-src-attr)
el.style.prop = … ALLOWED
Keep style-src strict: external stylesheet + CSSOM. Skill forbids inline
<style> and style= — both fail silently (unstyled element, no error).
Also confirmed: window.origin === "null"; localStorage throws; cookies
throw (stronger than r3's "empty string"); event.origin is the literal
"null" so the wrapper must authenticate on event.source (forged message
correctly rejected); connect-src 'none' blocks the page's own data.json,
so page data must ship as executed JS.
Server boundary all pass: encoded traversal 400, backslash 400, ADS 400,
dotfile 400, listing 404, POST/DELETE 405, Host: evil.com 400, bridge
without correct Origin 403.
validateFilePath confirmed unsuitable BY MEASUREMENT, not inference. A
symlink in the page dir pointing at a credentials.enc under $HOME:
validateFilePath() ACCEPTED -> …/.craft-agent/credentials.enc
our containment guard 400 Rejected
Refinement: validateFilePath does carry a 9-pattern sensitive-file
denylist, but credentials.enc matches none of them (credentials\.json$
needs .json; secrets?\. needs "secret."). The hole is narrower than
"allows all of $HOME" but lands exactly on the credential store.
Not yet run: Firefox, Safari, Windows, Linux. Required before WS2 lands.
Adds the WS0 decision record (docs/adr/0001-craft-pages-trust-model.md) and
extends the spike to a second engine. Safari/WebKit produced one finding
that Chromium alone could never have surfaced.
HEADLINE: never combine the iframe sandbox attribute with the CSP sandbox
header.
variant Chromium WebKit
iframe sandbox attr + CSP header scripts run NO SCRIPTS AT ALL
CSP sandbox header only scripts run scripts run
With both applied Safari executed nothing — not even the first <head>
script — and the wrapper timed out with no results. Plan r3 explicitly
recommended keeping the attribute "as belt-and-braces"; that advice is
withdrawn. Header-only works in both engines and still enforces
containment. This is a silent total failure that presents as a blank page,
which makes it worse than either control alone.
It also explains an earlier confusing run: the first Safari framed attempt
reported "no results from frame within 8s" with zero beacons. That was
this bug, not a reporting failure.
Cross-engine agreement otherwise total. WebKit matched Chromium on all 14
measured behaviours, including the two the plan's architecture depends on:
framed self-navigation blocked in both (wrapper frame-src)
top-level self-navigation SUCCEEDED in both (nothing stops it)
So the §2.5 capability split is cross-engine, not a Chromium artefact.
Instrumentation added so non-Electron engines are measurable at all:
- exfil-catcher.ts listens on the self-nav target port, making "did the
exfil succeed?" a network observation rather than an engine-specific
devtools query
- /internal/beacon uses img-src 'self' (permitted under connect-src
'none') to report script progress from inside the sandbox
- /internal/results lets the non-sandboxed wrapper POST its findings
Neither needs WebDriver, so the same rig will run on Firefox, Windows and
Linux unchanged.
METHODOLOGY FIX worth recording: the first top-level runs reported
"self-navigation blocked" in BOTH engines. That was an artefact — the
harness's own top-nav probe does window.top.location.href = …, and when
the document IS top-level, window.top === window, so the probe navigated
the page away mid-run and destroyed every later measurement. Corrected by
skipping that probe when top-level; the real result is the opposite. A
probe that destroys its own experiment is easy to reintroduce, so it is
documented in FINDINGS.md.
WS0 exit criteria are met for Chromium and WebKit. Gecko deferred by
decision (not a target platform; rig runs there unchanged). Windows and
Linux remain unverified — the containment guard's path handling is the
risk, to be covered by OS-independent unit tests before WS2 lands.
First shipping code for Craft Pages. Registry-mode tools, so both the
Claude and Pi backends pick them up with no backend-specific code and
neither parity assertion applies (verified: the only backend-mode tools
remain call_llm, spawn_session, browser_tool).
Two tools, not one. safeMode is a single value per tool def, so folding
delete into craft_page (safeMode 'allow') would make destructive deletion
reachable in Explore — the read-only mode. craft_page_delete is therefore
separate with safeMode 'block', and additionally requires confirm: true so
one malformed call cannot destroy work.
DEVIATION FROM plan.md §2.3 — no current.json pointer.
The plan proposed writing a revision then atomically swapping a small
pointer file, because a directory cannot be renamed over an existing
non-empty directory. The reasoning is right but the pointer reintroduces
the same problem one level down: replacing an existing FILE by rename is
not reliably atomic on Windows either (fs.rename can EPERM when the target
is open), which is exactly why persistence-queue.ts:159 unlinks first and
accepts a gap.
Renaming a directory onto a name that does NOT yet exist IS atomic
everywhere. So a revision is staged at revisions/.staging-{n} and renamed
to revisions/{n} as the single commit step, and the current revision is
the highest complete revision directory. A crash leaves either a complete
revision or a staging dir that is ignored on read and swept on next write.
No pointer, no gap, one fewer failure mode. Covered by three crash-safety
tests.
pages/naming.ts is deliberately PURE STRING LOGIC with no fs access. WS0
could only be run on macOS (ADR 0001, "Not yet verified"), and the
containment guard's biggest risk is Windows path handling. Making the
rules pure means they are testable from any OS today rather than whenever
a Windows machine appears. It rejects, with tests for each:
backslash separators drive-relative paths and ADS (":")
reserved device names trailing dot/space (Windows strips these,
incl. CON.txt, com1.png so "a " and "a" silently collide)
percent-encoding dotfiles at any depth
"." / ".." segments extensions outside an allowlist
case-only collisions (round-trip on Linux, overwrite on macOS/Windows)
Slug capped at 48 chars for MAX_PATH headroom — the full prefix is
{workspaceRoot}/sessions/{uuid}/data/pages/{slug}/revisions/{n}/public/...
Other decisions worth noting:
- update PATCHES by default and carries forward unmentioned files;
replaceAll must be asked for explicitly. Defaulting to replacement would
make a model editing one file silently delete the rest of the page.
- expectedRev gives optimistic concurrency; a mismatch is a recoverable
tool error telling the model to re-read, not a silent clobber.
- pageId is minted app-side with randomUUID and never accepted from the
agent — a model-chosen id could collide with or impersonate another page.
- page.json lives OUTSIDE public/, so the served tree cannot expose it.
Asserted by test.
- The fence returned to the model carries {pageId, rev}. rev is not
decoration: WS5 keys the preview on pageId:rev, and without it an edited
page re-renders the cached revision and appears not to have changed.
- ctx.pageCatalog is optional and the handlers degrade gracefully without
it, matching the createTask precedent. Its docstring records WHY the
implementation must serialize mutations.
Tool descriptions state the constraints WS0 measured, because every one of
them fails silently: external scripts only (no inline, no type="module"),
external stylesheet only (no <style>, no style=, use CSSOM), data as JS
not fetched JSON, no localStorage, no <form> submit, no external network.
Tests: 59 new (26 naming, 21 store, 12 handler). session-tools-core suite
148 pass / 0 fail. typecheck:all clean. Both derivation paths verified —
Claude .shape and Pi getToolDefsAsJsonSchema, including the nested files
array.
validate:ci still fails only on the pre-existing flaky
default-thinking-level test (12 heavy bun subprocess spawns inside one
test against bun's hard-coded 5s default). Unrelated to this branch;
confirmed earlier by stashing. Every other leg exits 0.
Two things that were blocking or would silently rot. 1. validate:ci is now green (exit 0) default-thinking-level.test.ts "supports every thinking level" spawns 2 bun subprocesses per level — 12 total, each importing storage.ts and its dependency graph. At ~600ms a spawn that is ~7s of real work against bun's hard-coded 5s default, so it passed only when spawns averaged under ~415ms. It failed on a loaded machine and passed on an idle one, which is what made it read as flaky. Fixed with an explicit 60s timeout on that single test. No assertion changed — all 6 levels still round-trip through a real subprocess. Only the budget now matches the work. This was pre-existing and unrelated to the pages branch (confirmed by stashing and reproducing on a clean tree), but it made every "CI is green" claim conditional, so it is fixed rather than carried. 2. lint:page-sandbox guards ADR 0001 D2 WS0 measured that combining an iframe `sandbox` attribute with the CSP `sandbox` response header makes WebKit execute NO scripts at all, while Chromium is unaffected. The failure is a blank page in Safari with no error and nothing in a diff to notice. Adding the attribute "as well, to be safe" is the natural instinct — it is exactly what plan r3 recommended before the spike disproved it — so a reviewer will not reliably catch its return. It gets a lint guard instead of a comment. Scoped to files that render or serve Craft Pages content, so legitimate sandboxed iframes elsewhere (html-preview, OAuth) are untouched. Matches HTML and JSX, across newlines, attributes in any order. The guard refuses to report success if none of its scopes exist (exit 2) — a lint that checks nothing while printing OK is worse than no lint, the same principle applied to check-raw-sends.sh earlier on this branch. Negative-tested: reintroducing the attribute in the spike wrapper produces exit 1 with the ADR reference; removing it returns to exit 0. Wired into both `lint` and `validate:ci`.
Server-core half of Step 3. Dedicated listener, real containment, and the workspace index that makes a pageId resolvable in a cold session. 53 tests, including the WS0 security matrix as a suite that now runs in CI rather than living in a spike. containment.ts — NOT validateFilePath Two independent layers. The string rules are IMPORTED from session-tools-core rather than reimplemented, so the authoring side and the serving side cannot drift — two copies of a traversal guard diverging is precisely how holes appear. Added @craft-agent/session-tools-core as a server-core dependency for this; verified acyclic (session-tools-core has no craft deps). The fs layer adds what strings cannot see: canonicalise, contain, and reject symlinked components. Rejecting ANY symlinked component matters — checking only the final realpath misses a symlinked *directory* mid-path, which is covered by a test. Double-encoding gets explicit handling: %252e%252e decodes once to %2e%2e, which a second decode turns into "..". We decode exactly once and then refuse any remaining "%", so a double-encoded value is rejected rather than quietly normalised. Pretty URLs: an extensionless final segment resolves to its index.html. The reference target uses /en/ style URLs and links routinely drop the trailing slash, so 400-ing those would break the exact site class this feature exists for. Done as a pure string rule, no filesystem probe before containment is established. catalog.ts — serialized, rebuildable Every mutation goes through one promise chain. Unserialized read-modify-write on a single JSON file loses entries even inside one event loop: `await readFile` yields, a second caller reads the same stale copy, last writer wins. Mutation-tested — removing the chain fails exactly the two concurrency tests and nothing else. A failed write does not poison the chain (the queue swallows rejection while the caller still sees it), and reconcile() rebuilds the index from per-page manifests, so a corrupt or deleted catalog costs nothing. The manifests are the source of truth; this is only an index. handler.ts / server.ts Own node:http listener, loopback only, EADDRINUSE fall-forward. Never the RPC port: WsRpcServer attaches its WebSocketServer to the same http server and does no Origin check on upgrade, so a shared origin would let page JS upgrade a socket carrying the HttpOnly session cookie. Written against node:fs with no Bun globals, because webui/http-server.ts is Bun-only and would throw under Electron's Node. Separate CSPs, asserted by test: page content gets `sandbox allow-scripts` + connect-src 'none'; the wrapper gets no sandbox and frame-src 'self', which is what blocks framed self-navigation off-origin in every browser. The wrapper is inlined as strings, not shipped as files. electron-builder `files` globs silently no-op on missing directories — resources/ session-mcp-server/** and resources/pi-agent-server/** are both listed today and neither exists — so a file-based wrapper would go missing only in packaged builds. Also fixes a false positive in check-page-sandbox.ts: a test ASSERTING the attribute is absent necessarily contains both tokens, and an unclosed '<iframe' string fragment made the tag regex over-match. Test files are now excluded; re-verified that the guard still catches a real violation in a non-test file. validate:ci exit 0.
Written test-first this time, per review feedback. Each test was observed failing before the code that satisfies it existed; the progression is visible in the ordering below. Closes four of the five wiring gaps that left WS1/WS2 as 112 passing tests of components nothing in the app touched. 1. FEATURE_FLAGS.craftPages — default off, CRAFT_FEATURE_CRAFT_PAGES=1|0 Tests written first, failed at import (no such export), then passed. One test pins that the flag re-evaluates at ACCESS time rather than being captured at module load — otherwise gating would depend on import order. 2. ctx.pageCatalog binding — the load-bearing one Without it craft_page succeeds, writes files, and produces a page the server can never resolve. The handler degrades gracefully by design, so the failure mode is an INVISIBLE page rather than an error. That is exactly the kind of gap a test-after would have missed, because the implementation "worked". Follows the existing createTask pattern: a field on the callback registry plus a lazy getter. A test pins that it resolves from the registry on every access, not at attach time — the context is constructed before the backend registers callbacks, so a captured value would be permanently undefined. The end-to-end test caught a real gap the unit tests could not: handleCraftPage was not exported from the session-tools-core barrel, so the handler was unreachable from the package's public API. 3. PagesRuntime — per-workspace catalog + listener lifecycle New module rather than more logic inside SessionManager (5000+ lines), which also makes the lifecycle testable without standing up a session. Gating is enforced HERE, not at the call site: ensureStarted() returns null when the flag is off so no listener is ever bound. A flag consulted only where prompt text is assembled would leave a live HTTP server. Carries an in-flight guard because two sessions in one workspace can call ensureStarted() simultaneously and would otherwise bind two listeners and leak one. Mutation-tested: removing the guard fails exactly that test and nothing else. Reconciles the catalog from per-page manifests on start, fail-soft, so pages created before a restart remain resolvable and a broken index is never a reason to refuse to serve. 4. pages:getUrl / pages:list channels, classified LOCAL_ONLY Nice property of the existing tooling: adding the channels made the repo's own routing exhaustiveness guard fail until they were classified, so the second failing test came for free. LOCAL_ONLY is deliberate. Pages are served by a loopback listener on the machine owning the workspace; on a thin client the remote's 127.0.0.1 URL is unreachable, so proxying would hand back a dead address. Craft Pages is explicitly unsupported for remote workspaces (ADR 0001 §9), and this classification makes that fail cleanly rather than silently. Still unwired (WS3, needs the app running to verify): SessionManager does not yet construct a PagesRuntime or register the catalog into the per-session callbacks, and no RPC handler implements the two channels. The seams they plug into are now tested and in place. validate:ci exit 0. Flag off verified end-to-end: ensureStarted -> null, isRunning -> false, no port bound.
…eardown Test-first throughout; every test observed failing before its implementation existed. Closes the fifth wiring gap: craft_page now registers what it creates, and the renderer has a sanctioned way to get a page URL. pages RPC handlers The renderer never builds a page URL itself — the port is chosen at runtime and moves on conflict, so a client-side URL would go stale silently. GET_URL always returns the WRAPPER (/w/), never /p/ directly: a page loaded top-level loses the frame-src protection that blocks self-navigation exfiltration (ADR 0001 D6), and a test asserts the returned URL contains no /p/. Every handler degrades to null/[] rather than throwing when the feature is off or no runtime is wired. A UI asking for a page URL on a host that cannot serve one should render nothing, not an error dialog. PagesRuntime.ensureStarted now fails closed for a missing workspace Found by a test I expected to pass. resolvePageCatalogForSession's "never throws" case returned a catalog for a bogus path: nothing threw, so a listener was bound for a workspace that does not exist, yielding a catalog that can never resolve anything — a page URL 404ing forever looks like a bug in the feature rather than a bad path. The same investigation turned up why the new test initially failed even after the fix: PageCatalogService.write() mkdirs the workspace root, so an earlier run of the failing test CREATED the directory it asserts is absent. The test now nests its path under a fresh mkdtemp so no prior run can pollute it. SessionManager - constructs one PagesRuntime; lazily starts per workspace - hands each session its catalog via mergeSessionScopedToolCallbacks - cleanup() disposes the listeners Gating and start-ordering live in resolvePageCatalogForSession rather than inline, so they are testable without instantiating SessionManager (5000+ lines) — the same shape the repo already uses for archive-guards. Ordering matters: the catalog does not exist until the runtime has started, so asking for it first silently yields undefined. The teardown test is a real integration test, not a proxy: it constructs a SessionManager, starts a listener, calls cleanup(), and asserts the port is released. Without it the loopback servers outlive the manager and every workspace switch leaks a port. pages:getUrl / pages:list classified LOCAL_ONLY, wired into registerCoreRpcHandlers, HandlerDeps extended with an optional pagesRuntime. 150 Craft Pages tests across three packages, 0 fail. validate:ci exit 0. Remaining for WS3, all Electron-side and unverifiable without launching the app: dedicated session partition with webRequest egress deny, adding that partition to network-proxy.ts, and the renderer frame-src CSP change.
Found by loading a page in Chrome. The entire feature was broken and all 150 tests passed. Cross-Origin-Resource-Policy: same-origin was applied to page content. A sandboxed page has an OPAQUE origin, which is never same-origin (or same-site) with anything — so the browser fetched every subresource and then DISCARDED the response. No script executed, no stylesheet applied. The failure was near-invisible from the server side: the requests genuinely arrive, so the access log looks perfectly healthy. Chrome fetched index.html, styles.css, data.js, app.js and the image, and rendered nothing. Diagnosing it needed staged beacons — a probe fired at the very top of app.js, before anything could throw — to distinguish "script never ran" from "script threw partway". bun's fetch() does not enforce CORP, which is why the whole suite was green against a feature that could not work. This is the class of defect only a real browser finds, and the argument for verifying in one before calling a serving layer done. Fix: page content gets 'cross-origin'; the wrapper, a normal same-origin document, keeps 'same-origin'. Not a weakening — page resources sit behind Host pinning on a loopback listener, carry no CORS headers (a foreign origin can embed but never READ them), and are addressed by an unguessable pageId. Two regression tests added first, both observed failing: - page content must not send same-origin or same-site - the wrapper must still send same-origin Verified in Chrome against the unmodified production handler: scripts ran (all 5 staged probes) data-as-JS 3 classes <- connect-src 'none' workaround holds external CSS rgb(122, 75, 42) <- style-src 'self' applies CSSOM write rgb(26, 127, 55) <- permitted without unsafe-inline origin null <- opaque, as designed localStorage threw, handled <- guidance is correct relative img loaded <- relative paths resolve That also confirms every constraint the authoring skill will teach is actually workable, not just theoretically permitted. validate:ci exit 0.
Test-first. The egress decision is a pure predicate so the policy is testable without launching Electron. Second layer, not the primary one. The wrapper's frame-src blocks a FRAMED page from navigating itself off-origin and is browser-native (ADR 0001 D6). But frame-src does not apply to a TOP-LEVEL document, and WS0 measured that a top-level sandboxed page CAN navigate itself anywhere — no sandbox flag restricts it and CSP's navigate-to was removed from the spec. This deny-list is what catches that case in Electron. Nothing catches it in a third-party browser, which is why live-data pages stay in-app. A mutation test caught my own bad test. Replacing the origin comparison with `url.startsWith(pagesOrigin)` left all 11 tests GREEN, so the prefix-collision case I had written proved nothing. Both variants — `http://127.0.0.1:51234.evil.com` (non-numeric port) and `http://127.0.0.1:512345` (port > 65535) — are rejected by URL parsing before the origin check is ever reached. The case that actually bites is a userinfo collision: `http://127.0.0.1:51234@evil.com/x` parses cleanly to host evil.com — the part before "@" is userinfo — while the string begins with the pages origin verbatim. Re-running the mutation now fails exactly that assertion. Both variants are kept, relabelled to say which layer each exercises. Worth recording because the first mutation run looked like a pass and I nearly reported it as proof. Also wired: - CRAFT_PAGES_SESSION_PARTITION, distinct from the browser-pane partition. Sharing that jar would put agent-authored pages in the same cookie and storage space as every site the agent has browsed. - network-proxy.ts applies the proxy to the pages partition. Omitting it silently breaks every corporate-proxy user: pages load nothing, with no actionable error. - renderer index.html gains `frame-src 'self' http://127.0.0.1:*`. The meta CSP was default-src 'self' with NO frame-src, so it fell back to 'self' and the wrapper iframe was blocked outright. This is the first time the app shell frames a foreign origin and wants a deliberate look. It is scoped to loopback, and the pages listener binds 127.0.0.1 only. applyPagesEgressPolicy takes a getPagesOrigin CALLBACK rather than a value: the port is resolved at runtime and can change on conflict, so capturing it once would pin a stale origin and block the real one. 12 tests. validate:ci exit 0. Not yet done: constructing the partition and attaching the policy inside Electron's window creation — that needs the app running to verify, and is the last piece of WS3.
Two things, both from wiring WS3 and finding the policy had nowhere correct
to attach yet.
1. createPagesSession owns its own fromPartition call
applyPagesEgressPolicy took a session as a parameter, which made it possible
to hand it defaultSession — and a default-deny egress filter on
defaultSession cancels every request the ENTIRE APP makes: the RPC socket,
model calls, OAuth, updates. That is a catastrophic-if-wrong API taking the
dangerous value as an argument.
Creation is now part of the function, so the wrong session cannot be passed.
Tested via an injected factory, asserting the partition it asks for.
2. ADR 0001 D6a — the deny-list has a limited attachment point
An <iframe> in the renderer runs in the MAIN WINDOW's session. Electron has
no per-iframe partition and webviewTag is false
(window-manager.ts:261), so persist:craft-pages CANNOT cover the in-app
iframe surface WS5 plans. I nearly wired this into window creation where it
would have been inert.
The controls divide cleanly instead:
framed iframe in renderer -> wrapper frame-src 'self' (browser-native,
measured in Chromium and WebKit)
dedicated WebContentsView -> pages partition + webRequest deny-list
top-level, third-party -> nothing; hence live-data pages are in-app only
This narrows D6, it does not contradict it: "live-data pages are always
framed" holds either way.
Leaves one genuine WS5 decision: whether the in-app surface is an iframe
(simple, relies solely on frame-src) or a WebContentsView in the pages
partition (isolated, egress enforced, heavier lifecycle and layout).
17 tests. validate:ci exit 0.
Visual verification of the production stack in Chrome. The feature works — the page renders correctly framed in the wrapper, with classes and glaze swatches generated by app.js, external CSS applied, CSSOM styling working, origin=null and localStorage=SecurityError handled. But two tooling artifacts made a WORKING page look broken, and both cost real time. Recorded so WS5 does not pay for them again. 1. A screenshot of an out-of-process iframe can capture blank The wrapper appeared to render nothing: dark and empty, repeatedly. The frame HAD loaded — load fired, contentWindow present, box measured 1200x1200 — and the same page rendered perfectly standalone. Forcing a reflow made it appear instantly. A cross-origin iframe composites out-of-process and the capture can beat the paint. I chased this as a real bug through three wrong hypotheses (CSP inheritance into sandboxed children, flexbox sizing, CORP) before a controlled experiment disproved the first and a live reflow exposed the truth. Wait or force a reflow before screenshotting a framed page. 2. Devtools instrumentation does not cross the opaque-origin boundary For a sandboxed frame the extension's network log shows the frame document but NOT its subresources, and its console shows nothing from inside. I read "no subresource requests" as "subresources blocked" — wrong, they are merely invisible. Caught by checking the WS0 spike, whose frame demonstrably ran (it POSTed results) while showing the same empty list. The reliable instrument remains the one the spike already used: an img-src 'self' beacon reporting to the server. Works in every engine, needs no automation, sees what devtools cannot. Synthetic click/type did not reach the framed document either; script execution, DOM generation, CSS and CSSOM were verified visually instead. Worth stating plainly: the CSP inheritance hypothesis was DISPROVED by a controlled experiment — 'self' resolves correctly in a framed sandboxed document, so ADR 0001 D4 stands unchanged. No production code changed. validate:ci exit 0.
The iframe surface, per the D6a decision. Verified end to end in Chrome: fence -> card -> RPC-resolved wrapper URL -> trusted wrapper -> sandboxed iframe -> agent-authored page, with classes from data.js, glaze swatches set via CSSOM, and origin=null / localStorage=SecurityError confirming the sandbox is real. Compact card, not an inline live iframe. Every authoring turn emits a new fence in a new message, so an inline design leaves N live pages stacked in the transcript, each its own renderer process. The card is cheap; the page expands on demand. rev is REQUIRED in the fence, and parsing rejects a spec without it rather than defaulting. The frame key is pageId:rev, so an edited page genuinely remounts — key on pageId alone and React reuses the element, the browser reuses the cached document, and "make the header blue" appears to do nothing. That is the single most likely way this feature feels broken, so the parser refuses to let it happen silently. 11 tests, written first. Both fence dispatch sites wired. The map in Markdown.tsx is duplicated (inline ~296 and block ~438); my first patch also over-applied because the 8-space pattern is a substring of the 10-space one, producing three dispatches. Verified there are now exactly two. The iframe carries no sandbox attribute (ADR D2) and points at the WRAPPER, never /p/ directly — framing is what supplies frame-src 'self', the control that blocks a page navigating itself off-origin. check-page-sandbox.ts now covers the new component (17 files) and passes. onResolvePageUrl added to PlatformActions: the renderer must never build a page URL itself, because the listener picks its port at runtime and can move. Resolution goes through pages:getUrl. Playground entry under Markdown -> CraftPageBlock, with variants for the ready state, a bumped rev, no title, and both malformed cases. Backed by scripts/dev/craft-pages-demo.ts, which serves a sample page through the REAL handler so what the browser exercises is what ships. playground.html CSP needed the same treatment as the renderer: it had no frame-src and no 127.0.0.1 in connect-src, so the fetch and the frame were both blocked. Good confirmation that the renderer CSP change in WS3 was load-bearing rather than precautionary — this is exactly the failure it prevents. i18n: 5 keys across all 7 locales, parity and coverage green. validate:ci exit 0.
WS6. Without these the model reaches for type="module", inline styles and
fetch() by default — and every one of those fails SILENTLY, so it cannot
self-correct.
Built-in skill tier (ADR 0001 §7)
loadAllSkills() scans ~/.agents/skills, {workspace}/skills and
{project}/.agents/skills. All three are user-owned directories the app must
not write to, so a skill that ships WITH the app and updates WITH it had
nowhere to live. An earlier revision of the plan proposed syncing into
~/.craft-agent/ — which is not a skill search path at all, so the skill
would simply never have loaded.
Copying into ~/.agents/skills instead forces a choice between clobbering
user edits on every update and going permanently stale. A fourth 'builtin'
tier read from getBundledAssetsDir('skills'), loaded FIRST so all three
existing tiers override it, avoids both: it updates with the app, and a
user copy always wins. Six tests, including override-by-workspace and
override-by-project.
Packaging needs no new glob: copy-assets.ts does cpSync('resources',
'dist/resources', {recursive:true}), so resources/skills ships via the
existing dist/**/* rule. That matters — electron-builder globs silently
no-op on missing directories, and two such dead globs are already in the
config.
The skill itself states only measured constraints, each with the reason it
fails silently. Nothing in it is guessed; all of it came out of WS0 and the
Chrome verification.
Prompt section
Extracted as getCraftPagesPromptSection() rather than inlined, because
testing it through getSystemPrompt() requires a real ~/.craft-agent install
(config-defaults.json) — that would have made it an environment test rather
than a behaviour one. Same shape the repo already uses for archive-guards.
Returns '' when the flag is off, so the model is never told about a tool
that is not registered. A test pins that it is evaluated per call rather
than captured at module load, and another caps its length: the system
prompt is static per session and must stay small for prompt caching, so the
section points at the skill instead of restating it.
Verified end to end: with the flag on and bundled assets rooted at
apps/electron, loadAllSkills finds craft-pages with source 'builtin'.
validate:ci exit 0.
Page content is session-scoped, so deleting a chat deletes its pages. Two obligations, both now met. Tell the user first. countPagesInSession() feeds the confirmation, and the i18n keys are phrased "Delete chat and 2 pages" / "Cancel" — deliberately NOT a Yes/No. Declining cannot mean "delete the chat but keep the pages", because that outcome does not exist, and a Yes/No dialog implies it does. The count is read from DISK rather than the catalog: a page whose catalog entry was lost is still about to be deleted, and the user should be warned about it. It also works when the pages runtime was never started. Clean up the index. purgeSessionPages() drops catalog entries before SessionManager removes the session directory, so the catalog never points at directories that no longer exist. Two deliberate non-behaviours, each pinned by a test: - purge does NOT delete page files. The caller removes the session directory wholesale; two owners of the same destructive operation is how partial deletions happen. - purge never throws. Deletion is already underway by the time it runs, and a catalog problem must not leave the user unable to delete a session. The test injects a catalog that throws and asserts the call still resolves. 10 tests. pages:countForSession added, classified LOCAL_ONLY, exposed through channel-map. Plural-aware i18n across all 7 locales. validate:ci exit 0.
WS4 plus the packaging guard from WS6.
pages:getUrl now returns { url, canOpenExternally } instead of a bare
string. The flag exists NOW, before grants do, so WS7 cannot forget it:
ADR 0001 D6 says a page holding connector grants must never load as a
top-level document, because frame-src — the control that blocks a page
navigating itself off-origin — does not apply there, and nothing replaces
it in a third-party browser. Grantless pages hold nothing worth
exfiltrating, so today it is always true; WS7 swaps the constant for a
grant-store lookup and the UI needs no change.
The card only renders "Open in browser" when the SERVER says so. The
renderer does not decide this, and cannot construct the URL itself either —
the listener picks its port at runtime and can move.
Packaging assertion in scripts/build/win32.ts: the shipped skill must exist
in dist/resources after the copy, or the build fails. electron-builder
`files` globs silently no-op on a missing directory, and with asar:false
there is no integrity error either, so a missing bundled asset surfaces
only in a packaged build — as a feature that quietly does nothing. Two dead
globs (resources/session-mcp-server, resources/pi-agent-server) are already
in electron-builder.yml, which is exactly the failure this prevents.
Verified the recursive copy does carry resources/skills through.
i18n across all 7 locales. validate:ci exit 0.
The card existed but nothing in the app supplied onResolvePageUrl, so every
page rendered its "unavailable" state outside the playground. Wired into
App.tsx's platformActions alongside the existing file/URL capabilities.
Derives windowWorkspaceRootPath from the bound workspace, because the pages
RPCs are keyed on the absolute path — a page lives under
{workspaceRoot}/sessions/{id}/data/pages.
Returns null on any failure (feature off, unknown page, no workspace bound)
so the card degrades to "unavailable" rather than throwing. The renderer
never constructs the URL itself: the listener picks its port at runtime and
can move if the preferred one is taken.
ElectronAPI.getPageUrl signature updated to the { url, canOpenExternally }
shape — the typechecker caught the mismatch, which is the argument for the
type living in one place.
Note for anyone running the shared prompt tests: six in
src/prompts/__tests__ fail on a machine with no ~/.craft-agent install,
because getSystemPrompt() reads config-defaults.json. Pre-existing and
environmental — verified by seeding the file, after which all 22 pass. It
is also why the Craft Pages section is an extracted, separately-testable
unit rather than inlined.
validate:ci exit 0.
The authorization half of live connector data, before any wire is connected. 39 tests. Constrained parameter schemas, not JSON Schema A grant's parameter schema is authored by the AGENT, which in this threat model is attacker-influenced — a hostile email can steer what the model writes. Accepting arbitrary JSON Schema would hand a hostile page a parser to attack, so the vocabulary is closed: string with a length cap, bounded integer, boolean, bounded enum. $ref, pattern (ReDoS), objects, arrays and unbounded strings are all rejected at APPROVAL time, before any value is ever evaluated against them. Undeclared parameters are rejected, never ignored — silently dropping a key hides a mismatch between what the page asked for and what actually ran. __proto__ is rejected explicitly, since a JSON payload can carry it as an own property. Curated allowlist, checked twice A tool name in a grant is not authorization. MCP tool names and readOnlyHint annotations come from the MCP server itself, so a compromised one can call a mutating tool read-only. The allowlist is ours, per source, and consulted at approval AND at execution — a tool can leave the list after a grant was issued, and an old grant must not outlive that decision. Curated, never inferred: `list_everything_ever` is not trusted because of its shape. A test pins that. Grant store outside agent-writable space page.json carries the agent's REQUEST; page-grants.json carries the user's DECISION, in the workspace root rather than any session directory. That is what makes "hand-editing page.json grants nothing" a fact rather than a claim. Reads fail CLOSED — an unreadable authorization record means nobody is authorised. Mutations serialized, like PageCatalogService. querySetHash drives re-consent, so restyling a dashboard does not nag the user about permissions they already gave. A mutation test corrected a wrong claim of mine I asserted resolveArgs spreads fixedArgs last "so a page cannot override them", and mutation-testing showed reversing that spread fails NOTHING. The ordering is unobservable: approve() rejects grants whose parameters collide with fixed arguments, so the objects always have disjoint keys. The real guarantee is that collision check — removing it fails immediately. The test is renamed to say what it actually verifies, and both the test and the implementation now record why the ordering is belt-and-braces. Kept, so the safe order survives if the collision check is ever relaxed. Also mutation-verified: dropping the execution-time allowlist recheck fails exactly its test. validate:ci exit 0. Not yet wired: the bridge endpoint, the workspace pool, and consent UI.
…sent RPC Completes live connector data. 314 Craft Pages tests across five packages. The bridge — the one endpoint a hostile page can reach Origin pinned to the pages origin, no CORS headers, preflights refused rather than answered, POST only. Body size-capped BEFORE parsing, at both the transport layer and the bridge, because Content-Length is a hint a chunked body can lie about. Per-PAGE rate limit, keyed on pageId rather than grantId so a page cannot multiply its budget by holding several grants. Timeout, response-size cap, Cache-Control: no-store. Errors are OPAQUE. A test feeds the executor "401 Unauthorized: token abc123 for https://gmail.googleapis.com/v1" and asserts none of the token, host or status reaches the page — it gets `upstream_error`, the log gets the detail. An upstream error echoed into the sandbox is an information channel straight out of it. Workspace-scoped pool McpClientPool is per-session and torn down on close, so nothing is connected when a user opens a saved page. This one's lifetime is the workspace: lazily built, idle-shutdown so stdio MCP subprocesses do not linger, with an in-flight guard because two concurrent page loads would otherwise each spawn a full set and leak all but one. Mutation-verified. Proxy names go through proxyToolName, now exported from the mcp barrel — per packages/shared CLAUDE.md that builder must be the only implementation or the dispatch key drifts (craft-ai-agents#864). Framed-only enforcement is now real, not a constant canOpenExternally is a grant-store lookup, and it fails CLOSED: if the store cannot be consulted we refuse to offer "open in browser". Refusing costs a convenience; wrongly offering costs the guarantee. /p/* refuses Sec-Fetch-Dest: document for a grant-holding page. An end-to-end test asserts the same URL returns 403 top-level and 200 framed — the supported way to view it is the way that supplies frame-src. The wrapper chrome now names the live connectors, because a page reading your mail must not look identical to a static one. A bug I introduced and caught Wiring the bridge made /internal/query a POST, but nodeAdapter never read request bodies — it was written when only GET/HEAD reached it. Every live query would have arrived as malformed JSON. The comment I wrote claiming it forwarded bodies was simply false. Test added first, then fixed, with its own size cap so the transport does not buffer an unbounded upload just to hand it to something that will reject it. Consent RPC: listGrants / approveGrants / revokeGrants, LOCAL_ONLY. Approval is a USER action routed through the app — never something a page or an agent can perform. Approval is per-query so one bad request does not silently drop the rest, and the UI can say which was refused and why. validate:ci exit 0.
SessionManager now installs a real pool builder, so the bridge reaches actual connectors rather than throwing "no connector pool configured". The workspace pool is deliberately NARROWER than a session pool: only sources exposing a tool on the trusted read-only allowlist are connected at all. Connecting anything else would spawn stdio MCP subprocesses and refresh OAuth tokens for capability a page can never call, and a source that is never connected cannot be called by mistake. A failing source is logged and skipped rather than taking the pool down — the page simply cannot use that connector. ADR marked implemented through WS7, with three notes the build surfaced: - the resolveArgs merge order is belt-and-braces; the collision check in approve() is the actual guarantee (mutation-verified) - canOpenExternally fails closed when the grant store is unreadable - the workspace pool's narrowing, and why plan.md now states what is actually left, which is not code: - Consent UI. The RPC exists and approval is a user action routed through the app, but the dialog is unbuilt. Until it is, no page can hold grants, so live data is inert and every page is grantless and static. - Windows/Linux verification of the filesystem containment guard — the largest carried risk, and one this machine cannot close. - Gecko, deferred by decision. validate:ci exit 0.
The plan still described a current.json pointer that the implementation
deliberately does not use, and two later sections referenced that file as
something the containment guard must reject.
The pointer's reasoning was right as far as it went — a directory cannot be
renamed over an existing non-empty directory — but replacing an existing
FILE by rename is not reliably atomic on Windows either (fs.rename can
EPERM when the target is open), which is why persistence-queue.ts:159
unlinks first and accepts a gap. A pointer moves the same problem down one
level.
Renaming a directory onto a name that does not yet exist IS atomic
everywhere, so a revision stages at revisions/.staging-{n} and renames to
revisions/{n} as the single commit, with the current revision being the
highest complete directory. A design doc that describes something the code
does not do is worse than no doc.
validate:ci exit 0.
A full-stack integration test found that the live-data path was never
connected: WS7 built the grant store, bridge and connector pool, but the
wrapper still answered every query with `live_data_unavailable`. Nothing
caught it because every test entered at the bridge — which is where the
wrapper enters, not where a page does. A page cannot reach /internal/query
itself (CSP `connect-src 'none'`, opaque origin refused by the Origin pin),
so the first real hop is a postMessage into the wrapper, and that hop had
no tests at all.
wrapper-asset.test.ts evaluates the shipped WRAPPER_JS string against a
minimal DOM harness rather than pulling in jsdom for one file. The harness
models fetch as async on purpose: real fetch rejects rather than throwing
synchronously, and a harness that threw would let a missing .catch pass.
The wrapper forwards grantId and params and nothing else, and collapses
anything the bridge did not itself name to `request_failed` — a status line
or an HTML error body is detail a sandboxed page has no business seeing.
integration.test.ts covers the seams end to end: tool -> store -> catalog
-> server -> wrapper -> bridge -> grants -> pool. Each assertion was
mutation-tested. Two carried a correction:
- "survives a restart" passed with reconcile() disabled, so it was
testing persistence, not recovery. It now destroys the index first
(deleted and corrupt), which is the case the manifests exist for.
- the tool-smuggling test does not fail on a wrapper-only regression,
because the bridge reads the tool from the grant. Kept as an
end-to-end assertion and labelled as such, with the wrapper's own
duty covered by its unit test.
Mutation testing found that removing `res.ok` from the success condition failed nothing — the guard existed but no test held it. An untested branch is either worth a test or worth deleting; this one is worth a test, since trusting the envelope alone would let any upstream able to shape a response hand a page data the bridge refused.
The plan claimed 'what remains is not code'. That was wrong — the wrapper hop was missing, and an integration test is what found it. Both documents now say so, along with the generalisable lesson: when a component can only be reached through another, testing it at its own front door proves nothing about whether anything can get there. The ADR gains a measured table from driving the whole path in Chrome: live data rendered, direct fetch blocked by CSP, ungranted query refused, opaque origin with localStorage and cookies blocked, top-level navigation to a grant-holding page refused, exactly one connector call with merged arguments.
Verified against the code rather than from memory. The status section said only the consent dialog was missing. In fact nothing in front of a grant exists: craft_page has no parameter for declaring a query, so no grant can be proposed; the skill teaches the static-page story only; and no UI calls the grant RPC, which is otherwise fully registered and exposed. Also records that FEATURE_FLAGS.craftPagesLiveData — the separate flag this plan says WS7 ships behind — was never implemented. craftPages alone gates both static pages and the bridge.
plan.md said WS7 ships behind its own flag, after static pages. It never did — craftPages alone gated both, so enabling static pages also bound /internal/query. Modelled as a SUB-flag: only ever on when craftPages is on, because live data means nothing without a page to hold the grant, a listener to serve it and a wrapper to broker it. One flag per gate would make 'pages off, live data on' a reachable state no code path expects. With it off the runtime builds no grant store, no connector pool and no bridge, so /internal/query is absent rather than unauthorised (404 live_data_unavailable, exactly as before WS7) and every page is grantless — none becomes framed-only, none loses 'open in browser'.
Third of the three missing pieces in front of a grant. craft_page gains a 'queries' parameter: an agent names the data a page needs, and the request lands on the manifest for the user to approve or refuse. The name is the load-bearing part. An agent cannot know a grant id while authoring — the user has not approved anything yet — so a page refers to its data by a handle it chose, and the wrapper resolves handle to grant at runtime. Nothing here is a security control, and the module says so: page.json is agent-writable, so a hand-edited manifest can claim any query it likes. The controls are the allowlist, schema validation and the fixed-argument collision check at approval time. What this buys is that a malformed or abusive request fails at the tool boundary with an error the agent can act on, rather than reaching the user as an unreadable consent dialog. Bounded at 8 queries per page: a page asking for dozens is a consent-fatigue attack, where the dialog becomes unreadable and the user approves it to make it stop. Names are unique case-insensitively, because two names differing only in case are confusable in page code and would resolve through the same lookup. Validation runs BEFORE any write, so a rejected request leaves no page behind for the agent to describe as working. The tool result states plainly that requesting is not having, so the model does not announce a live dashboard the moment the call succeeds.
A page refers to its data by the name it chose ('unread'), and the wrapper
resolves that to whatever grant the user approved for THIS page. Three
reasons it works this way:
- An agent cannot know a grant id while authoring; the user has not
approved anything yet. The name is the indirection between authoring
time and approval time.
- A page that could pass a raw grantId could try ids it was never given.
A handle only ever resolves to something approved for that page.
- Handles are page-scoped, so two pages both naming a query 'unread' reach
their own grant or none — never each other's.
The map is inlined into the server-rendered wrapper document rather than
fetched, because the wrapper already knows the page; the alternative is a
round trip on every load to learn what the user already decided. It is
built with Object.create(null): a plain object literal would resolve
'constructor', 'toString' and every other inherited name to a truthy value.
GrantStore enforces one live grant per (page, name) — two would make lookup
order decide which of the user's approvals a call actually used.
Adds pages:listQueryRequests, which pairs what the page asked for with what
the user has already decided, so a consent dialog has something to render.
It re-validates the manifest rather than trusting it: page.json is
agent-writable, and a hand-edited one must not put arbitrary text in front
of the user inside a consent dialog.
The skill and prompt now teach live data, which means craftQuery has to exist. Served at /w-assets/craft-query.js and referenced with a script tag rather than injected into agent-authored HTML: rewriting a page's markup to insert a script works right up until someone writes unusual HTML. script-src 'self' permits this even though the page's origin is opaque — a header-delivered policy takes its self-origin from the response URL (CSP3 4.1), not the document's origin. Measured in WS0. It is served with the PAGE's CORP, not the wrapper's. The sandboxed page is cross-origin to this server, so same-origin CORP would make the browser fetch the file and discard it, leaving the page with no craftQuery and no error anywhere — the same failure that made every page blank in 2564d98. bun's fetch does not enforce CORP, so there is now an explicit assertion standing in for a browser. The helper resolves rather than rejects on refusal: a rejected promise with no handler is an unhandled rejection in a page whose only sin was having access revoked. The prompt section is gated on the live-data sub-flag, so an agent is never taught to request queries no dialog will show.
Every rule lives in craft-page-consent.ts, which is pure and tested; the React component renders the model and reports the press. That follows how this repo tests UI (craft-page-spec.ts + CraftPageBlock.tsx) and keeps the one security-relevant decision out of a component where it cannot be tested. The property that matters: what the user SEES and what gets SENT are derived from the same list, so the panel cannot approve something it did not display. Mutation-tested — dropping that check, or making blocked rows selectable, each fail specific tests. Three choices worth naming: - Non-approvable requests are SHOWN, not hidden. A dropped row leaves the user wondering why the page is broken and the agent's claim about it unexplained. - The panel distinguishes what the user is fixing (fixedArgs) from what the page varies at runtime (paramSchema). That distinction is the entire point of fixed arguments, and it is invisible unless stated. - Consent sits above the preview, not below: the decision belongs in front of the thing it is about, and a user who has scrolled past a rendered page has stopped reading. Default selection is every pending row, matching the trust model's unit of consent — the query SET. Per-query prompting is the consent fatigue the ADR rejects. Nothing is approved without an explicit press. Approval and revocation both collapse the preview, because handles are inlined into the wrapper document at render time and a live frame would keep the pre-decision set.
Every other integration test starts after approval, so none would notice if requesting were impossible. This one goes agent -> request -> approve -> page reads, and pins two properties the earlier tests cannot: - A page that merely ASKED holds nothing: an empty handle map, no connector call, and still openable in a browser. Treating a request like a grant would penalise asking. - It becomes framed-only at the moment access actually exists, not when it was requested.
Records what the audit found and what was built in response: craft_page queries, the craftQuery helper, the consent panel, the live-data sub-flag, and handle-based grant resolution. The ADR gains the reasoning behind handles (an agent cannot know a grant id while authoring, and a page that could pass a raw id could try ids it was never given), the request/having separation extending to the tool result, and why non-approvable requests are shown rather than dropped. Re-verified in Chrome with the page calling the real craftQuery helper: live data rendered, unapproved handle refused, direct fetch still blocked by CSP, origin still opaque, exactly one connector call.
Verifying the containment guard on real Linux (bun 1.3.10, aarch64, container) rather than reasoning about it. The full suite now gives results identical to macOS — 3990 pass, 10 fail, the same ten pre-existing environmental failures on both — and D9's symlink rejection passes on a filesystem where /tmp is a real directory rather than a symlink, which is the case macOS cannot exercise. Doing it found a real cross-platform bug. MEASURED: writing "café.html" as NFC and again as NFD yields ONE file on APFS and TWO on ext4, so the same tool call produced different page content per machine. findCaseCollisions folded case but not normalisation; it now folds both. Normalisation is the worse half of the pair — the colliding names are visually IDENTICAL, so a reviewer reading the file list cannot see the collision at all. Both folds are mutation-tested. Also fixes two test files that replaced globalThis.fetch and never restored it. Bun runs every test file in one process, so those stubs applied to every file that ran afterwards, silently breaking any later suite that makes a real request. That was 24 phantom failures across the pages and webui servers in a full-suite run, on both platforms — invisible because validate:ci never ran those suites and per-package runs never hit the ordering. Full-suite failures drop from 35 to 10. `bun run test:pages` joins validate:ci, and CI already runs on ubuntu-latest, so Linux stays verified instead of being a one-off. Windows remains unverified and is now the only substantive gap. Linux does not reduce it: every Windows rule concerns a fold or rewrite that no POSIX filesystem performs.
Craft Pages is the one feature whose correctness is filesystem behaviour, and the three platforms genuinely disagree about when two distinct names are the same file. naming.ts encodes those rules as pure string logic so they can be tested anywhere — which is precisely why the Windows rules had never run on Windows. Rules written for a platform and never executed there are a hypothesis, not a guard. fail-fast is off deliberately: a Windows-only failure is the point, and cancelling the other two legs would hide whether a failure is platform-specific or universal. report-fs-behaviour.ts prints what the runner's filesystem actually does — case folding, unicode normalisation, trailing-space handling, and whether symlinks can be created at all. Diagnostic, never a gate. It exists so a CI log explains a platform-specific result without a re-run, and so that a platform which CANNOT exercise a security property says so out loud: symlink creation needs privilege on Windows, and three containment tests depend on it (ADR 0001 D9). A silent skip there would quietly retire the guard rather than verify it. Windows runners default to core.autocrlf=true, which rewrites line endings at checkout; that is disabled before checkout so a byte comparison never fails for a reason unrelated to the code.
The first Windows run failed in 'Install dependencies', before a single test executed: electron's postinstall was fetching its ~100MB binary and the socket hung up. These tests never launch Electron — they exercise filesystem rules, a loopback HTTP listener, and pure logic. Downloading the binary only adds a network dependency to a job with no use for it, so the fix is to not fetch it rather than to retry a flake.
The Windows run gave the answer this workflow was built for: 489 of 490 pages tests pass there, all three symlink-escape tests among them, so the D9 containment property is now verified on Windows rather than assumed. The single failure was in the TEST, not the guard. The guard canonicalises the root with fs/promises realpath and builds the candidate from it, so its containment check compares like with like. The test canonicalised with realpathSync — a different API, and the two disagree on Windows about 8.3 short names: the runner's tmpdir is C:\Users\RUNNER~1\… and only one of them expands it. The assertion was measuring which realpath Node happened to use, not whether the path was contained. On failure it now prints both paths, so a future disagreement is diagnosed from the log instead of another CI round trip. Also adds the workflow and probe to the push paths filter — editing the workflow previously could not re-trigger it, which is how the first fix had to be dispatched by hand.
The pages suite now runs on windows-latest: 490 pass, 0 fail, including all
three symlink-escape tests, so D9 containment is enforced on Windows rather
than argued from naming.ts being 'written for Windows'.
The run contradicted two things this ADR was partly written from. Windows
PRESERVES unicode normalisation — macOS is the one that folds — and the
runner preserved a trailing space rather than stripping it. Both rules stay:
they have to hold on the union of platforms and toolchains, and a filename
whose identity depends on which API last touched it is not one to allow. But
the rationale now cites measurement instead of reputation.
Records the remaining gap honestly: MAX_PATH is still unexercised, since CI
paths are short and a 48-character slug under revisions/{n}/public/ could
exceed 260 characters in a deep user directory.
Release blocker, correctly found in review and reproduced: with CRAFT_FEATURE_CRAFT_PAGES=0 the runtime refused to start, but craft_page and craft_page_delete were still advertised to the model — getSessionToolDefs filtered only send_developer_feedback. That is the worst of the three possible states. The tool succeeds, writes files and returns a fence, while catalog registration is silently skipped because the handler degrades gracefully by design. The model then tells the user their page is ready and the preview resolves to nothing — the exact "invisible page" failure the wiring tests were written to prevent. includeCraftPages defaults to FALSE, so a backend that is never updated ships the feature hidden rather than exposed. craft_page_delete is filtered too: it exists as a separate tool precisely to carry safeMode 'block', and hiding only the authoring tool would leave the one that destroys work. Tests pin all three derived surfaces — names, registry and JSON schema — since a filter applied to one and forgotten in another lets a backend advertise a tool the others do not implement. Also corrects a comment in CraftPageBlock that claimed the page "opens in a fullscreen overlay". It does not: expanding renders a fixed-height inline iframe below the card. The absent overlay, side panel, source view and revision picker are now recorded there rather than quietly dropped.
The new visibility test failed to TYPECHECK, which is the drift it was written to catch showing up a layer earlier than expected. getToolDefsAsJsonSchema declared an inline copy of the filter options and forwarded a single field by name, so includeCraftPages applied to the defs, the registry and the name set — and silently did not apply here. A backend built from this schema would have advertised craft_page while the registry behind it refused to implement it. It now takes SessionToolNameOptions and passes the options through wholesale, so a future flag cannot apply to three surfaces and miss the fourth.
The curated list stays curated for a reason: MCP tool names and readOnlyHint annotations come from the MCP server, so a careless or compromised server can call a mutating tool read-only. That argument forbids extension BY THE CONNECTOR — it does not forbid extension by the user. A local MAVIR, weather or water-level source now works by adding an entry to page-tool-allowlist.json, which is the user vouching for the tool rather than the source asserting it. Three properties keep it safe enough to offer: - ADDITIVE ONLY, and curated sources are CLOSED. The file may introduce new sources; it can never remove a built-in, and it cannot add a tool to a source we curate. That last rule has a real cost — a legitimate new Gmail read tool needs a code change — and is taken anyway, because the alternative makes "gmail.send_message" a one-line file edit. - ALL OR NOTHING. One bad entry rejects the whole file. Partial application means the file did something other than what it says. - A SNAPSHOT, loaded once per workspace start, so authorisation cannot change midway through a request. It lives beside page-grants.json, outside every agent-writable directory, for the same reason that file does. Wiring it exposed two latent asymmetries, both of which would have made an extended allowlist half-work: - approve() consulted the curated module directly while resolveArgs() used the injectable one, so an extended workspace could execute a tool it could never approve. Both paths now share one injected allowlist. - eligibleSourcesForPages narrowed to built-in slugs, so a user-declared source was approvable but never connected — the grant would exist and every query against it would fail to find a connector. And one real bug in the pre-existing allowlist: TRUSTED_READ_ONLY was an object literal, so TRUSTED_READ_ONLY['__proto__'] returned Object.prototype — truthy, with no .has — and isTrustedReadOnlyTool THREW a TypeError instead of answering "no". A security predicate that throws on an attacker-chosen value is not a predicate. It is now a Map, which has no inherited keys.
Copies the current revision to <workspace>/exports/<slug>/ as plain files
needing no server. Useful by itself, and the prerequisite for any publishing
story later.
The destination is DERIVED, never supplied. An agent-chosen path is a write
anywhere on disk wearing an "export" label, and the agent is the caller.
The load-bearing part is what happens to a live-data page. `craftQuery` comes
from /w-assets/craft-query.js, served by the app; exported as-is the first
call is a ReferenceError and the whole page dies, including everything that
never needed live data. So the export ships an offline shim that RESOLVES
with {error} — matching the online contract exactly — and the page takes the
empty-state branch the skill already tells agents to write. It also rewrites
the absolute script src, which would otherwise resolve against the host root
and 404 anywhere but the top level.
Verified in Chrome, served from a PARENT directory so an unrewritten absolute
path would fail: styles applied, shim resolved, page rendered its empty state
and its static content, no 404s.
Exports replace rather than merge. A file left from an earlier export is
worse than a missing one — the page would load it and behave like a version
nobody chose.
The tool result states plainly that live queries do not work in the copy, so
the model does not hand the user an export it describes as a live dashboard.
Skill prerequisites are registered only for skills the user explicitly mentions (base-agent's parseMentions), so an ordinary "build me a page" never loads craft-pages. The prompt then said "read the craft-pages skill" without saying where it lives, leaving the model to go looking — or not. That matters more for this skill than most, because the constraints it documents fail SILENTLY: a page using inline styles, `fetch` or a module script renders wrong with no error anywhere. The prompt already inlines those five, which caps the damage, but the tool contract and layout defaults were being missed. getSystemPrompt already has the workspace root, so the bundled skill is resolved there and the prompt names the exact file. Fail-soft: an unresolvable path degrades to asking for it by name, never to silence.
Author
|
@rjulius23 this is where I am currently with the craft pages feature. I had some time to hack it and produce stuff locally like |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Craft Pages: the agent authors a real web page, it is served from a local
loopback listener, and the user views it in-app or in their browser. Optionally the
page can read the user's connected sources — but only queries the user has explicitly
approved.
Both flags default off:
CRAFT_FEATURE_CRAFT_PAGES, and the sub-flagCRAFT_FEATURE_CRAFT_PAGES_LIVE_DATA. With live data off, no grant store, connectorpool or bridge is constructed at all, so
/internal/queryis absent rather thanmerely unauthorised.
Trust model
The page is written by an LLM that has read the user's mail and tickets, so it is
treated as hostile, not merely untrusted. Design and measurements are in
docs/adr/0001-craft-pages-trust-model.md; the spike behind them is inspike/ws0-pages-security/FINDINGS.md.The load-bearing decisions, each measured in Chromium and WebKit:
sandboxattribute.With both applied WebKit executes no scripts at all — a silent total failure that
looks like a broken page. "Defence in depth" is the instinct that produces it.
event.source, neverevent.origin:a sandboxed frame reports origin
"null", so accepting that string would accepta message from any sandboxed frame.
frame-src, which doesnot protect a top-level document — hence a page holding grants is framed-only and
loses "open in browser".
authoring, and a page that could pass a raw id could try ids it was never given.
Verification
Added
.github/workflows/validate-pages.ymlso this stays true — the naming ruleswere written for Windows and had never once run there.
The live-data path was also driven end-to-end in Chrome: real connector data rendered
in the sandboxed frame, an unapproved handle refused, the page's own
fetchblockedby CSP, origin opaque with
localStorageand cookies blocked, exactly one connectorcall with the page's parameter merged onto the user's fixed argument.
Bugs found by verifying rather than reasoning
Cross-Origin-Resource-Policy: same-originmade every page blank in a realbrowser. bun's
fetchdoes not enforce CORP, so no unit test could have caught it.café.htmlwritten as NFC and again as NFD is one file on APFS and two onext4 — the same page differing per machine. Now rejected.
all built and tested, but the wrapper still answered every query with
live_data_unavailable. Every test entered at the bridge, which is where thewrapper enters, not where a page does.
globalThis.fetchand never restored it. Since bun runsevery file in one process, that broke any later suite making a real request — 24
phantom failures in a full-suite run, on every platform.
Not verified
MAX_PATHon Windows (CI paths are short). Gecko is deferred by decision.