Skip to content

Surface-render route is overloaded with the permalink route — give hosts an unambiguous render endpoint #180

Description

@benvinegar

Summary

GET /s/:id (and its alias /p/:id) serves two different things, distinguished only by the presence of a ?part= / ?surface= query:

  • no part → the full viewer HTML page (a human permalink / new-tab open)
  • ?part=N → one surface rendered as a themed, sandboxed document, which the viewer embeds in a sandbox="allow-scripts" iframe (Card.tsx)

See server/app.ts:

// server/app.ts:965
const renderSurfacePage = async (c) => {
  const surface = await store.getPost(c.req.param("id"));
  if (!surface) return c.text("Post not found", 404);
  const partParam = c.req.query("surface") ?? c.req.query("part");
  if (partParam == null) return c.html(configuredViewerHtml(c, surface)); // ← full page
  // ...else render just surface N as a sandboxed document
};
app.get("/s/:id", renderSurfacePage);   // server/app.ts:1046
app.get("/p/:id", renderSurfacePage);   // both paths, both meanings

This works fine for self-hosted (one origin owns everything). It's a problem for a host that wraps the engine (e.g. sideshow-cloud), because the host has to decide, per request, "is this an engine subresource fetch I should forward to the app, or a human navigation I should answer with my own shell?" — and the only signal is an undocumented, implicit query-param convention the host must reverse-engineer and keep in sync across repos.

How this already bit us (the motivating incident)

sideshow-cloud owns its URLs/routing and serves its own app shell, forwarding app traffic to the per-account Durable Object running this engine. It treated /s/:id as a post-permalink alias and redirected it to a canonical session URL → its shell. That swallowed every surface render (/s/:id?part=N), because the edge matched on the path without the query.

Result: each card's sandbox="allow-scripts" iframe (null origin) received the cloud shell instead of the surface; the shell's import("/engine/engine.js") is then a cross-origin module fetch from a null origin → CORS-blocked → every card renders blank. Console fingerprint:

Access to script at .../engine/engine.js from origin 'null' blocked by CORS

The cloud-side fix (modem-dev/sideshow-cloud#114) re-derives the engine's own convention: forward when ?part= is present, redirect otherwise — plus an e2e that drives the real edge + real engine. It works, but it's a workaround that hardcodes an internal engine detail, which is exactly the coupling the host-inversion model is meant to avoid (the engine should be a black box behind the Host contract, not something the host reverse-engineers URLs for).

Concrete brittleness, today

  • The discriminator is an unspecified internal convention, not a documented contract. A future refactor of the render URL (rename part, move the route, switch to POST) re-breaks every host silently — host unit tests stay green; only production blanks.
  • renderSurfacePage accepts both ?part= and ?surface= (server/app.ts:968), but the cloud guard only checks part. If any engine code path emits ?surface=N, the host mis-routes again. Two spellings = twice the contract to get right.

Proposal

A — (recommended) a dedicated, collision-proof render path. Serve the surface document from a path that can't be mistaken for a human page, e.g. GET /s/:id/render?part=N (or /render/s/:id). Keep /s/:id + /p/:id as the human page only. Update the viewer to emit the new path:

  • viewer/src/Card.tsx (iframe src, ~:271 and the version <select> ~:216)
  • viewer/src/api.ts (postLink :94, postPng :101)
  • viewer/src/host.ts (surface nav URL :181-183)

A host can then route by path prefix with zero query heuristics. Keep the old /s/:id?part=N working as a deprecated alias for one release so old tabs/links don't break.

B — (complements A) declare it in the Host contract. Rather than have every host hardcode URL shapes, expose the engine's URL conventions through viewer/src/host.ts — e.g. a typed surfaceRenderUrl(id, part) builder and/or an isEngineSubresource(url) predicate the host can call. This turns an implicit cross-repo contract into an explicit, versioned one and matches the "engine is an embeddable black box" direction.

C — (cheap interim, if A/B are deferred) document + narrow the existing convention. Promote "/s/:id?part=N = surface document; bare /s/:id = page" to a documented, stable contract, and collapse the part/surface duplication to a single spelling. Smaller blast radius, but the overload (and its silent-failure mode) remains.

Constraints / non-goals

  • Self-hosted parity is sacred. Any change must keep self-hosted behavior identical, including the response-header sandbox CSP that makes a top-level load of a surface document safe (server/app.ts ~:990-999).
  • Preserve back-compat for existing /s/:id and /p/:id links during a deprecation window.

Acceptance criteria

  • A surface document is reachable at a path that cannot collide with a human permalink/page, distinguishable by path alone (no query sniffing).
  • The viewer emits the new path from Card.tsx, api.ts, host.ts.
  • Old /s/:id?part=N keeps working (deprecated alias) for one release.
  • Single canonical spelling for the surface index (drop the part/surface alias, or document both as permanent).
  • Self-hosted behavior + the top-level-load sandbox CSP unchanged.
  • Once shipped + pinned, sideshow-cloud can delete its ?part= guard (modem-dev/sideshow-cloud#114) in favor of the explicit contract.

References

  • Cloud incident + workaround: modem-dev/sideshow-cloud#114 (src/index.ts ?part= guard; test/e2e/surface-render.spec.ts)
  • Regression that exposed it (cloud side): the permalink alias was widened to /[ps]/ in cloud PR fix(store): derive comment sessionId from its surface #94
  • Engine: server/app.ts:965 (renderSurfacePage), :1046-1047 (both routes), :968 (part/surface alias); viewer/src/Card.tsx, viewer/src/api.ts, viewer/src/host.ts

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions