Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/daemon/studio-db-broker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@
*
* 1. **Nothing without a live grant.** No grant, an unknown token, a revoked one or an expired one is
* refused before a statement is prepared. Read grants cannot write.
* 2. **Refusals never leave residue.** Every refusal is decided BEFORE the storage is touched, and every
* write runs inside one transaction, so an op either completes or leaves the table byte-identical.
* 2. **Refusals never leave residue.** Every refusal is decided BEFORE the protected table is touched;
* the schema-skew decision reads only the migration ledger. Every write runs inside one transaction,
* so an op either completes or leaves the table byte-identical.
* 3. **Identifiers are never interpolated from the wire.** Table names come from the CLOSED contract set;
* column names are checked against the table's real columns read from the database itself. Values are
* always bound. A wire that names a column the table does not have is a malformed op, not a refusal —
Expand Down Expand Up @@ -328,8 +329,9 @@ function isRefusal(value: BrokerGrant | BrokerRefusal): value is BrokerRefusal {
/**
* Execute one op against the shared cache.
*
* Every decision that can refuse happens above the first prepared statement, and every write runs inside
* one transaction, so a refused op and a failed op both leave the table exactly as they found it.
* Every decision that can refuse protected-table access happens above the first statement against that
* table, and every write runs inside one transaction, so a refused op and a failed op both leave the
* table exactly as they found it.
* Reads answer rows; writes answer the rows they wrote — `insert` echoes what landed (so a caller learns
* the rowid it did not supply), `update` and `delete` answer the affected count as one row, because the
* count is the only thing SQLite will tell us without a second read the caller did not ask for.
Expand All @@ -356,6 +358,15 @@ export function executeBrokerOp(
const authorized = grants.authorize(op.grant, table, access);
if (isRefusal(authorized)) return authorized;

const liveSchemaHead = schemaHead(db);
if (liveSchemaHead > authorized.schemaHead) {
grants.revoke(authorized.token, 'schema_skew');
return { ok: false, reason: 'grant_revoked', table };
}
// Equality is the schema the grant accepted. A lower live head is not a forward shared-cache
// migration, so it remains subject to the broker's normal live-table checks rather than being
// mislabeled as schema_skew.

if (op.kind === 'read') {
if (!Number.isInteger(op.limit) || op.limit <= 0) {
throw new BrokerOpError('read.limit must be a positive integer');
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/studio-annotations-broker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
BrokerGrantStore,
BrokerOpError,
executeBrokerOp,
schemaHead,
} from '../../src/daemon/studio-db-broker.js';
import { BROKER_TABLES } from '../../src/companion-contract/index.js';
import type { BrokerOp, BrokerRefusal, BrokerRow } from '../../src/companion-contract/index.js';
Expand Down Expand Up @@ -63,7 +64,7 @@ describe('studio_annotations over the companion broker', () => {
return grants.issue({
mode,
tables: tables as Parameters<BrokerGrantStore['issue']>[0]['tables'],
schemaHead: 1,
schemaHead: schemaHead(db()),
}).token;
}

Expand Down
3 changes: 2 additions & 1 deletion tests/integration/studio-memories-broker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
BrokerGrantStore,
BrokerOpError,
executeBrokerOp,
schemaHead,
} from '../../src/daemon/studio-db-broker.js';
import { BROKER_TABLES } from '../../src/companion-contract/index.js';
import type { BrokerOp, BrokerRefusal, BrokerRow } from '../../src/companion-contract/index.js';
Expand Down Expand Up @@ -50,7 +51,7 @@ describe('studio_memories over the companion broker', () => {
return grants.issue({
mode,
tables: tables as Parameters<BrokerGrantStore['issue']>[0]['tables'],
schemaHead: 1,
schemaHead: schemaHead(db()),
}).token;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/integration/studio-sd10-tables-broker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest';

import { resetConfig } from '../../src/config.js';
import { closeDatabase, getDatabase, initDatabase } from '../../src/cache/db.js';
import { BrokerGrantStore, executeBrokerOp } from '../../src/daemon/studio-db-broker.js';
import { BrokerGrantStore, executeBrokerOp, schemaHead } from '../../src/daemon/studio-db-broker.js';
import { BROKER_TABLES } from '../../src/companion-contract/index.js';
import type { BrokerOp, BrokerRefusal, BrokerRow, BrokerTable } from '../../src/companion-contract/index.js';

Expand Down Expand Up @@ -81,7 +81,7 @@ describe('the SD10 tables over the companion broker', () => {
});

function token(tables: readonly BrokerTable[]): string {
return grants.issue({ mode: 'readwrite', tables, schemaHead: 1 }).token;
return grants.issue({ mode: 'readwrite', tables, schemaHead: schemaHead(getDatabase()) }).token;
}

function run(op: BrokerOp): ReturnType<typeof executeBrokerOp> {
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/studio-sd9-tables-broker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
BrokerGrantStore,
BrokerOpError,
executeBrokerOp,
schemaHead,
} from '../../src/daemon/studio-db-broker.js';
import { BROKER_TABLES } from '../../src/companion-contract/index.js';
import type { BrokerOp, BrokerRefusal, BrokerRow, BrokerTable } from '../../src/companion-contract/index.js';
Expand Down Expand Up @@ -99,7 +100,7 @@ describe('the SD9 tables over the companion broker', () => {
return grants.issue({
mode,
tables: tables as Parameters<BrokerGrantStore['issue']>[0]['tables'],
schemaHead: 1,
schemaHead: schemaHead(db()),
}).token;
}

Expand Down
3 changes: 2 additions & 1 deletion tests/integration/studio-site-profiles-broker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
BrokerGrantStore,
BrokerOpError,
executeBrokerOp,
schemaHead,
} from '../../src/daemon/studio-db-broker.js';
import { BROKER_TABLES } from '../../src/companion-contract/index.js';
import type { BrokerOp, BrokerRefusal, BrokerRow, BrokerTable } from '../../src/companion-contract/index.js';
Expand Down Expand Up @@ -93,7 +94,7 @@ describe('the studio_site_* tables over the companion broker', () => {
return grants.issue({
mode,
tables: tables as Parameters<BrokerGrantStore['issue']>[0]['tables'],
schemaHead: 1,
schemaHead: schemaHead(db()),
}).token;
}

Expand Down
72 changes: 64 additions & 8 deletions tests/unit/daemon/companion-broker.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import Database from 'better-sqlite3';

import { resetConfig } from '../../../src/config.js';
import { initDatabase, getDatabase, closeDatabase } from '../../../src/cache/db.js';
import { applyMigrations, MIGRATIONS } from '../../../src/cache/migrations/runner.js';
import {
BrokerGrantStore,
BrokerOpError,
Expand Down Expand Up @@ -112,6 +114,56 @@ describe('companion broker — grant-scoped table access', () => {
expect(schemaHead(db())).toBe(0);
});

it('revokes a grant when the shared database migrates past its issued head', () => {
const migratedDb = new Database(':memory:');
try {
migratedDb.exec(
'CREATE TABLE schema_migrations (name TEXT PRIMARY KEY, applied_at INTEGER NOT NULL)',
);
const nextMigration = [...MIGRATIONS].reverse().find((migration) => !migration.requiresVec);
if (!nextMigration) throw new Error('expected a non-vector migration');
const recordMigration = migratedDb.prepare(
'INSERT INTO schema_migrations (name, applied_at) VALUES (?, ?)',
);
for (const migration of MIGRATIONS) {
if (migration === nextMigration) break;
if (migration.requiresVec) continue;
migratedDb.transaction(() => {
migratedDb.exec(migration.sql);
migration.postStep?.(migratedDb);
recordMigration.run(migration.name, 1);
})();
}

const issuedHead = schemaHead(migratedDb);
const grant = grants.issue({ mode: 'read', tables: ['studio_runs'], schemaHead: issuedHead });

applyMigrations(migratedDb, { vecLoaded: false });
expect(schemaHead(migratedDb)).toBe(issuedHead + 1);

const op = {
grant: grant.token,
kind: 'read' as const,
table: 'studio_runs' as const,
limit: 10,
};
const first = executeBrokerOp(migratedDb, grants, op);

expect(refusalOf(first)).toEqual({ ok: false, reason: 'grant_revoked', table: 'studio_runs' });
expect(grants.revocationOf(grant.token)).toEqual({
token: grant.token,
revokedAt: clock,
reason: 'schema_skew',
});

const second = executeBrokerOp(migratedDb, grants, op);
expect(refusalOf(second)).toEqual({ ok: false, reason: 'grant_revoked', table: 'studio_runs' });
expect(grants.revocationOf(grant.token)?.reason).toBe('schema_skew');
} finally {
migratedDb.close();
}
});

it('revokes idempotently and keeps the FIRST reason', () => {
const grant = grants.issue({ mode: 'readwrite', tables: ['studio_runs'], schemaHead: 1 });

Expand Down Expand Up @@ -283,7 +335,7 @@ describe('companion broker — grant-scoped table access', () => {
});

it('refuses write_not_granted for a read grant and leaves the table untouched', () => {
const grant = grants.issue({ mode: 'read', tables: BROKER_TABLES, schemaHead: 1 });
const grant = grants.issue({ mode: 'read', tables: BROKER_TABLES, schemaHead: schemaHead(db()) });
const before = runRows();

const result = executeBrokerOp(db(), grants, {
Expand All @@ -300,7 +352,7 @@ describe('companion broker — grant-scoped table access', () => {
});

it('refuses row_limit_exceeded above the wire ceiling without running the read', () => {
const grant = grants.issue({ mode: 'read', tables: BROKER_TABLES, schemaHead: 1 });
const grant = grants.issue({ mode: 'read', tables: BROKER_TABLES, schemaHead: schemaHead(db()) });

const ok = executeBrokerOp(db(), grants, {
grant: grant.token,
Expand All @@ -326,7 +378,7 @@ describe('companion broker — grant-scoped table access', () => {
});

function grant() {
return grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 }).token;
return grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: schemaHead(db()) }).token;
}

it('round-trips insert → read → update → delete on the run projection tables', () => {
Expand Down Expand Up @@ -413,7 +465,7 @@ describe('companion broker — grant-scoped table access', () => {
});

it('rejects a column the table does not have as a malformed op, not a refusal', () => {
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 }).token;
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: schemaHead(db()) }).token;
const before = runRows();

expect(() =>
Expand All @@ -432,7 +484,7 @@ describe('companion broker — grant-scoped table access', () => {
});

it('binds a value that looks like SQL rather than interpolating it', () => {
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 }).token;
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: schemaHead(db()) }).token;

executeBrokerOp(db(), grants, {
grant: token,
Expand All @@ -446,7 +498,7 @@ describe('companion broker — grant-scoped table access', () => {
});

it('refuses an unfiltered update or delete — a whole-table mutation asked for by omission', () => {
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 }).token;
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: schemaHead(db()) }).token;

expect(() =>
executeBrokerOp(db(), grants, { grant: token, kind: 'update', table: 'studio_runs', row: { status: 'x' } }),
Expand All @@ -463,7 +515,7 @@ describe('companion broker — grant-scoped table access', () => {
describe('in-flight atomicity', () => {
it('leaves the table byte-identical when a write violates a constraint mid-op', () => {
seedRun();
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 }).token;
const token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: schemaHead(db()) }).token;
const before = runRows();

// Same primary key as the seeded run: SQLite aborts inside the transaction.
Expand All @@ -481,7 +533,11 @@ describe('companion broker — grant-scoped table access', () => {

it('does not roll back an op that already completed when a later one is refused', () => {
seedRun();
const grantRecord = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 });
const grantRecord = grants.issue({
mode: 'readwrite',
tables: BROKER_TABLES,
schemaHead: schemaHead(db()),
});

const first = executeBrokerOp(db(), grants, {
grant: grantRecord.token,
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/daemon/studio-db-broker-bounds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';

import { resetConfig } from '../../../src/config.js';
import { initDatabase, getDatabase, closeDatabase } from '../../../src/cache/db.js';
import { BrokerGrantStore, BrokerOpError, executeBrokerOp } from '../../../src/daemon/studio-db-broker.js';
import {
BrokerGrantStore,
BrokerOpError,
executeBrokerOp,
schemaHead,
} from '../../../src/daemon/studio-db-broker.js';
import { BROKER_TABLES, MAX_BROKER_ROWS } from '../../../src/companion-contract/index.js';

/**
Expand Down Expand Up @@ -40,7 +45,7 @@ describe('companion broker — the row bound', () => {
resetConfig();
initDatabase(':memory:');
grants = new BrokerGrantStore();
token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 }).token;
token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: schemaHead(db()) }).token;
});

afterEach(() => {
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/daemon/studio-db-broker-run-log.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';

import { resetConfig } from '../../../src/config.js';
import { initDatabase, getDatabase, closeDatabase } from '../../../src/cache/db.js';
import { BrokerGrantStore, executeBrokerOp } from '../../../src/daemon/studio-db-broker.js';
import { BrokerGrantStore, executeBrokerOp, schemaHead } from '../../../src/daemon/studio-db-broker.js';
import { BROKER_TABLES } from '../../../src/companion-contract/index.js';
import type { BrokerRow } from '../../../src/companion-contract/index.js';

Expand Down Expand Up @@ -56,7 +56,7 @@ describe('companion broker — the run-log surface is reachable as table ops', (
resetConfig();
initDatabase(':memory:');
grants = new BrokerGrantStore();
token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: 1 }).token;
token = grants.issue({ mode: 'readwrite', tables: BROKER_TABLES, schemaHead: schemaHead(db()) }).token;
op({
grant: token,
kind: 'insert',
Expand Down
Loading