diff --git a/src/app.test.ts b/src/app.test.ts
index 0fbc46b..1e26154 100644
--- a/src/app.test.ts
+++ b/src/app.test.ts
@@ -5,7 +5,6 @@ import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { after, before, test, type TestContext } from "node:test";
import { fileURLToPath } from "node:url";
-import * as ejs from "ejs";
import { createApp } from "./app.ts";
import type { Plugin } from "./plugin.ts";
import { contentTypeFor, resolveStaticPath, routePublic } from "./static.ts";
@@ -51,7 +50,7 @@ test("renders branding from the menu config into the shell: logo + default theme
assert.match(html, /id="theme-dark"\s+checked/); // config default theme reaches the switch
});
-test("serves a static file: GET sends body + content-type, HEAD sends headers only", async () => {
+test("static serving: GET sends body + content-type, HEAD headers only, unsafe paths → 403", async () => {
const get = await fetch(base + "/public/css/styles.css");
assert.equal(get.status, 200);
assert.match(get.headers.get("content-type") ?? "", /text\/css/);
@@ -60,6 +59,10 @@ test("serves a static file: GET sends body + content-type, HEAD sends headers on
assert.equal(head.status, 200);
assert.ok(Number(head.headers.get("content-length")) > 0);
assert.equal((await head.text()).length, 0);
+
+ // Encoded traversal and a NUL byte are refused before touching the filesystem.
+ assert.equal((await fetch(base + "/public/..%2f..%2fapp.ts")).status, 403);
+ assert.equal((await fetch(base + "/public/%00")).status, 403);
});
// Production caches compiled templates; rendering must stay correct across repeated requests.
@@ -102,13 +105,6 @@ test("renders the 500 HTML page when a handler throws", async () => {
}
});
-// 403 has no first-party route yet (guards land in §4), so assert the template renders.
-test("renders the 403 error page as HTML", async () => {
- const html = await ejs.renderFile(join(viewsDir, "403.ejs"), { title: "Forbidden" });
- assert.match(html, /403/);
- assert.match(html, /styles\.css/);
-});
-
// A test plugin exercising each RouteResult shape, a path param, and the permission gate.
const demoPlugin: Plugin = {
apiVersion: "1.0.0",
@@ -166,8 +162,12 @@ test("mounts plugin routes: params, html/json/redirect/view results, and the per
assert.match(await css.text(), /\.demo/);
assert.equal((await fetch(url + "/public/demo/..%2f..%2fplugin.ts")).status, 403); // traversal still blocked
- // gated route with no session → 403
- assert.equal((await fetch(url + "/demo/secret")).status, 403);
+ // gated route with no session → the rendered 403 page (covers the gate + 403.ejs over HTTP)
+ const denied = await fetch(url + "/demo/secret");
+ assert.equal(denied.status, 403);
+ const deniedBody = await denied.text();
+ assert.match(deniedBody, /403/);
+ assert.match(deniedBody, /styles\.css/);
// known path + wrong method → 405 with Allow; unknown path → 404
const wrong = await fetch(url + "/demo/data", { method: "DELETE" });
@@ -199,11 +199,6 @@ test("plugin hooks: onRequest can short-circuit a request and onResponse observe
assert.ok(seen.includes("/hooked/ok:handler ran"));
});
-test("rejects unsafe static request paths (encoded traversal, NUL) with 403", async () => {
- assert.equal((await fetch(base + "/public/..%2f..%2fapp.ts")).status, 403);
- assert.equal((await fetch(base + "/public/%00")).status, 403);
-});
-
test("resolveStaticPath blocks traversal and control chars, allows nested files", () => {
assert.equal(resolveStaticPath("/srv/public", "../app.ts"), null);
assert.equal(resolveStaticPath("/srv/public", "a\x00b"), null);
diff --git a/src/view-resolver.test.ts b/src/view-resolver.test.ts
index 71e1acc..38546cd 100644
--- a/src/view-resolver.test.ts
+++ b/src/view-resolver.test.ts
@@ -8,16 +8,13 @@ import { renderPluginView, resolveViewPath } from "./view-resolver.ts";
const coreViewsDir = join(dirname(fileURLToPath(import.meta.url)), "..", "views");
-test("resolveViewPath resolves names + nested subfolders within the plugin's views dir", () => {
+test("resolveViewPath resolves names/nested subfolders within the views dir, rejects traversal + control chars", () => {
const dir = "/srv/plugins";
assert.equal(resolveViewPath(dir, "demo", "page"), "/srv/plugins/demo/views/page.ejs");
assert.equal(resolveViewPath(dir, "demo", "shifts/edit"), "/srv/plugins/demo/views/shifts/edit.ejs");
assert.equal(resolveViewPath(dir, "demo", "page.ejs"), "/srv/plugins/demo/views/page.ejs"); // extension not doubled
-});
-
-test("resolveViewPath rejects traversal and control chars", () => {
- assert.equal(resolveViewPath("/srv/plugins", "demo", "../../secret"), null);
- assert.equal(resolveViewPath("/srv/plugins", "demo", "a\x00b"), null);
+ assert.equal(resolveViewPath(dir, "demo", "../../secret"), null); // traversal escapes the dir
+ assert.equal(resolveViewPath(dir, "demo", "a\x00b"), null); // control char
});
test("renderPluginView: a (nested) view includes a core building-block partial and its own partial", async (t: TestContext) => {
diff --git a/todo.md b/todo.md
index eb5352a..f858486 100644
--- a/todo.md
+++ b/todo.md
@@ -54,7 +54,7 @@ everything via Docker.
- [x] Wire branding into the app shell. → Completes the §2 branding chain (name/sub already flowed). `shell.ejs` now renders `brand.logo` as `
` when set, else the default `#i-box` brand-mark; the `theme` local (already forwarded to the theme-switch) is now supplied. `buildDashboardModel` puts `menu.branding.logo` into `shell.brand` and `menu.branding.theme` into `shell.theme` (both omitted when unset, so a clean clone is unchanged → brand-mark + auto theme); `views/index.ejs` forwards `theme` to the shell. Added a `.brand-logo` CSS rule (22px, matches `.brand-mark` sizing). Tests-first: `shell.test.ts` (logo replaces the mark + default theme checked; no-logo ⇒ mark + auto) + extended `dashboard.test.ts` (logo→brand, theme→shell.theme) + an `app.test.ts` integration rendering `createApp({ menu })` end-to-end (logo `
` + `theme-dark` checked on `/`). Default-app shell rendering is byte-equivalent, so the visual E2E is unaffected; typecheck + 109 units green. The §2 plugin host is feature-complete (remaining §2 items are the project-wide review + comment/test cleanup).
- [x] Run the architecture _and_ the stability reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on all of `src/`, `views/`, `config/`, Docker/tsconfig. Verdict: architecture sound + disciplined, no crash/security defect in the current path (fail-loud, traversal guards, JWT/cookie defenses all confirmed). **Fixed now:** (1) HIGH — `PluginHooks` was typed+documented but never invoked; wired it (`src/hooks.ts`: `runBootHooks`/`runRequestHooks`/`runResponseHooks`) — `server.ts` runs `onBoot` after discovery before listen, `app.ts` runs `onRequest` (before routing, first non-void short-circuits, renders against its plugin) + `onResponse` (after handler, observer, throw→500); skipped entirely when no plugin declares a hook (hot path free); `hooks.test.ts` + an `app.test.ts` integration. (2) `discovery.ts` `fail` helper retyped `: void`. (3) Documented the template trust boundary in `docs/plugin-contract.md` (raw `html`/`*.html` fields; URL sinks escaped but not scheme-checked) + tightened the Hooks prose to the wired semantics. **Deferred (reviewer-scoped, not §2):** extract a shared `buildShellContext` out of `dashboard.ts` and route the built-in screens through `matchRoute`/`isAuthorized` → §5 (premature at one call site); a `safeUrl()` helper for href sinks → §4 (no untrusted URLs until upstream data flows); doc/type-duplication + non-local `§N` refs → the §2 comment-cleanup item; HEAD-render cost + dev empty-secret fallback → negligible. typecheck + 113 units green; boot smoke-tested.
- [x] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. → Pass over the §2 accretion (the §0/§1 cleanup at line 21 stands). Tightened the verbose module-header blocks (`plugin.ts`, `discovery.ts`, `router.ts`, `dashboard.ts`) and collapsed the `checkApiVersion` rule comment to a one-liner that points at the contract doc (the if-chain + messages already document it). Removed now-stale forward-refs ("router wiring is the next §2 item", "rendered in the shell — next §2 item"). README: corrected the **Status** note (it undersold — §1 design system + the whole §2 plugin host are built, not just a scaffold), dropped the stale `_(planned)_`/"planned to extract" markers on **Building a plugin** and **Building blocks** (both shipped; auth guards still flagged §4), and named the real helpers. Left the security-rationale comments (jwt/cookie/static/paginate) and the EJS partials' config-doc headers intact — they carry vital info / are the only schema for untyped locals. No anchor links broke; typecheck + 113 units green.
-- [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us.
+- [x] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. → Reviewed all 24 test files. The suite already follows the deliberate per-module "matrix + edge" pattern from the §0/§1 merge (line 22), so most files carry no fat and force-merging distinct concerns would only hurt readability. Removed the genuine §2-era overlaps, all in `app.test.ts`: merged the two HTTP static tests into one (GET/HEAD + traversal/NUL→403), and dropped the standalone "renders the 403 error page" `ejs.renderFile` stopgap (its comment even said "403 has no first-party route yet") — the gated plugin route now exercises 403 over HTTP, so the template assertions (status + 403.ejs body + stylesheet link) moved there; also dropped the now-unused `ejs` import. Unified `view-resolver.test.ts`'s two `resolveViewPath` cases (resolve + reject) into one. 113 → 110 tests, zero coverage lost; typecheck + tests green.
## 3. Ory stack — compose + config
- [ ] `postgres` service (pinned tag); separate DB/schema per Kratos/Keto/Hydra.