Address Copilot review on JavaScriptSolidServer#285

melvincarvalho · melvincarvalho · commit e459a36cc131 · 2026-04-22T04:00:30.000+02:00
1. XSS-safe inline script. previewConfig is now escaped against the three sequences that can break out of an inline <script> context: `</script>` (via `<` → \u003c), and the U+2028 / U+2029 line separators that JS treats as line terminators. Plain JSON.stringify doesn't handle these on its own. 2. Client/server validation alignment. The HTML pattern now mirrors the server's "..-rejected" rule via a negative lookahead, and in subdomain mode it tightens to alphanumeric + hyphen only (matching server-side behaviour added below). title text updated to match. 3. Subdomain-mode regex. server.js refuses to route multi-level subdomains, so `alice.smith` would create a non-addressable pod. handleRegisterPost now picks /^[a-z0-9]([a-z0-9-]{1,30}[a-z0-9])?$/ when request.subdomainsEnabled && request.baseDomain, and uses a distinct error message so the user knows why dot/underscore is out. 4. Lowercase normalisation. The username field is now coerced to lowercase as the user types, so the live preview matches what the server will accept (it rejects uppercase outright). 5. Test coverage. Adds 9 path-mode register cases (accepted: alice, alice-smith, alice.smith, alice_work; rejected: leading separator, trailing separator, consecutive dots, uppercase, too-short) and 3 subdomain-mode cases (dash accepted; dot + underscore rejected). Also picks up the .jsonld pod creation flow from JavaScriptSolidServer#283 transitively.
diff --git a/src/idp/interactions.js b/src/idp/interactions.js
@@ -403,9 +403,20 @@ export async function handleRegisterPost(request, reply, issuer, inviteOnly = fa
   // `alice_work`, and so on. No leading/trailing separators (avoids the
   // `.hidden` / trailing-dot footguns), no `..` (path traversal hygiene
   // even though storage already guards against it).
-  const usernameRegex = /^[a-z0-9]([a-z0-9._-]{1,30}[a-z0-9])?$/;
+  //
+  // In subdomain mode the username becomes a single-level subdomain — DNS
+  // hostnames don't allow `.` or `_`, and `server.js` already refuses to
+  // route multi-level subdomains as pods. So we restrict to alphanumeric +
+  // hyphen there to keep the username and the pod actually addressable.
+  const subdomainMode = !!(request.subdomainsEnabled && request.baseDomain);
+  const usernameRegex = subdomainMode
+    ? /^[a-z0-9]([a-z0-9-]{1,30}[a-z0-9])?$/
+    : /^[a-z0-9]([a-z0-9._-]{1,30}[a-z0-9])?$/;
   if (!usernameRegex.test(username)) {
-    return reply.type('text/html').send(registerPage(uid, 'Username must be lowercase letters, numbers, or . _ - (start and end alphanumeric)', null, inviteOnly, ctx));
+    const msg = subdomainMode
+      ? 'Username must be lowercase letters, numbers, or - (subdomain mode disallows . and _)'
+      : 'Username must be lowercase letters, numbers, or . _ - (start and end alphanumeric)';
+    return reply.type('text/html').send(registerPage(uid, msg, null, inviteOnly, ctx));
   }
   if (username.includes('..')) {
     return reply.type('text/html').send(registerPage(uid, 'Username cannot contain ".."', null, inviteOnly, ctx));
diff --git a/src/idp/views.js b/src/idp/views.js
@@ -560,14 +560,28 @@ export function registerPage(uid = null, error = null, success = null, inviteOnl
              placeholder="Enter your invite code" style="text-transform: uppercase;">
   ` : '';
 
-  // Embed the values the live preview needs. Treat as opaque strings — the
-  // template substitution is JSON-serialised, so quotes / scripts inside
-  // baseUri can't break out.
+  // Embed the values the live preview needs. Escape characters that are
+  // unsafe in inline <script> contexts so values like "</script>" or
+  // U+2028 / U+2029 line separators can't terminate the script tag or
+  // confuse the parser when template-substituted.
   const previewConfig = JSON.stringify({
     baseUri: ctx.baseUri || '',
     subdomainsEnabled: !!ctx.subdomainsEnabled,
     baseDomain: ctx.baseDomain || '',
-  });
+  })
+    .replace(/</g, '\\u003c')
+    .replace(/\u2028/g, '\\u2028')
+    .replace(/\u2029/g, '\\u2029');
+
+  // Server validates more strictly than the HTML pattern can express; mirror
+  // as much as possible client-side so the browser catches obvious mistakes
+  // before submit. Subdomain mode drops dot/underscore (DNS hostname rules).
+  const usernamePattern = (ctx.subdomainsEnabled && ctx.baseDomain)
+    ? '[a-z0-9](?:[a-z0-9-]{1,30}[a-z0-9])?'
+    : '(?!.*\\.\\.)[a-z0-9](?:[a-z0-9._-]{1,30}[a-z0-9])?';
+  const usernameTitle = (ctx.subdomainsEnabled && ctx.baseDomain)
+    ? 'Lowercase letters, numbers, or - (start and end alphanumeric, 3–32 chars). Subdomain mode disallows . and _.'
+    : 'Lowercase letters, numbers, or . _ - (start and end alphanumeric, 3–32 chars, no consecutive dots)';
 
   return `
 <!DOCTYPE html>
@@ -618,8 +632,8 @@ export function registerPage(uid = null, error = null, success = null, inviteOnl
       <label for="username">Username</label>
       <input type="text" id="username" name="username" required ${!inviteOnly ? 'autofocus' : ''}
              placeholder="Choose a username" minlength="3" maxlength="32"
-             pattern="[a-z0-9]([a-z0-9._-]{1,30}[a-z0-9])?"
-             title="Lowercase letters, numbers, or . _ - (start and end alphanumeric, 3–32 chars)">
+             pattern="${usernamePattern}"
+             title="${usernameTitle}">
 
       <div class="preview" id="preview" aria-live="polite">
         <div><span class="label">WebID</span><span id="preview-webid" class="placeholder">choose a username to preview</span></div>
@@ -651,7 +665,12 @@ export function registerPage(uid = null, error = null, success = null, inviteOnl
     if (!input || !webEl || !storEl) return;
 
     function render() {
-      var u = (input.value || '').trim().toLowerCase();
+      // Server rejects uppercase outright, so normalise the field as the
+      // user types — keeps the preview honest and avoids a confusing
+      // post-submit error.
+      var normalised = (input.value || '').toLowerCase();
+      if (input.value !== normalised) input.value = normalised;
+      var u = normalised.trim();
       if (!u) {
         webEl.textContent = 'choose a username to preview';
         webEl.className = 'placeholder';
diff --git a/test/idp.test.js b/test/idp.test.js
@@ -181,6 +181,125 @@ describe('Identity Provider', () => {
       assert.ok(res.status >= 200 && res.status < 600, `got valid HTTP status ${res.status}`);
     });
   });
+
+  // Regression coverage for #284 — relaxed username regex + `..` rejection.
+  // Each register call below also exercises the .jsonld pod-creation flow
+  // from #283, since handleRegisterPost calls createPodStructure on success.
+  describe('Register username validation (path mode)', () => {
+    async function tryRegister(username) {
+      const res = await fetch(`${baseUrl}/idp/register`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
+        body: new URLSearchParams({ username, password: 'secret-password', confirmPassword: 'secret-password' }),
+      });
+      const body = await res.text();
+      return { status: res.status, body };
+    }
+
+    it('accepts plain alphanumeric (alice)', async () => {
+      const r = await tryRegister('alice');
+      assert.match(r.body, /Account created/);
+    });
+
+    it('accepts dash (alice-smith)', async () => {
+      const r = await tryRegister('alice-smith');
+      assert.match(r.body, /Account created/);
+    });
+
+    it('accepts dot (alice.smith)', async () => {
+      const r = await tryRegister('alice.smith');
+      assert.match(r.body, /Account created/);
+    });
+
+    it('accepts underscore (alice_work)', async () => {
+      const r = await tryRegister('alice_work');
+      assert.match(r.body, /Account created/);
+    });
+
+    it('rejects leading separator (.alice)', async () => {
+      const r = await tryRegister('.alice');
+      assert.match(r.body, /lowercase letters, numbers/);
+    });
+
+    it('rejects trailing separator (alice-)', async () => {
+      const r = await tryRegister('alice-');
+      assert.match(r.body, /lowercase letters, numbers/);
+    });
+
+    it('rejects consecutive dots (alice..bob)', async () => {
+      const r = await tryRegister('alice..bob');
+      // Quotes are HTML-escaped (&quot;) in the rendered error banner.
+      assert.match(r.body, /cannot contain (?:"|&quot;)\.\.(?:"|&quot;)/);
+    });
+
+    it('rejects uppercase (Alice)', async () => {
+      const r = await tryRegister('Alice');
+      assert.match(r.body, /lowercase letters, numbers/);
+    });
+
+    it('rejects too short (ab)', async () => {
+      const r = await tryRegister('ab');
+      // Two-char names fail the regex (min 3 enforced by the pattern itself).
+      assert.match(r.body, /lowercase letters, numbers|at least 3/);
+    });
+  });
+});
+
+// Subdomain mode: usernames become hostname components, so `.` and `_` are
+// not allowed (server.js refuses to route multi-level subdomains).
+describe('Identity Provider - Subdomain mode register validation', () => {
+  let server;
+  let baseUrl;
+  const SUBDOMAIN_DATA_DIR = './test-data-idp-subdomain';
+
+  before(async () => {
+    await fs.remove(SUBDOMAIN_DATA_DIR);
+    await fs.ensureDir(SUBDOMAIN_DATA_DIR);
+
+    const port = await getAvailablePort();
+    baseUrl = `http://${TEST_HOST}:${port}`;
+
+    server = createServer({
+      logger: false,
+      root: SUBDOMAIN_DATA_DIR,
+      idp: true,
+      idpIssuer: baseUrl,
+      subdomains: true,
+      baseDomain: TEST_HOST,
+      forceCloseConnections: true,
+    });
+
+    await server.listen({ port, host: TEST_HOST });
+  });
+
+  after(async () => {
+    await server.close();
+    await fs.remove(SUBDOMAIN_DATA_DIR);
+  });
+
+  async function tryRegister(username) {
+    const res = await fetch(`${baseUrl}/idp/register`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
+      body: new URLSearchParams({ username, password: 'secret-password', confirmPassword: 'secret-password' }),
+    });
+    return { status: res.status, body: await res.text() };
+  }
+
+  it('accepts dash (alice-smith)', async () => {
+    const r = await tryRegister('alice-smith');
+    assert.match(r.body, /Account created/);
+  });
+
+  it('rejects dot (alice.smith) — would not be a single-level subdomain', async () => {
+    const r = await tryRegister('alice.smith');
+    assert.match(r.body, /subdomain mode disallows/);
+  });
+
+  it('rejects underscore (alice_work) — invalid in DNS hostnames', async () => {
+    const r = await tryRegister('alice_work');
+    assert.match(r.body, /subdomain mode disallows/);
+  });
 });
 
 describe('Identity Provider - Accounts', () => {
