Spillway CMS security review, second pass (2026-11-05): package registry and domain importer

Scope: the working tree of branch phase-4 on the day the two surfaces landed, read file by file. Reviewed: the package registry (packages/core/src/registry/**, routes/registry.ts, modules/tar.ts reader and writer, modules/registry-client.ts, kits/registry.ts, sso/github.ts with the GitHub branch of routes/sso.ts, the registry blocks packages/blocks/src/registry-*.tsx and markdown.ts) and the domain importer (packages/core/src/importer/**, routes/imports.ts, routes/imports-plan.ts, routes/imports-theme.ts, importer/preview.tsx, the admin screens under packages/admin/src/screens/import, the import and publish CLI commands). Out of scope and untouched, because other people were writing them concurrently: packages/astro, packages/next, kits/ (except kits/registry.ts), forms/ (read only, for the SSRF guard), sso/scim* and routes/scim.ts.

Method: the threat model in docs/security.md and the first review were taken as the baseline (SSRF guard with DNS pinning, body limits, CSP nonces, the rich-text sanitizer, permissions per route, audit rows). Every new route was enumerated and checked for its permission and site scoping; every outbound fetch was traced to the guard and through its redirect handling; every parser that reads what a third party serves (tar, gzip, HTML, selectors, sitemaps, robots.txt, CSS, Markdown, JSON:API documents) was fed pathological input in a scratch script against the built dist and timed; every value that reaches a page, a stylesheet, a redirect or a menu was traced to its sanitizer. CONFIRMED findings were reproduced (a timing, a thrown RangeError, or a traced path end to end); PLAUSIBLE ones depend on deployment shape. Line numbers refer to the tree before this pass.

Summary

Everything Medium and above was fixed in the same pass, each with a regression test (packages/core/src/test/registry.test.ts, importer-crawl.test.ts, importer-plan.test.ts, importer-theme.test.ts); docs/security.md, docs/importer.md and docs/registry.md describe the new behavior. pnpm build and the four test files are green; node e2e/smoke-registry.mjs and node e2e/smoke-import.mjs pass. pnpm typecheck fails on src/test/scim.test.ts:547 (a concurrent SCIM change, outside this scope).

G-01 (High, CONFIRMED): duplicate tar entries let a sandboxed archive install a trusted module

Where: packages/core/src/modules/tar.ts:63-98 (readTar appends every entry; nothing checks for a repeated path), registry/checks.ts:85-95 (findEntry → entries.find(...), the first copy), modules/system.ts:387-410 (entries.find((e) => e.path === 'manifest.json') validated and consented, then for (const e of entries) await fsp.writeFile(...): the last copy wins on disk), modules/system.ts:142-183 (syncFromDisk at boot: updateModule(existing.id, { manifest, version: manifest.version, tier: manifest.tier }) with the row still enabled).

Exploit: a publisher packs manifest.json (sandboxed, main: main.wasm), main.wasm (a harmless module), then a second manifest.json (tier: "trusted", main: "index.js") and index.js. The registry's checks read the first manifest (sandboxed), smoke-load the Wasm, list the package as sandboxed; a manager (site.settings) installs it from the registry: fetchTarball compares the listed tier with the first manifest (match), the file loop writes the trusted manifest last. On the next restart syncFromDisk flips the row to trusted, loadTrusted imports index.js into the Node process. SPILLWAY_TRUSTED_MODULE_INSTALL and the users.manage gate of F-01 never apply because no trusted install was ever requested. The same archive uploaded through POST /api/modules/install does the same.

Fix: readTar keeps a set of lower-cased paths and throws Duplicate tar entry on a repeat (case-insensitive, because findEntry compares case-insensitively and macOS volumes do too); writeTar matches. syncFromDisk refuses a tier change: the row is disabled with lastError naming both tiers and the module is skipped. The on-disk tier can still never exceed what the row says.

G-02 to G-06 (Medium, CONFIRMED): parsers that are not linear in what the source serves

Each was timed against the built dist (scratch script, Node 22):

The importer's are all reachable by the site being imported (which a manager picks, but which may be compromised or hostile), and they run on the event loop of the whole instance: during the 20 s hangs every other request waits. The README one is reachable by any signed-in user with registry.publish (every GitHub sign-in on the registry host) and lands on the public package page.

Fixes: sitemap.ts is an indexOf scanner; robots.ts has wildcardMatch (greedy two-pointer); links.ts has globMatch (a row per token over the path) and isSafeRegexSource (no quantified group holding a quantifier, no backreferences, ≤ 12 quantifiers, ≤ 200 characters; also applied by routes/imports.ts with a 400 and by normaliseOptions); html.ts caps nesting at MAX_NESTING (512), memoises matchesChain per select/selectOne/matches call and throws on more than MAX_SELECTOR_COMPOUNDS (8), which sources.ts refuses first; markdown.ts caps block quotes at 8 and lists at 16 levels and builds lists from arrays; theme.ts caps CSS nesting at 32.

G-07 (Medium, CONFIRMED): no freshness on the signed index

Where: packages/core/src/modules/registry-client.ts:227-245 (load: signature verified, then normaliseIndex(raw)), registry/service.ts:613-640 (buildIndex writes generatedAt: now() inside the signed document).

A valid ed25519 signature proves who produced the index, not when. A CDN that serves a stale object, a DNS or BGP interposition, or a mirror operator can hand out an index captured before a version was yanked (registry.yank removes it from the index, nothing else reaches the client) or before an advisory-fixed release, and every consuming site keeps installing the old version. docs/registry.md noted that the replay "at least says when it was made" but no code read it.

Fix: checkFreshness after verification: generatedAt older than maxIndexAgeMs (REGISTRY_MAX_INDEX_AGE_DAYS, default 30, 0 off) → registry_stale; older than the newest generatedAt this process accepted → registry_rollback; an unreadable generatedAt → registry_invalid; absent (a spillway registry sign static index) → not aged, documented. Advisories themselves are not in the index; the agent manifest and package pages carry them.

G-08 (Medium, CONFIRMED): registry redirects are followed by undici, not by the guard

Where: packages/core/src/modules/registry-client.ts:118-150 (guardedFetch(url, { signal, headers }) with the default redirect: 'follow'), forms/http.ts:118-135 (the pinned agent's lookup is only consulted for host names; net.connect to an IP literal never calls it).

A registry (or whoever can answer for its host) returns 302 Location: http://127.0.0.1:3000/api/… for a tarball: undici follows inside the same dispatcher, the lookup is skipped for the literal, the request reaches the loopback. The sha256 check means the attacker learns only whether the bytes matched, but the request is made with the registry's accept header against anything on the private network (blind SSRF), and in the no-pinning fallback a redirect to a private name also goes through. A Location: file:///… hop would have been refused by undici itself; a https:// → http:// downgrade with REGISTRY_ALLOW_HTTP unset was not.

Fix: readUrl sends redirect: 'manual', follows at most three hops itself, checks each hop's scheme (https:, or http: only with REGISTRY_ALLOW_HTTP) and runs guardedFetch per hop (fresh resolution, classification and pin). The test drives file:, a downgrade, four hops and a CDN hop.

G-09 and G-10 (Medium, CONFIRMED): the importer as a tool against third parties

rateLimitMs: 0 with four workers and respectRobots: false lets one job hit a public host as fast as it answers for an hour (the time budget), 5,000 pages at a time, with a clear user agent but no floor. The Drupal adapter additionally follows whatever links.*.href and links.next say, so a source can point the importer at a third host (the guard refuses private targets, not public ones).

Fixes: ImporterService.minRateLimitMs (default IMPORT_MIN_RATE_LIMIT_MS = 100 ms; the in-process tests pass 0) floors the per-host gap in the runner, so a host never sees more than ten request starts a second from one job and one job runs per instance; adapterContext.getJson returns null for any URL off the source's host (www. folded) or a host under it.

G-11 to G-16 (Low, CONFIRMED): fixed as well

• G-11 importableCssValue (routes/imports-theme.ts): the site theme's cssValue is meant for values editors type; values the extractor pulled out of a stranger's stylesheet additionally lose anything that fetches or evaluates.
• G-12 httpUrl in routes/registry.ts and the same check in RegistryService.upsertPublisher: zod's .url() is a WHATWG parse, so javascript:alert(1) and data:text/html,… passed; React 19 blocks javascript: hrefs in the blocks, but data: and future renderers are not covered by that.
• G-13 the files endpoint shares downloadLimiter.
• G-14 CSS_MAX_NESTING.
• G-15 DOWNLOAD_DAILY_RETENTION_DAYS in jobs/retention.ts.
• G-16 assertPublishRegistryUrl in cli.ts.

What is done well

• The guard is used everywhere it should be: the crawler, the stylesheet fetcher, asset downloads, the logo fetch, the adapters and the registry client all go through guardedFetch/assertPublicUrl; the crawler follows redirects by hand and re-vets each hop; bodies are read under a cap while streaming; normalizeUrl strips credentials, fragments and tracking parameters before a URL is stored or logged.
• Tar: links, absolute paths, .., NUL, base-256 sizes and oversize archives refused; gunzip with maxOutputLength; resolveInside on every write; the writer applies the same rules so the CLI cannot produce what the reader refuses; registry storage paths come from a name regex and a semver parse.
• Registry flow: versions are immutable and must increase; the tier of a package cannot change; trusted modules are pending_review and absent from search and the index until a users.manage holder approves; approve/reject need pending_review, so a yanked or rejected version cannot come back; yanking recomputes latest; namespace ownership is settled on first publish and checked on every later one; publishes are rate-limited per user and downloads per address; download counts are deduplicated per day under an HMAC of the address; every mutation evicts the tagged public pages and writes an audit row.
• Public rendering: the Markdown converter escapes before it adds markup, keeps only http(s), mailto and relative links, and adds rel="noopener nofollow"; package names, handles and versions are regex-validated at publish; the agent manifest's shell command interpolates only validated names; screenshots are magic-checked and served only when the manifest lists them and the archive held them, with nosniff.
• Importer data handling: extracted main content is sanitized before storage and again at conversion (sanitizeHtml with the embed allow-list); raw pages are served under sandbox; default-src 'none'; script-src 'none' and viewed in sandbox="" iframes; redirect targets and menu links are scheme-checked; menu keys and type keys are regex-validated; job directories are keyed by a validated UUID and sha1(url) file names; asset bytes are MIME-sniffed and type-allow-listed; nothing lands published without content.publish.
• GitHub sign-in: state in a signed HttpOnly cookie bound to the provider slug, constant-time comparison, verified-email-only identity, same-origin returnTo, login limiter shared with the password form.
• Sandbox smoke load: declared memory bounds, only the three host imports, guest and wall budgets, terminate() on timeout, host calls stubbed so nothing reaches the network or the database.

Routes reviewed

/api/imports* (14 routes in routes/imports.ts, 7 in routes/imports-plan.ts, 3 in routes/imports-theme.ts): all requirePermission(c, 'imports.manage') and requireJob(id, currentSite(c).id) (or getJob + 404), sub-resources loaded through the job id. /api/registry/* (15 routes): requireUser, requirePermission('registry.publish' | 'users.manage') or requireManage (publisher or namespace maintainer); /registry/v1/* (public JSON): read-only, CORS *, 60 s cache, publish bearer-only; /api/registry/status any signed-in user. The catch-all public page cannot shadow any of them (G-20).

Spillway 0.1.0 · Edit this page on GitHub