diff --git a/app/annotations.js b/app/annotations.js index aeae901..e87f5a5 100644 --- a/app/annotations.js +++ b/app/annotations.js @@ -124,7 +124,14 @@ export function attach(shell, { store, persistence = null } = {}) { // frame against a shell that predates the seam. const view = () => { if (typeof shell.target === "function") { - try { const v = shell.target(); if (v?.doc && v?.rect) return v; } catch {} + let v = null; + try { v = shell.target(); } catch { v = null; } + // A cross-origin declared target is SPATIAL-ONLY: a rect but no readable + // doc. Return it as-is and do NOT fall back to the surface document — the + // record's selector was meant for the cross-origin frame, and matching it + // against the outer host is the false anchor the contract forbids. + if (v?.spatialOnly && v?.rect) return { doc: null, rect: v.rect }; + if (v?.doc && v?.rect) return v; } const frame = active().frame; try { return frame?.contentDocument ? { doc: frame.contentDocument, rect: frame.getBoundingClientRect() } : null; } @@ -142,7 +149,10 @@ export function attach(shell, { store, persistence = null } = {}) { }; const elementAnchor = (selector) => { const v = view(); - if (!v || !selector) return null; + // No readable doc (a cross-origin spatial-only target) means NO element + // anchoring: the selector stays unresolved and the mark is spatial/missing, + // never falsely anchored to the wrong document. + if (!v || !v.doc || !selector) return null; let element; try { element = v.doc.querySelector(selector); } catch { return null; } if (!element) return null; diff --git a/app/shell.html b/app/shell.html index 6d46cab..9316e19 100644 --- a/app/shell.html +++ b/app/shell.html @@ -781,21 +781,33 @@ const outer = frames.get(activeId); if (!outer) return null; const outerRect = outer.getBoundingClientRect(); - const fallback = () => { + // The surface's OWN document, element-anchorable. Used when no nested + // target is declared, or the declared one is not a frame. + const surfaceDoc = () => { try { return outer.contentDocument ? { doc: outer.contentDocument, rect: outerRect } : null; } catch { return null; } }; const frameId = declaredTargets.get(activeId) ?? null; - if (!frameId) return fallback(); + if (!frameId) return surfaceDoc(); try { const nested = outer.contentDocument?.getElementById(frameId); - const doc = nested && nested.tagName === "IFRAME" ? nested.contentDocument : null; - if (!doc) return fallback(); - const innerRect = nested.getBoundingClientRect(); - return { doc, rect: { - left: outerRect.left + innerRect.left, top: outerRect.top + innerRect.top, - width: innerRect.width, height: innerRect.height } }; - } catch { return fallback(); } + if (!nested || nested.tagName !== "IFRAME") return surfaceDoc(); + let doc = null; + try { doc = nested.contentDocument; } catch { doc = null; } + if (doc) { + const innerRect = nested.getBoundingClientRect(); + return { doc, rect: { + left: outerRect.left + innerRect.left, top: outerRect.top + innerRect.top, + width: innerRect.width, height: innerRect.height } }; + } + // CROSS-ORIGIN declared target: SPATIAL-ONLY. Its document cannot be + // read for element identity, so it must NOT fall back to the surface + // document — a record's selector was meant for the cross-origin frame, + // and resolving it against the outer host is the exact false anchor the + // contract forbids (a host and its nested doc both carrying `#publish` + // would anchor to the wrong one). A rect for spatial marks, no doc. + return { spatialOnly: true, rect: outerRect }; + } catch { return surfaceDoc(); } }, scope: () => { const context = declaredContexts.get(activeId) ?? null; diff --git a/test/annotations.test.mjs b/test/annotations.test.mjs index 03e1990..c82882e 100644 --- a/test/annotations.test.mjs +++ b/test/annotations.test.mjs @@ -232,8 +232,21 @@ test("the nested-frame target seam is wired, guarded, and falls back — STRUCTU "target() does not add the outer frame offset — nested anchors will be off by the surface frame's position"); assert.match(target[1], /catch/, "target() does not guard the cross-origin read; a cross-origin nested frame would throw instead of degrading"); - assert.match(target[1], /fallback\(\)/, - "target() has no fallback to the surface frame"); + assert.match(target[1], /surfaceDoc\(\)/, + "target() has no fallback to the surface frame for the no-target / not-a-frame cases"); + // review.r2 IMPORTANT — the cross-origin FALSE ANCHOR. A cross-origin declared + // target must degrade SPATIAL-ONLY (a rect, no readable doc), NOT fall back to + // the surface document: resolving a record's selector against the outer host + // when it was meant for the cross-origin frame is a false anchor (host and + // nested doc both carrying `#publish`). target() returns spatialOnly for that + // case, and the layer refuses to element-anchor a doc-less view. + assert.match(target[1], /spatialOnly:\s*true/, + "a cross-origin declared target still falls back to the surface document — the false anchor the contract forbids"); + const layer = fs.readFileSync(path.join(REPO, "app", "annotations.js"), "utf8"); + assert.match(layer, /spatialOnly[\s\S]*return \{ doc: null/, + "the layer's view() does not honour spatialOnly — it would fall back to the surface document"); + assert.match(layer, /if \(!v \|\| !v\.doc \|\| !selector\) return null/, + "elementAnchor no longer refuses a doc-less (spatial-only) view — a cross-origin selector could resolve"); }); test("the layer anchors through the shell's view, not the surface frame directly", () => { @@ -265,12 +278,12 @@ test("positive control: the two checks above can fail", () => { "the offset survived being removed, so that assertion would pass with the chain walk gone"); const layer = fs.readFileSync(path.join(REPO, "app", "annotations.js"), "utf8"); - const regressed = layer.replace(/const v = view\(\);\n if \(!v \|\| !selector\) return null;/, - "const v = { doc: active().frame.contentDocument, rect: active().frame.getBoundingClientRect() };\n if (!v || !selector) return null;"); - const a = regressed.match(/const elementAnchor = \(selector\) => \{([\s\S]*?)\n \};/); - assert.ok(a, "the mutation broke the elementAnchor extractor"); - assert.match(a[1], /active\(\)\.frame/, - "the regression could not be planted, so the doesNotMatch assertion proves nothing"); + // Regress the cross-origin doc-guard back to the pre-fix form (dropping the + // v.doc check), and prove the assertion that requires it would then fail. + const regressed = layer.replace("if (!v || !v.doc || !selector) return null;", "if (!v || !selector) return null;"); + assert.notEqual(regressed, layer, "the v.doc guard could not be located to mutate — the assertion proves nothing"); + assert.doesNotMatch(regressed, /if \(!v \|\| !v\.doc \|\| !selector\) return null/, + "the v.doc guard survived removal, so the doc-guard assertion above cannot fail"); }); test("the refresh hook re-measures WITHOUT reloading — STRUCTURAL", () => { diff --git a/test/compose-rail.test.mjs b/test/compose-rail.test.mjs index 9053723..2626577 100644 --- a/test/compose-rail.test.mjs +++ b/test/compose-rail.test.mjs @@ -277,6 +277,22 @@ test("a malformed provider.json REFUSES rather than silently falling back", () = "a broken provider declaration quietly fell back to the app's copy"); }); +test("I2: a self-served surface.url app composes as a URL row, with no entry to copy", () => { + // review.r2 IMPORTANT: the app contract defines surface.url for a self-served + // app, mutually exclusive with path and with no entry. Composition + // unconditionally joined m.surface.entry, so a schema-valid URL app could not + // compose at all — it either threw or was reported missing. It must produce a + // URL row and copy nothing. + const t = tree({ apps: { remote: { surface: { url: "https://example.test/app", glyph: "◆", hint: "remote" } } } }); + const out = t.run(["remote"]); + + assert.deepEqual(out.missing, [], `a schema-valid URL app failed to compose: ${out.missing.join(", ")}`); + const row = out.rows.find((r) => r.id === "remote"); + assert.ok(row, "the URL app produced no rail row"); + assert.equal(row.url, "https://example.test/app", "the URL row does not carry the self-served url"); + assert.ok(!("path" in row), "a URL app emitted a path row — url and path are mutually exclusive"); +}); + test("positive control: this suite is capable of failing", () => { assert.throws(() => assert.equal(1, 2)); }); diff --git a/tools/compose-rail.mjs b/tools/compose-rail.mjs index af2fd8d..1beebee 100644 --- a/tools/compose-rail.mjs +++ b/tools/compose-rail.mjs @@ -112,28 +112,38 @@ export function composeRail({ appsRoot, enabled, doors = [], runtimeDir, studioR if (!fs.existsSync(manifestPath)) { missing.push(`${id}: no app.json at ${appDir}`); continue; } const m = readJson(manifestPath); - const entry = path.join(appDir, m.surface.entry); - if (!fs.existsSync(entry)) { missing.push(`${id}: surface entry missing (${m.surface.entry})`); continue; } + // A SELF-SERVED app (`surface.url`) has no `entry` to copy and no overlay + // `path` — something else already serves it, and the contract makes `url` and + // `path` mutually exclusive (app-manifest.md). Composition emitted only a + // path row and unconditionally joined `m.surface.entry`, so a schema-valid + // URL app could not compose at all (review.r2). It gets a URL row and skips + // the copy; it may still declare a provider and calls below. + if (typeof m.surface.url === "string" && m.surface.url) { + rows.push({ id: m.id, name: m.name, glyph: m.surface.glyph, url: m.surface.url, hint: m.surface.hint }); + } else { + const entry = path.join(appDir, m.surface.entry); + if (!fs.existsSync(entry)) { missing.push(`${id}: surface entry missing (${m.surface.entry})`); continue; } - // The seam resolves /surfaces/ to /, flat. - fs.copyFileSync(entry, path.join(surfacesOut, path.basename(m.surface.path))); + // The seam resolves /surfaces/ to /, flat. + fs.copyFileSync(entry, path.join(surfacesOut, path.basename(m.surface.path))); - // An app's vendored assets travel with it. canvas ships 3.4MB of tldraw - // deliberately so the box never calls a CDN. - const appVendor = path.join(appDir, "vendor"); - if (fs.existsSync(appVendor)) { - for (const entryName of fs.readdirSync(appVendor)) { - fs.cpSync(path.join(appVendor, entryName), path.join(vendorOut, entryName), { recursive: true }); + // An app's vendored assets travel with it. canvas ships 3.4MB of tldraw + // deliberately so the box never calls a CDN. + const appVendor = path.join(appDir, "vendor"); + if (fs.existsSync(appVendor)) { + for (const entryName of fs.readdirSync(appVendor)) { + fs.cpSync(path.join(appVendor, entryName), path.join(vendorOut, entryName), { recursive: true }); + } } - } - rows.push({ - id: m.id, - name: m.name, - glyph: m.surface.glyph, - path: m.surface.path, - hint: m.surface.hint, - }); + rows.push({ + id: m.id, + name: m.name, + glyph: m.surface.glyph, + path: m.surface.path, + hint: m.surface.hint, + }); + } // Carry the provider's RUN SPEC, not just its name. The box used to know // each provider by name and hardcode how to start it, which meant a sixth // app could not run without editing box code — the opposite of installable.