§7 comment cleanup (todo §7); targeted density pass over the §7 accretion. The §7 modules were authored dense (the reference plugin is a teaching artifact, the host additions concise), so two wins: tightened chrome.ts's module header (7→5 lines, dropped the input-list duplicated by ChromeOptions + the nav-composition restatement already carried by the nav field/markCurrent comments); fixed a stale forward-ref in docs/plugin-contract.md (the safeUrl() helper said "§5/§7" but §7 deferred it to §9). Left intact: the reference plugin's instructive comments, the EJS view config-doc headers, and the contract doc + plugin README (authored concise in §7). README Status/_(planned)_/Layout refresh stays §9. Docs/comments-only; typecheck + 301 units green.
This commit is contained in:
@@ -166,8 +166,7 @@ safety of the data it renders**:
|
|||||||
item, a breadcrumb, `brand.logo` — is emitted as-is inside the attribute: a `javascript:` or
|
item, a breadcrumb, `brand.logo` — is emitted as-is inside the attribute: a `javascript:` or
|
||||||
`data:` URL from upstream/user data becomes live XSS. When a URL comes from data you don't
|
`data:` URL from upstream/user data becomes live XSS. When a URL comes from data you don't
|
||||||
control, restrict it to a relative (`/`, `?`, `#`) or `http(s):` URL before handing it to a
|
control, restrict it to a relative (`/`, `?`, `#`) or `http(s):` URL before handing it to a
|
||||||
partial. (A shared `safeUrl()` helper will land with the first plugin that renders untrusted
|
partial. (A shared `safeUrl()` helper is planned for §9, with the redirect-URI allowlist work.)
|
||||||
URL data, §5/§7.)
|
|
||||||
|
|
||||||
## RequestContext
|
## RequestContext
|
||||||
|
|
||||||
|
|||||||
@@ -1,10 +1,8 @@
|
|||||||
// Page chrome for plugin pages (todo §7): the brand / global-nav / user / theme / csrf block a
|
// Page chrome for plugin pages (todo §7): the brand / global-nav / user / theme / csrf block a
|
||||||
// plugin view hands to partials/shell so its page looks native — the same shell the dashboard and
|
// plugin view hands to partials/shell so its page looks native — the same shell the dashboard and
|
||||||
// admin screens render. Pure; the host builds it once per plugin request (it has the menu config,
|
// admin screens render. Pure; the host builds it per plugin request and exposes it on ctx.chrome.
|
||||||
// the discovered plugins, the signed-in user and the request CSRF token) and exposes it on
|
// nav is the global menu — Dashboard + every plugin's fragment + the gated admin section — run
|
||||||
// ctx.chrome. The nav is the global menu — a Dashboard home link, every discovered plugin's nav
|
// through composeNav (override + per-user filter) and current-marked for the request path.
|
||||||
// fragment, and the gated admin section — run through composeNav (override + per-user role filter),
|
|
||||||
// with the node whose href matches the current path marked `current` (its ancestors opened).
|
|
||||||
|
|
||||||
import { adminSection } from "./admin-nav.ts";
|
import { adminSection } from "./admin-nav.ts";
|
||||||
import type { User } from "./context.ts";
|
import type { User } from "./context.ts";
|
||||||
|
|||||||
2
todo.md
2
todo.md
@@ -113,7 +113,7 @@ everything via Docker.
|
|||||||
- [x] Reference plugin (e.g. people directory or scheduling): list page fetching upstream data, a form that forwards writes upstream, permission-gated nav. → `plugins/scheduling/` is the worked example the docs already reference (so contract + reference agree). `shifts.ts` = an injectable-`fetch` upstream REST client (`createUpstream`, stand-in for the customer's backend — the plugin is stateless) + thin handler **factories** bound to it: `listShifts` fetches `/shifts`, filters by `?q`, renders the data-table (upstream down ⇒ a recoverable error page, never a host 500); `newShiftForm` renders the form; `createShift` reads its own body, **CSRF-guards via `ctx.verifyCsrf`** (403 on a bad token), validates, forwards the create upstream, then POST-redirect-GET (a 4xx upstream ⇒ a recoverable 502 form keeping the input). `plugin.ts` = the manifest: `apiVersion` literal, namespaced `scheduling:read`/`scheduling:write` perms, **permission-gated nav** ("Shifts" gated on `read` so the whole "Scheduling" header vanishes for non-holders), routes gated `read`/`write`. Views (`shifts.ejs`, `shift-new.ejs` + the plugin's **own** `partials/shift-form.ejs`) compose the core building blocks (shell/nav-tree/filter-bar/data-table/field/alert via `include()`) around the **native app shell**. **New host capability so a plugin page is native + secure** (`src/chrome.ts` `buildPluginChrome`): `ctx.chrome` = brand/global-nav/user/theme/csrf the view hands to `partials/shell` — the global menu (a Dashboard link + every discovered plugin's nav fragment + the gated admin section), composed + role-filtered + current-marked by request path; `ctx.verifyCsrf(submitted)` = the host's bound double-submit verifier (plugin never sees the secret). Both added to `RequestContext` (defaulted in `buildContext`, anonymous chrome / fail-closed verify), built per plugin route in `app.ts` (CSRF cookie set when fresh so forms carry a token). The dashboard now merges plugin nav fragments too (reachable from `/`; gated ⇒ invisible to anonymous, so the visual E2E is byte-identical). Out of the box: bootstrap now grants the demo admin `scheduling:read`/`scheduling:write` (generalized `seedAdmin` to a roles list, env `ADMIN_ROLES`); the dev compose runs a tiny stdlib mock upstream (`examples/shifts-upstream/`, `SCHEDULING_UPSTREAM`) so `docker compose up` shows it working. Tooling: `plugins/` added to tsconfig + the `npm test` glob (so plugin authors' tests run via `docker compose run web npm test`). Tests-first: `plugins/scheduling/shifts.test.ts` (client w/ mock fetch · validation · list/create handlers incl. CSRF-403, validation-400, PRG, upstream-502 · form model), `src/chrome.test.ts` (brand/nav/role-filter/current/branding), `app.test.ts` (a plugin view renders the chrome + the CSRF round-trip over HTTP), `dashboard.test.ts` (plugin-fragment merge, gated), `bootstrap.test.ts` (multi-role grant). README **Building a plugin** + Layout and `docs/plugin-contract.md` (the `ctx.chrome`/`ctx.verifyCsrf` additions, the upstream pattern, the dev/test pointer) updated. typecheck + **296 units** green; the Ory-free **visual E2E** (real built image) confirms the plugin is discovered at boot, the routes/nav are permission-gated (anonymous → 403, hidden from the dashboard), and the dashboard still renders identically; live full-stack boot-verified — the stack comes up with the plugin + mock upstream, the upstream serves the seeded shifts and is reachable from `web`, and bootstrap grants the admin `admin`/`scheduling:read`/`scheduling:write` in real Keto (all `allowed:true`); torn down. The authenticated browser happy-path (login → rendered list) is deferred to §8's full E2E (line 114 verifies the contract end-to-end) — it needs the cross-host Playwright login infra, not curl. `apiVersion` stays `1.0.0` (the contract is still being assembled in §7, so chrome/verifyCsrf are part of the initial surface — no minor bump, no warn noise).
|
- [x] Reference plugin (e.g. people directory or scheduling): list page fetching upstream data, a form that forwards writes upstream, permission-gated nav. → `plugins/scheduling/` is the worked example the docs already reference (so contract + reference agree). `shifts.ts` = an injectable-`fetch` upstream REST client (`createUpstream`, stand-in for the customer's backend — the plugin is stateless) + thin handler **factories** bound to it: `listShifts` fetches `/shifts`, filters by `?q`, renders the data-table (upstream down ⇒ a recoverable error page, never a host 500); `newShiftForm` renders the form; `createShift` reads its own body, **CSRF-guards via `ctx.verifyCsrf`** (403 on a bad token), validates, forwards the create upstream, then POST-redirect-GET (a 4xx upstream ⇒ a recoverable 502 form keeping the input). `plugin.ts` = the manifest: `apiVersion` literal, namespaced `scheduling:read`/`scheduling:write` perms, **permission-gated nav** ("Shifts" gated on `read` so the whole "Scheduling" header vanishes for non-holders), routes gated `read`/`write`. Views (`shifts.ejs`, `shift-new.ejs` + the plugin's **own** `partials/shift-form.ejs`) compose the core building blocks (shell/nav-tree/filter-bar/data-table/field/alert via `include()`) around the **native app shell**. **New host capability so a plugin page is native + secure** (`src/chrome.ts` `buildPluginChrome`): `ctx.chrome` = brand/global-nav/user/theme/csrf the view hands to `partials/shell` — the global menu (a Dashboard link + every discovered plugin's nav fragment + the gated admin section), composed + role-filtered + current-marked by request path; `ctx.verifyCsrf(submitted)` = the host's bound double-submit verifier (plugin never sees the secret). Both added to `RequestContext` (defaulted in `buildContext`, anonymous chrome / fail-closed verify), built per plugin route in `app.ts` (CSRF cookie set when fresh so forms carry a token). The dashboard now merges plugin nav fragments too (reachable from `/`; gated ⇒ invisible to anonymous, so the visual E2E is byte-identical). Out of the box: bootstrap now grants the demo admin `scheduling:read`/`scheduling:write` (generalized `seedAdmin` to a roles list, env `ADMIN_ROLES`); the dev compose runs a tiny stdlib mock upstream (`examples/shifts-upstream/`, `SCHEDULING_UPSTREAM`) so `docker compose up` shows it working. Tooling: `plugins/` added to tsconfig + the `npm test` glob (so plugin authors' tests run via `docker compose run web npm test`). Tests-first: `plugins/scheduling/shifts.test.ts` (client w/ mock fetch · validation · list/create handlers incl. CSRF-403, validation-400, PRG, upstream-502 · form model), `src/chrome.test.ts` (brand/nav/role-filter/current/branding), `app.test.ts` (a plugin view renders the chrome + the CSRF round-trip over HTTP), `dashboard.test.ts` (plugin-fragment merge, gated), `bootstrap.test.ts` (multi-role grant). README **Building a plugin** + Layout and `docs/plugin-contract.md` (the `ctx.chrome`/`ctx.verifyCsrf` additions, the upstream pattern, the dev/test pointer) updated. typecheck + **296 units** green; the Ory-free **visual E2E** (real built image) confirms the plugin is discovered at boot, the routes/nav are permission-gated (anonymous → 403, hidden from the dashboard), and the dashboard still renders identically; live full-stack boot-verified — the stack comes up with the plugin + mock upstream, the upstream serves the seeded shifts and is reachable from `web`, and bootstrap grants the admin `admin`/`scheduling:read`/`scheduling:write` in real Keto (all `allowed:true`); torn down. The authenticated browser happy-path (login → rendered list) is deferred to §8's full E2E (line 114 verifies the contract end-to-end) — it needs the cross-host Playwright login infra, not curl. `apiVersion` stays `1.0.0` (the contract is still being assembled in §7, so chrome/verifyCsrf are part of the initial surface — no minor bump, no warn noise).
|
||||||
- [x] Verify the full plugin contract end-to-end against the README. → Cross-checked every contract claim in `README.md` (**Building a plugin**, **Where plugins live**, **Layout**) + `docs/plugin-contract.md` + `plugins/scheduling/README.md` against the implementation (`plugin.ts`/`discovery.ts`/`router.ts`/`view-resolver.ts`/`chrome.ts`/`context.ts`/`guards.ts`/`app.ts` `sendResult`) and the shipped reference plugin. **Contract holds end-to-end**: derived id+mount (no `id`/`basePath` in the manifest), `apiVersion` literal + `checkApiVersion` table, permission-gated nav (the "Scheduling" header vanishes without `scheduling:read`) + gated routes, every `RouteResult` shape incl. `view`+`status` and `redirect`-PRG, `ctx.chrome` → native app shell, `ctx.verifyCsrf` double-submit (403 on bad token), `can()`/`GuardError(403)`, the view-resolver rendering `plugins/<id>/views/*` while `include()`-ing **both** core partials (shell/filter-bar/data-table/field/alert/nav-tree — all present in `views/partials/`) and the plugin's own `partials/shift-form`, per-plugin static, upstream-fetch statelessness; all three reference-plugin icons (`i-cal`/`i-plus`/`i-user`) are registered in `src/icons.ts`. **One material doc drift found + fixed:** `docs/plugin-contract.md`'s reserved-id list was stale — it named 8 ids but `RESERVED_PLUGIN_IDS` has 10 (the §5 `admin` + §6 `oauth2` mounts were missing), so an author would wrongly think `plugins/admin/` or `plugins/oauth2/` was allowed when discovery refuses them (test-covered in `discovery.test.ts`). Updated the doc to list all 10 accurately (docs-only, no behaviour change). typecheck + 296 units green.
|
- [x] Verify the full plugin contract end-to-end against the README. → Cross-checked every contract claim in `README.md` (**Building a plugin**, **Where plugins live**, **Layout**) + `docs/plugin-contract.md` + `plugins/scheduling/README.md` against the implementation (`plugin.ts`/`discovery.ts`/`router.ts`/`view-resolver.ts`/`chrome.ts`/`context.ts`/`guards.ts`/`app.ts` `sendResult`) and the shipped reference plugin. **Contract holds end-to-end**: derived id+mount (no `id`/`basePath` in the manifest), `apiVersion` literal + `checkApiVersion` table, permission-gated nav (the "Scheduling" header vanishes without `scheduling:read`) + gated routes, every `RouteResult` shape incl. `view`+`status` and `redirect`-PRG, `ctx.chrome` → native app shell, `ctx.verifyCsrf` double-submit (403 on bad token), `can()`/`GuardError(403)`, the view-resolver rendering `plugins/<id>/views/*` while `include()`-ing **both** core partials (shell/filter-bar/data-table/field/alert/nav-tree — all present in `views/partials/`) and the plugin's own `partials/shift-form`, per-plugin static, upstream-fetch statelessness; all three reference-plugin icons (`i-cal`/`i-plus`/`i-user`) are registered in `src/icons.ts`. **One material doc drift found + fixed:** `docs/plugin-contract.md`'s reserved-id list was stale — it named 8 ids but `RESERVED_PLUGIN_IDS` has 10 (the §5 `admin` + §6 `oauth2` mounts were missing), so an author would wrongly think `plugins/admin/` or `plugins/oauth2/` was allowed when discovery refuses them (test-covered in `discovery.test.ts`). Updated the doc to list all 10 accurately (docs-only, no behaviour change). typecheck + 296 units green.
|
||||||
- [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §7 reference-plugin surfaces). Architecture: **no Critical**; Product: **no Critical** this checkpoint. **Fixed now (tests-first):** (1) HIGH (arch + product Blocker) — the contract claimed declaring `permissions` "seeds Keto" but nothing wired it, and the host hardcoded the plugin-specific `scheduling:read`/`write` into `ADMIN_ROLES` (`compose.yml` + `bootstrap.ts`) — core knowing about a plugin. Made the claim true *and* removed the coupling: new pure `seedRoles(adminRolesEnv, declaredTokens)` (union+dedup, trims both sides); `bootstrap.main()` now `discoverPlugins()` → seeds the demo admin `admin`(/`ADMIN_ROLES`) ∪ every discovered plugin's declared tokens; `compose.yml` `ADMIN_ROLES` default dropped to just `admin` (host names no plugin). Clean-clone behaviour is unchanged (admin still ends up with the scheduling tokens, now derived via discovery, not hardcoded — `readRoles` enumerates the Role namespace so a granted token reaches the JWT). Docs corrected: a route/nav `permission` **is a coarse role** granted as a Keto `Role:<token>#members` tuple (vs the separate fine-grained `Resource` tier). (2) HIGH (arch) — no delineated plugin SDK: the reference plugin imported six deep `src/*` internals, none of them the documented-stable surface. Added `src/plugin-api.ts` — the one barrel a plugin imports (`definePlugin` + manifest/handler/ctx types + guards + body/CSRF/list-query helpers); repointed `plugins/scheduling/{plugin,shifts}.ts` to it, so the contract boundary now lives in code (the host can refactor internals freely behind it). (3) MEDIUM (arch L1 + product) — per-plugin CSS was documented (`/public/<id>/`) but unusable (the shell `<head>` had no slot) and the reference shipped no `public/`. Added an optional `styles` slot to `shell.ejs` (extra stylesheet hrefs, default `[]`, backward-compatible) + `plugins/scheduling/public/scheduling.css` linked from both reference views (`.scheduling-page` scope) — closes the only unexercised contract feature + the anatomy-diagram drift. (4) MEDIUM (arch M4) — the reference demonstrated no hook; added `hooks.onBoot` validating `SCHEDULING_UPSTREAM` fail-loud at boot (`assertHttpUrl` — unparseable or non-http(s) aborts the boot, a typo no longer degrades every request), closing the hooks-coverage gap. (5) MEDIUM (arch M3) — `ctx.chrome` was built twice on a matched plugin request when any onRequest hook exists; now built at most once per request (memoized lazy `chrome()`), hot path still chrome-free. (6) HIGH/MEDIUM (product) — doc honesty: fixed the false `visual.spec.ts` comment ("covered by the full-stack suites" — it isn't), softened the contract's "every plugin ships *with* a Playwright test" to match reality (the reference's authed list/form happy-path is the §8 item), and added an **Upstream contract** block (routes/JSON shape/status-code expectations) to `plugins/scheduling/README.md`. Stability-reviewer run as a local PR: **APPROVE, no Critical/High**; addressed both Low nits (`seedRoles` now trims/dedups declared tokens too; added an `onBoot`-binding seam test on the real manifest). typecheck + **301 units** green (296 → 301). **Deferred (reviewer-scoped, not the §7 checkpoint):** the host **internal route-table** (fold the admin/oauth if-ladder into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` from it — arch M1) → **§9** (now the top structural item; standalone change, not a review-fix); the `safeUrl()` href helper + a reference row that renders an untrusted URL through it (arch L2 / product 🟢) → **§9** (first untrusted-URL flow / redirect-allowlist work); the data-table **empty state** + **success-flash** after writes (product 🟡, recurring §5/§6 deferral, now visible in the exemplar) → **§8/polish**; compile-time `apiVersion`-literal enforcement (arch M2) → accept prose-only; rename route/nav `permission` → `requireRole` to match the mechanism (arch H1.3) → a future minor `apiVersion` bump.
|
- [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §7 reference-plugin surfaces). Architecture: **no Critical**; Product: **no Critical** this checkpoint. **Fixed now (tests-first):** (1) HIGH (arch + product Blocker) — the contract claimed declaring `permissions` "seeds Keto" but nothing wired it, and the host hardcoded the plugin-specific `scheduling:read`/`write` into `ADMIN_ROLES` (`compose.yml` + `bootstrap.ts`) — core knowing about a plugin. Made the claim true *and* removed the coupling: new pure `seedRoles(adminRolesEnv, declaredTokens)` (union+dedup, trims both sides); `bootstrap.main()` now `discoverPlugins()` → seeds the demo admin `admin`(/`ADMIN_ROLES`) ∪ every discovered plugin's declared tokens; `compose.yml` `ADMIN_ROLES` default dropped to just `admin` (host names no plugin). Clean-clone behaviour is unchanged (admin still ends up with the scheduling tokens, now derived via discovery, not hardcoded — `readRoles` enumerates the Role namespace so a granted token reaches the JWT). Docs corrected: a route/nav `permission` **is a coarse role** granted as a Keto `Role:<token>#members` tuple (vs the separate fine-grained `Resource` tier). (2) HIGH (arch) — no delineated plugin SDK: the reference plugin imported six deep `src/*` internals, none of them the documented-stable surface. Added `src/plugin-api.ts` — the one barrel a plugin imports (`definePlugin` + manifest/handler/ctx types + guards + body/CSRF/list-query helpers); repointed `plugins/scheduling/{plugin,shifts}.ts` to it, so the contract boundary now lives in code (the host can refactor internals freely behind it). (3) MEDIUM (arch L1 + product) — per-plugin CSS was documented (`/public/<id>/`) but unusable (the shell `<head>` had no slot) and the reference shipped no `public/`. Added an optional `styles` slot to `shell.ejs` (extra stylesheet hrefs, default `[]`, backward-compatible) + `plugins/scheduling/public/scheduling.css` linked from both reference views (`.scheduling-page` scope) — closes the only unexercised contract feature + the anatomy-diagram drift. (4) MEDIUM (arch M4) — the reference demonstrated no hook; added `hooks.onBoot` validating `SCHEDULING_UPSTREAM` fail-loud at boot (`assertHttpUrl` — unparseable or non-http(s) aborts the boot, a typo no longer degrades every request), closing the hooks-coverage gap. (5) MEDIUM (arch M3) — `ctx.chrome` was built twice on a matched plugin request when any onRequest hook exists; now built at most once per request (memoized lazy `chrome()`), hot path still chrome-free. (6) HIGH/MEDIUM (product) — doc honesty: fixed the false `visual.spec.ts` comment ("covered by the full-stack suites" — it isn't), softened the contract's "every plugin ships *with* a Playwright test" to match reality (the reference's authed list/form happy-path is the §8 item), and added an **Upstream contract** block (routes/JSON shape/status-code expectations) to `plugins/scheduling/README.md`. Stability-reviewer run as a local PR: **APPROVE, no Critical/High**; addressed both Low nits (`seedRoles` now trims/dedups declared tokens too; added an `onBoot`-binding seam test on the real manifest). typecheck + **301 units** green (296 → 301). **Deferred (reviewer-scoped, not the §7 checkpoint):** the host **internal route-table** (fold the admin/oauth if-ladder into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` from it — arch M1) → **§9** (now the top structural item; standalone change, not a review-fix); the `safeUrl()` href helper + a reference row that renders an untrusted URL through it (arch L2 / product 🟢) → **§9** (first untrusted-URL flow / redirect-allowlist work); the data-table **empty state** + **success-flash** after writes (product 🟡, recurring §5/§6 deferral, now visible in the exemplar) → **§8/polish**; compile-time `apiVersion`-literal enforcement (arch M2) → accept prose-only; rename route/nav `permission` → `requireRole` to match the mechanism (arch H1.3) → a future minor `apiVersion` bump.
|
||||||
- [ ] 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.
|
- [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 §7 accretion (reference plugin + the §7-review host additions). The §7 modules were authored dense — the reference plugin (`plugin.ts`/`shifts.ts`/views/`examples/shifts-upstream`) is a deliberate teaching artifact, and the host bits (`context.ts`/`plugin-api.ts`/`bootstrap.ts`/`app.ts` §7 region) were written concise — so the wins are targeted: (1) tightened `chrome.ts`'s module header (7→5 lines) — dropped the parenthetical input-list (visible in `ChromeOptions`) and the nav-composition restatement (already carried by the `nav` field comment + `markCurrent`'s own comment). (2) Fixed a stale forward-ref in `docs/plugin-contract.md`: the `safeUrl()` helper note said "will land … §5/§7", but §7 deferred it to §9 (line 115) — now "planned for §9, with the redirect-URI allowlist work". Left intact: the reference plugin's instructive comments (it's the copy-target), the EJS view config-doc headers (the only schema for untyped locals), and the contract doc + plugin README (authored/refined concise in the §7 verify + review items). README **Building a plugin**/Layout/bootstrap prose was already tight (§7) — untouched; the README **Status**/`_(planned)_`/Layout refresh stays §9 (line 133). Docs/comments-only (per AGENTS.md, no stability reviewer); typecheck + 301 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.
|
- [ ] 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.
|
||||||
|
|
||||||
## 8. Testing & CI
|
## 8. Testing & CI
|
||||||
|
|||||||
Reference in New Issue
Block a user