From 2368647a01f004a7eea9bdb632770a468f69d8f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 15:28:26 +0000 Subject: [PATCH] Upgrade to core 0.13 line and adopt the 404/403 anti-oracle rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps @haverstack/core to ^0.13.1, wire-types to ^0.12.0, adapter-local to ^0.12.0, commons to ^0.5.0, and conformance-fixtures to ^0.6.0 (dev), resolving core to 0.13.1 and record-adapter-sqlite to 0.5.1 transitively. ScopedStack.get() now returns null (rather than throwing) for a record the requester can't read, and the write/history mutators throw StackNotFoundError instead of StackPermissionError in that case, so 403 is now earned by readability rather than by holding a valid id. Record ids encode their creation millisecond, so a bare 403 to an unauthenticated caller would let anyone confirm a guessed id's same-millisecond siblings — errorMiddleware now answers an anonymous 404 with a Bearer WWW-Authenticate challenge, keeping the login prompt reachable without reopening that distinction. entity.ts's cached owner-record id is now only evicted by the owner's own null read, since a non-owner's null is ambiguous under the new rule (denied vs. gone) and previously forced a wasted re-resolve query on every forbidden GET. Also folds in two wire-contract changes that shipped in the same dependency range and are needed to get the suite green: soft-delete, associate, dissociate, and setPermissions now answer 200 with the updated record instead of 204 (hard-delete, which produces no record, is unchanged); and a non-author PATCH now stamps updatedBy/updatedVia on the record, which ScopedStack already does from the session with no route change needed. The two new change-feed discovery fixtures are parked in conformance tests' SKIPPED set pending #82/#83. --- package.json | 10 ++-- pnpm-lock.yaml | 78 +++++++++++++++---------------- src/middleware/errors.ts | 9 ++++ src/routes/entity.ts | 14 +++++- src/routes/records.ts | 36 +++++++------- tests/conformance.test.ts | 71 ++++++++++++++++++++++++++-- tests/routes/associations.test.ts | 20 ++++---- tests/routes/entity.test.ts | 26 ++++++++++- tests/routes/records.test.ts | 67 +++++++++++++++++--------- 9 files changed, 233 insertions(+), 98 deletions(-) diff --git a/package.json b/package.json index f9ab2ca..e1318c9 100644 --- a/package.json +++ b/package.json @@ -17,10 +17,10 @@ "format:check": "prettier --check ." }, "dependencies": { - "@haverstack/adapter-local": "^0.10.0", - "@haverstack/commons": "^0.3.0", - "@haverstack/core": "^0.11.1", - "@haverstack/wire-types": "^0.9.0", + "@haverstack/adapter-local": "^0.12.0", + "@haverstack/commons": "^0.5.0", + "@haverstack/core": "^0.13.1", + "@haverstack/wire-types": "^0.12.0", "@hono/node-server": "^2.1.1", "hono": "^4.13.3", "pino": "^10.3.1", @@ -28,7 +28,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", - "@haverstack/conformance-fixtures": "^0.3.0", + "@haverstack/conformance-fixtures": "^0.6.0", "@types/node": "^26.2.0", "eslint": "^10.8.1", "eslint-config-prettier": "^10.1.8", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 32865e7..56985e0 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -9,17 +9,17 @@ importers: .: dependencies: '@haverstack/adapter-local': - specifier: ^0.10.0 - version: 0.10.0 + specifier: ^0.12.0 + version: 0.12.0 '@haverstack/commons': - specifier: ^0.3.0 - version: 0.3.0 + specifier: ^0.5.0 + version: 0.5.0 '@haverstack/core': - specifier: ^0.11.1 - version: 0.11.1 + specifier: ^0.13.1 + version: 0.13.1 '@haverstack/wire-types': - specifier: ^0.9.0 - version: 0.9.0 + specifier: ^0.12.0 + version: 0.12.0 '@hono/node-server': specifier: ^2.1.1 version: 2.1.1(hono@4.13.3) @@ -37,8 +37,8 @@ importers: specifier: ^10.0.1 version: 10.0.1(eslint@10.8.1) '@haverstack/conformance-fixtures': - specifier: ^0.3.0 - version: 0.3.0 + specifier: ^0.6.0 + version: 0.6.0 '@types/node': specifier: ^26.2.0 version: 26.2.0 @@ -264,28 +264,28 @@ packages: resolution: {integrity: sha512-+CNAzxglkrpNf/kKywqQfk74QjtceuOE7Qm+AF8miRvPF/wmmK5+OJOgVh3AVTT3RP2mH3+FOaxlE5v72owk0A==} engines: {node: ^20.19.0 || ^22.13.0 || >=24} - '@haverstack/adapter-local@0.10.0': - resolution: {integrity: sha512-h3dH0l2z9SXVCJ8MKaIYU1tTS37ONioAYWZpPB5861P7BVlp71/6v2P2svooorToZyeNaqM9tgyf7YzMeS62cQ==} + '@haverstack/adapter-local@0.12.0': + resolution: {integrity: sha512-9DSURgL5TZIQvv2+otq7UsxM/WVPkwDrlEZNQCYwtl/YtFoIuCILRPTEH4bRb0FXZaRsGYcI9i7UUzPmVIrkyg==} engines: {node: '>=22.5.0'} - '@haverstack/blob-adapter-disk@0.9.0': - resolution: {integrity: sha512-/TLyuqzG6yfuAfSRGoMq0FxTryAE2DzYKId6pc4eqggYTeru6huHxHjmOwterVy2mt/CInpBmhF5W3j2pBMEdw==} + '@haverstack/blob-adapter-disk@0.11.0': + resolution: {integrity: sha512-diO90GNQdwMza71caT0YNczOV5BuYYE66eMaSKylErlOout2a5ZnDHpuCPntF9rPCngzBuunROhX10WwrETqhQ==} - '@haverstack/commons@0.3.0': - resolution: {integrity: sha512-tqjbJJw9DvfgSxMWANrPXTLj22Skqq1SU+P8vFVJgtWqJvZLSJjpoPamyd7qacDnj4eYtv3ejpEMDJFv2dI3xg==} + '@haverstack/commons@0.5.0': + resolution: {integrity: sha512-lk6e5nz+fjyklKv81snC/H984hGmwZPXz7DGLbSiKtZ1PJDl93wgVGlhHtXN355ZuhPQRn0SsFEq0h6HG11Hug==} - '@haverstack/conformance-fixtures@0.3.0': - resolution: {integrity: sha512-CVuLTol0+gaN2xJRvcErbyHWvUXO4PR3+ViGIxQVu6tHthMWMv+XSiVcksdn10Fbir+fFmjYqMLxeCcr4c5fqQ==} + '@haverstack/conformance-fixtures@0.6.0': + resolution: {integrity: sha512-CLUuxrbIcHrR0dVKsqlW5w+ISUQdVFNyC2NSAbRDa1nWE66AXRmBKjgqnWseS2XHGqceDoPwURLVbrZgwlsOrw==} - '@haverstack/core@0.11.1': - resolution: {integrity: sha512-kNF8m8c/gQvjGqFa6WNNfOZzEdw+FRaya/olIK9u+cGG9Pzp62ynvHX+pCwYB5Q4Sg05ohmkVvidw/IKanssVQ==} + '@haverstack/core@0.13.1': + resolution: {integrity: sha512-ggP9RPiZ7uhRaZf0iM6HXBZHbjHjJ2zlgtsc1OSnlfWtDPgDx23EBs4fFltXLedf4n2be7A+yid2McwABAMPfw==} - '@haverstack/record-adapter-sqlite@0.3.0': - resolution: {integrity: sha512-PCIxC8qy/rkKMsq815KGhR0APy8NBu3TCFgXugpPmQOiucUjr6FWLSnvYjhIbWGsuTAYUCMyvxevRN+SsUIZ3w==} + '@haverstack/record-adapter-sqlite@0.5.1': + resolution: {integrity: sha512-XG2Sz4yYzmL57vVAHNEixjUiez9TF/jWFLfxPGPb0CaLESNHwUr066OhUIuVdAwRuweSjuVgRsionXFTNsEtig==} engines: {node: '>=22.5.0'} - '@haverstack/wire-types@0.9.0': - resolution: {integrity: sha512-UwPJxzN1mhi3Zuftby5kECc1CmlnXs4ela+VwqK47OXizoWnX2We5kCc7m3sqti/qFEc2UhKGtKZjQqtalWN3w==} + '@haverstack/wire-types@0.12.0': + resolution: {integrity: sha512-MfQptvdv/xd1l9ZhCgB/TQTO0c+K8JvbErGaDFVxfN1LrAR0VvVbhIm80VCQhaWVRcmul/D1eGsYSw5LAQiZhQ==} '@hono/node-server@2.1.1': resolution: {integrity: sha512-ELuehkj5VCBdgEw9zs+ivkKwyzzUCSQuE96YmiPvn1ECBoZCczbFXJLeEGMTYjphP6gydh4pHMqEYPVMYUVgQg==} @@ -1261,33 +1261,33 @@ snapshots: '@eslint/core': 1.2.1 levn: 0.4.1 - '@haverstack/adapter-local@0.10.0': + '@haverstack/adapter-local@0.12.0': dependencies: - '@haverstack/blob-adapter-disk': 0.9.0 - '@haverstack/core': 0.11.1 - '@haverstack/record-adapter-sqlite': 0.3.0 + '@haverstack/blob-adapter-disk': 0.11.0 + '@haverstack/core': 0.13.1 + '@haverstack/record-adapter-sqlite': 0.5.1 - '@haverstack/blob-adapter-disk@0.9.0': + '@haverstack/blob-adapter-disk@0.11.0': dependencies: - '@haverstack/core': 0.11.1 + '@haverstack/core': 0.13.1 - '@haverstack/commons@0.3.0': + '@haverstack/commons@0.5.0': dependencies: - '@haverstack/core': 0.11.1 + '@haverstack/core': 0.13.1 - '@haverstack/conformance-fixtures@0.3.0': + '@haverstack/conformance-fixtures@0.6.0': dependencies: - '@haverstack/wire-types': 0.9.0 + '@haverstack/wire-types': 0.12.0 - '@haverstack/core@0.11.1': {} + '@haverstack/core@0.13.1': {} - '@haverstack/record-adapter-sqlite@0.3.0': + '@haverstack/record-adapter-sqlite@0.5.1': dependencies: - '@haverstack/core': 0.11.1 + '@haverstack/core': 0.13.1 - '@haverstack/wire-types@0.9.0': + '@haverstack/wire-types@0.12.0': dependencies: - '@haverstack/core': 0.11.1 + '@haverstack/core': 0.13.1 '@hono/node-server@2.1.1(hono@4.13.3)': dependencies: diff --git a/src/middleware/errors.ts b/src/middleware/errors.ts index 947d0f0..b3b4bc0 100644 --- a/src/middleware/errors.ts +++ b/src/middleware/errors.ts @@ -33,6 +33,15 @@ export function errorMiddleware(logger: Logger): ErrorHandler { 'Denied a verified requester', ); } + // An anonymous requester gets the same 404 for a private record as for + // a missing one (docs/spec/wire-format.md § Server implementation + // checklist): a bare 403 or 401 here would confirm the record exists + // to a caller who presented no credential at all. `WWW-Authenticate` + // keeps the login prompt reachable without reopening that + // distinction — RFC 7235's standard scheme for a bearer-token API is + // `Bearer` (RFC 6750 §3), not the higher-level `did-challenge` + // exchange discovery advertises for obtaining that token. + if (wire.status === 404 && !auth) c.header('WWW-Authenticate', 'Bearer'); return c.json(wire.body, wire.status as ContentfulStatusCode); } logger.error({ err, requestId: c.get('requestId') }, 'Unhandled request error'); diff --git a/src/routes/entity.ts b/src/routes/entity.ts index b324526..889ee8b 100644 --- a/src/routes/entity.ts +++ b/src/routes/entity.ts @@ -47,7 +47,16 @@ export function entityRoutes(ctx: StackContext): Hono { const id = await resolveOwnerRecordId(); const record = id ? await stack.forSession(auth).get(id) : null; if (!record) { - cachedOwnerRecordId = null; + // get() now returns null both for "doesn't exist" and "exists but + // this caller can't read it" (the anti-oracle rule — see #79), so a + // non-owner's null says nothing about whether the card is actually + // gone. Only the owner's own null is a reliable deletion signal; + // evicting the cache on anyone else's denial would force a + // re-resolve query on every subsequent forbidden GET for a card that + // never moved. + const ownerActingAlone = + auth.principalId === ownerEntityId && auth.subjectId === ownerEntityId; + if (ownerActingAlone) cachedOwnerRecordId = null; throw new StackNotFoundError('Entity record not found'); } return c.json(serializeRecord(record)); @@ -64,6 +73,9 @@ export function entityRoutes(ctx: StackContext): Hono { .forSession(auth) .update(id, (body.content ?? {}) as Record); } catch (err) { + // requireOwner() above already restricts this handler to the owner + // acting alone, so unlike GET's, a StackNotFoundError here can only + // mean the card is genuinely gone — never a permission denial. if (err instanceof StackNotFoundError) cachedOwnerRecordId = null; throw err; } diff --git a/src/routes/records.ts b/src/routes/records.ts index 7a6d3c5..4731630 100644 --- a/src/routes/records.ts +++ b/src/routes/records.ts @@ -144,16 +144,19 @@ export function recordRoutes(ctx: StackContext, queryTimeoutMs: number): Hono { const id = c.req.param('id'); const auth = c.get('auth')!; const hard = new URL(c.req.url).searchParams.get('hard') === 'true'; + const session = stack.forSession(auth); - await stack - .forSession(auth) - .delete(id, { hard, ifVersion: parseIfMatch(c.req.header('If-Match')) }); - return c.body(null, 204); + await session.delete(id, { hard, ifVersion: parseIfMatch(c.req.header('If-Match')) }); + if (hard) return c.body(null, 204); + return c.json(serializeRecord((await session.get(id))!)); }); // POST /records/:id/undelete — reverses a soft delete; idempotent @@ -183,10 +186,11 @@ export function recordRoutes(ctx: StackContext, queryTimeoutMs: number): Hono(c); if (!Array.isArray(body.permissions)) throw new StackQueryError('permissions must be an array'); - await stack - .forSession(auth) - .setPermissions(id, body.permissions, { ifVersion: parseIfMatch(c.req.header('If-Match')) }); - return c.body(null, 204); + const session = stack.forSession(auth); + await session.setPermissions(id, body.permissions, { + ifVersion: parseIfMatch(c.req.header('If-Match')), + }); + return c.json(serializeRecord((await session.get(id))!)); }); // ------------------------------------------------------------------ @@ -211,10 +215,9 @@ export function recordRoutes(ctx: StackContext, queryTimeoutMs: number): Hono(c); if (!body.kind || !body.label) throw new StackQueryError('kind and label are required'); - await stack - .forSession(auth) - .associate(id, body, { ifVersion: parseIfMatch(c.req.header('If-Match')) }); - return c.body(null, 204); + const session = stack.forSession(auth); + await session.associate(id, body, { ifVersion: parseIfMatch(c.req.header('If-Match')) }); + return c.json(serializeRecord((await session.get(id))!)); }); // POST, not DELETE — a DELETE request body has no defined semantics @@ -224,10 +227,9 @@ export function recordRoutes(ctx: StackContext, queryTimeoutMs: number): Hono(c); - await stack - .forSession(auth) - .dissociate(id, body, { ifVersion: parseIfMatch(c.req.header('If-Match')) }); - return c.body(null, 204); + const session = stack.forSession(auth); + await session.dissociate(id, body, { ifVersion: parseIfMatch(c.req.header('If-Match')) }); + return c.json(serializeRecord((await session.get(id))!)); }); // ------------------------------------------------------------------ diff --git a/tests/conformance.test.ts b/tests/conformance.test.ts index a9e1581..c1de9e8 100644 --- a/tests/conformance.test.ts +++ b/tests/conformance.test.ts @@ -160,7 +160,15 @@ describe('discovery fixtures', () => { assertCoverage( discoveryFixtures.map((f) => f.name), handled, - new Set(), + new Set([ + // This server has no change feed yet — no src/routes/changes.ts, no + // `changes` key in discovery. Advertising one before GET /changes + // exists would be worse than not advertising it at all (clients call + // supportsChangeFeed() and fail locally when absent). Land with #82 + // (GET /changes) and #83 (discovery advertisement) — see #78. + 'discovery-advertises-a-change-feed', + 'discovery-advertises-a-feed-that-neither-resumes-nor-includes-records', + ]), ); }); }); @@ -371,6 +379,31 @@ describe('patchContent fixtures', () => { expect(d.version).toBe(fixture.responseBody!.version); }); + test('patch-record-restamps-the-actor', async () => { + const fixture = patchContentFixtures.find((f) => f.name === 'patch-record-restamps-the-actor')!; + handled.add(fixture.name); + const record = await t.ctx.stack.create( + NOTE_TYPE, + { title: 'original' }, + { + entityId: TEST_ENTITY_ID, + permissions: [{ access: 'entity', entityId: CONTRIBUTOR_ID, read: true, write: true }], + }, + ); + const { token } = await t.ctx.adapter.createToken(CONTRIBUTOR_ID); + const { status, data } = await req(t.app, 'PATCH', `/records/${record.id}`, { + token, + body: fixture.requestBody, + }); + expect(status).toBe(fixture.responseStatus); + const d = data as Record; + expect(d.content).toEqual(fixture.responseBody!.content); + // Authorship (entityId) is untouched by a non-author write; updatedBy + // moves to the requester who made this edit. + expect(d.entityId).toBe(TEST_ENTITY_ID); + expect(d.updatedBy).toBe(CONTRIBUTOR_ID); + }); + test('coverage', () => { assertCoverage( patchContentFixtures.map((f) => f.name), @@ -549,8 +582,11 @@ describe('setPermissions fixtures', () => { body: fixture.requestBody, }); expect(status).toBe(fixture.responseStatus); - const anon = await req(t.app, 'GET', `/records/${record.id}`); - expect(anon.status).toBe(403); + // Anonymous can't tell "made private" from "never existed" (#79's + // anti-oracle rule) — 404 + WWW-Authenticate, not 403. + const anon = await t.app.request(`/records/${record.id}`); + expect(anon.status).toBe(404); + expect(anon.headers.get('WWW-Authenticate')).toBe('Bearer'); }); test('coverage', () => { @@ -763,14 +799,41 @@ describe('error response fixtures', () => { } } - test('error-permission-denied — write without a grant', async () => { + test('error-permission-denied — can read, no write grant', async () => { const fixture = find('error-permission-denied'); + // Readability is what earns the 403 (see error-not-found-record-the- + // requester-cannot-read below) — a write-only-denied requester still + // needs an explicit read grant, or this would hit the anti-oracle 404 + // instead of the permission check this fixture pins. + const record = await t.ctx.stack.create( + NOTE_TYPE, + { title: 'x' }, + { permissions: [{ access: 'entity', entityId: CONTRIBUTOR_ID, read: true, write: false }] }, + ); + const { token } = await t.ctx.adapter.createToken(CONTRIBUTOR_ID); + const { status, data } = await dispatch(fixture, token, `/records/${record.id}`); + expectError(status, data, fixture); + }); + + test('error-not-found-record-the-requester-cannot-read — the anti-oracle rule', async () => { + const fixture = find('error-not-found-record-the-requester-cannot-read'); const record = await t.ctx.stack.create(NOTE_TYPE, { title: 'x' }); const { token } = await t.ctx.adapter.createToken(CONTRIBUTOR_ID); const { status, data } = await dispatch(fixture, token, `/records/${record.id}`); expectError(status, data, fixture); }); + test('error-validation-permission-write-without-read', async () => { + const fixture = find('error-validation-permission-write-without-read'); + const record = await t.ctx.stack.create(NOTE_TYPE, { title: 'x' }); + const { status, data } = await dispatch( + fixture, + TEST_TOKEN, + `/records/${record.id}/permissions`, + ); + expectError(status, data, fixture); + }); + test('error-permission-denied-versions-read-only — can read, cannot write', async () => { const fixture = find('error-permission-denied-versions-read-only'); const record = await t.ctx.stack.create( diff --git a/tests/routes/associations.test.ts b/tests/routes/associations.test.ts index 724e956..92969d6 100644 --- a/tests/routes/associations.test.ts +++ b/tests/routes/associations.test.ts @@ -19,13 +19,16 @@ describe('Associations', () => { return t.ctx.stack.create(TYPE_ID, { text: 'Hello' }); } - it('POST adds a tag association', async () => { + it('POST adds a tag association and answers with the updated record', async () => { const record = await seedRecord(); - const { status } = await req(t.app, 'POST', `/records/${record.id}/associations`, { + const { status, data } = await req(t.app, 'POST', `/records/${record.id}/associations`, { token: TEST_TOKEN, body: { kind: 'tag', label: 'starred' }, }); - expect(status).toBe(204); + expect(status).toBe(200); + expect((data as Record).associations).toEqual([ + { kind: 'tag', label: 'starred' }, + ]); }); it('GET returns all associations', async () => { @@ -53,14 +56,15 @@ describe('Associations', () => { expect(assocs.every((a) => a.kind === 'tag')).toBe(true); }); - it('POST .../associations/delete removes an association', async () => { + it('POST .../associations/delete removes an association and answers with the updated record', async () => { const record = await seedRecord(); await t.ctx.adapter.associate(record.id, { kind: 'tag', label: 'starred' }); - const { status } = await req(t.app, 'POST', `/records/${record.id}/associations/delete`, { + const { status, data } = await req(t.app, 'POST', `/records/${record.id}/associations/delete`, { token: TEST_TOKEN, body: { kind: 'tag', label: 'starred' }, }); - expect(status).toBe(204); + expect(status).toBe(200); + expect((data as Record).associations).toBeUndefined(); const after = await t.ctx.adapter.getRecord(record.id); expect(after?.associations?.some((a) => a.label === 'starred')).toBeFalsy(); }); @@ -73,7 +77,7 @@ describe('Associations', () => { body: { kind: 'tag', label: 'starred' }, headers: { 'If-Match': `"${record.version}"` }, }); - expect(status).toBe(204); + expect(status).toBe(200); }); it('POST /associations returns 412 version_conflict on an If-Match mismatch', async () => { @@ -96,7 +100,7 @@ describe('Associations', () => { body: { kind: 'tag', label: 'starred' }, headers: { 'If-Match': `"${current!.version}"` }, }); - expect(status).toBe(204); + expect(status).toBe(200); }); it('POST /associations/delete returns 412 version_conflict on an If-Match mismatch', async () => { diff --git a/tests/routes/entity.test.ts b/tests/routes/entity.test.ts index 63483d3..c514485 100644 --- a/tests/routes/entity.test.ts +++ b/tests/routes/entity.test.ts @@ -61,11 +61,33 @@ describe('GET /entity', () => { expect(content.name).toBe('Test Entity'); }); - it('returns 403 for a non-owner authenticated entity', async () => { + it('returns 404, not 403, for a non-owner authenticated entity (anti-oracle rule)', async () => { await seedEntityRecord(t.ctx); const { token } = await t.ctx.adapter.createToken(OTHER_ENTITY_ID); const { status } = await req(t.app, 'GET', '/entity', { token }); - expect(status).toBe(403); + expect(status).toBe(404); + }); + + it("a non-owner's denial does not evict the cached owner record id", async () => { + await seedEntityRecord(t.ctx); + const { token } = await t.ctx.adapter.createToken(OTHER_ENTITY_ID); + const querySpy = vi.spyOn(t.ctx.stack, 'query'); + + // Populate the cache, then have a non-owner get denied against it. The + // denied request may itself run a query internally to evaluate the + // permission grant — that's unrelated to the id cache this test is + // pinning down, so only the *delta* across it matters below. + await req(t.app, 'GET', '/entity', { token: TEST_TOKEN }); + const { status: deniedStatus } = await req(t.app, 'GET', '/entity', { token }); + expect(deniedStatus).toBe(404); + const countAfterDenial = querySpy.mock.calls.length; + + // The card was never deleted, so a well-behaved cache doesn't re-resolve + // it on the next request — a non-owner's null read() is ambiguous + // (denied vs. gone) and must not be trusted to evict a valid entry. A + // stale eviction would show up here as an extra query call. + await req(t.app, 'GET', '/entity', { token: TEST_TOKEN }); + expect(querySpy.mock.calls.length).toBe(countAfterDenial); }); it('returns 401 for an unauthenticated request', async () => { diff --git a/tests/routes/records.test.ts b/tests/routes/records.test.ts index b88f4a0..13603c1 100644 --- a/tests/routes/records.test.ts +++ b/tests/routes/records.test.ts @@ -107,10 +107,11 @@ describe('Records', () => { expect(status).toBe(404); }); - it('anonymous gets 403 for a private record', async () => { + it('anonymous gets 404 + WWW-Authenticate for a private record, not 403', async () => { const record = await seedRecord(t.ctx); - const { status } = await req(t.app, 'GET', `/records/${record.id}`); - expect(status).toBe(403); + const res = await t.app.request(`/records/${record.id}`); + expect(res.status).toBe(404); + expect(res.headers.get('WWW-Authenticate')).toBe('Bearer'); }); it('anonymous can read a public record', async () => { @@ -139,11 +140,14 @@ describe('Records', () => { expect(status).toBe(200); }); - it('entity without a grant gets 403', async () => { + it('entity without a grant gets 404, not 403, and no WWW-Authenticate (already authenticated)', async () => { const record = await seedRecord(t.ctx); const { token } = await t.ctx.adapter.createToken(OTHER_ENTITY_ID); - const { status } = await req(t.app, 'GET', `/records/${record.id}`, { token }); - expect(status).toBe(403); + const res = await t.app.request(`/records/${record.id}`, { + headers: { Authorization: `Bearer ${token}` }, + }); + expect(res.status).toBe(404); + expect(res.headers.get('WWW-Authenticate')).toBeNull(); }); }); @@ -325,10 +329,13 @@ describe('Records', () => { }); describe('DELETE /records/:id', () => { - it('soft-deletes by default', async () => { + it('soft-deletes by default, answering with the deleted record', async () => { const record = await seedRecord(t.ctx); - const { status } = await req(t.app, 'DELETE', `/records/${record.id}`, { token: TEST_TOKEN }); - expect(status).toBe(204); + const { status, data } = await req(t.app, 'DELETE', `/records/${record.id}`, { + token: TEST_TOKEN, + }); + expect(status).toBe(200); + expect((data as Record).deletedAt).toBeDefined(); const after = await t.ctx.adapter.getRecord(record.id); expect(after?.deletedAt).toBeDefined(); }); @@ -352,7 +359,7 @@ describe('Records', () => { ); const { token } = await t.ctx.adapter.createToken(OTHER_ENTITY_ID); const { status } = await req(t.app, 'DELETE', `/records/${record.id}`, { token }); - expect(status).toBe(204); + expect(status).toBe(200); const after = await t.ctx.adapter.getRecord(record.id); expect(after?.deletedAt).toBeDefined(); }); @@ -377,7 +384,7 @@ describe('Records', () => { token: TEST_TOKEN, headers: { 'If-Match': `"${record.version}"` }, }); - expect(status).toBe(204); + expect(status).toBe(200); }); it('returns 412 version_conflict on an If-Match mismatch, leaving the record untouched', async () => { @@ -499,10 +506,21 @@ describe('Records', () => { expect(status).toBe(403); }); - it('returns 403 when non-owner tries to soft-DELETE a grant record', async () => { - const [grantRecord] = await t.ctx.stack.grant(OTHER_ENTITY_ID, [ - { actions: ['read-own'], typeId: NOTE_TYPE_ID }, - ]); + it('returns 403 when non-owner tries to soft-DELETE a grant record, even with direct write permission on it', async () => { + // stack.grant() doesn't give the grantee read access to the grant + // record itself, only to what it grants — which would now 404 under + // the anti-oracle rule rather than exercise the write-protection fence + // this test targets. Set an explicit read+write permission directly on + // the grant record instead (same approach as the PATCH sibling above) + // so the requester can read it, and the refusal proven here is really + // "_grant records refuse deletion" rather than "can't read it". + const grantRecord = await t.ctx.stack.create( + GRANT_TYPE_ID, + { typeId: NOTE_TYPE_ID, actions: ['read-own'] }, + { + permissions: [{ access: 'entity', entityId: OTHER_ENTITY_ID, read: true, write: true }], + }, + ); const { token } = await t.ctx.adapter.createToken(OTHER_ENTITY_ID); const { status } = await req(t.app, 'DELETE', `/records/${grantRecord.id}`, { token }); @@ -532,7 +550,7 @@ describe('Records', () => { const { status } = await req(t.app, 'DELETE', `/records/${grantRecord.id}`, { token: TEST_TOKEN, }); - expect(status).toBe(204); + expect(status).toBe(200); const after = await t.ctx.adapter.getRecord(grantRecord.id); expect(after?.deletedAt).toBeDefined(); }); @@ -942,15 +960,16 @@ describe('Records', () => { }); describe('PUT/GET /records/:id/permissions', () => { - it('PUT replaces permissions and returns 204 with no body', async () => { + it('PUT replaces permissions and returns the updated record', async () => { const record = await seedRecord(t.ctx); const res = await t.app.request(`/records/${record.id}/permissions`, { method: 'PUT', headers: { Authorization: `Bearer ${TEST_TOKEN}`, 'Content-Type': 'application/json' }, body: JSON.stringify({ permissions: [{ access: 'public' }] }), }); - expect(res.status).toBe(204); - expect(await res.text()).toBe(''); + expect(res.status).toBe(200); + const body = (await res.json()) as { permissions: unknown[] }; + expect(body.permissions).toEqual([{ access: 'public' }]); const { token } = await t.ctx.adapter.createToken(OTHER_ENTITY_ID); const { status: anonStatus } = await req(t.app, 'GET', `/records/${record.id}`); @@ -969,8 +988,12 @@ describe('Records', () => { token: TEST_TOKEN, body: { permissions: [] }, }); - const { status } = await req(t.app, 'GET', `/records/${record.id}`); - expect(status).toBe(403); + // Anonymous can no longer tell "made private" from "never existed" — + // the anti-oracle rule (#79) — so this is 404 + WWW-Authenticate, not + // 403 or a bodyless 401. + const res = await t.app.request(`/records/${record.id}`); + expect(res.status).toBe(404); + expect(res.headers.get('WWW-Authenticate')).toBe('Bearer'); }); it('GET returns the current permissions', async () => { @@ -1009,7 +1032,7 @@ describe('Records', () => { body: { permissions: [{ access: 'public' }] }, headers: { 'If-Match': `"${record.version}"` }, }); - expect(status).toBe(204); + expect(status).toBe(200); }); it('PUT returns 412 version_conflict on an If-Match mismatch', async () => {