feat(theming): dropdown sections and added links in the public header

Phase 10 of docs/website/THEMING_AND_NAV.md, asked for before the edge -> main
cutover. An admin can now create dropdown sections in the public header, organise
the coded entries into them, and add links of their own.

This deliberately amends §7, which said the override layer "cannot introduce a
`to` that is not already in the hardcoded NAV array". That stays true of every
CODED entry; an admin may now also add a link, restricted to a same-origin path —
no scheme, no protocol-relative //host. A link carries no gate of its own and
needs none: the page behind it enforces its own access, so an added link
advertises a route and never grants one.

The invariant is kept structurally rather than by vigilance. Coded entries live
in an `items` map whose keys must be routes the base array declares, so that map
cannot invent a route; everything that CAN name an arbitrary path lives in
`links`, which is the one place the path rule is applied — on both the write and
the read path.

nav_public therefore grew a { items, sections, links } wrapper. A bare map still
reads as the items map, and a nav with no sections still stores one, so this
changed nothing for a nav that does not use it. Free to do now because nothing
has shipped; after the cutover it would have needed a migration.

The Public tab gets its own editor. A public section is an entry in the
top-level order that the admin created and can drag among the pills, unlike the
admin sidebar's four coded sections, where only membership moves — that is a tree
rather than a list of groups. Deleting a section returns its entries to the top
level rather than removing them, which is the one destructive act this screen
could otherwise commit.

The dropdown opens on click and never on hover, and its trigger is not a link: a
hover menu is unusable on touch, and a trigger that navigates means tapping to
open takes you somewhere instead. Escape closes and returns focus, an outside
press closes, navigating closes, and Arrow Up/Down walk the items.

pruneNav applies the shard-feature gate inside a section and drops one it leaves
empty, so a dropdown never opens onto nothing.

Also fixes a bug this surfaced in the phase 6-8 code: the save path judged "does
this route still exist?" against the palette — the base array already filtered to
what the editing admin can see — so on the public header a feature-gated row's
override could never be carried through and would have been silently reset.
Membership is now judged against the full coded nav while the rows still come
from the palette.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-08 00:42:19 -05:00
parent 32a3ff104a
commit b517d7b2df
10 changed files with 1335 additions and 51 deletions

View File

@@ -34,7 +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.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 key coded entries by their existing route and carry only label/order/hidden/group/section; whether a key names a route the nav declares is settled client-side at merge time. nav_public may additionally carry admin-created dropdown `sections` and admin-authored `links` — the only place an arbitrary path may be named, and therefore restricted to same-origin paths (no scheme, no protocol-relative host). Sections and links are dropped for the other two navs, which cannot render them.'
/* #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" } } } } */

View File

