feat(theming): wire the three navs and add the admin nav builder

Phases 6-8 of docs/website/THEMING_AND_NAV.md. The public header, the admin
sidebar and the player portal now read their override row, and /admin/navigation
writes them: rename, reorder by drag, hide, and — on the admin sidebar — move a
row into another existing section.

The merge always runs BEFORE the role and shard-feature filters in the layouts,
which are unchanged and remain the boundary. An override is presentation: it
cannot introduce a route, cannot touch a `roles` or `feature` gate, and a stored
`hidden: false` on a gated item shows nobody anything.

The design scoped these phases as client work, but the server had no way to
store a nav row: updateSettings validates and stringifies theme_visual and
brand_assets and lets everything else through, so a nav object would have been
written as "[object Object]" and read as absent for ever. utils/navOverrides.js
mirrors utils/brandAssets.js — strict on write with the offending key named,
forgiving on read. It validates shape only; whether a `to` exists is settled
client-side at merge time, because the base NAV arrays are client constants and
a server-side copy would be a second source of truth that drifts.

The nav editor cannot be hidden — its own toggle is disabled, the write path
drops `hidden` on that one `to`, and AdminLayout strips it again before merging,
which also covers a row edited straight in the database.

Orders are written only when the sequence actually differs from the code's, and
the comparison is restricted to the rows the editing admin can see, so renaming
one item does not pin the position of every other one and a role- or
feature-gated item missing from their palette is not mistaken for a reorder.

Co-Authored-By: Claude <noreply@anthropic.com>

server/src/router/v1/admin/admin.controller.js

@@ -14,6 +14,7 @@ const { cleanBody } = require('../../../utils/sanitizeHtml')
 const { parseJsonSetting } = require('../../../utils/settingsJson')
 const { validateThemeVisual } = require('../../../utils/themeResolve')
 const { validateBrandAssets, resolveBrandAssets } = require('../../../utils/brandAssets')
+const { validateNavOverrides, resolveNavOverrides, NAV_KEYS } = require('../../../utils/navOverrides')
 const htmlShell = require('../../../utils/htmlShell')
 
 const log = require('../../../utils/logger')('admin')
@@ -566,6 +567,23 @@ async function updateSettings(req, res) {
     if (!check.ok) return res.status(400).json({ message: check.message })
     updates.brand_assets = JSON.stringify(resolveBrandAssets(parsed))
   }
+  // The three nav rows are JSON too, and without this they would reach
+  // settingsDb.set as objects and be stored as the string "[object Object]".
+  // Shape only — whether a key names a route the nav actually declares is the
+  // client's question, and utils/navOverrides.js says why. Resolved on the way
+  // in so the stored row carries no dead fields, and so `hidden` can never land
+  // on the nav editor's own row.
+  for (const key of NAV_KEYS) {
+    if (!(key in updates)) continue
+    const raw = updates[key]
+    const parsed = typeof raw === 'string' ? parseJsonSetting(raw) : raw
+    if (typeof raw === 'string' && parsed === null) {
+      return res.status(400).json({ message: `${key} must be a JSON object` })
+    }
+    const check = validateNavOverrides(parsed, key)
+    if (!check.ok) return res.status(400).json({ message: check.message })
+    updates[key] = JSON.stringify(resolveNavOverrides(parsed, key))
+  }
   try {
     await settings.setMany(updates, req.user.id)
     // The HTML shell is templated from brand_assets and theme_visual, and is

server/src/router/v1/admin/settings.router.js

@@ -34,6 +34,7 @@ settingsRouter.put(
   // #swagger.tags = ['Admin · Settings']
   // #swagger.summary = 'Update site settings (admin only)'
   // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }]
+  // #swagger.description = 'Writes the given keys. The JSON-valued theming keys (theme_visual, brand_assets, nav_public, nav_admin, nav_player) accept an object or its stringified form, are validated strictly with the offending field named in the 400, and are stored stringified with unusable fields dropped. Nav overrides carry only label/order/hidden/group; whether a key names a route the nav declares is settled client-side at merge time.'
   /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: "object", additionalProperties: true, description: "An object of key/value settings." } } } } */
   /* #swagger.responses[200] = { description: 'Updated settings', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */
   /* #swagger.responses[400] = { description: 'Body must be an object of key/value settings', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */

