fix(authz): fix prod boot-order (C1), dev HMR staleness (I2), add prod coverage (I4)
Fix round 2 for Task 14, addressing a critical review finding reproduced on
a real built server.
C1 (critical): the generated production entry set the authz catalog inside
createProductionServer's BODY, but ES modules evaluate every static import
(including app middleware, emitted as a static import) before the importing
module's body runs. Middleware reading getAuthzCatalog() at module scope —
the same eager shape authzMiddleware({ catalog, ... }) itself requires, and
the pattern app/middleware/logger.ts's `export default requestLogger({...})`
already uses — saw an unset catalog and crashed the whole process at import
time, after every other gate (typecheck/lint/tests/a plain `bun run build`)
stayed green.
Fix: packages/cli/src/build.ts now emits a small side-effecting
`.authz-setup.ts` module containing the static imports of every
app/authz/*.ts declaration plus a call to the new
applyAuthzManifestEarly(entries) (packages/dev-server/src/prod.ts), and
imports THAT MODULE FIRST in the generated entry — before pages, api,
realtime, middleware, components, and layouts. applyAuthzManifestEarly is
deliberately silent (no missing-default-export warnings, though a genuine
conflict still throws and fails the boot at import time); createProductionHandlers
keeps its own unconditional merge+set as an idempotent, always-warning second
pass, so an adapter that bypasses the generated entry and calls it directly
still gets a correctly merged, validated catalog, and so the function stays
independently testable.
I3: corrected packages/authz/src/client.ts's WRN-AUTHZ-SETUP message, which
claimed prod always sets the catalog before middleware runs — true again for
the generated entry after the C1 fix, but not for a custom entry that calls
createProductionHandlers directly.
I2: dev HMR editing app/authz/*.ts reloaded the page while the OLD catalog
stayed authoritative (watch.ts classifies any non-CSS change as "server";
hotUpdate had no authz/ branch) — a false security signal, since tightening
or removing a permission looked like it took effect but didn't until a
restart. Added the branch (packages/dev-server/src/index.ts), and gave
loadAppAuthzCatalog (authz-boot.ts) an injectable importer: a raw import()
would have silently no-op'd on the re-import (Bun caches local TS/JS modules
by filesystem path and ignores query strings), so the hot path routes through
loadModule (pipeline.ts) instead, which copies the edited file to a versioned
sibling specifically to defeat that cache.
I4: added direct createProductionHandlers/applyAuthzManifestEarly tests
(packages/dev-server/test/authz-prod.test.ts: conflict throws naming both
files, missing default export warns and skips, empty array yields an empty
catalog, a second call re-validates rather than trusting a stale singleton)
and the regression test that matters most
(packages/cli/test/authz-prod-coldstart.test.ts): a real `runBuild` + a real
`bun dist/server.js` boot, with a middleware module reading
getAuthzCatalog() at module scope, asserting it actually serves a request.
M5: startServer built its own router once, then loadAppAuthzCatalog built a
second one from scratch on every dev boot and every authz/ hot reload.
loadAppAuthzCatalog now accepts either an appDir (still used standalone, e.g.
by the test suite) or an already-built Router, and both call sites in
index.ts now pass the router they already have.
Every fix in this round was verified non-vacuous by sabotaging it and
confirming the corresponding test fails, then reverting.
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { pathToFileURL } from "node:url";
|
||||
import { buildRouter } from "@wrnexus/router";
|
||||
import { buildRouter, type Router } from "@wrnexus/router";
|
||||
import {
|
||||
emptyCatalog,
|
||||
mergeCatalogs,
|
||||
@@ -8,14 +8,39 @@ import {
|
||||
type CatalogSource,
|
||||
} from "@wrnexus/authz";
|
||||
|
||||
/** Imports one declaration module. Defaults to a raw `import()`; the hot-reload
|
||||
* call site passes `loadModule` instead (see the note below on why). */
|
||||
export type AuthzImporter = (file: string) => Promise<{ default?: AuthzModule }>;
|
||||
|
||||
const rawImport: AuthzImporter = (file) =>
|
||||
import(pathToFileURL(file).href) as Promise<{ default?: AuthzModule }>;
|
||||
|
||||
/**
|
||||
* Load and merge every `app/authz/*.ts` declaration. Conflicts throw so a
|
||||
* misconfigured catalog fails the boot rather than silently changing who can
|
||||
* do what. An app with no `app/authz/` directory gets an empty catalog rather
|
||||
* than an error, since not every app uses permissions.
|
||||
*
|
||||
* Accepts either an app directory — the original, standalone shape, still
|
||||
* used by the test suite and by any caller without a router on hand — or an
|
||||
* already-built `Router`. `startServer` passes its own router (built once at
|
||||
* `:315` with the full `componentDirs`/`externalRoutes`/`middlewareFiles`
|
||||
* options) to avoid a second, redundant filesystem scan of the whole `app/`
|
||||
* tree on every dev boot and on every hot reload of an `app/authz/*.ts` file.
|
||||
*
|
||||
* `importModule` defaults to a raw dynamic `import()`, correct for the
|
||||
* initial boot. On a HOT reload, the caller must instead pass `loadModule`
|
||||
* (from `./pipeline.ts`): Bun caches local TS/JS modules by filesystem path
|
||||
* and ignores query strings, so re-`import()`-ing the same absolute path
|
||||
* after an edit silently returns the stale, already-cached module —
|
||||
* `loadModule` is what copies an edited file to a versioned sibling path
|
||||
* specifically to defeat that cache.
|
||||
*/
|
||||
export async function loadAppAuthzCatalog(appDir: string): Promise<AuthzCatalog> {
|
||||
const router = buildRouter(appDir);
|
||||
export async function loadAppAuthzCatalog(
|
||||
appDirOrRouter: string | Router,
|
||||
importModule: AuthzImporter = rawImport,
|
||||
): Promise<AuthzCatalog> {
|
||||
const router = typeof appDirOrRouter === "string" ? buildRouter(appDirOrRouter) : appDirOrRouter;
|
||||
if (!router.authz.length) return emptyCatalog();
|
||||
const sources: CatalogSource[] = [];
|
||||
for (const entry of router.authz) {
|
||||
@@ -24,7 +49,7 @@ export async function loadAppAuthzCatalog(appDir: string): Promise<AuthzCatalog>
|
||||
// same failure class as a genuine conflict (a broken/misconfigured catalog),
|
||||
// and letting it propagate fails the boot loudly instead of silently
|
||||
// producing a partial catalog. Do not "helpfully" wrap this in a try/catch.
|
||||
const imported = (await import(pathToFileURL(entry.file).href)) as { default?: AuthzModule };
|
||||
const imported = await importModule(entry.file);
|
||||
if (!imported.default) {
|
||||
console.warn(`[wrnexus] authz declaration ${entry.file} has no default export; skipping.`);
|
||||
continue;
|
||||
|
||||
@@ -32,7 +32,7 @@ import {
|
||||
} from "@wrnexus/db";
|
||||
import { connectFromConfig } from "@wrnexus/db/connect";
|
||||
import { configureStorage, type StorageConfig } from "@wrnexus/uploader";
|
||||
import { setAuthzCatalog, type AuthzCatalog } from "@wrnexus/authz";
|
||||
import { setAuthzCatalog, type AuthzCatalog, type AuthzModule } from "@wrnexus/authz";
|
||||
import { loadAppAuthzCatalog } from "./authz-boot.ts";
|
||||
import { realtimeBusFromConfig } from "./realtime-bus.ts";
|
||||
import {
|
||||
@@ -344,8 +344,11 @@ export async function startServer(opts: ServeOptions): Promise<RunningServer> {
|
||||
// the framework never installs one) runs at request time and needs
|
||||
// getAuthzCatalog() already populated by then. An app with no declarations
|
||||
// gets an empty catalog; a genuine conflict between declarations throws and
|
||||
// fails this boot loudly.
|
||||
const authzCatalog: AuthzCatalog = await loadAppAuthzCatalog(appDir);
|
||||
// fails this boot loudly. Pass the already-built `router` (not `appDir`):
|
||||
// it was just built above with the full componentDirs/externalRoutes/
|
||||
// middlewareFiles options, so this avoids a second, redundant filesystem
|
||||
// scan of the whole app/ tree on every dev boot.
|
||||
const authzCatalog: AuthzCatalog = await loadAppAuthzCatalog(router);
|
||||
setAuthzCatalog(authzCatalog);
|
||||
|
||||
// i18n is opt-in by the presence of app/locales/*.json.
|
||||
@@ -615,6 +618,33 @@ export async function startServer(opts: ServeOptions): Promise<RunningServer> {
|
||||
middleware.invalidate();
|
||||
|
||||
const appFiles = files.filter((file) => !isAbsolute(file));
|
||||
// Without this branch, editing app/authz/*.ts reloaded the page (watch.ts
|
||||
// classifies any non-CSS change as "server") while the OLD catalog stayed
|
||||
// authoritative — a false security signal: tightening or removing a
|
||||
// permission LOOKS like it took effect but does not until a restart. A
|
||||
// raw `import()` here would silently no-op: Bun caches local TS/JS
|
||||
// modules by filesystem path and ignores query strings, so the edited
|
||||
// file must be re-imported through `loadModule` (pipeline.ts), which
|
||||
// copies it to a versioned sibling path specifically to defeat that
|
||||
// cache — the same mechanism every other hot-reloaded module already
|
||||
// uses. `router` was just rebuilt above, so this reuses it rather than
|
||||
// re-scanning the filesystem a third time.
|
||||
if (appFiles.some((file) => file === "authz" || file.startsWith("authz/"))) {
|
||||
try {
|
||||
const nextAuthzCatalog = await loadAppAuthzCatalog(
|
||||
router,
|
||||
(file) => loadModule(file) as Promise<{ default?: AuthzModule }>,
|
||||
);
|
||||
setAuthzCatalog(nextAuthzCatalog);
|
||||
runtimeDeps.authz = nextAuthzCatalog;
|
||||
} catch (error) {
|
||||
console.error(
|
||||
"[wrnexus] authz hot update failed — the PREVIOUS catalog remains authoritative " +
|
||||
"until this is fixed and the file saved again",
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
if (appFiles.some((file) => file === "schemas" || file.startsWith("schemas/"))) {
|
||||
assets.updateSchemas(await schemaRuntime(router));
|
||||
}
|
||||
@@ -730,5 +760,11 @@ export type {
|
||||
|
||||
// Deployment: the portable production handler + the node:http adapter.
|
||||
export { createProductionServer, createProductionHandlers } from "./prod.ts";
|
||||
// Internal: called only by the generated `.authz-setup.ts` module (see
|
||||
// packages/cli/src/build.ts) to populate the authorization catalog before any
|
||||
// other static import — including app middleware — evaluates. Not meant for
|
||||
// direct use by application code.
|
||||
export { applyAuthzManifestEarly } from "./prod.ts";
|
||||
export type { AuthzManifestEntry } from "./prod.ts";
|
||||
export { toRequest, writeResponse, nodeListener, serveNode } from "./adapters/node.ts";
|
||||
export type { FetchHandler } from "./adapters/node.ts";
|
||||
|
||||
@@ -109,10 +109,14 @@ export interface ProdOptions {
|
||||
* `app/authz/*.ts`, statically imported into the generated entry (the
|
||||
* catalog holds policy FUNCTIONS, so — unlike `schemasJs` — it cannot be
|
||||
* JSON-serialised). `module` is `undefined` for a file with no default
|
||||
* export; `createProductionHandlers` warns and skips it, then merges the
|
||||
* rest into the process-wide catalog before the server accepts traffic.
|
||||
* export. In the NORMAL generated-entry build, the catalog is already set
|
||||
* by the generated `.authz-setup.ts` module before this ever runs (see
|
||||
* `applyAuthzManifestEarly` below); `createProductionHandlers` merges this
|
||||
* same list again as an idempotent second pass — with its warnings — so a
|
||||
* caller that bypasses the generated entry and calls it directly still gets
|
||||
* a correctly merged catalog.
|
||||
*/
|
||||
authz?: { source: string; module?: AuthzModule }[];
|
||||
authz?: AuthzManifestEntry[];
|
||||
/** Resolved i18n bundle (default lang + locale messages). */
|
||||
i18n?: ResolvedI18n;
|
||||
/** Default database connection (driver + url); enables `getDb()`. */
|
||||
@@ -198,6 +202,40 @@ export function resolveProductionHostname(
|
||||
return environmentHostname?.trim() || explicit || "0.0.0.0";
|
||||
}
|
||||
|
||||
/** One `app/authz/*.ts` declaration as passed through `ProdOptions.authz`. */
|
||||
export interface AuthzManifestEntry {
|
||||
source: string;
|
||||
/** Undefined when the declaration file has no default export. */
|
||||
module?: AuthzModule;
|
||||
}
|
||||
|
||||
function resolveAuthzSources(
|
||||
entries: AuthzManifestEntry[],
|
||||
): { source: string; module: AuthzModule }[] {
|
||||
return entries.flatMap((entry) =>
|
||||
entry.module ? [{ source: entry.source, module: entry.module }] : [],
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge + `setAuthzCatalog` as EARLY as possible, deliberately silently (no
|
||||
* missing-default-export warnings). Called ONLY from the generated
|
||||
* `.authz-setup.ts` module that `wrnexus build` imports FIRST in the
|
||||
* production entry — before any other static import, including app
|
||||
* middleware — so that a middleware module reading `getAuthzCatalog()` at its
|
||||
* own module scope (the same eager shape `authzMiddleware({ catalog, ... })`
|
||||
* itself requires) sees a populated catalog. `createProductionHandlers` below
|
||||
* performs the exact same merge again, WITH its warnings, as the canonical,
|
||||
* always-warns second pass — this function stays silent specifically so the
|
||||
* normal boot path does not print the same "no default export" warning
|
||||
* twice. A genuine conflict still throws here (via `mergeCatalogs`), which
|
||||
* fails the boot at import time — before the entry body, and thus
|
||||
* `createProductionHandlers`, ever runs.
|
||||
*/
|
||||
export function applyAuthzManifestEarly(entries: AuthzManifestEntry[]): void {
|
||||
setAuthzCatalog(mergeCatalogs(resolveAuthzSources(entries)));
|
||||
}
|
||||
|
||||
/** Build the route-matching tables + a module map from the manifest. */
|
||||
function buildProdRouter(manifest: ProdManifest): {
|
||||
router: Router;
|
||||
@@ -371,14 +409,23 @@ export function createProductionHandlers(
|
||||
// Bun.serve path in createProductionServer below. The app still registers
|
||||
// authzMiddleware itself with its own store; this only makes the merged
|
||||
// catalog reachable. No declarations -> an empty catalog, no error.
|
||||
const authzSources = (opts.authz ?? []).flatMap((entry) => {
|
||||
if (!entry.module) {
|
||||
console.warn(`[wrnexus] authz declaration ${entry.source} has no default export; skipping.`);
|
||||
return [];
|
||||
}
|
||||
return [{ source: entry.source, module: entry.module }];
|
||||
});
|
||||
const authzCatalog = mergeCatalogs(authzSources);
|
||||
//
|
||||
// In the NORMAL generated-entry build, this is a deliberately redundant
|
||||
// SECOND pass: the generated `.authz-setup.ts` module already ran this
|
||||
// exact merge (silently, via applyAuthzManifestEarly above) before this
|
||||
// function was ever called, specifically so a middleware module that reads
|
||||
// getAuthzCatalog() at its own module scope sees a populated catalog — this
|
||||
// function's body runs too late for that (it is reached only once every
|
||||
// OTHER static import, including middleware, has already evaluated). This
|
||||
// pass still runs unconditionally (not skipped when the catalog is already
|
||||
// set) so a direct caller that bypasses the generated entry — and thus
|
||||
// never ran that early pass — still gets a correctly merged, validated
|
||||
// catalog, and so this function's own authorization handling stays fully
|
||||
// testable in isolation.
|
||||
for (const missing of (opts.authz ?? []).filter((entry) => !entry.module)) {
|
||||
console.warn(`[wrnexus] authz declaration ${missing.source} has no default export; skipping.`);
|
||||
}
|
||||
const authzCatalog = mergeCatalogs(resolveAuthzSources(opts.authz ?? []));
|
||||
setAuthzCatalog(authzCatalog);
|
||||
|
||||
// Middleware is already an ordered array of functions.
|
||||
|
||||
Reference in New Issue
Block a user