@@ -25,10 +25,11 @@
// 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']
// The overridable fields on a CODED item. `group` is only meaningful on the
// grouped admin nav and `section` only on the public header, but accepting both
// everywhere costs nothing — the merge util drops a group the base nav does not
// declare, and a section id no `sections` entry declares.
const FIELDS = ['label', 'order', 'hidden', 'group', 'section']
// 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
@@ -38,6 +39,20 @@ const MAX_ENTRIES = 200
const MAX_PATH = 128
const MAX_LABEL = 64
const MAX_GROUP = 64
const MAX_SECTIONS = 12
const MAX_LINKS = 40
// Only the public header supports admin-created dropdown sections and
// admin-authored links (THEMING_AND_NAV.md §7, Phase 10). The admin sidebar has
// its own coded sections and the player portal is three flat rows, so both keep
// the bare items map; `sections`/`links` are dropped for them rather than
// rejected, the same posture as every other unusable field here.
const SECTIONED_KEYS = ['nav_public']
// Generated by the editor, never typed. Constrained so a stored id is safe to
// use as a React key and as a DOM id fragment without further escaping.
const SECTION_ID = /^sec_[a-z0-9]{4,16}$/
const LINK_ID = /^lnk_[a-z0-9]{4,16}$/
// 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
@@ -61,6 +76,100 @@ function isNavPath(value) {
return true
}
/**
* Split a stored value into its three parts.
*
* The public header grew dropdown sections in Phase 10, so `nav_public` may be
* a wrapper — `{ items, sections, links }` — while the other two navs stay the
* bare items map phases 6-8 wrote. **A bare map is still read as the items
* map**, which is unambiguous because every item key is a path beginning with
* `/` and so can never be the string `items`.
*
* @param {object} value a parsed, non-array object
* @returns {{items: object, sections: unknown, links: unknown, wrapped: boolean}}
*/
function unwrap(value) {
const wrapped = value.items && typeof value.items === 'object' && !Array.isArray(value.items)
if (!wrapped) return { items: value, sections: undefined, links: undefined, wrapped: false }
return { items: value.items, sections: value.sections, links: value.links, wrapped: true }
}
// A section is a dropdown an admin created: a label and a position, no route.
// It is never itself a link — it only opens — so there is no `to` to validate.
function validateSections(sections, key) {
if (sections === undefined || sections === null) return { ok: true }
if (!Array.isArray(sections)) return { ok: false, message: `${key}.sections must be an array` }
if (sections.length > MAX_SECTIONS) {
return { ok: false, message: `${key} may hold at most ${MAX_SECTIONS} sections` }
}
const seen = new Set()
for (const section of sections) {
if (!section || typeof section !== 'object' || Array.isArray(section)) {
return { ok: false, message: `${key}.sections entries must be objects` }
}
if (typeof section.id !== 'string' || !SECTION_ID.test(section.id)) {
return { ok: false, message: `${key}.sections has an entry with an invalid id` }
}
if (seen.has(section.id)) {
return { ok: false, message: `${key}.sections has a duplicate id '${section.id}'` }
}
seen.add(section.id)
if (typeof section.label !== 'string' || !section.label.trim() || section.label.length > MAX_LABEL) {
return { ok: false, message: `${key}.sections['${section.id}'].label must be text of at most ${MAX_LABEL} characters` }
}
if (section.order !== undefined && (typeof section.order !== 'number' || !Number.isFinite(section.order))) {
return { ok: false, message: `${key}.sections['${section.id}'].order must be a number` }
}
}
return { ok: true }
}
// A link is the one thing an admin may ADD to a nav, and the only place a `to`
// is not required to already exist in code. It is kept in its own array rather
// than in `items` on purpose: `items` may only key routes the base array
// declares, so an override structurally cannot invent a route, and everything
// that CAN name an arbitrary path is here where the path rule is applied.
//
// A link carries no `roles` or `feature` of its own. It does not need one: the
// page behind it enforces its own access, so a link to somewhere the viewer
// cannot reach 403s exactly as typing the URL would (§7).
function validateLinks(links, key) {
if (links === undefined || links === null) return { ok: true }
if (!Array.isArray(links)) return { ok: false, message: `${key}.links must be an array` }
if (links.length > MAX_LINKS) {
return { ok: false, message: `${key} may hold at most ${MAX_LINKS} added links` }
}
const seen = new Set()
for (const link of links) {
if (!link || typeof link !== 'object' || Array.isArray(link)) {
return { ok: false, message: `${key}.links entries must be objects` }
}
if (typeof link.id !== 'string' || !LINK_ID.test(link.id)) {
return { ok: false, message: `${key}.links has an entry with an invalid id` }
}
if (seen.has(link.id)) {
return { ok: false, message: `${key}.links has a duplicate id '${link.id}'` }
}
seen.add(link.id)
if (typeof link.label !== 'string' || !link.label.trim() || link.label.length > MAX_LABEL) {
return { ok: false, message: `${key}.links['${link.id}'].label must be text of at most ${MAX_LABEL} characters` }
}
// The whole point of the restriction: an added link points somewhere on this
// site. No scheme, no `//host` — the nav is not a place to send visitors off
// to an origin the operator does not control.
if (!isNavPath(link.to)) {
return { ok: false, message: `${key}.links['${link.id}'].to must be a path on this site, such as /wiki/new-player-guide` }
}
if (link.order !== undefined && (typeof link.order !== 'number' || !Number.isFinite(link.order))) {
return { ok: false, message: `${key}.links['${link.id}'].order must be a number` }
}
if (link.section !== undefined && link.section !== null && typeof link.section !== 'string') {
return { ok: false, message: `${key}.links['${link.id}'].section must be a section id` }
}
}
return { ok: 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
@@ -72,7 +181,16 @@ function validateNavOverrides(value, key = 'nav') {
if (typeof value !== 'object' || Array.isArray(value)) {
return { ok: false, message: `${key} must be a JSON object` }
}
const entries = Object.entries(value)
const { items, sections, links } = unwrap(value)
if (!items || typeof items !== 'object' || Array.isArray(items)) {
return { ok: false, message: `${key}.items must be a JSON object` }
}
const sectionCheck = validateSections(sections, key)
if (!sectionCheck.ok) return sectionCheck
const linkCheck = validateLinks(links, key)
if (!linkCheck.ok) return linkCheck
const entries = Object.entries(items)
if (entries.length > MAX_ENTRIES) {
return { ok: false, message: `${key} may hold at most ${MAX_ENTRIES} entries` }
}
@@ -96,6 +214,9 @@ function validateNavOverrides(value, key = 'nav') {
if (field === 'order' && (typeof fieldValue !== 'number' || !Number.isFinite(fieldValue))) {
return { ok: false, message: `${key}['${to}'].order must be a number` }
}
if (field === 'section' && fieldValue !== null && typeof fieldValue !== 'string') {
return { ok: false, message: `${key}['${to}'].section must be a section id` }
}
// `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
@@ -117,15 +238,78 @@ function validateNavOverrides(value, key = 'nav') {
* was customised" (§4.1);
* • reading — a hand-edited entry is dropped and its neighbours kept.
*
* Sections and added links are honored only for the navs that can render them
* (`nav_public`), and a `section` naming no surviving section falls back to the
* top level rather than stranding the item in a dropdown that is not there.
*
* The return shape mirrors the input: a nav with no sections and no added links
* resolves to the bare items map phases 6-8 wrote, so adding this feature
* changed nothing at all for a nav that does not use it.
*
* @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') {
if (!value || typeof value !== 'object' || Array.isArray(value)) return {}
const { items, sections, links } = unwrap(value)
if (!items || typeof items !== 'object' || Array.isArray(items)) return {}
const sectioned = SECTIONED_KEYS.includes(key)
const cleanSections = sectioned ? resolveSections(sections) : []
const known = new Set(cleanSections.map((s) => s.id))
const cleanLinks = sectioned ? resolveLinks(links, known) : []
const out = resolveItems(items, key, known)
if (cleanSections.length === 0 && cleanLinks.length === 0) return out
// A section with nothing in it renders as an empty dropdown, so an admin who
// emptied one has simply stopped using it — but it is theirs to keep until
// they delete it, and the editor is where that happens. Kept here; the
// renderer drops it (client/src/lib/navOverrides.js pruneNav).
const wrapper = { items: out }
if (cleanSections.length) wrapper.sections = cleanSections
if (cleanLinks.length) wrapper.links = cleanLinks
return wrapper
}
function resolveSections(sections) {
const out = []
const seen = new Set()
if (!Array.isArray(sections)) return out
for (const section of sections.slice(0, MAX_SECTIONS)) {
if (!section || typeof section !== 'object' || Array.isArray(section)) continue
if (typeof section.id !== 'string' || !SECTION_ID.test(section.id) || seen.has(section.id)) continue
if (typeof section.label !== 'string' || !section.label.trim() || section.label.length > MAX_LABEL) continue
seen.add(section.id)
const clean = { id: section.id, label: section.label.trim() }
if (typeof section.order === 'number' && Number.isFinite(section.order)) clean.order = section.order
out.push(clean)
}
return out
}
function resolveLinks(links, knownSections) {
const out = []
const seen = new Set()
if (!Array.isArray(links)) return out
for (const link of links.slice(0, MAX_LINKS)) {
if (!link || typeof link !== 'object' || Array.isArray(link)) continue
if (typeof link.id !== 'string' || !LINK_ID.test(link.id) || seen.has(link.id)) continue
if (typeof link.label !== 'string' || !link.label.trim() || link.label.length > MAX_LABEL) continue
if (!isNavPath(link.to)) continue
seen.add(link.id)
const clean = { id: link.id, label: link.label.trim(), to: link.to }
if (typeof link.order === 'number' && Number.isFinite(link.order)) clean.order = link.order
if (typeof link.section === 'string' && knownSections.has(link.section)) clean.section = link.section
out.push(clean)
}
return out
}
function resolveItems(items, key, knownSections) {
const out = {}
if (!value || typeof value !== 'object' || Array.isArray(value)) return out
const unhideable = UNHIDEABLE[key] || []
for (const [to, entry] of Object.entries(value)) {
for (const [to, entry] of Object.entries(items)) {
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
@@ -140,6 +324,10 @@ function resolveNavOverrides(value, key = 'nav') {
if (typeof entry.group === 'string' && entry.group.trim() && entry.group.length <= MAX_GROUP) {
clean.group = entry.group.trim()
}
// Only a section that survived resolution: an item pointing at a deleted or
// malformed one belongs at the top level, visible, rather than inside a
// dropdown that no longer exists.
if (typeof entry.section === 'string' && knownSections.has(entry.section)) clean.section = entry.section
if (Object.keys(clean).length > 0) out[to] = clean
}
return out

View File

@@ -3463,7 +3463,7 @@
"Admin · Settings"
],
"summary": "Update site settings (admin only)",
"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.",
"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 key coded entries by their existing route and carry only label/order/hidden/group/section; whether a key names a route the nav declares is settled client-side at merge time. nav_public may additionally carry admin-created dropdown `sections` and admin-authored `links` — the only place an arbitrary path may be named, and therefore restricted to same-origin paths (no scheme, no protocol-relative host). Sections and links are dropped for the other two navs, which cannot render them.",
"responses": {
"200": {
"description": "Updated settings",

View File

@@ -169,3 +169,121 @@ test('a non-object resolves to {} rather than throwing', () => {
test('NAV_KEYS names the three rows the controller validates', () => {
assert.deepEqual(NAV_KEYS, ['nav_public', 'nav_admin', 'nav_player'])
})
// ── Sections and added links (phase 10) ───────────────────────────────────
//
// The public header may carry admin-created dropdown sections and links the
// admin authored. The invariant that has to survive is structural: `items` may
// only key routes the code declares, so it can never introduce one, while
// `links` is the one place an arbitrary path may be named — and is therefore
// the one place the path rule is applied.
const WRAPPED = {
items: { '/site/champs': { section: 'sec_abcd', order: 0 } },
sections: [{ id: 'sec_abcd', label: 'The World', order: 3 }],
links: [{ id: 'lnk_wxyz', label: 'Guide', to: '/wiki/new-player-guide', section: 'sec_abcd', order: 1 }],
}
test('a bare items map is still valid and still stored as-is', () => {
// Phases 6-8 wrote this shape, and a nav that does not use sections keeps it.
assert.equal(validateNavOverrides({ '/site/news': { label: 'N' } }, 'nav_public').ok, true)
assert.deepEqual(resolveNavOverrides({ '/site/news': { label: 'N' } }, 'nav_public'), {
'/site/news': { label: 'N' },
})
})
test('a wrapped value round-trips with its sections and links', () => {
assert.equal(validateNavOverrides(WRAPPED, 'nav_public').ok, true)
assert.deepEqual(resolveNavOverrides(WRAPPED, 'nav_public'), WRAPPED)
})
test('an added link must point at this site', () => {
for (const to of ['https://evil.example', '//evil.example/x', 'javascript:alert(1)', 'wiki/guide', '/a b', '/a"b']) {
const check = validateNavOverrides(
{ items: {}, links: [{ id: 'lnk_wxyz', label: 'Bad', to }] },
'nav_public',
)
assert.equal(check.ok, false, `${to} should be refused`)
assert.match(check.message, /must be a path on this site/)
}
})
test('a link to a path that happens to be gated is allowed — the page is the gate', () => {
// An added link carries no roles/feature of its own and does not need one: the
// route behind it enforces its own access, exactly as typing the URL would.
const check = validateNavOverrides(
{ items: {}, links: [{ id: 'lnk_wxyz', label: 'Admin', to: '/admin/users' }] },
'nav_public',
)
assert.equal(check.ok, true)
})
test('section and link ids are constrained, and duplicates refused', () => {
const bad = [
[{ sections: [{ id: 'nope', label: 'X' }] }, /invalid id/],
[{ sections: [{ id: 'sec_AB', label: 'X' }] }, /invalid id/],
[{ sections: [{ id: 'sec_abcd', label: '' }] }, /label must be text/],
[{ sections: [{ id: 'sec_abcd', label: 'A' }, { id: 'sec_abcd', label: 'B' }] }, /duplicate id/],
[{ links: [{ id: 'sec_abcd', label: 'X', to: '/x' }] }, /invalid id/],
[{ links: [{ id: 'lnk_abcd', label: 'A', to: '/a' }, { id: 'lnk_abcd', label: 'B', to: '/b' }] }, /duplicate id/],
]
for (const [extra, pattern] of bad) {
const check = validateNavOverrides({ items: {}, ...extra }, 'nav_public')
assert.equal(check.ok, false, JSON.stringify(extra))
assert.match(check.message, pattern)
}
})
test('sections and links are bounded', () => {
const sections = Array.from({ length: 13 }, (_, i) => ({ id: `sec_a${String(i).padStart(3, '0')}`, label: 'S' }))
assert.match(validateNavOverrides({ items: {}, sections }, 'nav_public').message, /at most 12 sections/)
const links = Array.from({ length: 41 }, (_, i) => ({ id: `lnk_a${String(i).padStart(3, '0')}`, label: 'L', to: '/x' }))
assert.match(validateNavOverrides({ items: {}, links }, 'nav_public').message, /at most 40 added links/)
})
test('sections and links are dropped for the navs that cannot render them', () => {
// The admin sidebar has its own coded sections and the player portal is three
// flat rows; only the public header supports this.
for (const key of ['nav_admin', 'nav_player']) {
const out = resolveNavOverrides(WRAPPED, key)
assert.equal(out.sections, undefined, key)
assert.equal(out.links, undefined, key)
// The item survives, minus the section it can no longer belong to.
assert.deepEqual(out, { '/site/champs': { order: 0 } })
}
})
test('an item or link naming a section that does not exist falls to the top level', () => {
const out = resolveNavOverrides(
{
items: { '/site/champs': { section: 'sec_gone', order: 2 } },
sections: [{ id: 'sec_abcd', label: 'Real' }],
links: [{ id: 'lnk_wxyz', label: 'L', to: '/x', section: 'sec_gone' }],
},
'nav_public',
)
assert.equal(out.items['/site/champs'].section, undefined)
assert.equal(out.links[0].section, undefined)
})
test('an unusable section or link is dropped, its neighbours kept', () => {
const out = resolveNavOverrides(
{
items: {},
sections: [{ id: 'sec_abcd', label: 'Keep' }, { id: 'bad', label: 'Drop' }],
links: [
{ id: 'lnk_aaaa', label: 'Keep', to: '/keep' },
{ id: 'lnk_bbbb', label: 'Drop', to: 'https://evil.example' },
],
},
'nav_public',
)
assert.deepEqual(out.sections.map((s) => s.label), ['Keep'])
assert.deepEqual(out.links.map((l) => l.label), ['Keep'])
})
test('a wrapper that resolves to nothing usable comes back empty', () => {
// The caller deletes the row rather than storing a wrapper that says nothing.
assert.deepEqual(resolveNavOverrides({ items: {}, sections: [], links: [] }, 'nav_public'), {})
assert.deepEqual(resolveNavOverrides({ items: 'nope' }, 'nav_public'), {})
})