feat: default landing page + ACL at server root (#433, supersedes #303)#434
Conversation
Phase 3 of #427. Supersedes the abandoned PR #303 (open since Apr 23, mergeable: CONFLICTING). On startup, seed DATA_ROOT/index.html, DATA_ROOT/.acl, and DATA_ROOT/index.html.acl with a minimal landing page and public-read ACLs. Skip-if-exists — operator files are never overwritten. Skipped entirely in read-only mode so startup never mutates DATA_ROOT. Landing page adapts to server mode: - multi-user + IDP → Create a pod / Sign in - single-user + IDP → Sign in - both → Docs link, version, mode, enabled-feature pills ACLs are portable by construction now that #428 / #430 made generatePublicReadAcl emit the resourceUrl verbatim. The seeded /.acl uses path-only "/" so it matches the request host, not whichever interface the server happened to bind on first start. Carries over the design + Copilot review fixes from PR #303: - generatePublicReadAcl reuse (no inline ACL JSON) - skip in read-only mode - check storage.write returns; abort ACL seeding on HTML write failure - real IDP routes (/idp/register, /idp — not /.account/new, /idp/auth) - full error logging (with stack) - terminal feature flag - resilient package.json read with version=unknown fallback - restore process.env.DATA_ROOT after operator-override test One additional fix flagged but not addressed on the original branch: String.prototype.replace with a string interprets `$&`, `$1`, etc. as substitution patterns, which would corrupt any interpolated value containing `$` (notably a singleUserName). Switched all template substitutions to the function form. Regression test asserts a singleUserName of "foo$&bar" survives intact. 811/811 tests pass.
There was a problem hiding this comment.
Pull request overview
Adds first-start seeding for a default server-root landing page and accompanying public-read ACLs so that GET / serves a minimal HTML page (while preserving operator-provided files and avoiding mutations in read-only mode).
Changes:
- Introduces a server-root HTML template plus renderer/seeder (
src/ui/server-root.{html,js}) to create/index.html,/.acl, and/index.html.aclon first start (skip-if-exists). - Hooks seeding into server startup via a Fastify
onReadyhook, including feature/mode-aware rendering context and resilient version detection. - Adds integration + unit tests covering seeding behavior, operator override preservation, mode-specific rendering, escaping, and
$-replacement regression.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/server-root.test.js |
Adds integration and unit tests for root seeding + renderer behavior (modes, features, escaping, $ regression). |
src/ui/server-root.js |
Implements renderer and first-start seeding of /index.html and public-read ACLs. |
src/ui/server-root.html |
Adds the default landing page template rendered/seeded at server root. |
src/server.js |
Adds an onReady seeding hook (skipped in read-only mode) to populate root landing page + ACLs. |
Comments suppressed due to low confidence (1)
src/ui/server-root.js:152
- For the resource ACL, consider using a relative accessTo (e.g. './index.html') instead of the absolute-path '/index.html' so the generated ACL follows the same portability conventions as the rest of the ACL generators (see generateOwnerAcl usage with './publicTypeIndex.jsonld' etc.). Using '/index.html' would resolve against the origin root and could be incorrect if the effective base URL ever includes a path prefix.
if (!(await storage.exists('/index.html.acl'))) {
const ok = await storage.write('/index.html.acl', serializeAcl(generatePublicReadAcl('/index.html')));
if (ok) seededPageAcl = true;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!(await storage.exists('/.acl'))) { | ||
| const ok = await storage.write('/.acl', serializeAcl(generatePublicReadAcl('/'))); | ||
| if (ok) seededAcl = true; |
There was a problem hiding this comment.
Addressed in 3a45d17: switched both seeded ACLs to './' / './index.html' to match the createPodStructure / createRootPodStructure pattern. Added a regression test that inspects the on-disk /.acl and /index.html.acl directly and asserts the relative form — a request smoke-test wouldn't catch this since both forms work when mounted at the origin root.
Address Copilot review on #434. The previous commit seeded /.acl with generatePublicReadAcl('/') and /index.html.acl with generatePublicReadAcl('/index.html'). Both work when JSS sits at the origin root (the two coincide there), but '/' resolves to the origin root regardless of any path prefix the server is mounted under — so JSS behind a reverse proxy at e.g. https://example/jss/ would seed an ACL whose accessTo points at https://example/ rather than https://example/jss/. Switch to './' / './index.html' (resolved against the .acl's own URL), matching createPodStructure / createRootPodStructure since #428 / #430. Add a regression test that inspects the on-disk .acl files directly and asserts the relative form. A request smoke-test wouldn't catch this since both forms work when mounted at the origin root. 812/812 tests pass.
|
|
||
| // Replacements use the function form, not the string form: a string | ||
| // replacement interprets `$&`, `$1`, etc. as substitution patterns, | ||
| // which would corrupt any interpolated value containing a `$` (e.g. | ||
| // a single-user name). The function form skips that interpretation | ||
| // entirely. See #433. | ||
| return tpl | ||
| .replace(/{{title}}/g, () => heading) | ||
| .replace(/{{heading}}/g, () => heading) | ||
| .replace(/{{subtitle}}/g, () => subtitle) | ||
| .replace(/{{description}}/g, () => description) | ||
| .replace(/{{actions}}/g, () => renderActions({ singleUser, idp })) | ||
| .replace(/{{version}}/g, () => escape(version)) | ||
| .replace(/{{mode}}/g, () => mode) | ||
| .replace(/{{features}}/g, () => features); |
There was a problem hiding this comment.
Addressed in 69b74ba: switched to a single-pass substitution — built a {key: value} map then ran one .replace(/{{(\w+)}}/g, (m, k) => values[k]) over the original template. Each token matches in tpl exactly once; substituted text is no longer re-scanned. Regression test asserts a singleUserName of "evil{{actions}}name" lands verbatim and the actions block isn't spliced in. The function form keeps the $& immunity from the previous round.
Address Copilot review (2nd round) on #434. The previous chain of sequential .replace() calls re-scanned already-substituted values, so a singleUserName containing a literal `{{actions}}` would land in `subtitle` and then get expanded by the later `.replace(/{{actions}}/g, …)` — letting any pod owner inject other template fragments via their name. Switch to a single pass: build a values map, then run one .replace(/{{(\w+)}}/g, (m, k) => values[k]) over the original template. Each token is matched against `tpl` exactly once and substituted from the map; substituted text is not re-scanned. Also keeps the `$&` immunity (still using the function form of replace). Regression test asserts a singleUserName of "evil{{actions}}name" appears verbatim in the output and does not get the actions block spliced in. 813/813 tests pass.
Summary
DATA_ROOT/index.html+DATA_ROOT/.acl+DATA_ROOT/index.html.aclon first start with a minimal landing page and public-read ACLs. Skip-if-exists — operator files are preserved.mergeable: CONFLICTING).Why this lands now
PR #303 stalled because the original design had to coexist with
createRootPodStructurewriting absolute-URI ACLs. Now that #428 (Phase 1) and #430 (Phase 2) made the generators emit relativeaccessTo/default/agent, the seeded/.aclis portable by construction — no design tension, no special-casing.Carried over from PR #303
This branch picks up the design + the entire Copilot review thread that went unaddressed on the original branch:
generatePublicReadAclreuse (no inline ACL JSON)storage.writereturns; abort ACL seeding if the HTML write fails (so we never end up with a public-read root ACL but no index page)/idp/register,/idp— not the non-existent/.account/newand/idp/auth)package.jsonread withversion="unknown"fallbackprocess.env.DATA_ROOTafter the operator-override testAdditional fix (one Copilot nit that was flagged but never landed)
String.prototype.replacewith a string interprets$&,$1, etc. as substitution patterns. Interpolated values can transitively contain$characters (notablysingleUserName), so any pod owner with a$in their name would have produced corrupted HTML. All template substitutions now use the function form (replace(/{{key}}/g, () => value)). Regression test asserts asingleUserNameoffoo$&barsurvives intact.Tests
test/server-root.test.js:GET /, public-read on direct/index.htmlfetch)/index.html, assert it isn't overwritten andGET /serves it)$&substitution gotchaTest plan
npm test— 811/811 passing locally$®ressionOut of scope
--admin-webid/ web-edit access for the landing page — future iteration.Closes #433. Closes #303 (superseded).