Skip to content

Commit 3780507

Browse files
committed
fix(metadata)!: one protocol seam keeps legacy organization rows out of every family read (#23429)
S2 re-review 6181366782, blocking fix 2. On a v17-shape database (which `os serve` refuses but the ceremony's own boot reads), these still served, locked, addressed or echoed a legacy organization's row as the environment's: - protocol: every sys_metadata / _audit / _commit / _history read now goes through findEnvironmentRows / findEnvironmentRow (the physical column via readLegacyOrganizationRows, joining context.transaction). That covers findServedOverlayRow (getMetaItem, getMetaItemLayered), overlayLockLayerAt (the item lock, lockWriteRefusal, assertLockAllowsDelete, authoredLockFamily), packagelessAddress (save, delete, readVersionToken, packagelessReadAddress) and every other find the record lists. Two keep legacy rows on purpose and read the column themselves: migrateStoredMetadata and revertCommit's commit read. Stale `organization_id null` comments fixed. - plugin-security security-catalog-echo and service-automation flow-credential-migration: the dead `row.organization_id` discriminators read the physical column. - metadata DatabaseLoader: its family reads leave legacy rows out, engine- or driver-backed. - Real-engine pins (cli, v17-shape SQLite, column added raw): a package-bound organization row never becomes the package-less address; an organization row carrying `_lock: full` is neither the item's lock (lock.verdict) nor the gate's refusal; the kernel:ready echo registers the environment's permission overlay and never the organization's. Ablation: overlayLockLayerAt back on the raw read turns the lock pin red; restored. - Non-blocking: es-ES/ja-JP/zh-CN environment_id help re-translated; TursoDriver's introspectSchema JSDoc re-attached and the remote catalog read's errors go through rawStatementFault; the changeset names the ApiError.code widening and service-automation; the repository's history reads and the tenant-chokepoint gate's not-covered note are documented. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WXLB6sUdwQZQ9NQS3mj3uZ
1 parent a62b26f commit 3780507

14 files changed

Lines changed: 330 additions & 108 deletions

File tree

‎.changeset/23429-sys-metadata-family-organization-column-retired.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
'@objectstack/objectql': minor
88
'@objectstack/plugin-security': minor
99
'@objectstack/service-datasource': minor
10+
'@objectstack/service-automation': minor
1011
'@objectstack/platform-objects': minor
1112
'@objectstack/driver-sql': minor
1213
'@objectstack/driver-turso': minor
@@ -28,6 +29,7 @@ FROM → TO:
2829
- An authored filter, list-view column, report grouping, formula or seed key naming `organization_id` on one of the four tables → delete it; no organization owns a row. The field resolver answers it as an unknown field.
2930
- `new SysMetadataRepository({ engine, organizationId, orgLabel })` → `new SysMetadataRepository({ engine, orgLabel })`. The `organizationId` option is gone: every repository reads and writes the environment's rows.
3031
- `DatabaseLoader`'s `organizationId` option (and so `MetadataManager.setDatabaseDriver` / `setDataEngine`'s `organizationId` argument) → accepted and ignored, like `environmentId`: no write stamps it and no read filters on it. Drop the argument.
32+
- `@objectstack/spec`: the published `ApiError.code` / `ErrorCode` enum widens by `LEGACY_ORGANIZATION_ROWS_UNREADABLE`, registered under `@objectstack/metadata-core`.
3133
- `isLegacyOrganizationRow(row)` (`@objectstack/metadata-core`) → `readLegacyOrganizationRows(driver, table?)`. The row-value test read a key the engine no longer returns, so it is removed. The new read answers from the physical column, through the table's driver (`engine.getDriverForObject`): id → organization, empty on a migrated or fresh database. A seam that cannot answer is refused (`LegacyOrganizationRowsUnreadable`), never read as "none".
3234
- An existing database → run `os migrate organization-ownership` (review its plan), then `os migrate organization-ownership --apply --plan-file FILE` with that plan file. The ceremony promotes or reports the legacy organization rows, then drops the physical column. Until it has run, the v18 runtime refuses to start against the database.
3335

@@ -47,12 +49,12 @@ What else changes:
4749
- Every `/meta` door serving these rows admits `studio.access`: `/meta/:type/:name/history` and `/audit` admit `studio.access`, `setup.access` or `manage_metadata`; `/packages/:id/commits` admits `studio.access` or `setup.access`. `requiredPermissions` is an AND-gate and cannot name an any-of set, and `setup.access` is held by organization admins, so it could not be the gate. A caller holding only `setup.access` or only `manage_metadata` therefore still reads these rows through `/meta`, but not through the data door.
4850
- `sys_metadata` stays ungated, before and after: its rows are what `/meta` already serves to every organization.
4951
- On a database the ceremony has not migrated (which `os serve` refuses, but every `os migrate` boot opens, the ceremony's own among them), the legacy organization rows are still told apart, off the physical column:
50-
- boot hydration, the `/meta` list, by-name and draft-preview reads and the authored hook and action binders leave them out;
52+
- every read the metadata protocol makes of the family goes through one seam that leaves them out. That covers boot hydration; the `/meta` list, by-name, layered and draft-preview reads; the item lock (`_lock`) the reads report and the save and delete gates enforce; the package-less write and read address; and the package, commit, audit, history and diff reads. Two reads keep them on purpose and read the column themselves: `os migrate meta --stored` reports such a row, and the commit revert refuses a legacy commit by name. The authored hook and action binders leave them out too;
5153
- the metadata repository's write never resolves to one: before, a save of an address that only a legacy organization's row held rewrote that row in place (measured on the ceremony's template promotion). Now the write targets the environment's row, and with none it creates one, which the re-keyed unique index refuses beside the organization's row until the ceremony settles it;
5254
- `os migrate meta --stored` reports such a row `skipped` and never re-saves it;
5355
- `os migrate security-catalog-overlays` neither lists nor deletes it;
5456
- a commit recorded in a legacy organization's layer is still refused by the revert;
55-
- the runtime-datasource reads and the packaged permission-set overlay reading leave them out.
57+
- the runtime-datasource reads, the packaged permission-set overlay reading, the kernel:ready security-catalog echo, the stored flow-credential move and a `DatabaseLoader` (engine- or driver-backed) leave them out.
5658
- `SqlDriver.listPhysicalColumns(object, options?)` (new, read-only): a table's physical column names, from the catalog, joining `options.transaction`. `TursoDriver`'s remote face answers it from the remote catalog. Every boot asks whether the family still carries the retired column, and it asks this way, not through `execute()`. So `os migrate plan` keeps its "a plan writes nothing" line on a migrated or fresh database. The legacy rows themselves are read with a raw statement only where the column exists.
5759
- `GET /forms/:slug` and `POST /forms/:slug/submit` no longer read the Default Organization's legacy layer. Every metadata read is now environment → code. The doors judge each candidate against the environment's view list, which holds one item per package of a view name. So a withdrawal in one package still closes that name's form in every package. On a deployment with no organization this closes more than before, which is the fail-closed direction. A withdrawal saved only at the organization layer is carried onto the environment by the ceremony's `metadata-promotion` step, which the runtime requires before it starts.
5860
- The protocol's `getMetaItems` drops its undocumented `legacyFormOrganizationId` request key. It was never in the spec's `GetMetaItemsRequest`, and only those doors passed it. A caller that still passes it now gets the environment's list.

‎packages/cli/src/commands/migrate/meta-stored-legacy-organization.integration.test.ts‎

Lines changed: 98 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,27 @@ const legacyFlow = (name: string) => ({
6565
const TEMPLATE = 'm23429_welcome';
6666
const template = (subject: string) => ({ name: TEMPLATE, label: 'Welcome', locale: 'en-US', subject, bodyHtml: `<p>${subject}</p>` });
6767

68-
interface SeedRow { id: string; type: string; name: string; organization_id: string | null; body: unknown }
68+
const PKG = 'com.example.m23429';
69+
const PKG_TEMPLATE = 'm23429_pkg_tpl';
70+
const LOCKED_TEMPLATE = 'm23429_locked_tpl';
71+
const tpl = (name: string, subject: string, extra: Record<string, unknown> = {}) =>
72+
({ name, label: name, locale: 'en-US', subject, bodyHtml: `<p>${subject}</p>`, ...extra });
73+
const permission = (name: string, label: string) => ({ name, label, objects: {} });
74+
75+
interface SeedRow { id: string; type: string; name: string; organization_id: string | null; package_id?: string | null; body: unknown }
6976
const SEED: SeedRow[] = [
7077
{ id: 'env_flow', type: 'flow', name: 'm23429_env_purge', organization_id: null, body: legacyFlow('m23429_env_purge') },
7178
{ id: 'org_flow', type: 'flow', name: 'm23429_org_purge', organization_id: 'org_a', body: legacyFlow('m23429_org_purge') },
7279
{ id: 'org_template', type: 'email_template', name: TEMPLATE, organization_id: 'org_a', body: template('Org A welcome') },
80+
// An organization row BOUND TO A PACKAGE: its binding must never become the
81+
// package-less write address (packagelessAddress) or the served row.
82+
{ id: 'org_pkg_template', type: 'email_template', name: PKG_TEMPLATE, organization_id: 'org_a', package_id: PKG, body: tpl(PKG_TEMPLATE, 'Org A packaged') },
83+
// An organization row CARRYING `_lock`: it must never lock the environment's item.
84+
{ id: 'org_locked_template', type: 'email_template', name: LOCKED_TEMPLATE, organization_id: 'org_a', body: tpl(LOCKED_TEMPLATE, 'Org A locked', { _lock: 'full' }) },
85+
// The kernel:ready security-catalog echo: an organization's permission
86+
// overlay, beside the environment's control.
87+
{ id: 'org_permission', type: 'permission', name: 'm23429_org_perm', organization_id: 'org_a', body: permission('m23429_org_perm', 'Org A') },
88+
{ id: 'env_permission', type: 'permission', name: 'm23429_env_perm', organization_id: null, body: permission('m23429_env_perm', 'Environment') },
7389
];
7490

7591
let dir: string;
@@ -153,7 +169,7 @@ beforeAll(async () => {
153169
const now = new Date().toISOString();
154170
for (const row of SEED) {
155171
await k('sys_metadata').insert({
156-
id: row.id, type: row.type, name: row.name, organization_id: row.organization_id, package_id: null,
172+
id: row.id, type: row.type, name: row.name, organization_id: row.organization_id, package_id: row.package_id ?? null,
157173
state: 'active', version: 1, checksum: `sha256:seed_${row.id}`, created_at: now, updated_at: now,
158174
metadata: JSON.stringify(row.body),
159175
});
@@ -206,8 +222,8 @@ describe('os migrate meta --stored on a v17-shape database — the legacy organi
206222
}, CASE_TIMEOUT_MS);
207223
});
208224

209-
/** The ceremony's `--apply` boot (`organization-ownership.ts`), and its `door.save`. */
210-
async function saveTemplateOnCeremonyBoot(db: string, subject: string): Promise<unknown> {
225+
/** The ceremony's `--apply` boot (`organization-ownership.ts`), with `fn` run against its kernel. */
226+
async function onCeremonyBoot<T>(db: string, fn: (kernel: { getService(name: string): any }) => Promise<T>): Promise<T> {
211227
const stack = await bootSchemaStack({
212228
jsonOutput: false,
213229
databaseUrl: `file:${db}`,
@@ -218,16 +234,21 @@ async function saveTemplateOnCeremonyBoot(db: string, subject: string): Promise<
218234
composeAuthGatedSecurity: true,
219235
});
220236
try {
221-
const protocol = (stack.kernel as { getService(name: string): any }).getService('protocol');
222-
await protocol.saveMetaItem({ type: 'email_template', name: TEMPLATE, item: template(subject), actor: 'os migrate organization-ownership' });
223-
return null;
224-
} catch (error) {
225-
return error;
237+
return await fn(stack.kernel as { getService(name: string): any });
226238
} finally {
227239
await stack.shutdown();
228240
}
229241
}
230242

243+
/** The ceremony's `door.save` (`protocol.saveMetaItem`, no `source`): `null`, or what it threw. */
244+
const doorSave = (kernel: { getService(name: string): any }, type: string, name: string, item: unknown) =>
245+
kernel.getService('protocol').saveMetaItem({ type, name, item, actor: 'os migrate organization-ownership' })
246+
.then(() => null, (error: unknown) => error);
247+
248+
async function saveTemplateOnCeremonyBoot(db: string, subject: string): Promise<unknown> {
249+
return onCeremonyBoot(db, (kernel) => doorSave(kernel, 'email_template', TEMPLATE, template(subject)));
250+
}
251+
231252
describe('the ceremony\'s template-promotion save on a v17-shape database', () => {
232253
it('a legacy organization template and no environment row: the save never rewrites the organization\'s row — it is refused, nothing written', async () => {
233254
const db = caseDb();
@@ -252,3 +273,71 @@ describe('the ceremony\'s template-promotion save on a v17-shape database', () =
252273
expect(rows[0]!.metadata).toMatchObject({ subject: 'Environment welcome' });
253274
}, CASE_TIMEOUT_MS);
254275
});
276+
277+
describe('[ADR-0131 D6/D7] the ceremony boot\'s readers ignore a legacy organization row (re-review 6181366782)', () => {
278+
it('door.save of a package-less item an organization holds BOUND TO A PACKAGE: the address is the environment\'s, never the organization row\'s package', async () => {
279+
const db = caseDb();
280+
const error = await onCeremonyBoot(db, (kernel) => doorSave(kernel, 'email_template', PKG_TEMPLATE, tpl(PKG_TEMPLATE, 'Environment')));
281+
expect(error, String((error as Error | null)?.message ?? error)).toBeNull();
282+
const rows = (await rawRows(db)).filter((r) => r.name === PKG_TEMPLATE);
283+
const org = rows.find((r) => r.id === 'org_pkg_template')!;
284+
expect(org).toMatchObject({ organization_id: 'org_a' });
285+
expect(org.metadata).toEqual(tpl(PKG_TEMPLATE, 'Org A packaged'));
286+
const env = rows.filter((r) => r.organization_id === null);
287+
expect(env).toHaveLength(1);
288+
expect(env[0]!.metadata).toMatchObject({ subject: 'Environment' });
289+
const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: db }, useNullAsDefault: true });
290+
try {
291+
const pkg = await (driver as any).knex('sys_metadata').where({ id: env[0]!.id }).first('package_id');
292+
expect(pkg.package_id ?? null, 'the organization row\'s binding is not the environment row\'s address').toBeNull();
293+
} finally {
294+
await driver.disconnect();
295+
}
296+
}, CASE_TIMEOUT_MS);
297+
298+
it('lock.verdict and the _lock gate: an organization row carrying _lock: full neither reads as the item\'s lock nor is the gate\'s refusal', async () => {
299+
const db = caseDb();
300+
const { read, error } = await onCeremonyBoot(db, async (kernel) => {
301+
const protocol = kernel.getService('protocol');
302+
// The ceremony's `lock.verdict` port reads `getMetaItem`.
303+
const read = await protocol.getMetaItem({ type: 'email_template', name: LOCKED_TEMPLATE }).catch(() => null);
304+
const error = await doorSave(kernel, 'email_template', LOCKED_TEMPLATE, tpl(LOCKED_TEMPLATE, 'Environment'));
305+
return { read, error };
306+
});
307+
expect((read as { lock?: unknown } | null)?.lock ?? 'none').not.toBe('full');
308+
expect((read as { editable?: unknown } | null)?.editable).not.toBe(false);
309+
// The `_lock` gate runs BEFORE the write. It did not refuse over the
310+
// organization's lock: the save reached the store, where the re-keyed
311+
// unique index refuses an environment row beside the organization's row of
312+
// the same key — the refusal the template pin above measures.
313+
expect(String((error as Error | null)?.message ?? error)).toMatch(/Duplicate record refused on 'sys_metadata'/);
314+
expect(String((error as Error | null)?.message ?? error)).not.toMatch(/lock/i);
315+
const org = (await rawRows(db)).find((r) => r.id === 'org_locked_template')!;
316+
expect(org.metadata).toEqual(tpl(LOCKED_TEMPLATE, 'Org A locked', { _lock: 'full' }));
317+
}, CASE_TIMEOUT_MS);
318+
319+
it('the kernel:ready security-catalog echo registers the environment\'s permission overlay and never an organization\'s', async () => {
320+
const db = caseDb();
321+
const saved = process.env.OS_AUTH_SECRET;
322+
process.env.OS_AUTH_SECRET = 'm23429-secret-'.padEnd(40, 'x');
323+
try {
324+
const { orgEntry, envEntry } = await onCeremonyBoot(db, async (kernel) => {
325+
const { createSecurityCatalogReader } = await import('@objectstack/core');
326+
const catalog = createSecurityCatalogReader({
327+
registry: kernel.getService('objectql').registry,
328+
metadata: kernel.getService('metadata'),
329+
});
330+
return {
331+
orgEntry: await catalog.resolve('permission', 'm23429_org_perm'),
332+
envEntry: await catalog.resolve('permission', 'm23429_env_perm'),
333+
};
334+
});
335+
expect(envEntry, 'control: the environment\'s overlay is in the catalog').toBeDefined();
336+
expect(orgEntry, 'an organization\'s overlay is not the environment\'s').toBeUndefined();
337+
} finally {
338+
if (saved === undefined) delete process.env.OS_AUTH_SECRET;
339+
else process.env.OS_AUTH_SECRET = saved;
340+
}
341+
}, CASE_TIMEOUT_MS);
342+
});
343+

‎packages/drivers/driver-turso/src/turso-driver.ts‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3883,16 +3883,6 @@ export class TursoDriver extends SqlDriver {
38833883
return super.analyzeQuery(object, query, options);
38843884
}
38853885

3886-
/**
3887-
* Schema introspection — refused on the REMOTE face. The inherited member
3888-
* reads every table's columns, foreign keys, primary keys and unique
3889-
* constraints through Knex (`columnInfo()` and the SQLite pragmas), so a
3890-
* remote answer would be a second copy of those four readers, the copy
3891-
* {@link refuseRemoteDriftDetection} declines for drift. Its callers already
3892-
* treat a throw as "could not introspect": the datasource connection test
3893-
* answers `ok: false` with this message, and the federation validation sweep
3894-
* rows the datasource `unreachable`.
3895-
*/
38963886
/**
38973887
* [ADR-0131 D7 · #23429] The physical column names of `object`'s table. The
38983888
* remote arm asks the remote catalog (`pragma_table_info`): the inherited
@@ -3906,12 +3896,29 @@ export class TursoDriver extends SqlDriver {
39063896
if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(table)) {
39073897
throw new Error(`listPhysicalColumns: "${table}" is not a table name the remote catalog read can quote.`);
39083898
}
3909-
const rows = await this.remoteTransport!.execute(`SELECT name FROM pragma_table_info('${table}')`);
3899+
const sql = `SELECT name FROM pragma_table_info('${table}')`;
3900+
let rows: unknown;
3901+
try {
3902+
rows = await this.remoteTransport!.execute(sql);
3903+
} catch (error) {
3904+
// One envelope for both transports, as `execute()` declares it.
3905+
throw this.rawStatementFault(sql, error);
3906+
}
39103907
return (Array.isArray(rows) ? rows : []).map((row) => String((row as { name?: unknown }).name ?? ''));
39113908
}
39123909
return super.listPhysicalColumns(object, options);
39133910
}
39143911

3912+
/**
3913+
* Schema introspection — refused on the REMOTE face. The inherited member
3914+
* reads every table's columns, foreign keys, primary keys and unique
3915+
* constraints through Knex (`columnInfo()` and the SQLite pragmas), so a
3916+
* remote answer would be a second copy of those four readers, the copy
3917+
* {@link refuseRemoteDriftDetection} declines for drift. Its callers already
3918+
* treat a throw as "could not introspect": the datasource connection test
3919+
* answers `ok: false` with this message, and the federation validation sweep
3920+
* rows the datasource `unreachable`.
3921+
*/
39153922
override async introspectSchema(): Promise<IntrospectedSchema> {
39163923
if (this.isRemote) {
39173924
refuseRemoteInheritedMember(

0 commit comments

Comments
 (0)