From dfa50f6b8b387e5e30d2981f047847afe6e6e8bc Mon Sep 17 00:00:00 2001 From: Vivek Date: Fri, 31 Jul 2026 02:26:06 +0530 Subject: [PATCH 01/11] feat: add asset() for content-hashed public urls An app's own asset urls sat at stable paths, so a CDN kept serving the previous bytes after a deploy. That caused two visible regressions on webjs.dev in one day, and the purge workflow that cleans up after it only exists for this repo: an app built with the framework has no such workflow, so the framework owed its users an answer. import { html, asset } from '@webjsdev/core'; html`` `asset()` follows the existing `cspNonce()` seam: the server installs a resolver at boot, the browser has none and the function returns the path unchanged. So it is safe to call from a layout, which must load in the browser to register its component imports. Why opt-in. The first attempt (#1196) rewrote asset urls by matching the assembled HTML. Two deep-review rounds found six major defects, five of them one bug: at that layer framework output and author data are indistinguishable, so the matcher kept editing things it did not own. It read and published a content hash for any file under the project root (`` became `/.env?v=`, leaking existence and a fingerprint of the bytes), rewrote hyphenated CUSTOM ELEMENTS' reactive props, mangled `#fragment` urls, and desynced `rel=preload` font hints from the `@font-face url()` that actually fetches them. Each fix narrowed one case, but the space is every tag times every attribute times every rel times every custom element times every data-driven value, so zero regressions was not provable at any amount of review. Marking the url where the author writes it makes the meaning unambiguous: the blast radius is exactly the marked set, and nothing else in the document is ever read or rewritten. The `public/` gate is kept anyway, because an app may pass user-derived data (`asset(user.avatarPath)`), and a fragment is split before hashing so `/x.svg#icon` cannot lose its query or its fragment. Adoption shows the point: the website marks tailwind.css, the two brand lockups, and the highlight script, and deliberately does NOT mark the font preloads (they must match the unversioned CSS `url()`) or the favicons (SEO repo-health tests parse those hrefs literally). The framework cannot get that choice wrong because it no longer makes it. Verified end to end: a prod handler renders the marked url with `?v=` and serves it `max-age=31536000, immutable` while a bare url gets 3600, a dev handler leaves urls untouched, and the full suite is regression- free against a clean-tree baseline under a real `npm ci`. Closes #1194 --- packages/core/index-browser.js | 1 + packages/core/index.js | 1 + packages/core/src/asset-url.js | 66 ++++++++ packages/server/src/asset-hash.js | 40 +++++ packages/server/src/dev.js | 18 ++- .../test/dev/asset-helper-serve.test.js | 98 ++++++++++++ .../test/importmap/resolve-asset-url.test.js | 148 ++++++++++++++++++ website/app/layout.ts | 6 +- website/lib/design/brand.ts | 15 +- 9 files changed, 384 insertions(+), 9 deletions(-) create mode 100644 packages/core/src/asset-url.js create mode 100644 packages/server/test/dev/asset-helper-serve.test.js create mode 100644 packages/server/test/importmap/resolve-asset-url.test.js diff --git a/packages/core/index-browser.js b/packages/core/index-browser.js index 6fa901f96..8ae6afe8e 100644 --- a/packages/core/index-browser.js +++ b/packages/core/index-browser.js @@ -30,6 +30,7 @@ export { render } from './src/render-client.js'; export { escapeText, escapeAttr } from './src/escape.js'; export { notFound, redirect, forbidden, unauthorized, isNotFound, isRedirect, isForbidden, isUnauthorized } from './src/nav.js'; export { cspNonce } from './src/csp-nonce.js'; +export { asset } from './src/asset-url.js'; export { repeat, isRepeat } from './src/repeat.js'; export { Suspense, isSuspense } from './src/suspense.js'; export { connectWS } from './src/websocket-client.js'; diff --git a/packages/core/index.js b/packages/core/index.js index aa426838a..f95a66ffd 100644 --- a/packages/core/index.js +++ b/packages/core/index.js @@ -15,6 +15,7 @@ export { render } from './src/render-client.js'; export { escapeText, escapeAttr } from './src/escape.js'; export { notFound, redirect, forbidden, unauthorized, isNotFound, isRedirect, isForbidden, isUnauthorized } from './src/nav.js'; export { cspNonce, setCspNonceProvider } from './src/csp-nonce.js'; +export { asset, setAssetUrlProvider } from './src/asset-url.js'; export { repeat, isRepeat } from './src/repeat.js'; export { Suspense, isSuspense } from './src/suspense.js'; export { connectWS } from './src/websocket-client.js'; diff --git a/packages/core/src/asset-url.js b/packages/core/src/asset-url.js new file mode 100644 index 000000000..e448d57b3 --- /dev/null +++ b/packages/core/src/asset-url.js @@ -0,0 +1,66 @@ +/** + * Resolve a `public/` asset url to its content-fingerprinted form, so a + * deploy that changes the file changes the url and no cache can serve the + * previous bytes. Isomorphic: importable from server-loaded AND browser-loaded + * modules, exactly like `cspNonce()` and for the same reason (a layout must + * load on the browser to register its component imports). + * + * import { html, asset } from '@webjsdev/core'; + * html`` + * + * On the server, `@webjsdev/server` installs a provider that appends + * `?v=`; the framework then serves a `?v=`-carrying request + * `immutable` for a year instead of the short fallback. On the browser there + * is no provider and the path is returned UNCHANGED, which is always a + * correct url, just an un-versioned one. + * + * Why this is opt-in rather than automatic. An earlier attempt (#1196) + * rewrote asset urls by matching the assembled HTML, and two deep-review + * rounds found six major defects, five of them the same bug: at that layer + * framework output and author data are indistinguishable, so the matcher kept + * touching things it did not own (a custom element's reactive prop, a + * rendered code sample, a `rel=preload` hint whose real request comes from + * CSS, a data-driven `src` pointing at `/.env`). Marking the url at the point + * the author writes it makes the meaning unambiguous, so the blast radius is + * exactly the set of urls someone deliberately marked and nothing else in the + * document is ever read or rewritten. + * + * Scope note: only a `public/` path is fingerprinted, matching what the + * static-asset route will actually serve. Anything else is returned + * unchanged rather than guessed at. + */ + +/** @type {((path: string) => string) | null} */ +let _provider = null; + +/** + * Internal: server-only wiring. `@webjsdev/server` calls this once at load + * time to install the real resolver. Browser builds never call it, so + * `asset()` stays an identity function there. + * + * @param {(path: string) => string} fn + * @returns {void} + */ +export function setAssetUrlProvider(fn) { + _provider = fn; +} + +/** + * The runtime function. Returns the fingerprinted url on the server, or the + * path unchanged when there is no provider (browser), when fingerprinting is + * off (dev, so dev output stays byte-identical), or when the file cannot be + * resolved. Every failure mode degrades to the plain path, never to a broken + * one. + * + * @param {string} path a root-absolute same-origin path, e.g. `/public/app.css` + * @returns {string} + */ +export function asset(path) { + if (typeof path !== 'string' || !path) return path; + if (!_provider) return path; + try { + return _provider(path) || path; + } catch { + return path; + } +} diff --git a/packages/server/src/asset-hash.js b/packages/server/src/asset-hash.js index 5796fb464..0a4176ef1 100644 --- a/packages/server/src/asset-hash.js +++ b/packages/server/src/asset-hash.js @@ -267,6 +267,46 @@ export function withAssetHash(url, basePath = '') { return `${url}${sepChar}v=${hash}`; } +/** + * Resolve one author-marked asset path to its fingerprinted url. This is the + * body behind the isomorphic `asset()` helper (#1194); `dev.js` installs it + * via `setAssetUrlProvider` at boot. + * + * The `public/` gate is defensive, not decorative. `resolveUrlToFile` maps any + * same-origin path under the PROJECT ROOT, so an app that passed request- or + * user-derived data in (`asset(user.avatarPath)`) would otherwise read and + * publish a content hash for `/.env`, `/db/app.db`, or a `.server.ts`, leaking + * both that the file exists and a fingerprint of its exact bytes for files the + * serve path deliberately 404s. Mirror the static-asset route instead: under + * `public/`, no traversal. Anything else returns unchanged WITHOUT touching + * the disk, so a rejected path costs nothing and reveals nothing. + * + * A fragment is split before hashing. `withAssetHash` appends the query, so + * passing the whole thing would emit `/x.svg#icon?v=H`: the browser then + * requests `/x.svg` with no query (losing the immutable caching this exists + * for) and the fragment becomes `icon?v=H`, matching no element id. + * + * @param {string} p the author-supplied path + * @param {string} [bp] the active base path + * @returns {string} the fingerprinted url, or `p` unchanged + */ +export function resolveAssetUrl(p, bp = '') { + if (typeof p !== 'string' || p[0] !== '/' || p[1] === '/') return p; + let probe = p; + if (bp && probe.startsWith(bp + '/')) probe = probe.slice(bp.length); + const cuts = [probe.indexOf('?'), probe.indexOf('#')].filter((i) => i !== -1); + let decoded = probe.slice(0, cuts.length ? Math.min(...cuts) : probe.length); + try { decoded = decodeURIComponent(decoded); } catch { /* keep raw */ } + if (decoded.includes('..') || !decoded.startsWith('/public/')) return p; + const h = p.indexOf('#'); + const path = h === -1 ? p : p.slice(0, h); + const frag = h === -1 ? '' : p.slice(h); + // An author-supplied query is left alone: it may carry meaning we do not + // own, and appending would merge with `&`. + if (path.includes('?')) return p; + return withAssetHash(path, bp) + frag; +} + /** * Static `import` specifier with positional indices. Mirrors module-graph.js's * `IMPORT_RE` (side-effect / default / namespace / named imports), but captures diff --git a/packages/server/src/dev.js b/packages/server/src/dev.js index eea248df6..17bc1265e 100644 --- a/packages/server/src/dev.js +++ b/packages/server/src/dev.js @@ -93,7 +93,8 @@ import { setVendorEntries, setCoreInstall, publishBuildId, setAppSourceId, setBa import { readBasePath, stripBasePath, withBasePath } from './base-path.js'; import { propagateTrustedRemoteIp } from './rate-limit.js'; import { readAllowedOrigins } from './csrf.js'; -import { setAssetRoots, clearAssetHashCache, setElisionFingerprint, withAssetHash, assetHashFor, versionModuleImports } from './asset-hash.js'; +import { setAssetUrlProvider } from '@webjsdev/core'; +import { setAssetRoots, clearAssetHashCache, setElisionFingerprint, withAssetHash, assetHashFor, versionModuleImports, resolveAssetUrl } from './asset-hash.js'; import { urlFromRequest } from './forwarded.js'; import { compileHeaderRules, applySecurityHeaders, webRequestIsHttps } from './headers.js'; import { @@ -624,6 +625,21 @@ export async function createRequestHandler(opts) { // id is a stable deploy fingerprint independent of per-file content hashes. setAssetRoots({ appDir, coreDir, enabled: !dev }); + // Install the resolver behind the isomorphic `asset()` helper (#1194), the + // same provider seam `cspNonce()` uses. An author writes + // `href=${asset('/public/app.css')}` and gets the fingerprinted url on the + // server; the browser has no provider and returns the path unchanged. + // + // The `public/` gate is defensive rather than decorative. `resolveUrlToFile` + // maps any same-origin path under the PROJECT ROOT, so an app that passed + // request- or user-derived data here (`asset(user.avatarPath)`) would + // otherwise read and publish a content hash for `/.env`, `/db/app.db`, or a + // `.server.ts`, leaking both existence and a fingerprint of the bytes for + // files the serve path deliberately 404s. Mirror the static-asset route: + // under `public/`, no traversal. Everything else returns unchanged, having + // never touched the disk. + setAssetUrlProvider((p) => resolveAssetUrl(p, basePath())); + // SSR action-result seeding (#472). Install the process-global module load // hook NOW, at boot, before any `'use server'` action module is imported (ESM // caches by URL, so a module loaded before the hook would never be faceted). diff --git a/packages/server/test/dev/asset-helper-serve.test.js b/packages/server/test/dev/asset-helper-serve.test.js new file mode 100644 index 000000000..4fed5e2fc --- /dev/null +++ b/packages/server/test/dev/asset-helper-serve.test.js @@ -0,0 +1,98 @@ +/** + * End-to-end serving test for the `asset()` helper (#1194). + * + * The unit tests for `resolveAssetUrl` prove the resolver. This proves the + * WIRING: that a real prod handler installs the provider at boot, that a + * layout calling `asset()` therefore renders a fingerprinted url, and that + * requesting that url gets `immutable` caching back. Without this, the two + * halves could each be correct while nothing connected them, which is exactly + * the failure mode a manual browser check kept hiding (a worktree resolves + * `@webjsdev/*` through node_modules, so a hand-booted server can silently be + * running a DIFFERENT checkout's code). + * + * `dev: false` matters: fingerprinting is prod-only, so a dev handler must + * leave the url untouched, which the second test pins. + */ +import { test, before, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync, symlinkSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { createRequestHandler } from '../../src/dev.js'; + +// The fixture app's own source does `import { asset } from '@webjsdev/core'`, +// and a temp dir outside the monorepo cannot resolve that. Link the repo's +// node_modules into each fixture so the app resolves the SAME core this test +// is exercising, rather than whatever a parent directory happens to expose. +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..', '..', '..', '..'); +const REPO_MODULES = join(REPO_ROOT, 'node_modules'); + +let tmpRoot; +before(() => { tmpRoot = mkdtempSync(join(tmpdir(), 'webjs-asset-serve-')); }); +after(() => { rmSync(tmpRoot, { recursive: true, force: true }); }); + +function makeApp(files) { + const appDir = mkdtempSync(join(tmpRoot, 'app-')); + for (const [rel, body] of Object.entries(files)) { + const abs = join(appDir, rel); + mkdirSync(dirname(abs), { recursive: true }); + writeFileSync(abs, body); + } + if (existsSync(REPO_MODULES)) symlinkSync(REPO_MODULES, join(appDir, 'node_modules'), 'dir'); + return appDir; +} + +const FILES = { + // A layout that marks one asset and leaves a second one bare, so the same + // render proves both that marking works and that nothing else is touched. + 'app/layout.ts': ` +import { html, asset } from '@webjsdev/core'; +export default ({ children }) => html\` + + +\${children}\`; +`, + 'app/page.ts': `import { html } from '@webjsdev/core';\nexport default () => html\`
hi
\`;`, + 'public/app.css': 'body{color:red}', + 'package.json': JSON.stringify({ name: 'asset-serve' }), +}; + +test('a prod handler renders asset() fingerprinted and serves it immutable', async () => { + const appDir = makeApp(FILES); + const app = await createRequestHandler({ appDir, dev: false }); + + const page = await app.handle(new Request('http://x/')); + assert.equal(page.status, 200); + const html = await page.text(); + + const m = html.match(//); + + // And the emitted url is really servable, with the caching that is the + // whole point: a hashed url is immutable for a year, a bare one is not. + const hashed = await app.handle(new Request(`http://x${m[1]}`)); + assert.equal(hashed.status, 200); + assert.match(hashed.headers.get('cache-control') || '', /immutable/); + + const bare = await app.handle(new Request('http://x/public/app.css')); + assert.equal(bare.status, 200); + assert.doesNotMatch(bare.headers.get('cache-control') || '', /immutable/); +}); + +test('a dev handler leaves asset() urls untouched', async () => { + // Fingerprinting is prod-only so dev output stays byte-identical, and a + // dev server never installs the resolver. + const appDir = makeApp(FILES); + const app = await createRequestHandler({ appDir, dev: true }); + const html = await (await app.handle(new Request('http://x/'))).text(); + assert.match(html, //); + assert.doesNotMatch(html, /\?v=/); +}); diff --git a/packages/server/test/importmap/resolve-asset-url.test.js b/packages/server/test/importmap/resolve-asset-url.test.js new file mode 100644 index 000000000..8bf23f64c --- /dev/null +++ b/packages/server/test/importmap/resolve-asset-url.test.js @@ -0,0 +1,148 @@ +/** + * Unit tests for `resolveAssetUrl` (#1194), the server-side body behind the + * isomorphic `asset()` helper. An author marks a url explicitly: + * + * html`` + * + * and gets `?v=` in production, which the static route serves + * `immutable` for a year, so a deploy that changes the file changes the url + * and no cache can serve the previous bytes. + * + * This replaces the approach in #1196, which matched asset urls in the + * assembled HTML. Two deep-review rounds found six major defects there, five + * of them the same bug: at that layer framework output and author data are + * indistinguishable, so the matcher kept rewriting things it did not own (a + * custom element's reactive prop, a rendered code sample, a `rel=preload` + * hint, a data-driven `src` pointing at `/.env`). Those whole classes are + * gone here by construction, because nothing is scanned. What remains worth + * testing is this function's own contract, and the security gate in + * particular, since an app may pass user-derived data to `asset()`. + */ +import { test, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { + setAssetRoots, + clearAssetHashCache, + withAssetHash, + resolveAssetUrl, +} from '../../src/asset-hash.js'; + +let root; +let appDir; +let coreDir; + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'webjs-asseturl-')); + appDir = join(root, 'app'); + coreDir = join(root, 'core'); + mkdirSync(join(appDir, 'public', 'brand'), { recursive: true }); + mkdirSync(coreDir, { recursive: true }); + writeFileSync(join(appDir, 'public', 'app.css'), 'body{color:red}'); + writeFileSync(join(appDir, 'public', 'brand', 'logo.svg'), ''); + clearAssetHashCache(); + setAssetRoots({ appDir, coreDir, enabled: true }); +}); + +afterEach(() => { + setAssetRoots({ appDir: '', coreDir: '', enabled: false }); + clearAssetHashCache(); + rmSync(root, { recursive: true, force: true }); +}); + +test('a public asset is fingerprinted', () => { + const out = resolveAssetUrl('/public/app.css'); + assert.match(out, /^\/public\/app\.css\?v=[0-9a-f]+$/); + assert.equal(out, withAssetHash('/public/app.css'), 'agrees with the framework hash'); +}); + +test('the hash changes when the bytes change', () => { + const before = resolveAssetUrl('/public/app.css'); + writeFileSync(join(appDir, 'public', 'app.css'), 'body{color:blue}'); + clearAssetHashCache(); + assert.notEqual(resolveAssetUrl('/public/app.css'), before); +}); + +test('a fragment survives and the query precedes it', () => { + // An SVG sprite (`#icon`) and a media fragment (`#t=10,20`) are real. + // Appending naively would emit `/logo.svg#icon?v=H`, which requests the + // file with NO query (losing immutable caching) and leaves a fragment that + // matches no element id. + const out = resolveAssetUrl('/public/brand/logo.svg#icon'); + assert.match(out, /^\/public\/brand\/logo\.svg\?v=[0-9a-f]+#icon$/); +}); + +test('a private file is never read, whatever the caller passes', () => { + // The gate is defensive: an app may pass user-derived data, e.g. + // `asset(user.avatarPath)`. Publishing a hash would leak that the file + // exists AND a stable fingerprint of its bytes, for files the serve path + // deliberately 404s. + writeFileSync(join(appDir, '.env'), 'DATABASE_URL=postgres://u:SECRET@h/db'); + mkdirSync(join(appDir, 'db'), { recursive: true }); + writeFileSync(join(appDir, 'db', 'app.db'), 'SQLITE FORMAT 3'); + mkdirSync(join(appDir, 'lib'), { recursive: true }); + writeFileSync(join(appDir, 'lib', 'session.server.ts'), 'export const KEY = 1'); + + for (const p of ['/.env', '/db/app.db', '/lib/session.server.ts', '/package.json']) { + assert.equal(resolveAssetUrl(p), p, `${p} must not be fingerprinted`); + } +}); + +test('a traversal out of public/ is refused', () => { + writeFileSync(join(appDir, '.env'), 'SECRET=1'); + for (const p of ['/public/../.env', '/public/%2E%2E/.env', '/public/%2e%2e/.env']) { + assert.equal(resolveAssetUrl(p), p, `${p} must not escape public/`); + } +}); + +test('non-public, cross-origin, relative, and queried paths pass through', () => { + for (const p of [ + 'https://cdn.example.com/a.css', + '//cdn.example.com/a.css', + './a.css', + '/public/app.css?theme=dark', + '/public/missing.css', + '', + ]) { + assert.equal(resolveAssetUrl(p), p); + } +}); + +test('a base path is stripped for resolution and kept on the url', () => { + // Compose order matches withAssetHash: basePath first, then the hash. + const out = resolveAssetUrl('/base/public/app.css', '/base'); + assert.match(out, /^\/base\/public\/app\.css\?v=[0-9a-f]+$/); +}); + +test('it is a no-op when fingerprinting is disabled', () => { + // Dev never enables it, so dev output stays byte-identical. + setAssetRoots({ appDir: '', coreDir: '', enabled: false }); + assert.equal(resolveAssetUrl('/public/app.css'), '/public/app.css'); +}); + +/* + * The isomorphic contract: `asset()` must be safe to call from a module that + * also loads in the browser, because a layout does exactly that to register + * its component imports. With no provider installed it returns the path + * unchanged, which is always a correct url, just an un-versioned one. + */ +test('asset() returns the path unchanged with no provider (the browser case)', async () => { + // Imported by source path, not by `@webjsdev/core`. The bare specifier + // resolves through node_modules, which in a git worktree can point at a + // DIFFERENT checkout of the monorepo, so the test would silently exercise + // another copy of this file. The relative path pins it to this tree. + const { asset, setAssetUrlProvider } = await import('../../../core/src/asset-url.js'); + setAssetUrlProvider(null); + assert.equal(asset('/public/app.css'), '/public/app.css'); + + setAssetUrlProvider((p) => resolveAssetUrl(p)); + assert.match(asset('/public/app.css'), /\?v=[0-9a-f]+$/); + + // A throwing provider must degrade to the plain path, never propagate. + setAssetUrlProvider(() => { throw new Error('boom'); }); + assert.equal(asset('/public/app.css'), '/public/app.css'); + setAssetUrlProvider(null); +}); diff --git a/website/app/layout.ts b/website/app/layout.ts index 083d161b7..f252ae20c 100644 --- a/website/app/layout.ts +++ b/website/app/layout.ts @@ -1,4 +1,4 @@ -import { html, cspNonce } from '@webjsdev/core'; +import { html, cspNonce, asset } from '@webjsdev/core'; import '#components/theme-toggle.ts'; import { DOCS_START_PATH, UI_PATH, GH_URL, NEW_TAB } from '#lib/links.ts'; import { siteFooter } from '#lib/ui/site-footer.ts'; @@ -229,7 +229,7 @@ export default function RootLayout({ children }: { children: unknown }) { }); - +