Skip to content

Commit 3a804e3

Browse files
melvincarvalhomelvincarvalho
committed
markets/: fix cookie sessions resolving to "[object Object]"
I widened the session verifier to return claims instead of an agent id and shipped it without updating its two callers, so every cookie session authenticated as the string "[object Object]". Every browser user shared one ledger row, ensureAccount minted a phantom grant against that key, the rate limiter was keyed on a fresh object per request (so it did nothing), and market creation from the UI 400'd because isAgentId() of an object fails. The conservation invariant was silently false and the journal would have replayed it that way forever. Sixty-two green tests missed this because they asserted status codes and never asserted WHICH agent a cookie resolved to. There are now tests for the identity itself, for two sessions not being the same account, and for creating a market through a cookie session. The epoch work that widening was for is now actually wired: tokens carry an epoch, sign-out and freeze bump it, and a replayed token after sign-out is dead. Previously DELETE /api/session only cleared the cookie while the self-verifying token stayed valid for its full 12 hours. Also from the review: - A failed journal rotation closed the fd and never reopened it, so every later commit died with EBADF -- every trade and settlement 500ing forever, logged as "harmless, will retry". It never retried. - eventsFor captured the segment list at boot, so a rotation at runtime made the operator's agent-history query blind to everything written since startup -- the same confidently-empty answer that query exists to prevent. - fs.writeSync's return value was ignored: a short write truncated an event and fsync made the truncation durable, in the one function whose entire purpose is durability. - An admin voiding a disputed market through /void rather than /adjudicate forfeited the disputer's bond despite vindicating them. - ownOrigin() can be null without baseUrl, which would have 403'd every browser mutation with no diagnostic; it now falls back to the request's Host. - The header diagram still described the old dispute-grace-to-void policy that this round deliberately reversed. 64 plugin tests.
1 parent 76af048 commit 3a804e3

4 files changed

Lines changed: 135 additions & 18 deletions

File tree

‎markets/README.md‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ Open `{prefix}/` for the UI; the JSON API is under `{prefix}/api`.
2424
| `POST /api/markets/:id/trade` | buy/sell, slippage guards, idempotency keys |
2525
| `.../close .../resolve .../dispute .../settle .../void` | lifecycle |
2626
| `GET /api/stats` | public conservation figures + journal seq |
27+
| `POST /api/admin/adjudicate` | uphold / re-resolve / void a disputed market |
28+
| `GET /api/admin/disputes` | the operator's dispute queue, soonest deadline first |
29+
| `GET /api/admin/agent` | one agent's journal history (support / adjudication) |
2730
| `POST /api/admin/{freeze,adjust,hide}` | operator plane (journalled) |
2831
| `WS {prefix}/ws` | `{market,trade,settle}` events |
2932

@@ -150,6 +153,17 @@ the walls point at.
150153
regression test for exactly this attack, and it caught a real bug: the
151154
price path was seeded with `m.history || [seed]`, and an empty array is
152155
truthy, so the TWAP degenerated to the post-pump spot price.
156+
- **A self-verifying credential needs an epoch, and the type of what
157+
`verify()` returns is a money bug.** Widening the session verifier from
158+
"returns the agent id" to "returns the claims" without updating its two
159+
callers made every cookie session authenticate as the string
160+
`[object Object]`: one shared ledger row for every browser user, a
161+
phantom grant minted against that key, a rate limiter keyed on a fresh
162+
object per request (so, disabled), and a conservation invariant that
163+
was silently false and would have replayed that way forever. Sixty-two
164+
green tests missed it because they asserted status codes and never once
165+
asserted *which agent* a cookie resolved to. Test the identity, not the
166+
200.
153167
- **Dropping a torn journal tail is only half of crash recovery.** The
154168
fragment must also be TRUNCATED before reopening for append —
155169
otherwise the next acknowledged, fsync'd event is welded onto the
@@ -171,7 +185,7 @@ the walls point at.
171185

172186
## Tests
173187

174-
`node --test --test-concurrency=1 markets/test.js` — 62 tests: LMSR and
188+
`node --test --test-concurrency=1 markets/test.js` — 64 tests: LMSR and
175189
TWAP math, session/CSRF/rate-limit units, hardened headers, cookie
176190
scoping, prototype-key ids, grants, escrow, stake-first quotes,
177191
quote↔trade parity, slippage guards (including the NaN-fails-closed case),

