diff --git a/.changeset/quiet-ledger-readback.md b/.changeset/quiet-ledger-readback.md new file mode 100644 index 000000000..e391d234e --- /dev/null +++ b/.changeset/quiet-ledger-readback.md @@ -0,0 +1,5 @@ +--- +'@opensaas/stack-core': patch +--- + +Security fix: a `create`/`update`/`delete` result now respects operation-level `query` access. A session that cannot query the written row gets only the list's system fields back (the write still persists); `sudo()` is unchanged. diff --git a/packages/core/src/context/write-pipeline.ts b/packages/core/src/context/write-pipeline.ts index b7dd6bd70..564e59d58 100644 --- a/packages/core/src/context/write-pipeline.ts +++ b/packages/core/src/context/write-pipeline.ts @@ -34,7 +34,7 @@ import { type WriteCollection, type WriteScope, } from '../secured/write.js' -import { visibleWrittenRow } from '../secured/read.js' +import { visibleWrittenRow, writtenRowQueryable } from '../secured/read.js' import { resolveWhere, type WherePlan } from '../secured/vocabulary.js' import { hookPipeline } from './hook-pipeline.js' import { lowerRelationInput, refuseNestedRelationInput } from './relationship-input.js' @@ -529,7 +529,8 @@ async function runWriteInTransaction( ) // ── Phase 11: Field Visibility (filter readable fields + resolveOutput) ───── - return visibleWrittenRow({ listName, listConfig, ormHandle: tx, context, config }, item) + const binding = { listName, listConfig, ormHandle: tx, context, config } + return visibleWrittenRow(binding, item, await writtenRowQueryable(binding, item.id)) } /** @@ -602,6 +603,9 @@ async function runDeletePath(args: { context, }) + const binding = { listName, listConfig, ormHandle, context, config } + const queryable = await writtenRowQueryable(binding, item.id) + // ── Phase 8: DB delete ────────────────────────────────────────────────────── const deleted = await strategy.persist(collection, ops, scope, {}) if (deleted === null) return null @@ -626,7 +630,7 @@ async function runDeletePath(args: { item, // original row before deletion ) - return visibleWrittenRow({ listName, listConfig, ormHandle, context, config }, deleted) + return visibleWrittenRow(binding, deleted, queryable) } // ── Per-operation strategies ────────────────────────────────────────────────── diff --git a/packages/core/src/context/write-result-query-access.test.ts b/packages/core/src/context/write-result-query-access.test.ts new file mode 100644 index 000000000..9803cac14 --- /dev/null +++ b/packages/core/src/context/write-result-query-access.test.ts @@ -0,0 +1,103 @@ +import { afterAll, beforeAll, describe, expect, test } from 'vitest' +import type { OpenSaasConfig } from '../config/types.js' +import { text } from '../fields/index.js' +import { createTestContext, type TestContext } from '../testing/context.js' + +const BOOT = 120_000 + +const config: OpenSaasConfig = { + db: { provider: 'postgresql' }, + lists: { + Ledger: { + fields: { note: text(), ownerId: text() }, + access: { + operation: { + query: () => false, + create: () => true, + update: () => true, + delete: () => true, + }, + }, + }, + Doc: { + fields: { note: text(), ownerId: text() }, + access: { + operation: { + query: ({ session }) => ({ ownerId: { equals: String(session?.userId) } }), + create: () => true, + update: () => true, + delete: () => true, + }, + }, + }, + }, +} + +describe('a write result respects operation-level query access', () => { + let harness: TestContext + + beforeAll(async () => { + harness = await createTestContext(config, { userId: 'u1' }) + }, BOOT) + + afterAll(async () => { + await harness?.close() + }) + + test( + 'create on a list the session cannot query returns only the id, and persists', + async () => { + const row = await harness.context.db.Ledger.create({ data: { note: 'n', ownerId: 'u1' } }) + expect(row).not.toBeNull() + expect(Object.keys(row ?? {})).toEqual(['id']) + const stored = await harness.context.sudo().db.Ledger.all() + expect(stored).toHaveLength(1) + }, + BOOT, + ) + + test( + 'filter rule: own row returns the full row, someone else’s returns system fields only', + async () => { + const own = await harness.context.db.Doc.create({ data: { note: 'a', ownerId: 'u1' } }) + expect(own).toMatchObject({ note: 'a', ownerId: 'u1' }) + const other = await harness.context.db.Doc.create({ data: { note: 'b', ownerId: 'u2' } }) + expect(Object.keys(other ?? {})).toEqual(['id']) + }, + BOOT, + ) + + test( + 'update that moves a row out of scope returns system fields only', + async () => { + const own = await harness.context.db.Doc.create({ data: { note: 'c', ownerId: 'u1' } }) + const moved = await harness.context.db.Doc.update({ + where: { id: String(own?.id) }, + data: { ownerId: 'u2' }, + }) + expect(Object.keys(moved ?? {})).toEqual(['id']) + }, + BOOT, + ) + + test( + 'delete of a row the session could not query returns system fields only', + async () => { + const row = await harness.context.sudo().db.Doc.create({ data: { note: 'd', ownerId: 'u2' } }) + const deleted = await harness.context.db.Doc.delete({ where: { id: String(row?.id) } }) + expect(Object.keys(deleted ?? {})).toEqual(['id']) + }, + BOOT, + ) + + test( + 'sudo returns the full row', + async () => { + const row = await harness.context + .sudo() + .db.Ledger.create({ data: { note: 'z', ownerId: 'u9' } }) + expect(row).toMatchObject({ note: 'z' }) + }, + BOOT, + ) +}) diff --git a/packages/core/src/secured/read.ts b/packages/core/src/secured/read.ts index 25ff56291..9eccb59eb 100644 --- a/packages/core/src/secured/read.ts +++ b/packages/core/src/secured/read.ts @@ -58,10 +58,11 @@ import { selectionScope, type ProjectionPlan, } from './select.js' -import type { - DependencyAdditions, - FieldSelectionScope, - ReducedDeclaredKeys, +import { + getListDependencies, + type DependencyAdditions, + type FieldSelectionScope, + type ReducedDeclaredKeys, } from '../access/declared-dependencies.js' import { aggregations, checkSpec, specKeys, zeroed, type AggregateBuild } from './aggregate.js' import { distanceToScore, requireVector, vectorDistance } from './vector.js' @@ -1265,16 +1266,57 @@ async function visibleRows( return results as VisibleRow[] } +/** + * Whether the session's operation-level `query` access reaches the row with + * this id, resolved as a read would: a filter rule is run as a scoped read of + * the row's identity on the binding's own handle. + */ +export async function writtenRowQueryable( + binding: Omit, + id: unknown, +): Promise { + if (binding.context._isSudo === true) return true + if (typeof id !== 'string' && typeof id !== 'number') return false + const state: QueryState = { + predicates: [{ id: { equals: id } }], + orders: [], + includes: [], + fields: ['id'], + distincts: [], + lock: false, + } + const plan = await resolvePlan(binding, state) + if (plan === null) return false + if (plan.predicates.length === 1) return true + const collection = scope( + binding, + plan, + await whereCombinators(), + FIRST_DISPOSITIONS, + unreachableRefusal, + ) + return (await withOrigin('engine', () => collection.first())) !== null +} + /** * What a write hands back: the row Field Visibility leaves, then the * foreign-key pass a read of the same row would give it, so `create()` and - * `update()` never return an id `first()` hides. + * `update()` never return an id `first()` hides. A row the session cannot + * `query` comes back as the list's system fields alone. */ export async function visibleWrittenRow( binding: Omit, row: OrmRow, + queryable: boolean, ): Promise { const { listConfig, context, config, listName } = binding + if (!queryable) { + const kept: OrmRow = {} + for (const key of getListDependencies(config, listName).systemFields) { + if (key in row) kept[key] = row[key] + } + return kept + } const filtered = await filterReadableFields( row, listConfig.fields,