From be3bc2bdbbfdf07741aff97575763dbafcfa9e89 Mon Sep 17 00:00:00 2001 From: lilleman Date: Mon, 3 Aug 2026 23:51:40 +0200 Subject: [PATCH] Stability fixes: plugin-scoped contexts for owned pages, absent-href guard, checked locale mounts --- AGENTS.md | 14 ++++++- README.md | 6 ++- .../admin/views/partials/client-form-body.ejs | 2 +- .../views/partials/group-detail-body.ejs | 4 +- .../admin/views/partials/group-form-body.ejs | 2 +- .../views/partials/permission-detail-body.ejs | 4 +- .../views/partials/permission-form-body.ejs | 2 +- .../admin/views/partials/user-form-body.ejs | 6 +-- .../scheduling/views/partials/shift-form.ejs | 2 +- src/http/app.test.ts | 27 +++++++++++++ src/http/app.ts | 38 ++++++++++++------- src/http/builtin-routes.ts | 7 +++- src/i18n/catalog.ts | 2 +- src/i18n/load.ts | 23 ++++++----- src/i18n/locale.test.ts | 4 ++ src/i18n/locale.ts | 13 +++++-- src/i18n/locales/en-US.ts | 1 - src/i18n/locales/sv-SE.ts | 1 - src/i18n/translate.ts | 4 ++ src/i18n/view-locals.test.ts | 32 ++++++++++++++++ src/plugin-host/hooks.test.ts | 8 +++- src/plugin-host/hooks.ts | 14 +++++-- src/ui/shell-context.ts | 2 +- views/index.ejs | 2 + views/oauth-consent.ejs | 4 +- views/partials/auth-card.ejs | 2 +- views/partials/menu.ejs | 6 +-- views/partials/shell.ejs | 2 +- 28 files changed, 176 insertions(+), 58 deletions(-) create mode 100644 src/i18n/view-locals.test.ts diff --git a/AGENTS.md b/AGENTS.md index 8669410..b972e4a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,13 +117,25 @@ them. Revisit only if the stated reason stops holding. whole query string and no href wrapper can reach it. Putting the obligation on each call site was tried first and missed five of eight sites inside one commit — including the admin screens. `ctx.localeHref` remains for hrefs a plugin's own markup emits (the admin example's delete links). + **A form's `action` counts as a link** — a POST replaces the URL as completely as a GET submit, so + the sign-out, consent and auth-card forms carry it too; without that, picking a language and then + saving anything drops back to `Accept-Language`. The one round-trip that cannot carry it is the + Kratos sign-in POST, whose action is an absolute off-site URL. Decided 2026-08-03 after an architecture review; a second pass then found breadcrumbs still raw, so: when a link renders from the core chrome, it is the chrome's job to carry the locale. - **`locale` is a host-owned query param.** It is in `parseListQuery`'s reserved set (`list-query.ts`), so a localized list page doesn't hand a plugin a phantom `locale` filter; the i18n view locals (`t`, `locale`, `locales`, `localeHref`, `localeParam`, `localeSwitch`, `dir`) are likewise reserved names, merged after a handler's `data` so a collision loses the key instead of breaking the shell. -- **`locales/` at the repo root is a drop-in mount, like `plugins/` and `config/`.** A catalog there +- **A plugin-owned render always runs on that plugin's context.** The landing slots (`home`, + `dashboard`) and an `onRequest` short-circuit dispatch a plugin's handler, so they build the + context with `contextFor(pluginId)` exactly as a plugin route does — otherwise `ctx.t` is the core + translator and the plugin's own keys render as bare keys on the pages it owns. Found by review + 2026-08-03 after all three paths shipped with the host's context. +- **`locales/` at the repo root is a drop-in mount, like `plugins/` and `config/`.** The SHIPPED + `en-US` stays the parity baseline even when the mount replaces it, so a mounted catalog is checked + rather than trusted (a mounted `en-US` compared only against itself would boot green with the + whole UI rendering keys). A catalog there for a new tag adds a language; one for a tag the image ships replaces that catalog wholesale, held to the same parity check. Adding a language must not require forking the image. - **An unknown translation key renders as itself.** That single rule is what lets a nav label, diff --git a/README.md b/README.md index 7ae4dd1..d996dec 100644 --- a/README.md +++ b/README.md @@ -1001,8 +1001,10 @@ Three rules worth knowing: - **The core building blocks carry the locale for you** — every href they render (menu, breadcrumbs, pagination, sort headers, row actions, the auth card's links) goes through `localeHref`, and their GET forms carry it as a hidden field, since a GET submit replaces the whole query string. - `ctx.localeHref` is for hrefs your own markup emits, and `localeParam` (a view local: the tag, or - null) for your own GET forms. `locale` is reserved: `parseListQuery` never returns it as a filter. + `ctx.localeHref` is for hrefs and form actions your own markup emits (a POST replaces the URL just + as a GET submit does), and `localeParam` (a view local: the tag, or null) for your own GET forms. + `locale` is reserved: `parseListQuery` never returns it as a filter. Responses carry + `Vary: Accept-Language`, so a cache in front of the app keys on the language too. - **Reuse the core words.** Generic UI verbs live in the core catalog — `common.add/cancel/delete/ edit/new/remove/save`, `filter.*`, `pagination.*`, `table.*` — and a plugin's lookup falls through to them. Keep your catalog for your domain words, so N plugins don't re-translate "Cancel" N times. diff --git a/examples/plugins/admin/views/partials/client-form-body.ejs b/examples/plugins/admin/views/partials/client-form-body.ejs index 63eae55..e55be3c 100644 --- a/examples/plugins/admin/views/partials/client-form-body.ejs +++ b/examples/plugins/admin/views/partials/client-form-body.ejs @@ -10,7 +10,7 @@ <% if (locals.error) { -%> <%- include("partials/alert", { text: locals.error, tone: "neg" }) %> <% } -%> -
+ <%- include("partials/field", form.nameField) %>
diff --git a/examples/plugins/admin/views/partials/group-detail-body.ejs b/examples/plugins/admin/views/partials/group-detail-body.ejs index 2171a61..6b0fcba 100644 --- a/examples/plugins/admin/views/partials/group-detail-body.ejs +++ b/examples/plugins/admin/views/partials/group-detail-body.ejs @@ -21,7 +21,7 @@ <% if (members.rows.length) { -%>
<% members.rows.forEach((m) => { -%> - + <% }) -%>
<%= t("admin.groups.membersOf", { name: group.name }) %>
<%= t("admin.common.member") %><%= t("admin.common.type") %><%= t("table.actions") %>
<%= m.label %><%= m.kind === "group" ? t("admin.common.group") : t("admin.common.user") %>
<%= m.label %><%= m.kind === "group" ? t("admin.common.group") : t("admin.common.user") %>
<% } else { -%> @@ -31,7 +31,7 @@

<%= t("admin.groups.addMember") %>

<% if (add.options.length) { -%> -
+
<% } else { -%>

<%= t("admin.groups.allMembers") %>

<% } -%> diff --git a/examples/plugins/admin/views/partials/group-form-body.ejs b/examples/plugins/admin/views/partials/group-form-body.ejs index dd10dea..a3880d5 100644 --- a/examples/plugins/admin/views/partials/group-form-body.ejs +++ b/examples/plugins/admin/views/partials/group-form-body.ejs @@ -10,7 +10,7 @@ <% if (locals.error) { -%> <%- include("partials/alert", { text: locals.error, tone: "neg" }) %> <% } -%> -
+ <%- include("partials/field", form.nameField) %>
diff --git a/examples/plugins/admin/views/partials/permission-detail-body.ejs b/examples/plugins/admin/views/partials/permission-detail-body.ejs index 1cc1be3..14470bd 100644 --- a/examples/plugins/admin/views/partials/permission-detail-body.ejs +++ b/examples/plugins/admin/views/partials/permission-detail-body.ejs @@ -23,7 +23,7 @@ <% if (members.rows.length) { -%>
<% members.rows.forEach((m) => { -%> - + <% }) -%>
<%= t("admin.groups.membersOf", { name: permission.name }) %>
<%= t("admin.common.member") %><%= t("admin.common.type") %><%= t("table.actions") %>
<%= m.label %><%= m.kind === "group" ? t("admin.common.group") : t("admin.common.user") %>
<%= m.label %><%= m.kind === "group" ? t("admin.common.group") : t("admin.common.user") %>
<% } else { -%> @@ -46,7 +46,7 @@

<%= t("admin.permissions.assign") %>

<% if (add.options.length) { -%> -
+
<% } else { -%>

<%= t("admin.permissions.allAssigned") %>

<% } -%> diff --git a/examples/plugins/admin/views/partials/permission-form-body.ejs b/examples/plugins/admin/views/partials/permission-form-body.ejs index dfb99da..81f748b 100644 --- a/examples/plugins/admin/views/partials/permission-form-body.ejs +++ b/examples/plugins/admin/views/partials/permission-form-body.ejs @@ -10,7 +10,7 @@ <% if (locals.error) { -%> <%- include("partials/alert", { text: locals.error, tone: "neg" }) %> <% } -%> -
+ <%- include("partials/field", form.nameField) %>
diff --git a/examples/plugins/admin/views/partials/user-form-body.ejs b/examples/plugins/admin/views/partials/user-form-body.ejs index 920e065..1de94a7 100644 --- a/examples/plugins/admin/views/partials/user-form-body.ejs +++ b/examples/plugins/admin/views/partials/user-form-body.ejs @@ -16,7 +16,7 @@ <% if (recovery) { -%>
<%= t("admin.users.recovery.title") %><%- t("admin.users.recovery.body") %><% if (recovery.code) { %><%= recovery.code %><% } %>
<% } -%> - + <% form.fields.forEach((field) => { -%> <%- include("partials/field", field) %> @@ -28,8 +28,8 @@ <% if (edit) { -%>
"> -
-
+
+
<%= t("admin.users.delete") %>
<% } -%> diff --git a/examples/plugins/scheduling/views/partials/shift-form.ejs b/examples/plugins/scheduling/views/partials/shift-form.ejs index 65f230b..1d8ed5d 100644 --- a/examples/plugins/scheduling/views/partials/shift-form.ejs +++ b/examples/plugins/scheduling/views/partials/shift-form.ejs @@ -9,7 +9,7 @@ <% if (locals.formError) { -%> <%- include("partials/alert", { text: locals.formError, tone: "neg" }) %> <% } -%> -
+ <% form.fields.forEach((field) => { -%> <%- include("partials/field", field) %> diff --git a/src/http/app.test.ts b/src/http/app.test.ts index 75ddb71..768c1bd 100644 --- a/src/http/app.test.ts +++ b/src/http/app.test.ts @@ -1459,3 +1459,30 @@ test("the error pages speak the visitor's language too", async (t) => { assert.match(html, / { + const dir = mkdtempSync(join(tmpdir(), "pp-i18n-plugin-")); + mkdirSync(join(dir, "demo", "i18n"), { recursive: true }); + t.after(() => rmSync(dir, { force: true, recursive: true })); + writeFileSync(join(dir, "demo", "i18n", "en-US.ts"), 'const m = { "demo.hello": "Hello from the plugin" };\nexport default m;\n'); + + // Every plugin-owned render path: the public landing, the gated dashboard, and a hook short-circuit. + const demo: Plugin = { + apiVersion: "1.0.0", + dashboard: (ctx) => ({ html: ctx.t("demo.hello") }), + home: (ctx) => ({ html: ctx.t("demo.hello") }), + hooks: { onRequest: (ctx) => (ctx.url.pathname === "/hooked" ? { html: ctx.t("demo.hello") } : undefined) }, + id: "demo", + }; + const i18n = createI18n(await loadI18n({ pluginIds: ["demo"], pluginsDir: dir })); + const app = createApp({ i18n, jwks: staticJwks([ecJwk]), plugins: [demo], pluginsDir: dir }); + await new Promise((r) => app.listen(0, r)); + t.after(() => app.close()); + const url = `http://localhost:${(app.address() as AddressInfo).port}`; + const nowSec = Math.floor(Date.now() / 1000); + const cookie = `${SESSION_COOKIE}=${mintJwt({ email: "a@b", exp: nowSec + 600, permissions: [], sub: "u1" })}`; + + assert.equal(await (await fetch(`${url}/`)).text(), "Hello from the plugin"); + assert.equal(await (await fetch(`${url}/hooked`)).text(), "Hello from the plugin"); + assert.equal(await (await fetch(`${url}/dashboard`, { headers: { cookie } })).text(), "Hello from the plugin"); +}); diff --git a/src/http/app.ts b/src/http/app.ts index 585b023..592d84e 100644 --- a/src/http/app.ts +++ b/src/http/app.ts @@ -3,7 +3,7 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse } import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import ejs from "ejs"; -import { type BuiltinRoute, matchBuiltinRoute, type RequestCsrf } from "./builtin-routes.ts"; +import { type BuiltinRoute, matchBuiltinRoute, type PluginContextFactory, type RequestCsrf } from "./builtin-routes.ts"; import { buildPluginChrome, type PageChrome } from "../ui/chrome.ts"; import { buildContext, type RequestContext, type User } from "./context.ts"; import { csrfCookie, ensureCsrfToken, verifyCsrfRequest } from "../auth/csrf.ts"; @@ -132,12 +132,14 @@ export function createApp(options: AppOptions = {}): Server { // (rendered against its own views, native shell via ctx.chrome, with a fresh CSRF cookie for // any form it ships). Else the built-in intro page with prominent sign-in / register links // (`user` picks "go to dashboard" vs sign-in; the shell's Sign-out form needs the CSRF cookie). - const serveHome = async (ctx: RequestContext, csrf: RequestCsrf): Promise => { + const serveHome = async (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory): Promise => { csrf.setCookie(); if (homePlugin) { - const result = (await homePlugin.home(ctx)) ?? null; - if (anyResponseHooks) await runResponseHooks(plugins, ctx, result); - await sendResult(ctx.res, result, pluginViewsFor(ctx, homePlugin.id), ctx.localeHref); + // The plugin owns this page, so it runs on its own context — its catalog first, then core. + const pluginCtx = contextFor(homePlugin.id); + const result = (await homePlugin.home(pluginCtx)) ?? null; + if (anyResponseHooks) await runResponseHooks(plugins, pluginCtx, result); + await sendResult(ctx.res, result, pluginViewsFor(pluginCtx, homePlugin.id), pluginCtx.localeHref); return null; } return { data: { chrome: ctx.chrome, user: ctx.user }, view: "home" }; @@ -147,14 +149,15 @@ export function createApp(options: AppOptions = {}): Server { // in, remembering /dashboard as return_to. A plugin may fully own it via `dashboard` — its // handler renders against its own views, same path as a plugin route. Else the built-in // mock-data People list with the one global menu (ctx.chrome.nav) + branding from config/menu.ts. - const serveDashboard = async (ctx: RequestContext, csrf: RequestCsrf): Promise => { + const serveDashboard = async (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory): Promise => { if (!ctx.user) return { redirect: loginRedirect(ctx), status: 303 }; // The page carries the Sign-out form, so Set-Cookie a fresh CSRF token here when absent. csrf.setCookie(); if (dashboardPlugin) { - const result = (await dashboardPlugin.dashboard(ctx)) ?? null; - if (anyResponseHooks) await runResponseHooks(plugins, ctx, result); - await sendResult(ctx.res, result, pluginViewsFor(ctx, dashboardPlugin.id), ctx.localeHref); + const pluginCtx = contextFor(dashboardPlugin.id); // as serveHome: the owner's own translator + const result = (await dashboardPlugin.dashboard(pluginCtx)) ?? null; + if (anyResponseHooks) await runResponseHooks(plugins, pluginCtx, result); + await sendResult(ctx.res, result, pluginViewsFor(pluginCtx, dashboardPlugin.id), pluginCtx.localeHref); return null; } return { data: { model: buildDashboardModel({ csrfToken: csrf.token, menu, user: ctx.user, nav: ctx.chrome.nav, t: ctx.t }) }, view: "index" }; @@ -183,6 +186,9 @@ export function createApp(options: AppOptions = {}): Server { // Set before any branch so every response — static/redirect/error included — inherits them // (writeHead merges these with its own headers; a plugin's RouteResult.headers can override). for (const [name, value] of secHeaderEntries) res.setHeader(name, value); + // The same URL renders in different languages depending on Accept-Language, so a cache in + // front of us must key on it — otherwise the first visitor's language is served to everyone. + res.setHeader("vary", "accept-language"); if (pathname.startsWith("/public/") && (method === "GET" || method === "HEAD")) { // /public//… serves a plugin's public/; everything else the core public/. @@ -261,18 +267,22 @@ export function createApp(options: AppOptions = {}): Server { t: pluginId === undefined ? t : i18n.translator(locale, pluginId), }); - // base context (no route params yet); reused for onRequest hooks and the landing routes. + // base context (no route params yet); reused for the built-in routes. A plugin-owned render + // (a landing slot, a hook short-circuit, a plugin route) gets `contextFor(id)` instead, so its + // own catalog is what `ctx.t` reads. const ctx = buildContext(req, res, { chrome, user, ...i18nFor(), log: reqLog, verifyCsrf, ...(system ? { system } : {}) }); + const contextFor = (pluginId: string, params?: Record): RequestContext => + buildContext(req, res, { chrome, user, ...i18nFor(pluginId), log: reqLog, ...(params ? { params } : {}), verifyCsrf, ...(system ? { system } : {}) }); renderPage = viewsFor(ctx); // Plugin onRequest hooks run before routing and may short-circuit the request. if (anyRequestHooks) { - const short = await runRequestHooks(plugins, ctx); + const short = await runRequestHooks(plugins, contextFor); if (short) { // Set the fresh CSRF cookie like every other page-emitting path, so a form the hook // renders (its token is in ctx.chrome.csrfToken) has the matching double-submit cookie. csrfMint.setCookie(); - await sendResult(res, short.result, pluginViewsFor(ctx, short.plugin.id), carryLocale); + await sendResult(res, short.result, pluginViewsFor(short.ctx, short.plugin.id), carryLocale); return; } } @@ -282,7 +292,7 @@ export function createApp(options: AppOptions = {}): Server { // CSRF cookie is set so those forms have a valid double-submit token. const match = matchRoute(plugins, method, pathname); if (match) { - const routeCtx = buildContext(req, res, { chrome, user, ...i18nFor(match.plugin.id), log: reqLog, params: match.params, verifyCsrf, ...(system ? { system } : {}) }); + const routeCtx = contextFor(match.plugin.id, match.params); if (!isAuthorized(match.route, routeCtx.permissions)) { // Anonymous → sign in (like the built-in screens' requireSession), remembering the page as // return_to; a signed-in user who simply lacks the permission gets the 403 page. @@ -303,7 +313,7 @@ export function createApp(options: AppOptions = {}): Server { // null means the handler wrote to ctx.res itself. const builtin = matchBuiltinRoute(builtinRoutes, method, pathname); if (builtin) { - await sendResult(res, await builtin.handler(ctx, csrfMint), viewsFor(ctx), carryLocale); + await sendResult(res, await builtin.handler(ctx, csrfMint, contextFor), viewsFor(ctx), carryLocale); return; } diff --git a/src/http/builtin-routes.ts b/src/http/builtin-routes.ts index a4eaefa..b1877a9 100644 --- a/src/http/builtin-routes.ts +++ b/src/http/builtin-routes.ts @@ -14,10 +14,15 @@ export interface RequestCsrf { token: string; } +// A context scoped to a plugin: same request, but `t` reads that plugin's catalog first. The +// landing slots run a plugin's handler, so they must hand it one of these rather than the host's +// own context — otherwise the plugin's keys render as bare keys on the pages it owns. +export type PluginContextFactory = (pluginId: string) => RequestContext; + export interface BuiltinRoute { // Returns a RouteResult, or null when the handler wrote to ctx.res itself // (the landing slots dispatch a plugin's own result against that plugin's views). - handler: (ctx: RequestContext, csrf: RequestCsrf) => Promise | RouteResult | null; + handler: (ctx: RequestContext, csrf: RequestCsrf, contextFor: PluginContextFactory) => Promise | RouteResult | null; method: "GET" | "POST"; // a GET route also answers HEAD, like plugin routes path: string; // exact pathname } diff --git a/src/i18n/catalog.ts b/src/i18n/catalog.ts index b8d58c0..9864eae 100644 --- a/src/i18n/catalog.ts +++ b/src/i18n/catalog.ts @@ -63,7 +63,7 @@ export function checkCatalog({ baseline, baselineLocale, catalog, locale }: Pari } for (const key of Object.keys(catalog)) { - if (!(key in baseline)) problems.push(`unknown key "${key}" — add it to ${baselineLocale} first`); + if (!Object.hasOwn(baseline, key)) problems.push(`unknown key "${key}" — add it to ${baselineLocale} first`); } return problems; diff --git a/src/i18n/load.ts b/src/i18n/load.ts index c3be096..7faee39 100644 --- a/src/i18n/load.ts +++ b/src/i18n/load.ts @@ -21,9 +21,10 @@ const rootDir = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); export const LOCALES_DIR = join(dirname(fileURLToPath(import.meta.url)), "locales"); export const MOUNTED_LOCALES_DIR = join(rootDir, "locales"); -// A catalog file is named for the full locale it holds — sv-SE.ts, never sv.ts. Anything else in -// the folder is a mistake worth stopping for. -const LOCALE_FILE = /^([a-z]{2,3}-[A-Z]{2})\.ts$/; +// A catalog file is named for the full locale it holds — sv-SE.ts, never sv.ts — with the script +// subtag when the language needs one (sr-Latn-RS). Anything else in the folder is a mistake worth +// stopping for. +const LOCALE_FILE = /^([a-z]{2,3}(?:-[A-Z][a-z]{3})?-[A-Z]{2})\.ts$/; export interface LoadI18nOptions { localesDir?: string; @@ -46,10 +47,15 @@ export async function loadI18n(options: LoadI18nOptions = {}): Promise>(); @@ -62,7 +68,7 @@ export async function loadI18n(options: LoadI18nOptions = {}): Promise !set.has(locale)); @@ -103,8 +109,7 @@ async function readSet(dir: string, label: string, errors: string[]): Promise, label: string, errors: string[]): void { - const baseline = set.get(DEFAULT_LOCALE); +function checkSet(set: Map, label: string, baseline: Catalog | undefined, errors: string[]): void { if (baseline === undefined) return; // already reported; nothing to compare against for (const [locale, catalog] of set) { for (const problem of checkCatalog({ baseline, baselineLocale: DEFAULT_LOCALE, catalog, locale })) { diff --git a/src/i18n/locale.test.ts b/src/i18n/locale.test.ts index 5f914ef..5470c48 100644 --- a/src/i18n/locale.test.ts +++ b/src/i18n/locale.test.ts @@ -59,6 +59,10 @@ test("localeHref carries the locale on host-relative links only", () => { assert.equal(localeHref("https://example.com/x", "sv-SE"), "https://example.com/x"); // off-site assert.equal(localeHref("//example.com/x", "sv-SE"), "//example.com/x"); // protocol-relative is off-site too assert.equal(localeHref("", "sv-SE"), ""); + // The building blocks document href as optional (an unlinked page item, a header with no sort + // target) — an absent one must not throw, or the page breaks only for visitors who chose a language. + assert.equal(localeHref(undefined as unknown as string, "sv-SE"), undefined); + assert.equal(localeHref(null as unknown as string, "sv-SE"), null); }); test("textDirection reads the script direction, defaulting to ltr", () => { diff --git a/src/i18n/locale.ts b/src/i18n/locale.ts index f6d6925..7a60432 100644 --- a/src/i18n/locale.ts +++ b/src/i18n/locale.ts @@ -58,10 +58,17 @@ export function resolveLocale({ acceptLanguage, available, param }: ResolveInput // Carry `locale` on a host-relative link. Off-site and protocol-relative URLs are left alone — the // locale is ours to state, not theirs. `locale` null (the visitor never asked for one) ⇒ unchanged. export function localeHref(href: string, locale: string | null): string { - if (locale === null || href === "" || !href.startsWith("/") || href.startsWith("//")) return href; - const url = new URL(href, "http://localhost"); + // An absent href is a shape the building blocks document as optional (a non-linked page item, a + // header with no sort target) — it must not throw here, or a page renders for every visitor + // except the ones who chose a language. + if (locale === null || !href || href.startsWith("//")) return href; + // A query-only href ("?" — the filter bar's documented "clear" target) keeps that shape; anything + // else must be host-relative, or it is someone else's URL to state a language for. + const queryOnly = href.startsWith("?"); + if (!queryOnly && !href.startsWith("/")) return href; + const url = new URL(href, "http://localhost/"); url.searchParams.set("locale", locale); - return `${url.pathname}${url.search}${url.hash}`; + return queryOnly ? `${url.search}${url.hash}` : `${url.pathname}${url.search}${url.hash}`; } // Both are asked for on every render (the tag, the language picker) but depend only on the diff --git a/src/i18n/locales/en-US.ts b/src/i18n/locales/en-US.ts index 084f13d..0129513 100644 --- a/src/i18n/locales/en-US.ts +++ b/src/i18n/locales/en-US.ts @@ -45,7 +45,6 @@ const messages = { "common.edit": "Edit", "common.new": "New", "common.remove": "Remove", - "common.save": "Save", "consent.allow": "Allow", "consent.deny": "Deny", diff --git a/src/i18n/locales/sv-SE.ts b/src/i18n/locales/sv-SE.ts index f8ed9da..e09a926 100644 --- a/src/i18n/locales/sv-SE.ts +++ b/src/i18n/locales/sv-SE.ts @@ -36,7 +36,6 @@ const messages: CoreMessages = { "common.edit": "Redigera", "common.new": "Ny", "common.remove": "Ta bort", - "common.save": "Spara", "consent.allow": "Tillåt", "consent.deny": "Neka", diff --git a/src/i18n/translate.ts b/src/i18n/translate.ts index c21056f..717899f 100644 --- a/src/i18n/translate.ts +++ b/src/i18n/translate.ts @@ -24,6 +24,9 @@ const pluralRules = new Map(); export function createTranslator({ catalogs, locale }: TranslatorOptions): Translate { return (key, vars) => { for (const catalog of catalogs) { + // Own keys only: `t("toString")` must fall through to the key itself like any other unknown + // one, not pick up Object.prototype. + if (!Object.hasOwn(catalog, key)) continue; const message = catalog[key]; if (message === undefined) continue; return interpolate(isPluralMessage(message) ? selectPlural(message, locale, vars?.["count"]) : message, vars); @@ -56,6 +59,7 @@ function rulesFor(locale: string): Intl.PluralRules { function interpolate(text: string, vars: TranslateVars | undefined): string { if (vars === undefined) return text; return text.replace(PLACEHOLDER, (whole, name: string) => { + if (!Object.hasOwn(vars, name)) return whole; const value = vars[name]; return value === undefined ? whole : String(value); }); diff --git a/src/i18n/view-locals.test.ts b/src/i18n/view-locals.test.ts new file mode 100644 index 0000000..c29345b --- /dev/null +++ b/src/i18n/view-locals.test.ts @@ -0,0 +1,32 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { ENGLISH } from "./english.ts"; +import { localeHref } from "./locale.ts"; +import { i18nLocals, type I18nRequest } from "./view-locals.ts"; + +const request = (overrides: Partial = {}): I18nRequest => ({ + locale: "sv-SE", + localeHref: (href) => href, + locales: ["en-US", "sv-SE"], + t: ENGLISH, + url: new URL("http://localhost/admin/users?q=ada"), + ...overrides, +}); + +test("localeSwitch offers this same page in every installed locale, marking the current one", () => { + const locals = i18nLocals(request()); + assert.deepEqual(locals.localeSwitch.map((c) => c.href), ["/admin/users?q=ada&locale=en-US", "/admin/users?q=ada&locale=sv-SE"]); + assert.deepEqual(locals.localeSwitch.map((c) => c.current), [false, true]); + assert.match(locals.localeSwitch[1]?.label ?? "", /svenska/i); // named in its own language +}); + +test("localeParam is the tag only when the URL asked — it is what the GET forms carry", () => { + // The probe asks the very function that decides, so the two can't drift apart. + assert.equal(i18nLocals(request()).localeParam, null); // identity localeHref ⇒ nothing was chosen + assert.equal(i18nLocals(request({ localeHref: (href) => localeHref(href, "sv-SE") })).localeParam, "sv-SE"); +}); + +test("dir follows the locale's script", () => { + assert.equal(i18nLocals(request()).dir, "ltr"); + assert.equal(i18nLocals(request({ locale: "ar-EG" })).dir, "rtl"); +}); diff --git a/src/plugin-host/hooks.test.ts b/src/plugin-host/hooks.test.ts index 7c25f40..1acb126 100644 --- a/src/plugin-host/hooks.test.ts +++ b/src/plugin-host/hooks.test.ts @@ -24,18 +24,22 @@ test("runBootHooks runs each onBoot in order, skips plugins without one, and a t test("runRequestHooks short-circuits on the first RouteResult (with its plugin); later hooks skipped", async () => { const calls: string[] = []; + const scoped: string[] = []; // each hook is handed a context built for its own plugin + const contextFor = (pluginId: string) => { scoped.push(pluginId); return ctx; }; const short = await runRequestHooks([ plugin("a", { onRequest: () => void calls.push("a") }), // returns void → continue plugin("b", { onRequest: () => { calls.push("b"); return { html: "stop" }; } }), plugin("c", { onRequest: () => void calls.push("c") }), // never reached - ], ctx); + ], contextFor); assert.deepEqual(short?.result, { html: "stop" }); assert.equal(short?.plugin.id, "b"); // the owning plugin (so a `view` result resolves correctly) + assert.equal(short?.ctx, ctx); // …and the context it ran on, for rendering its view assert.deepEqual(calls, ["a", "b"]); + assert.deepEqual(scoped, ["a", "b"]); // a plugin without the hook never builds a context // No hook short-circuits → null (proceed with normal routing). - assert.equal(await runRequestHooks([plugin("a", { onRequest: () => {} })], ctx), null); + assert.equal(await runRequestHooks([plugin("a", { onRequest: () => {} })], contextFor), null); }); test("runResponseHooks runs every onResponse as an observer with the result; a throw fails", async () => { diff --git a/src/plugin-host/hooks.ts b/src/plugin-host/hooks.ts index 85a68f5..ceae572 100644 --- a/src/plugin-host/hooks.ts +++ b/src/plugin-host/hooks.ts @@ -13,11 +13,17 @@ export async function runBootHooks(plugins: Plugin[]): Promise { // Before route matching. The first hook to return a RouteResult short-circuits the request — its // result becomes the response and later hooks + the route handler are skipped. Returns that result -// with its owning plugin (so a `view` result resolves against that plugin's views), or null to proceed. -export async function runRequestHooks(plugins: Plugin[], ctx: RequestContext): Promise<{ plugin: Plugin; result: RouteResult } | null> { +// with its owning plugin (so a `view` result resolves against that plugin's views), or null to +// proceed. Each hook gets a context scoped to its own plugin, so `ctx.t` reads that plugin's catalog. +export async function runRequestHooks( + plugins: Plugin[], + contextFor: (pluginId: string) => RequestContext, +): Promise<{ ctx: RequestContext; plugin: Plugin; result: RouteResult } | null> { for (const plugin of plugins) { - const result = await plugin.hooks?.onRequest?.(ctx); - if (result != null) return { plugin, result }; + if (!plugin.hooks?.onRequest) continue; + const ctx = contextFor(plugin.id); + const result = await plugin.hooks.onRequest(ctx); + if (result != null) return { ctx, plugin, result }; } return null; } diff --git a/src/ui/shell-context.ts b/src/ui/shell-context.ts index 3d96abf..aa173a2 100644 --- a/src/ui/shell-context.ts +++ b/src/ui/shell-context.ts @@ -41,7 +41,7 @@ export interface ShellModel { export function shellUser(user: User | null | undefined, t: Translate = ENGLISH): ShellUser { if (!user) { const guest = t("shell.guest"); - return { email: "", initials: guest.slice(0, 1).toUpperCase(), name: guest }; + return { email: "", initials: ([...guest][0] ?? "?").toUpperCase(), name: guest }; // by character: the word is translated } const local = user.email.split("@")[0] || user.email; return { email: user.email, initials: (local.slice(0, 2) || "U").toUpperCase(), name: local }; diff --git a/views/index.ejs b/views/index.ejs index a2a9a0a..7d17fb4 100644 --- a/views/index.ejs +++ b/views/index.ejs @@ -5,6 +5,8 @@ handler; this renders until then. Data: model { nav, shell }. %><% const nav = include("partials/nav-tree", { nodes: model.nav }); + // These four messages carry markup ( spans), so they render raw — the documented + // markup-carrying case. They interpolate nothing, so there is no untrusted data to escape. const body = `
diff --git a/views/oauth-consent.ejs b/views/oauth-consent.ejs index fee58a2..150839a 100644 --- a/views/oauth-consent.ejs +++ b/views/oauth-consent.ejs @@ -25,14 +25,14 @@ <%= brand %>
<%- include("partials/auth-card", { - action: "/oauth2/consent", + action: localeHref("/oauth2/consent"), body, method: "post", sub: t("consent.sub", { client: consent.client }), title: t("consent.title", { client: consent.client }), }) %> <% if (consent.account) { %> - + "> <%= t("consent.notYou") %> diff --git a/views/partials/auth-card.ejs b/views/partials/auth-card.ejs index 358c9be..d7e3a8b 100644 --- a/views/partials/auth-card.ejs +++ b/views/partials/auth-card.ejs @@ -17,7 +17,7 @@ const providers = (sso && sso.providers) || []; const alt = locals.alt; -%> -
action="<%= locals.action %>"<% } %>> + action="<%= localeHref(locals.action) %>"<% } %>>
<% if (back) { %><%= back.label %><% } %>

<%= locals.title %>

<% if (locals.sub) { %>

<%= locals.sub %>

<% } %>
<% if (providers.length) { -%>
"> diff --git a/views/partials/menu.ejs b/views/partials/menu.ejs index d630138..200ec67 100644 --- a/views/partials/menu.ejs +++ b/views/partials/menu.ejs @@ -11,14 +11,14 @@ Item ∈ { head } · { sep } · { label, icon?, href? ⇒ , hreflang?, current?, danger? } (default: menu-item button) · { group: { legend?, name, control?(="checkbox"|"radio"), options:{value,label,checked?}[] } } %><% - const t = locals.trigger || {}; - const sumCls = "class" in t ? t.class : "btn"; + const trigger = locals.trigger || {}; // not `t` — that name is the translator in every view + const sumCls = "class" in trigger ? trigger.class : "btn"; const items = locals.items || []; const popCls = "menu-pop" + (locals.align === "left" ? " left" : "") + (locals.up ? " up" : ""); const width = locals.width; -%>
"<%= locals.open ? " open" : "" %>> - class="<%= sumCls %>"<% } %><% if (t.label) { %> aria-label="<%= t.label %>"<% } %>><% if (t.html != null) { %><%- t.html %><% } else { if (t.icon) { %><% } if (t.text) { %><%= t.text %><% } } %> + class="<%= sumCls %>"<% } %><% if (trigger.label) { %> aria-label="<%= trigger.label %>"<% } %>><% if (trigger.html != null) { %><%- trigger.html %><% } else { if (trigger.icon) { %><% } if (trigger.text) { %><%= trigger.text %><% } } %>
style="min-width:<%= typeof width === "number" ? width + "px" : width %>"<% } %>> <% items.forEach((it) => { -%> <% if (it.head != null) { -%> diff --git a/views/partials/shell.ejs b/views/partials/shell.ejs index cac2628..4226976 100644 --- a/views/partials/shell.ejs +++ b/views/partials/shell.ejs @@ -70,7 +70,7 @@
<%= t("shell.signedInAs", { name: user.name }) %>
<%# Sign out is a state change → a POST form (not a GET link), CSRF-guarded by app.ts %> - + ">