fix(shard): enforce visibility on the REST reads that bypassed it
Protocol 3.0 Part A follow-up, found by the live five-rung smoke test.
Part A implemented the visibility framework correctly on the SSE path
and on /guilds + /governors, but the remaining public REST reads never
called into it. The result was that one event was projected live and
served verbatim from history:
* GET /public/shard/feed returned the stored payload as-is, so
actor.acct and actor.webId were readable ANONYMOUSLY for every
logged kind - player.death, player.murdered, mob.killed,
quest.complete, skill.gain, fame/karma.change, mob.login/logout,
guild.join. Broader than the guild-leader leak Part A set out to
close, since it covers every player rather than board holders.
* GET /public/shard/idoc returned ownerAcct - the house owner's game
account - to anonymous callers.
* The `houses` field rules (owner/price -> staff) were dead config:
neither getIdoc nor getHouses projected, so an admin could set them
in the panel and nothing happened.
* /feed filtered on PUBLIC_KINDS, a module-load constant derived from
the compiled DEFAULTS, so live audience changes did not reach it.
With `guilds` moved to staff, /guilds 403'd while /feed happily
served guild.join to anonymous.
Four fixes, all at the root rather than per-route:
1. Rule 1 now matches a field's MEANING, not one spelling. The wire
nests actors (leader.acct) but the read models flatten them
(shapeHouse -> ownerAcct, shapeGuild -> leaderWebId), and an
exact-key check missed every flattened one. isLockedField() locks a
key that is or ends in acct/webId, case-insensitively, so it fails
closed for shapes not yet written. The admin PUT rejects those
spellings too - `ownerAcct` is no longer configurable.
2. visibleKinds(level, config) resolves readable kinds from the LIVE
config; getFeed uses it and projects each row against its own kind's
feature. Deliberately independent of the `stream` flag, which governs
SSE fan-out only - so market history stays readable with its firehose
off. This makes the set a superset of PUBLIC_KINDS by exactly the two
vendor kinds.
3. getIdoc/getHouses/getChamps/getPresence project, so every shard
surface honours the same config.
4. shardEvents.db.list treats an EMPTY kinds array as "serve nothing".
It previously fell through to the unfiltered query, so a fully-gated
config would have dumped the whole event log, staff audit included.
Also fixes a bug introduced while wiring this up: projectValue recursed
into any object, so a Date column came back as {}. It now walks arrays
and plain objects only. The unit tests used JSON fixtures and could not
have caught it - the live /idoc read did.
Verified live against MariaDB + a stub sidecar, all five rungs: 13
routes x 5 rungs, defaults reproducing pre-v3 access exactly, zero
acct/webId below admin on any read, unmapped kinds (staff.command,
cheat.detect, login.attempt) reaching only admin on SSE, and audience /
enabled / stream changes taking effect live on an already-open stream.
Tests: 487 server (+9). Swagger regenerated; route manifest unchanged.
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -20,10 +20,22 @@ const shardEvents = require('../src/model/shardEvents/shardEvents.model')
|
||||
const shardState = require('../src/model/shardState/shardState.model')
|
||||
const uoLinkConfig = require('../src/model/uoLinkConfig/uoLinkConfig.model')
|
||||
const broadcast = require('../src/utils/shardBroadcast')
|
||||
const visibility = require('../src/utils/shardVisibility')
|
||||
const db = require('../src/utils/db')
|
||||
|
||||
after(() => db.close())
|
||||
|
||||
// The controller now resolves the visibility config and the caller's rung on
|
||||
// every read. Stub the MODEL rather than the util's exports: getConfig() and
|
||||
// project() call the module-internal getConfig, which an exports-level stub does
|
||||
// not intercept — it would still hit the closed DB port and cost a ~10s pool
|
||||
// timeout per test before falling back to these same defaults.
|
||||
const visibilityModel = require('../src/model/shardVisibility/shardVisibility.model')
|
||||
visibilityModel.listAll = async () => [] // no overrides ⇒ compiled defaults
|
||||
visibility.viewerLevel = async (req) => req?.viewerLevel || 'anonymous'
|
||||
|
||||
const DEFAULTS = visibility.compileDefaults()
|
||||
|
||||
function mockRes() {
|
||||
return {
|
||||
statusCode: 200,
|
||||
@@ -81,16 +93,65 @@ test('getFeed serves a specific kind when it IS public-safe', async () => {
|
||||
assert.equal(res.body[0].kind, publicKind)
|
||||
})
|
||||
|
||||
test('getFeed with no kind restricts the query to the whole public allowlist', async () => {
|
||||
test('getFeed with no kind restricts the query to the kinds THIS viewer may read', async () => {
|
||||
let seen
|
||||
shardEvents.list = async (opts) => {
|
||||
seen = opts
|
||||
return []
|
||||
}
|
||||
await ctrl.getFeed({ query: {} }, mockRes())
|
||||
assert.deepEqual(new Set(seen.kinds), broadcast.PUBLIC_KINDS)
|
||||
// Resolved from the LIVE config, not the module-load PUBLIC_KINDS constant, so
|
||||
// an admin re-gating a feature takes effect on the stored history too.
|
||||
assert.deepEqual(new Set(seen.kinds), new Set(visibility.visibleKinds('anonymous', DEFAULTS)))
|
||||
// Sanity: a known admin-only kind is absent from what the public feed queries.
|
||||
assert.ok(!seen.kinds.includes('staff.audit'))
|
||||
// The `stream` flag governs SSE fan-out only, so a feature whose live firehose
|
||||
// ships off is still readable from history — the one way this set is WIDER
|
||||
// than PUBLIC_KINDS.
|
||||
for (const kind of broadcast.PUBLIC_KINDS) assert.ok(seen.kinds.includes(kind))
|
||||
assert.ok(seen.kinds.includes('vendor.listing'))
|
||||
assert.ok(!broadcast.PUBLIC_KINDS.has('vendor.listing'))
|
||||
})
|
||||
|
||||
test('getFeed projects each row against ITS OWN kind\'s feature', async () => {
|
||||
shardEvents.list = async () => [
|
||||
{
|
||||
id: 1,
|
||||
kind: 'player.death',
|
||||
payload: { kind: 'player.death', actor: { serial: '0x1', name: 'Doomed', acct: 'secret', webId: 99 } },
|
||||
},
|
||||
{
|
||||
id: 2,
|
||||
kind: 'guild.join',
|
||||
payload: { kind: 'guild.join', actor: { serial: '0x2', name: 'Joiner', acct: 'secret2', webId: 98 } },
|
||||
},
|
||||
]
|
||||
const res = mockRes()
|
||||
await ctrl.getFeed({ query: {} }, res)
|
||||
for (const row of res.body) {
|
||||
assert.equal(row.payload.actor.acct, undefined, `${row.kind} leaked acct`)
|
||||
assert.equal(row.payload.actor.webId, undefined, `${row.kind} leaked webId`)
|
||||
assert.ok(row.payload.actor.name, 'the in-game name is still public')
|
||||
}
|
||||
})
|
||||
|
||||
test('getFeed serves nothing when the viewer may read no kinds at all', async () => {
|
||||
let queried = false
|
||||
shardEvents.list = async () => {
|
||||
queried = true
|
||||
return [{ kind: 'staff.audit' }]
|
||||
}
|
||||
const allGated = Object.fromEntries(
|
||||
Object.entries(DEFAULTS).map(([name, f]) => [name, { ...f, enabled: false }]),
|
||||
)
|
||||
visibility.getConfig = async () => allGated
|
||||
const res = mockRes()
|
||||
await ctrl.getFeed({ query: {} }, res)
|
||||
visibility.getConfig = async () => DEFAULTS
|
||||
assert.deepEqual(res.body, [])
|
||||
// An empty allowlist must never fall through to an unfiltered "give me
|
||||
// everything" query.
|
||||
assert.equal(queried, false)
|
||||
})
|
||||
|
||||
// ── getHouses: the public house view must strip owner/price ─────────────
|
||||
@@ -122,6 +183,66 @@ test('getHouses exposes only IDOC location fields and strips owner/price/decay',
|
||||
assert.equal(h.coOwners, undefined)
|
||||
})
|
||||
|
||||
// ── getIdoc: the flattened owner fields are a security boundary too ──────
|
||||
test('getIdoc never serves the owner game account to a viewer below admin', async () => {
|
||||
shardState.listIdoc = async () => [
|
||||
{
|
||||
serial: '0x1',
|
||||
name: 'Marble Tower',
|
||||
region: 'Britain',
|
||||
map: 'Felucca',
|
||||
x: 1,
|
||||
y: 2,
|
||||
z: 3,
|
||||
ownerSerial: '0x2A01',
|
||||
ownerName: 'Sir Cadmus',
|
||||
ownerAcct: 'cadmus_acct', // flattened spelling of the locked `acct`
|
||||
price: 1250000,
|
||||
isIdoc: true,
|
||||
},
|
||||
]
|
||||
for (const level of ['anonymous', 'logged_in', 'player', 'staff']) {
|
||||
const res = mockRes()
|
||||
await ctrl.getIdoc({ viewerLevel: level }, res)
|
||||
assert.equal(res.body[0].ownerAcct, undefined, `${level} saw the owner's game account`)
|
||||
}
|
||||
const res = mockRes()
|
||||
await ctrl.getIdoc({ viewerLevel: 'admin' }, res)
|
||||
assert.equal(res.body[0].ownerAcct, 'cadmus_acct', 'admin still sees it')
|
||||
})
|
||||
|
||||
test('getIdoc gates owner identity and price at `staff`, but never the location', async () => {
|
||||
shardState.listIdoc = async () => [
|
||||
{ serial: '0x1', name: 'Marble Tower', region: 'Britain', map: 'Felucca', x: 1, y: 2, z: 3,
|
||||
ownerSerial: '0x2A01', ownerName: 'Sir Cadmus', price: 1250000, isIdoc: true },
|
||||
]
|
||||
const anon = mockRes()
|
||||
await ctrl.getIdoc({ viewerLevel: 'anonymous' }, anon)
|
||||
assert.equal(anon.body[0].ownerName, undefined)
|
||||
assert.equal(anon.body[0].ownerSerial, undefined)
|
||||
assert.equal(anon.body[0].price, undefined)
|
||||
// The public IDOC board still renders: name, region and location survive.
|
||||
assert.equal(anon.body[0].name, 'Marble Tower')
|
||||
assert.equal(anon.body[0].region, 'Britain')
|
||||
assert.equal(anon.body[0].map, 'Felucca')
|
||||
|
||||
const staff = mockRes()
|
||||
await ctrl.getIdoc({ viewerLevel: 'staff' }, staff)
|
||||
assert.equal(staff.body[0].ownerName, 'Sir Cadmus')
|
||||
assert.equal(staff.body[0].price, 1250000)
|
||||
})
|
||||
|
||||
test('getIdoc preserves Date columns rather than flattening them to {}', async () => {
|
||||
const when = new Date('2026-07-06T19:32:29.000Z')
|
||||
shardState.listIdoc = async () => [
|
||||
{ serial: '0x1', name: 'Marble Tower', isIdoc: true, lastRefreshed: when, updatedAt: when },
|
||||
]
|
||||
const res = mockRes()
|
||||
await ctrl.getIdoc({ viewerLevel: 'anonymous' }, res)
|
||||
assert.ok(res.body[0].updatedAt instanceof Date, 'a Date must survive projection intact')
|
||||
assert.equal(res.body[0].updatedAt.toISOString(), when.toISOString())
|
||||
})
|
||||
|
||||
// ── getStatus assembles the summary ─────────────────────────────────────
|
||||
test('getStatus merges the sidecar config with the online count and latest economy', async () => {
|
||||
uoLinkConfig.getSafe = async () => ({
|
||||
|
||||
@@ -128,6 +128,67 @@ test('a stored rule trying to loosen a locked field is ignored', async () => {
|
||||
assert.equal('webId' in out.leader, false)
|
||||
})
|
||||
|
||||
test('rule 1 matches FLATTENED spellings, not just the two canonical keys', () => {
|
||||
const config = visibility.compileDefaults()
|
||||
// shapeHouse/shapeGuild flatten the actor into `<role>Acct` / `<role>WebId`.
|
||||
// An exact-key check missed every one of these, which is how GET
|
||||
// /public/shard/idoc served the owner's game account to anonymous callers.
|
||||
const row = {
|
||||
serial: '0x1',
|
||||
name: 'Marble Tower',
|
||||
ownerAcct: 'cadmus_acct',
|
||||
leaderWebId: 42,
|
||||
governorAcct: 'blackthorn_acct',
|
||||
}
|
||||
const out = visibility.projectFeature('houses', row, 'staff', config)
|
||||
assert.equal('ownerAcct' in out, false, 'staff must not see a flattened acct')
|
||||
assert.equal('leaderWebId' in out, false)
|
||||
assert.equal('governorAcct' in out, false)
|
||||
assert.equal(out.name, 'Marble Tower', 'ordinary fields are untouched')
|
||||
|
||||
const asAdmin = visibility.projectFeature('houses', row, 'admin', config)
|
||||
assert.equal(asAdmin.ownerAcct, 'cadmus_acct')
|
||||
})
|
||||
|
||||
test('isLockedField locks acct/webId and their suffixed forms, and nothing else', () => {
|
||||
for (const key of ['acct', 'webId', 'WEBID', 'ownerAcct', 'leaderWebId', 'governorAcct']) {
|
||||
assert.equal(visibility.isLockedField(key), true, `${key} must be locked`)
|
||||
}
|
||||
// Must not over-match: these are ordinary public fields.
|
||||
for (const key of ['name', 'serial', 'ownerName', 'price', 'contact', 'region']) {
|
||||
assert.equal(visibility.isLockedField(key), false, `${key} must stay configurable`)
|
||||
}
|
||||
})
|
||||
|
||||
test('a Date survives projection instead of collapsing to {}', () => {
|
||||
const config = visibility.compileDefaults()
|
||||
const when = new Date('2026-07-06T19:32:29.000Z')
|
||||
// The DB-backed read models carry real Date columns; rebuilding one key-by-key
|
||||
// yields `{}` because a Date has no enumerable own properties.
|
||||
const out = visibility.projectFeature('houses', { name: 'Keep', updatedAt: when }, 'anonymous', config)
|
||||
assert.ok(out.updatedAt instanceof Date)
|
||||
assert.equal(out.updatedAt.toISOString(), when.toISOString())
|
||||
})
|
||||
|
||||
test('visibleKinds tracks live config and stays independent of the stream flag', async () => {
|
||||
const config = visibility.compileDefaults()
|
||||
assert.ok(visibleIncludes(config, 'anonymous', 'guild.update'))
|
||||
// `stream: false` suppresses SSE fan-out only — the stored history stays readable.
|
||||
assert.ok(visibleIncludes(config, 'anonymous', 'vendor.listing'))
|
||||
assert.equal(visibility.kindVisibleTo('vendor.listing', 'anonymous', config), false)
|
||||
|
||||
const gated = { ...config, guilds: { ...config.guilds, audience: 'staff' } }
|
||||
assert.equal(visibleIncludes(gated, 'anonymous', 'guild.update'), false)
|
||||
assert.ok(visibleIncludes(gated, 'staff', 'guild.update'))
|
||||
|
||||
const off = { ...config, guilds: { ...config.guilds, enabled: false } }
|
||||
assert.equal(visibleIncludes(off, 'admin', 'guild.update'), false)
|
||||
// Rule 2 still holds: an unmapped kind is in nobody's readable set.
|
||||
assert.equal(visibleIncludes(config, 'admin', 'staff.audit'), false)
|
||||
})
|
||||
|
||||
const visibleIncludes = (config, level, kind) => visibility.visibleKinds(level, config).includes(kind)
|
||||
|
||||
test('projection recurses into arrays and nested actors', () => {
|
||||
const config = visibility.compileDefaults()
|
||||
const rows = [
|
||||
|
||||
Reference in New Issue
Block a user