‎markets/plugin.js‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
// | (funds are NEVER stuck:
3434
// | any holder disputes anyone may trigger this)
3535
// v
36-
// disputed --admin, or disputeGrace--> void at TWAP
36+
// disputed --admin: uphold / re-resolve / void--> settled
37+
// --disputeGrace with no admin--> the resolution STANDS
38+
// (bonds forfeited: silence must not be a free refund)
3739
//
3840
// Three separate defences against the oracle stealing the pool:
3941
// 1. the oracle and creator MAY NOT TRADE in their own market;
@@ -221,15 +223,27 @@ export async function activate(api) {
221223
* the pod bearer path is reserved for API clients that send it
222224
* explicitly.
223225
*/
226+
/** Resolve a session token to an agent id. A session is live only
227+
* while its epoch matches the agent's current epoch — that comparison
228+
* is what makes sign-out, freeze and revoke actually end a session,
229+
* since a self-verifying token is otherwise valid for its whole TTL. */
230+
function liveSession(token) {
231+
const claims = sessions.verify(token);
232+
if (!claims) return null;
233+
const row = state.ledger[claims.agent];
234+
if (row && (row.epoch || 0) !== claims.epoch) return null;
235+
return claims.agent;
236+
}
237+
224238
async function resolveAgent(request) {
225239
const cookie = cookieToken(request);
226240
if (cookie) {
227-
const agent = sessions.verify(cookie);
241+
const agent = liveSession(cookie);
228242
if (agent) return agent;
229243
}
230244
const auth = request.headers.authorization;
231245
if (auth && auth.startsWith('Bearer v1.')) {
232-
const agent = sessions.verify(auth.slice(7));
246+
const agent = liveSession(auth.slice(7));
233247
if (agent) return agent;
234248
}
235249
return api.auth.getAgent(request);
@@ -241,14 +255,24 @@ export async function activate(api) {
241255
/** Guard for every mutating route: same-origin required whenever the
242256
* credential is ambient (cookie/TLS cert), because those are exactly
243257
* the credentials a cross-origin page can borrow. */
258+
/** The origin to compare against, preferring configured/serverInfo and
259+
* falling back to the request's own Host so a deployment without
260+
* baseUrl doesn't silently refuse every browser mutation. */
261+
function originFor(request) {
262+
const known = ownOrigin();
263+
if (known) return known;
264+
const host = request.headers.host;
265+
return host ? `${request.protocol || 'http'}://${host}` : null;
266+
}
267+
244268
function csrfOk(request) {
245269
// A session cookie makes the request ambient REGARDLESS of any
246270
// Authorization header: resolveAgent checks the cookie first, so an
247271
// attacker could otherwise bolt on a junk bearer to look
248272
// "explicitly credentialed", skip this check, and still be
249273
// authenticated by the victim's cookie.
250274
if (!cookieToken(request) && !isAmbientCredential(request)) return true;
251-
return isSameOrigin(request, ownOrigin());
275+
return isSameOrigin(request, originFor(request));
252276
}
253277

254278
async function authed(request, reply, { mutating = true } = {}) {
@@ -294,7 +318,7 @@ export async function activate(api) {
294318

295319
api.fastify.addHook('onRequest', async (request, reply) => {
296320
if (!mine(request)) return undefined;
297-
const key = sessions.verify(cookieToken(request) || '') || request.ip;
321+
const key = liveSession(cookieToken(request) || '') || request.ip;
298322
const cost = request.method === 'GET' || request.method === 'HEAD' ? 1 : 4;
299323
const waitMs = limiter.take(key, cost);
300324
if (waitMs) {
@@ -684,15 +708,21 @@ export async function activate(api) {
684708
if (!csrfOk(request)) return err(reply, 403, 'cross-origin request refused');
685709
const agent = await api.auth.getAgent(request);
686710
if (!agent) return err(reply, 401, 'a pod bearer token is required to start a session');
687-
const token = sessions.mint(agent);
688711
ensureAccount(agent);
712+
const token = sessions.mint(agent, state.ledger[agent].epoch || 0);
689713
const secure = (ownOrigin() || '').startsWith('https:') ? ' Secure;' : '';
690714
reply.header('set-cookie',
691715
`${cookieName}=${encodeURIComponent(token)}; Path=${prefix}; HttpOnly; SameSite=Strict;${secure} Max-Age=${Math.floor(sessionTtlMs / 1000)}`);
692716
return reply.send({ agent, expiresIn: Math.floor(sessionTtlMs / 1000), balance: balanceOf(agent) / MICRO });
693717
});
694718

695719
api.fastify.delete(`${prefix}/api/session`, async (request, reply) => {
720+
if (!csrfOk(request)) return err(reply, 403, 'cross-origin request refused');
721+
// Clearing the cookie is cosmetic on its own — the token is
722+
// self-verifying, so a captured copy still worked for the full TTL.
723+
// Bump the epoch so every token minted before now stops verifying.
724+
const who = await resolveAgent(request);
725+
if (who && state.ledger[who]) store.commit({ type: 'session.revoke', agent: who });
696726
reply.header('set-cookie', `${cookieName}=; Path=${prefix}; HttpOnly; SameSite=Strict; Max-Age=0`);
697727
return reply.send({ ok: true });
698728
});
@@ -1043,8 +1073,10 @@ export async function activate(api) {
10431073
return err(reply, 409, `market is ${displayStatus(m)} — nothing to settle`);
10441074
});
10451075

1046-
// A holder can park a resolution they believe is wrong. That is the
1047-
// check on a unilateral oracle: disputed markets never auto-pay.
1076+
// A holder can park a resolution they believe is wrong, at the cost of
1077+
// a bond. An operator adjudicates; if none does before the grace
1078+
// expires the resolution stands and the bond is forfeited, because a
1079+
// dispute that cancels the market for free is just a refund button.
10481080
api.fastify.post(`${prefix}/api/markets/:id/dispute`, jsonOpts(1024), async (request, reply) => {
10491081
const agent = await authed(request, reply);
10501082
if (!agent) return reply;
@@ -1100,7 +1132,10 @@ export async function activate(api) {
11001132
: 'only the oracle may void before the settlement window expires');
11011133
}
11021134
if (!m.closedAt) store.commit({ type: 'market.close', marketId: m.id });
1103-
settleVoid(m);
1135+
// An operator voiding a DISPUTED market has sustained the dispute,
1136+
// whichever route they used to do it.
1137+
settleVoid(m, m.status === 'disputed' && admins.has(agent)
1138+
? { adjudicatedBy: agent, sustained: true } : {});
11041139
api.log.info(`markets: ${m.id} voided (redeemed at TWAP)`);
11051140
return reply.send(marketOut(m));
11061141
});

‎markets/store.js‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,9 @@ export function createStore({ dir, log, prices }) {
397397
function commit(ev) {
398398
ev.seq = state.seq + 1;
399399
ev.t = ev.t || Date.now();
400-
fs.writeSync(jfd, `${JSON.stringify(ev)}\n`);
400+
const line = Buffer.from(`${JSON.stringify(ev)}\n`, 'utf8');
401+
let off = 0;
402+
while (off < line.length) off += fs.writeSync(jfd, line, off, line.length - off);
401403
fs.fsyncSync(jfd);
402404
applyEvent(state, ev, prices);
403405
dirty++;
@@ -416,15 +418,21 @@ export function createStore({ dir, log, prices }) {
416418
// the file — then the retired segment is never needed for replay,
417419
// only for audit. Without this the journal grows forever and boot
418420
// is O(lifetime).
419-
try {
420-
if (fs.fstatSync(jfd).size >= ROTATE_BYTES) {
421+
if (fs.fstatSync(jfd).size >= ROTATE_BYTES) {
422+
const retired = path.join(path.dirname(journalFile), `journal.${state.seq}.jsonl`);
423+
try {
421424
fs.closeSync(jfd);
422-
fs.renameSync(journalFile, path.join(path.dirname(journalFile), `journal.${state.seq}.jsonl`));
425+
fs.renameSync(journalFile, retired);
426+
files.push(retired); // keep the audit query able to see it
427+
} catch (err) {
428+
// NOT harmless: leaving jfd closed makes every later commit
429+
// fail with EBADF, i.e. every trade and settlement 500s
430+
// forever. Always get a working descriptor back.
431+
log.error(`markets: journal rotation failed: ${err.message}`);
432+
} finally {
423433
jfd = fs.openSync(journalFile, 'a');
424-
log.info(`markets: rotated journal at seq ${state.seq} (prior segment retained for audit)`);
425434
}
426-
} catch (err) {
427-
log.warn(`markets: journal rotation failed (harmless, will retry): ${err.message}`);
435+
log.info(`markets: rotated journal at seq ${state.seq} (prior segment retained for audit)`);
428436
}
429437
} catch (err) {
430438
// Non-fatal by design: the journal is the durable record, so a
@@ -444,7 +452,15 @@ export function createStore({ dir, log, prices }) {
444452
// an EMPTY history after the first rotation, and a confidently empty
445453
// answer to "what happened to this account" is worse than an error.
446454
const all = [];
447-
for (const file of files) {
455+
// Re-scan: a rotation since boot moved the live file's contents into
456+
// a segment that wasn't in the boot-time list.
457+
const current = new Set(files);
458+
try {
459+
for (const f of fs.readdirSync(dir)) {
460+
if (/^journal\.\d+\.jsonl$/.test(f)) current.add(path.join(dir, f));
461+
}
462+
} catch { /* directory vanished */ }
463+
for (const file of [...current].sort()) {
448464
try { all.push(...fs.readFileSync(file, 'utf8').split('\n')); } catch { /* rotated away */ }
449465
}
450466
for (const line of all) {

‎markets/test.js‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,58 @@ describe('markets plugin', () => {
349349
await assertConserved('after create');
350350
});
351351

352+
it('a cookie session resolves to the RIGHT agent, and sessions are isolated', async () => {
353+
// The regression that 62 status-code-only tests missed: verify()
354+
// returns a claims OBJECT, and returning it as the agent id made
355+
// every cookie user share one ledger row keyed "[object Object]".
356+
for (const user of ['alice', 'bob', 'carol']) {
357+
const res = await fetch(`${mk}/me`, {
358+
headers: { cookie: cookie[user], 'sec-fetch-site': 'same-origin' },
359+
});
360+
const body = await json(res, 200);
361+
assert.strictEqual(typeof body.agent, 'string', 'the agent is an id, not a claims object');
362+
assert.ok(isAgentId(body.agent), `${user}'s cookie resolves to a WebID (got ${body.agent})`);
363+
const viaBearer = await me(user);
364+
assert.strictEqual(body.agent, viaBearer.agent,
365+
`${user}'s cookie and pod bearer must be the same identity`);
366+
}
367+
// …and two users must not be the same account.
368+
const a = await (await fetch(`${mk}/me`, { headers: { cookie: cookie.alice, 'sec-fetch-site': 'same-origin' } })).json();
369+
const b = await (await fetch(`${mk}/me`, { headers: { cookie: cookie.bob, 'sec-fetch-site': 'same-origin' } })).json();
370+
assert.notStrictEqual(a.agent, b.agent, 'cookie sessions are not a shared ledger row');
371+
// A cookie session can also create a market (isAgentId(obj) failed,
372+
// so the shipped UI could not create one at all).
373+
const created = await fetch(`${mk}/markets`, {
374+
method: 'POST',
375+
headers: { cookie: cookie.alice, 'content-type': 'application/json', 'sec-fetch-site': 'same-origin' },
376+
body: JSON.stringify({
377+
title: 'Created from a browser session',
378+
outcomes: ['Yes', 'No'],
379+
closesAt: new Date(Date.now() + 3600e3).toISOString(),
380+
b: 15,
381+
}),
382+
});
383+
assert.strictEqual(created.status, 201, await created.text());
384+
await assertConserved('after a cookie-session market');
385+
});
386+
387+
it('signing out actually revokes the session token', async () => {
388+
// Clearing the cookie is cosmetic: the token is self-verifying, so a
389+
// captured copy worked for the full TTL until epochs were added.
390+
const res = await call('carol', 'POST', '/session');
391+
const tok = res.headers.get('set-cookie').split(';')[0];
392+
assert.strictEqual((await fetch(`${mk}/me`, { headers: { cookie: tok, 'sec-fetch-site': 'same-origin' } })).status, 200);
393+
await fetch(`${mk}/session`, {
394+
method: 'DELETE',
395+
headers: { cookie: tok, 'sec-fetch-site': 'same-origin' },
396+
});
397+
const replayed = await fetch(`${mk}/me`, { headers: { cookie: tok, 'sec-fetch-site': 'same-origin' } });
398+
assert.strictEqual(replayed.status, 401, 'a replayed token after sign-out must be dead');
399+
// carol needs a working session again for later tests.
400+
const fresh = await call('carol', 'POST', '/session');
401+
cookie.carol = fresh.headers.get('set-cookie').split(';')[0];
402+
});
403+
352404
it('the creator and oracle may not trade in their own market', async () => {
353405
const out = await json(await call('alice', 'POST', `/markets/${market.id}/trade`,
354406
{ side: 'buy', outcome: 0, shares: 10 }), 403);

0 commit comments

Comments
 (0)