Skip to content

feat: default landing page + ACL at server root (#433, supersedes #303)#434

Merged
melvincarvalho merged 3 commits into
gh-pagesfrom
issue-433-landing-page-seeding
May 14, 2026
Merged

feat: default landing page + ACL at server root (#433, supersedes #303)#434
melvincarvalho merged 3 commits into
gh-pagesfrom
issue-433-landing-page-seeding

Conversation

@melvincarvalho
Copy link
Copy Markdown
Contributor

Summary

Why this lands now

PR #303 stalled because the original design had to coexist with createRootPodStructure writing absolute-URI ACLs. Now that #428 (Phase 1) and #430 (Phase 2) made the generators emit relative accessTo/default/agent, the seeded /.acl is 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:

  • generatePublicReadAcl reuse (no inline ACL JSON)
  • skip seeding in read-only deployments
  • check storage.write returns; abort ACL seeding if the HTML write fails (so we never end up with a public-read root ACL but no index page)
  • real IDP routes (/idp/register, /idp — not the non-existent /.account/new and /idp/auth)
  • full error logging (with stack)
  • terminal feature flag
  • resilient package.json read with version="unknown" fallback
  • restore process.env.DATA_ROOT after the operator-override test

Additional fix (one Copilot nit that was flagged but never landed)

String.prototype.replace with a string interprets $&, $1, etc. as substitution patterns. Interpolated values can transitively contain $ characters (notably singleUserName), 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 a singleUserName of foo$&bar survives intact.

Tests

  • 12 new tests in test/server-root.test.js:
    • 2 server-root integration tests (seeded HTML on GET /, public-read on direct /index.html fetch)
    • 2 operator-override tests (pre-write a custom /index.html, assert it isn't overwritten and GET / serves it)
    • 7 mode-specific renderer tests (multi-user + IDP buttons, single-user-only Sign-in, no-IDP Docs-only, single-user subtitle, feature listing, version interpolation, version XSS escape)
    • 1 regression for the $& substitution gotcha
  • 811/811 existing tests pass.

Test plan

  • npm test — 811/811 passing locally
  • Phase 3 tests cover seed + operator-override + mode-aware rendering + $& regression

Out of scope

Closes #433. Closes #303 (superseded).

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.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.acl on first start (skip-if-exists).
  • Hooks seeding into server startup via a Fastify onReady hook, 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.

Comment thread src/ui/server-root.js
Comment on lines +142 to +144
if (!(await storage.exists('/.acl'))) {
const ok = await storage.write('/.acl', serializeAcl(generatePublicReadAcl('/')));
if (ok) seededAcl = true;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread src/ui/server-root.js Outdated
Comment on lines +81 to +95

// 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);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 3: fold in #303 landing-page seeding on portable generators (#427)

2 participants