server/src/utils/navOverrides.js

@@ -0,0 +1,148 @@
+// Navigation overrides — the `nav_public` / `nav_admin` / `nav_player` rows.
+//
+//   { "/site/news": { "label": "Announcements", "order": 2 },
+//     "/site/market": { "hidden": true },
+//     "/admin/houses": { "order": 1, "group": "Moderation" } }
+//
+// Keyed by an item's existing `to`; every field is optional and an absent one
+// falls back to the code default (docs/website/THEMING_AND_NAV.md §6.4). The
+// merge itself happens on the client — client/src/lib/navOverrides.js — and the
+// role/feature filters in the layouts run *after* it, so this layer is
+// presentation and never authorization (§7).
+//
+// **What this module cannot check, deliberately: whether a `to` exists.** The
+// three base NAV arrays are client constants (SiteHeader.jsx, AdminLayout.jsx,
+// PlayerPortalLayout.jsx). Shipping a copy of them to the server would create a
+// second source of truth for navigation that drifts the first time a route is
+// added, and it would buy nothing: `applyNavOverrides` already drops an entry
+// whose `to` the base array does not declare, which is the right place for it —
+// deleting a route in code stops mattering immediately, with no migration and no
+// stale row doing something unexpected later. So the server validates *shape*
+// and the client owns *membership*.
+//
+// Same strict-on-write / forgiving-on-read asymmetry as the theme and the brand
+// assets (utils/themeResolve.js, utils/brandAssets.js): a bad write is rejected
+// with the offending key named, while a bad stored value is dropped entry by
+// entry so one hand-edited row does not cost the admin the rest of their nav.
+
+// The four overridable fields. `group` is only meaningful on the grouped admin
+// nav, but accepting it everywhere costs nothing — the merge util drops a group
+// the base nav does not declare, and the flat navs declare none at all.
+const FIELDS = ['label', 'order', 'hidden', 'group']
+
+// Bounds. None of these is a security control on its own — the row is written by
+// an admin and rendered as text by React — they keep a single settings row from
+// growing without limit, and they are what makes "the admin nav has 21 items"
+// the shape this store is sized for.
+const MAX_ENTRIES = 200
+const MAX_PATH = 128
+const MAX_LABEL = 64
+const MAX_GROUP = 64
+
+// The one item an override may never hide: the nav editor itself. An admin who
+// hid it would lose the only screen that can un-hide it, and "type the URL from
+// memory" is not a recovery path. Enforced here as well as in the editor's UI so
+// a hand-written row cannot do it either.
+const UNHIDEABLE = { nav_admin: ['/admin/navigation'] }
+
+/**
+ * Is this a usable key — that is, something that could be a `to` in a nav array?
+ * An app-internal path: absolute, same-origin, no scheme and no whitespace.
+ * Whether it *is* one of the declared routes is the client's question (above).
+ * @param {unknown} value
+ * @returns {boolean}
+ */
+function isNavPath(value) {
+  if (typeof value !== 'string' || value.length === 0 || value.length > MAX_PATH) return false
+  if (!value.startsWith('/')) return false
+  // `//host` is protocol-relative and would leave the origin despite looking
+  // like a path; whitespace and quotes have no business in a route.
+  if (value.startsWith('//') || /[\s<>"'\\]/.test(value)) return false
+  return true
+}
+
+/**
+ * Validate a nav-override object for WRITING. Strict: names the offending key.
+ * @param {unknown} value  the parsed object, or null to clear every override
+ * @param {string} [key]  which nav row this is, for the messages
+ * @returns {{ok: true} | {ok: false, message: string}}
+ */
+function validateNavOverrides(value, key = 'nav') {
+  if (value === null || value === undefined) return { ok: true }
+  if (typeof value !== 'object' || Array.isArray(value)) {
+    return { ok: false, message: `${key} must be a JSON object` }
+  }
+  const entries = Object.entries(value)
+  if (entries.length > MAX_ENTRIES) {
+    return { ok: false, message: `${key} may hold at most ${MAX_ENTRIES} entries` }
+  }
+  for (const [to, entry] of entries) {
+    if (!isNavPath(to)) {
+      return { ok: false, message: `${key} key '${to}' must be an app path such as /site/news` }
+    }
+    if (!entry || typeof entry !== 'object' || Array.isArray(entry)) {
+      return { ok: false, message: `${key}['${to}'] must be an object` }
+    }
+    for (const [field, fieldValue] of Object.entries(entry)) {
+      if (!FIELDS.includes(field)) {
+        return { ok: false, message: `Unknown nav field '${field}' on ${key}['${to}']` }
+      }
+      if (field === 'label' && (typeof fieldValue !== 'string' || fieldValue.length > MAX_LABEL)) {
+        return { ok: false, message: `${key}['${to}'].label must be text of at most ${MAX_LABEL} characters` }
+      }
+      if (field === 'group' && (typeof fieldValue !== 'string' || fieldValue.length > MAX_GROUP)) {
+        return { ok: false, message: `${key}['${to}'].group must be text of at most ${MAX_GROUP} characters` }
+      }
+      if (field === 'order' && (typeof fieldValue !== 'number' || !Number.isFinite(fieldValue))) {
+        return { ok: false, message: `${key}['${to}'].order must be a number` }
+      }
+      // `hidden: false` is not an error — it is simply the default, and the
+      // editor sends it while a row is being edited. It is dropped below, never
+      // stored, because hiding is subtractive only (§7): a stored `false` could
+      // read as "force visible" to a later reader, and nothing may un-hide.
+      if (field === 'hidden' && typeof fieldValue !== 'boolean') {
+        return { ok: false, message: `${key}['${to}'].hidden must be true or false` }
+      }
+    }
+  }
+  return { ok: true }
+}
+
+/**
+ * Keep only the entries and fields that would actually do something. Serves both
+ * directions, like resolveBrandAssets:
+ *
+ *   • writing — an admin who cleared every override stores nothing, and the
+ *     caller deletes the row instead, so "a row exists" keeps meaning "this nav
+ *     was customised" (§4.1);
+ *   • reading — a hand-edited entry is dropped and its neighbours kept.
+ *
+ * @param {object|null} value  an object, or a parseJsonSetting result
+ * @param {string} [key]  the settings key, so the un-hideable rule can apply
+ * @returns {object} a new object, `{}` when nothing survives
+ */
+function resolveNavOverrides(value, key = 'nav') {
+  const out = {}
+  if (!value || typeof value !== 'object' || Array.isArray(value)) return out
+  const unhideable = UNHIDEABLE[key] || []
+  for (const [to, entry] of Object.entries(value)) {
+    if (!isNavPath(to) || !entry || typeof entry !== 'object' || Array.isArray(entry)) continue
+    const clean = {}
+    // A label that is only whitespace is not a label — it would render an
+    // unclickable-looking gap — so it falls back to the coded one.
+    if (typeof entry.label === 'string' && entry.label.trim() && entry.label.length <= MAX_LABEL) {
+      clean.label = entry.label.trim()
+    }
+    if (typeof entry.order === 'number' && Number.isFinite(entry.order)) clean.order = entry.order
+    // Only the literal `true` is stored: `hidden: false` is the default and
+    // carrying it would suggest an override that can un-hide something.
+    if (entry.hidden === true && !unhideable.includes(to)) clean.hidden = true
+    if (typeof entry.group === 'string' && entry.group.trim() && entry.group.length <= MAX_GROUP) {
+      clean.group = entry.group.trim()
+    }
+    if (Object.keys(clean).length > 0) out[to] = clean
+  }
+  return out
+}
+
+module.exports = { validateNavOverrides, resolveNavOverrides, NAV_KEYS: ['nav_public', 'nav_admin', 'nav_player'] }

server/swagger/swagger-output.json

@@ -3463,7 +3463,7 @@
           "Admin · Settings"
         ],
         "summary": "Update site settings (admin only)",
-        "description": "",
+        "description": "Writes the given keys. The JSON-valued theming keys (theme_visual, brand_assets, nav_public, nav_admin, nav_player) accept an object or its stringified form, are validated strictly with the offending field named in the 400, and are stored stringified with unusable fields dropped. Nav overrides carry only label/order/hidden/group; whether a key names a route the nav declares is settled client-side at merge time.",
         "responses": {
           "200": {
             "description": "Updated settings",

server/test/navOverrides.test.js

@@ -0,0 +1,171 @@
+// Point the DB at a closed port before anything builds the pool — this file only
+// exercises pure functions, but requiring the util pulls in nothing else.
+process.env.DB_HOST = '127.0.0.1'
+process.env.DB_PORT = '59999'
+
+const { test } = require('node:test')
+const assert = require('node:assert/strict')
+
+// Phases 6-8 of docs/website/THEMING_AND_NAV.md: the server half of the nav
+// overrides. The merge itself is the client's (client/src/lib/navOverrides.js,
+// tested there); this module decides only what may be *stored*, and the two
+// properties worth locking are the asymmetry — strict on write, forgiving on
+// read — and the two things a stored row may never carry: a field that is not
+// one of the four, and `hidden` on the nav editor's own row.
+const { validateNavOverrides, resolveNavOverrides, NAV_KEYS } = require('../src/utils/navOverrides')
+
+// ── validateNavOverrides — strict, and names what it rejected ──────────
+
+test('null and undefined are valid — that is how every override is cleared', () => {
+  assert.equal(validateNavOverrides(null).ok, true)
+  assert.equal(validateNavOverrides(undefined).ok, true)
+})
+
+test('an empty object is valid — the caller deletes the row instead of storing it', () => {
+  assert.equal(validateNavOverrides({}).ok, true)
+})
+
+test('a non-object is rejected under the key it was written to', () => {
+  for (const bad of [4, 'x', true, [], [{ to: '/' }]]) {
+    const check = validateNavOverrides(bad, 'nav_public')
+    assert.equal(check.ok, false, `${JSON.stringify(bad)} should be rejected`)
+    assert.match(check.message, /nav_public must be a JSON object/)
+  }
+})
+
+test('a key that is not an app path is rejected and named', () => {
+  for (const bad of [
+    'site/news', // relative
+    '//evil.example/x', // protocol-relative: looks like a path, leaves the origin
+    'https://evil.example', // scheme
+    '/site/ news', // whitespace
+    '/site/"news"', // quotes
+    '',
+  ]) {
+    const check = validateNavOverrides({ [bad]: { order: 1 } }, 'nav_public')
+    assert.equal(check.ok, false, `'${bad}' should be rejected as a key`)
+    assert.match(check.message, /must be an app path/)
+  }
+})
+
+test('a valid app path is accepted as a key even when no nav declares it', () => {
+  // Membership is the client's question: applyNavOverrides drops an unknown `to`
+  // at merge time, so a route deleted in code needs no migration here.
+  assert.equal(validateNavOverrides({ '/site/gone': { order: 3 } }).ok, true)
+})
+
+test('an entry that is not an object is rejected', () => {
+  for (const bad of ['x', 4, null, []]) {
+    const check = validateNavOverrides({ '/site/news': bad }, 'nav_public')
+    assert.equal(check.ok, false, `${JSON.stringify(bad)} should be rejected as an entry`)
+    assert.match(check.message, /must be an object/)
+  }
+})
+
+test('an unknown field is rejected rather than silently ignored', () => {
+  // Storing a value that will never apply is a bad admin experience, and a
+  // field nobody validates is where a future `to`/`roles` would try to sneak in.
+  for (const field of ['to', 'roles', 'feature', 'icon', 'end', 'href']) {
+    const check = validateNavOverrides({ '/site/news': { [field]: 'x' } }, 'nav_public')
+    assert.equal(check.ok, false, `'${field}' should be rejected`)
+    assert.match(check.message, new RegExp(`Unknown nav field '${field}'`))
+  }
+})
+
+test('each of the four fields is type-checked and named on failure', () => {
+  const cases = [
+    [{ label: 4 }, /label must be text/],
+    [{ label: 'x'.repeat(65) }, /label must be text/],
+    [{ group: 4 }, /group must be text/],
+    [{ group: 'x'.repeat(65) }, /group must be text/],
+    [{ order: '1' }, /order must be a number/],
+    [{ order: Number.NaN }, /order must be a number/],
+    [{ order: Number.POSITIVE_INFINITY }, /order must be a number/],
+    [{ hidden: 'true' }, /hidden must be true or false/],
+    [{ hidden: 1 }, /hidden must be true or false/],
+  ]
+  for (const [entry, pattern] of cases) {
+    const check = validateNavOverrides({ '/site/news': entry }, 'nav_public')
+    assert.equal(check.ok, false, `${JSON.stringify(entry)} should be rejected`)
+    assert.match(check.message, pattern)
+    assert.match(check.message, /\/site\/news/)
+  }
+})
+
+test('all four fields together are accepted', () => {
+  const check = validateNavOverrides({
+    '/admin/houses': { label: 'Houses', order: 2, hidden: true, group: 'Moderation' },
+  })
+  assert.equal(check.ok, true)
+})
+
+test('an absurd number of entries is refused', () => {
+  const many = {}
+  for (let i = 0; i < 201; i += 1) many[`/site/p${i}`] = { order: i }
+  const check = validateNavOverrides(many, 'nav_public')
+  assert.equal(check.ok, false)
+  assert.match(check.message, /at most 200 entries/)
+})
+
+// ── resolveNavOverrides — forgiving, and drops what would do nothing ───
+
+test('unusable entries are dropped and their neighbours kept', () => {
+  const out = resolveNavOverrides({
+    '/site/news': { label: 'Announcements' },
+    'not-a-path': { label: 'Ignored' },
+    '/site/wiki': 'garbage',
+    '/site/market': { hidden: true },
+  })
+  assert.deepEqual(out, {
+    '/site/news': { label: 'Announcements' },
+    '/site/market': { hidden: true },
+  })
+})
+
+test('a label is trimmed, and a whitespace-only label falls back to the coded one', () => {
+  assert.deepEqual(resolveNavOverrides({ '/x': { label: '  News  ' } }), { '/x': { label: 'News' } })
+  assert.deepEqual(resolveNavOverrides({ '/x': { label: '   ' } }), {})
+})
+
+test('hidden: false is never stored — hiding is subtractive only', () => {
+  // A stored `false` could read to a later consumer as "force visible", and
+  // nothing in this layer may un-hide a role- or feature-gated item (§7).
+  assert.deepEqual(resolveNavOverrides({ '/x': { hidden: false } }), {})
+  assert.deepEqual(resolveNavOverrides({ '/x': { hidden: false, order: 2 } }), { '/x': { order: 2 } })
+})
+
+test('the nav editor cannot be hidden, even by a hand-written row', () => {
+  // Hiding /admin/navigation would remove the only screen that can un-hide it.
+  const out = resolveNavOverrides({ '/admin/navigation': { hidden: true, order: 9 } }, 'nav_admin')
+  assert.deepEqual(out, { '/admin/navigation': { order: 9 } })
+  // An entry that carried nothing else disappears entirely rather than storing
+  // an empty object.
+  assert.deepEqual(resolveNavOverrides({ '/admin/navigation': { hidden: true } }, 'nav_admin'), {})
+})
+
+test('the un-hideable rule is scoped to the admin nav', () => {
+  // The same path in another row is meaningless, but it is also not special:
+  // the rule protects the admin sidebar, which is the nav that renders it.
+  assert.deepEqual(resolveNavOverrides({ '/admin/navigation': { hidden: true } }, 'nav_public'), {
+    '/admin/navigation': { hidden: true },
+  })
+})
+
+test('an entry left with no usable field is dropped, so `{}` is never stored', () => {
+  assert.deepEqual(resolveNavOverrides({ '/x': {}, '/y': { label: 4 } }), {})
+})
+
+test('order survives as a number, including zero and negatives', () => {
+  const out = resolveNavOverrides({ '/a': { order: 0 }, '/b': { order: -3 }, '/c': { order: 1.5 } })
+  assert.deepEqual(out, { '/a': { order: 0 }, '/b': { order: -3 }, '/c': { order: 1.5 } })
+})
+
+test('a non-object resolves to {} rather than throwing', () => {
+  for (const bad of [null, undefined, 'x', 4, []]) {
+    assert.deepEqual(resolveNavOverrides(bad), {})
+  }
+})
+
+test('NAV_KEYS names the three rows the controller validates', () => {
+  assert.deepEqual(NAV_KEYS, ['nav_public', 'nav_admin', 'nav_player'])
+})

server/test/settingsTheming.test.js

@@ -272,6 +272,100 @@ test('an invalid theme_visual is rejected and nothing is written', async () => {
   }
 })
 
+// ── PUT /admin/settings — the nav rows (phases 6-8) ───────────────────────
+//
+// The nav keys reach the same validate-then-stringify block. Without it they
+// would fall through to settingsDb.set as objects and be stored as the string
+// "[object Object]" — a row that parses as absent forever, silently.
+
+test('a valid nav override is stored stringified', async () => {
+  signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' })
+  const written = {}
+  settingsDb.set = async (key, value) => {
+    written[key] = value
+  }
+  settingsDb.getAll = async () => []
+  const app = await startApp((a) => a.use('/api/v1/admin/settings', requireAuth, settingsRouter))
+  try {
+    const nav = { '/site/news': { label: 'Announcements', order: 1 }, '/site/market': { hidden: true } }
+    const res = await fetch(`${app.url}/api/v1/admin/settings`, {
+      method: 'PUT',
+      headers: { 'content-type': 'application/json' },
+      body: JSON.stringify({ nav_public: nav }),
+    })
+    assert.equal(res.status, 200)
+    assert.equal(typeof written.nav_public, 'string')
+    assert.notEqual(written.nav_public, '[object Object]')
+    assert.deepEqual(JSON.parse(written.nav_public), nav)
+  } finally {
+    settingsDb.set = originals.set
+    await app.close()
+  }
+})
+
+test('an invalid nav override is rejected, named, and nothing is written', async () => {
+  signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' })
+  settingsDb.set = () => assert.fail('an invalid nav override must not be stored')
+  const app = await startApp((a) => a.use('/api/v1/admin/settings', requireAuth, settingsRouter))
+  try {
+    const bad = [
+      { '//evil.example/x': { order: 1 } }, // protocol-relative key
+      { '/site/news': { roles: ['admin'] } }, // a gate is not overridable
+      { '/site/news': { to: '/elsewhere' } }, // an override cannot introduce a route
+      { '/site/news': { order: 'first' } },
+      { '/site/news': 'hidden' },
+      'not json',
+    ]
+    for (const nav_public of bad) {
+      const res = await fetch(`${app.url}/api/v1/admin/settings`, {
+        method: 'PUT',
+        headers: { 'content-type': 'application/json' },
+        body: JSON.stringify({ nav_public }),
+      })
+      assert.equal(res.status, 400, JSON.stringify(nav_public))
+      const body = await res.json()
+      assert.match(body.message, /nav_public|nav field/)
+    }
+  } finally {
+    settingsDb.set = originals.set
+    await app.close()
+  }
+})
+
+test('the write path drops hidden on the nav editor and never stores hidden: false', async () => {
+  signInAs({ id: 1, username: 'a', role: 'admin', status: 'active' })
+  const written = {}
+  settingsDb.set = async (key, value) => {
+    written[key] = value
+  }
+  settingsDb.getAll = async () => []
+  const app = await startApp((a) => a.use('/api/v1/admin/settings', requireAuth, settingsRouter))
+  try {
+    const res = await fetch(`${app.url}/api/v1/admin/settings`, {
+      method: 'PUT',
+      headers: { 'content-type': 'application/json' },
+      body: JSON.stringify({
+        nav_admin: {
+          '/admin/navigation': { hidden: true, order: 3 },
+          '/admin/posts': { hidden: false, label: 'Blog' },
+          '/admin/wiki': { hidden: true },
+        },
+      }),
+    })
+    assert.equal(res.status, 200)
+    // The editor keeps its order but not its hiding; a `hidden: false` is the
+    // default, so it is dropped rather than stored as an un-hide instruction.
+    assert.deepEqual(JSON.parse(written.nav_admin), {
+      '/admin/navigation': { order: 3 },
+      '/admin/posts': { label: 'Blog' },
+      '/admin/wiki': { hidden: true },
+    })
+  } finally {
+    settingsDb.set = originals.set
+    await app.close()
+  }
+})
+
 // ── GET /settings/theme-options — the catalog the admin form is built from ──
 
 test('GET /settings/theme/options serves the catalog to an authenticated caller', async () => {