Reviewer-run fixes (todo §4); re-mint try/catch degrades an Ory outage to anonymous (not 500), RESERVED_PLUGIN_IDS refuses a plugin folder that would shadow a host route
This commit is contained in:
@@ -242,6 +242,15 @@ test("session re-mint: an expired JWT backed by a live Kratos session is silentl
|
||||
const denied = await fetch(`http://localhost:${(dead.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired } });
|
||||
assert.equal(denied.status, 403);
|
||||
assert.match(denied.headers.get("set-cookie") ?? "", /^plainpages_jwt=;.*Max-Age=0/);
|
||||
|
||||
// Ory unreachable (not a dead session): whoami throws → degrade to anonymous (403, not 500),
|
||||
// and leave the cookie untouched so the token can re-mint once Ory recovers.
|
||||
const down = createApp({ jwks: staticJwks([ecJwk]), keto, kratos: withWhoami(async () => { throw new KratosError("kratos down", 503, ""); }), kratosAdmin: stubAdmin({}), plugins: [demoPlugin] });
|
||||
await new Promise<void>((r) => down.listen(0, r));
|
||||
t.after(() => down.close());
|
||||
const outage = await fetch(`http://localhost:${(down.address() as AddressInfo).port}/demo/secret`, { headers: { cookie: expired } });
|
||||
assert.equal(outage.status, 403);
|
||||
assert.equal(outage.headers.get("set-cookie"), null);
|
||||
});
|
||||
|
||||
test("guards map to responses: requireSession → /login, a failed can/check → 403, success runs the handler", async (t) => {
|
||||
|
||||
12
src/app.ts
12
src/app.ts
@@ -97,9 +97,15 @@ export function createApp(options: AppOptions = {}): Server {
|
||||
const auth = await resolveSession(req.headers.cookie, jwks, authOptions);
|
||||
user = auth.user;
|
||||
if (!user && auth.expired && keto && kratos && kratosAdmin) {
|
||||
const reminted = await remintSession({ keto, kratosAdmin, kratosPublic: kratos }, req.headers.cookie, { secure: secureCookies });
|
||||
user = reminted.user;
|
||||
res.appendHeader("set-cookie", reminted.setCookie);
|
||||
try {
|
||||
const reminted = await remintSession({ keto, kratosAdmin, kratosPublic: kratos }, req.headers.cookie, { secure: secureCookies });
|
||||
user = reminted.user;
|
||||
res.appendHeader("set-cookie", reminted.setCookie);
|
||||
} catch (err) {
|
||||
// Ory unreachable (Kratos/Keto 5xx, refused, timeout) — degrade to anonymous instead of
|
||||
// 500ing every lapsed request. Leave the cookie alone: it can re-mint once Ory recovers.
|
||||
console.error("session re-mint failed (Ory unreachable?):", err);
|
||||
}
|
||||
}
|
||||
}
|
||||
// CSRF token for this request's first-party forms: reuse a genuine cookie token, else mint
|
||||
|
||||
@@ -39,6 +39,7 @@ test("discovers each folder's manifest, sorted, id derived from the folder name"
|
||||
// Every per-plugin problem and every error-level conflict aborts boot with a message naming it.
|
||||
const badCases: Array<{ name: string; files: Record<string, string>; match: RegExp }> = [
|
||||
{ name: "invalid folder name", files: { "Bad_Name/plugin.ts": full("x") }, match: /Bad_Name/ },
|
||||
{ name: "reserved id shadows a host route", files: { "login/plugin.ts": full("login") }, match: /login.*reserved/s },
|
||||
{ name: "missing plugin.ts", files: { "broken/readme.txt": "x" }, match: /broken.*plugin\.ts/s },
|
||||
{ name: "no default export", files: { "named-only/plugin.ts": "export const x = 1;" }, match: /named-only.*default/s },
|
||||
{ name: "import throws", files: { "explodes/plugin.ts": "throw new Error('boom');" }, match: /explodes.*boom/s },
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
import { existsSync, readdirSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath, pathToFileURL } from "node:url";
|
||||
import { checkApiVersion, findConflicts, isValidPluginId, type Plugin, type PluginManifest } from "./plugin.ts";
|
||||
import { checkApiVersion, findConflicts, isValidPluginId, RESERVED_PLUGIN_IDS, type Plugin, type PluginManifest } from "./plugin.ts";
|
||||
|
||||
const rootDir = join(dirname(fileURLToPath(import.meta.url)), "..");
|
||||
|
||||
@@ -34,6 +34,7 @@ export async function discoverPlugins(options: DiscoverOptions = {}): Promise<Pl
|
||||
errors.push(`"${id}" is not a valid plugin folder name (lowercase a–z, digits, dashes)`);
|
||||
continue;
|
||||
}
|
||||
if (RESERVED_PLUGIN_IDS.has(id)) { fail(`"${id}" is a reserved id — it would shadow a built-in host route`); continue; }
|
||||
const file = join(dir, id, "plugin.ts");
|
||||
if (!existsSync(file)) { fail("no plugin.ts found"); continue; }
|
||||
|
||||
|
||||
@@ -77,6 +77,13 @@ export function isValidPluginId(id: string): boolean {
|
||||
return PLUGIN_ID.test(id);
|
||||
}
|
||||
|
||||
// Ids the host reserves for its own first-party mount segments (the auth flows, /auth/complete,
|
||||
// /logout, the dashboard's /public/ static). Plugin routes resolve before these, so a folder named
|
||||
// one of them would silently shadow a built-in route — discovery refuses it, loud like any conflict.
|
||||
export const RESERVED_PLUGIN_IDS: ReadonlySet<string> = new Set([
|
||||
"auth", "login", "logout", "public", "recovery", "registration", "settings", "verification",
|
||||
]);
|
||||
|
||||
export interface Semver {
|
||||
major: number;
|
||||
minor: number;
|
||||
|
||||
Reference in New Issue
Block a user