Skip to content

Commit 8f78495

Browse files
fix(rest): the import template answers to the import door's gates, not the export's (#20896) (#20977)
Closes #20896 Clause-②: no (no schema or published-export change; the route and its closed parameter set are unchanged, as the ruling states) Implements ruling A on the card (comment 5921162178, its execution parameters taken whole, per the claim 5921450654): `GET /api/v1/data/:object/export?template=true` is judged by the import door's gates, not the export door's. Branch base: `f6ccca4a44`. ## What changed - `packages/rest/src/rest-server.ts` - The export route picks its gates by what the caller asked for. A request whose `template` value reads `true` goes through the new `enforceImportTemplateGates`. The value is read by the template reader itself (`readTemplateMode`, given the `template` key alone), so no second spelling of that reading exists. Every other request goes through `enforceApiAccess(..., 'export')` and `enforceExportPermission` exactly as before. The gates still run before the query-string gates, in the same position, so `template=true&limit=5` from an importer is still refused `400` with the reason, not `403`. - `enforceImportTemplateGates`: 1. The object half is the import door's own first gate, the same call `POST /data/:object/import` makes before it parses: `enforceApiAccess(..., 'import')` (`404` not exposed, `405` neither create nor update exposed). 2. The caller half is the create permission: the security service's `explain({ object, operation: 'create' })`. `allowed: false`, or a throw, answers `403 PERMISSION_DENIED`. No security service, or one without `explain`, allows, the stance `enforceExportPermission` takes. - Docblocks: the `template` paragraph of `DATA_EXPORT_PARAMS` and `answerImportTemplate` now name the import door's gates. - `packages/rest/src/import-template-route.test.ts`: the pins below. The three field-level-security fixtures now grant create through `explain` instead of `canExport`, which no longer reaches the template. - `content/docs/permissions/permission-sets.mdx`: the `allowExport` section says the template is gated by create, never `allowExport`. - `.changeset/20896-template-import-door.md`: patch, new. The still-pending `.changeset/18386-export-import-template.md` is NOT edited: the maintainer answered B on the decision `5922804275` (「20991 20977 都不改」, 2026-10-01), and the one-sentence correction proposed at `501dca7347` was reverted at `8e2d1fda64` (the file is byte-identical to the merge base). ## Premise measured: there is no route-level caller create check to reuse The dispatch asked to reuse the create check the import door enforces. Read at `f6ccca4a44`: - The import door's route-level gates are `enforceApiAccess(..., 'import')` (stage 1) and `enforceApiAccess(..., 'import', { writeMode })` (stage 2). Both are OBJECT-level (`enable.apiMethods`, `404` / `405`). - The caller's create permission is never asked before a write. The engine's security middleware judges it on each written row, and `runImport` records the refusal as a failed row (`PERMISSION_DENIED`) inside a `200` report (the per-row catch in `import-runner.ts`). So the template branch reuses stage 1 verbatim and asks the security service for the middleware's create verdict through `ISecurityService.explain`. That member is not optional. The contract names it as the composition for an object-level verdict with no dedicated method, and `mayReadRunState` in `packages/runtime/src/domains/automation.ts` is the in-repo precedent. Nothing here is re-derived from permission sets. Stage 2 is not asked: it needs the write mode a request body names, and a template request names none. ## Fork clause: not triggered The template's columns do not depend on a write mode. `templateColumns(schema, { explicitFields, permitted })` takes none. `permitted` comes from `ISecurityService.getWritableFields(object, context)`, whose signature has no operation (plugin-security's implementation reads the field mask only). The import door's `writeMode` check is a gate (`404` / `405`) and hands nothing to the column computation. No mode was picked. ## Pins In `import-template-route.test.ts`: - create on the object, no `allowExport`: `200` and the workbook; `canExport` never asked. - no create: `403 PERMISSION_DENIED`, empty body, `answerImportTemplate` never called, `getWritableFields` never asked. - `allowExport` and no create: `403 PERMISSION_DENIED` on `template=true`, builder never called, `canExport` never asked. - a create verdict that throws: `403`, never a grant. - object half: an object exposing `create` without `list` serves the template; an object exposing `get` and `list` only answers `405 OBJECT_API_METHOD_NOT_ALLOWED`, builder never called. - preservation, non-template export: without `allowExport` it is `403 EXPORT_NOT_PERMITTED`, no row read, create verdict never asked. With `allowExport` and no create it is `200` with the pre-change headers, text and sha256, create verdict never asked. The existing byte-identity block is unchanged and green. Each pin is ablated after this PR opens: a mutation proven on disk, the run red, the restore proven, the run green. The readings go in the `os-dev-report` comment on the card, not here. ## Acceptance notes - One direction is stricter than a write. `explain` denies a caller whose permission sets resolve empty, where the middleware skips its CRUD gate. That is reachable only on a deployment with no baseline permission set, and it is the closed direction. `mayReadRunState` records the same boundary. - A caller who holds update but not create is refused the template, even though they could import in update mode. Ruling A names create. - The refusal code is `PERMISSION_DENIED`, a standard code, so the error-code ledger does not change. It is the code the import door's row report carries for the same caller. - The objectui#9600 pointer (the button needs create, not export) is the seat's act when this lands, per the claim. ## Verification at the current head `8e2d1fda64` - One appended commit restores the 18386 note from the merge base; the code is byte-identical to `501dca7347`. - `check-empty-changeset --base origin/main` exit 0; `check-changeset-no-major` and `check-adr-0087-registration` exit 0. - rest typecheck exit 0; rest suite 248 files / 4962 passed, exit 0. - `dispatch-gates`: 93 accounted, 92 run, 1 NOT MEASURED (`check:dual-build-cjs-loads`; CI builds it). Dev report `5923358262` on #20896. ## Verification at the prior head `501dca7347` - The 403 answers `{ success: false, error: { code: PERMISSION_DENIED, message, details: { object } } }` through `sendError` (`check:route-envelope` refused the flat form). - rest typecheck exit 0; rest suite 248 files / 4962 passed, exit 0. - Ablations A1–A3 and A5 went red as expected and were restored (blob equal to HEAD). - `dispatch-gates`: 93 accounted, 92 run, 1 NOT MEASURED (`check:dual-build-cjs-loads`, which needs a full build; CI builds it). `check-empty-changeset` was red by design at this head (the then-proposed correction of the pending #18386 note, since reverted). ## Verification at opening - `pnpm --filter @objectstack/rest exec vitest run --project local --maxWorkers=2 src/import-template-route.test.ts`: 37 passed, 0 failed, at `617255a015`. That is the code commit; the head `3c7d7dd4ef` adds only the changesets and the doc. - The package-wide `test` and `typecheck`, the ablations and `dispatch-gates` finish after opening. Their exit codes are in the report on the card. --- _Generated by [Claude Code](https://claude.ai/code/session_01Sfe5YjBLwB9J3y8fvm2xq1)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent bb2eccf commit 8f78495

4 files changed

Lines changed: 237 additions & 29 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
fix(rest): the import template (`GET /api/v1/data/:object/export?template=true`) is gated by the import door's permissions, not the export's
6+
7+
Clause-②: no
8+
9+
The template carries no records — only the columns the caller may write, one
10+
example row and an instructions sheet — so it answers to whoever may import, and
11+
the export permission (`allowExport`) neither admits nor refuses it:
12+
13+
- A caller with the create permission on the object gets the template, with or
14+
without `allowExport`.
15+
- A caller without the create permission gets `403 PERMISSION_DENIED`, with or
16+
without `allowExport`.
17+
- An object whose `enable.apiMethods` exposes neither `create` nor `update`
18+
answers `405 OBJECT_API_METHOD_NOT_ALLOWED`, as `POST /api/v1/data/:object/import`
19+
does. An object that exposes `create` without `list` serves the template.
20+
- Without `template=true` the export is unchanged: the same two export checks
21+
and the same bytes.
22+
23+
To let a role download the template, grant it create on the object.

‎content/docs/permissions/permission-sets.mdx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ opt-in. Grant it through an ordinary position-distributed set instead.
9595
Enforcement is server-side and covers both egress doors:
9696

9797
- `GET /api/v1/data/:object/export` answers `403 EXPORT_NOT_PERMITTED` before it
98-
reads the first row.
98+
reads the first row. Its import template (`?template=true`) is not a copy of
99+
data — it carries no records — so it is gated by the import instead: the create
100+
permission on the object, never `allowExport`.
99101
- A **report** rendered as `csv` or `json` is the same bulk copy and is gated the
100102
same way, whether run interactively or delivered by a schedule. `html_table`
101103
is a rendered view and stays a read.

‎packages/rest/src/import-template-route.test.ts‎

Lines changed: 115 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
* registry injects onto every object are really there to be excluded.
99
*
1010
* Also here: the proof that WITHOUT `?template=true` the export is byte for
11-
* byte what it was before the mode existed — see the last describe block.
11+
* byte what it was before the mode existed — see the last describe block —
12+
* and that the template is judged by the IMPORT door's gates, never the
13+
* export's (#20896 ruling A).
1214
*/
1315

1416
import { describe, it, expect, afterEach, vi } from 'vitest';
@@ -104,6 +106,12 @@ const DEAL = {
104106
/** The seven columns the registry injects onto every object (the card's table). */
105107
const INJECTED = ['organization_id', 'created_at', 'created_by', 'updated_at', 'updated_by', 'owner_id', 'owning_business_unit_id'];
106108

109+
/**
110+
* A security service's `explain` answering "may create" for every object —
111+
* the caller half of the import door's gates on `?template=true` (#20896).
112+
*/
113+
const MAY_CREATE = async (request: { operation: string }) => ({ allowed: request.operation === 'create' });
114+
107115
interface BootOptions {
108116
security?: Record<string, unknown>;
109117
}
@@ -130,7 +138,7 @@ async function boot(opts: BootOptions = {}) {
130138
await exportRoute.handler({ params: { object }, query, headers } as any, out.res);
131139
return out;
132140
};
133-
return { engine, protocol, findData, get, importRoute };
141+
return { engine, protocol, findData, get, importRoute, rest };
134142
}
135143

136144
const headerRow = async (bytes: Buffer, sheet = 0) => {
@@ -221,7 +229,7 @@ describe('?template=true — field-level security: the WRITE projection', () =>
221229

222230
it('a field the caller may read but not edit is absent, and the response names the write projection', async () => {
223231
const { get } = await boot({
224-
security: { canExport: async () => true, getReadableFields: async () => READABLE, getWritableFields: async () => WRITABLE },
232+
security: { explain: MAY_CREATE, getReadableFields: async () => READABLE, getWritableFields: async () => WRITABLE },
225233
});
226234
const out = await get({ template: 'true' });
227235
const { wb, header } = await headerRow(out.body());
@@ -233,7 +241,7 @@ describe('?template=true — field-level security: the WRITE projection', () =>
233241
});
234242

235243
it('a security service without getWritableFields: narrowed by the read projection, and the response says so', async () => {
236-
const { get } = await boot({ security: { canExport: async () => true, getReadableFields: async () => READABLE } });
244+
const { get } = await boot({ security: { explain: MAY_CREATE, getReadableFields: async () => READABLE } });
237245
const out = await get({ template: 'true' });
238246
const { wb, header } = await headerRow(out.body());
239247
expect(header).toContain('Salary');
@@ -244,7 +252,7 @@ describe('?template=true — field-level security: the WRITE projection', () =>
244252

245253
it('a security service that answers neither projection fails the request instead of an unnarrowed header', async () => {
246254
const { get } = await boot({
247-
security: { canExport: async () => true, getWritableFields: async () => undefined, getReadableFields: async () => undefined },
255+
security: { explain: MAY_CREATE, getWritableFields: async () => undefined, getReadableFields: async () => undefined },
248256
});
249257
const out = await get({ template: 'true' });
250258
expect(out.status()).toBe(500);
@@ -253,26 +261,85 @@ describe('?template=true — field-level security: the WRITE projection', () =>
253261
});
254262
});
255263

256-
describe('?template=true — the two existing gates still decide', () => {
257-
it('a caller without the export permission is refused 403 and gets no workbook', async () => {
258-
const { get } = await boot({ security: { canExport: async () => false, getReadableFields: async () => ['title'] } });
264+
describe('?template=true — the IMPORT door\'s gates decide, not the export\'s (#20896 ruling A)', () => {
265+
// The template carries no records, so it answers to whoever may import:
266+
// the object's `import` exposure, then the caller's create permission. The
267+
// export gates (`export` exposure, `canExport`) neither admit nor refuse it.
268+
const noCreate = async () => ({ allowed: false });
269+
270+
it('create on the object and no allowExport → 200 and the workbook', async () => {
271+
const canExport = vi.fn(async () => false);
272+
const explain = vi.fn(MAY_CREATE);
273+
const { get } = await boot({ security: { canExport, explain, getWritableFields: async () => ['title', 'stage'] } });
274+
const out = await get({ template: 'true' });
275+
expect(out.status()).toBe(200);
276+
expect(out.headers['Content-Type']).toBe('application/vnd.openxmlformats-officedocument.spreadsheetml.sheet');
277+
expect(out.headers['X-Export-Template']).toBe('true');
278+
expect((await headerRow(out.body())).header).toEqual(['Title *', 'Stage']);
279+
expect(explain).toHaveBeenCalledWith({ object: 'deal', operation: 'create' }, expect.objectContaining({ userId: 'test-user' }));
280+
expect(canExport).not.toHaveBeenCalled();
281+
});
282+
283+
it('no create → 403 PERMISSION_DENIED from the import door, and the template builder is never called', async () => {
284+
const getWritableFields = vi.fn(async () => ['title']);
285+
const { get, rest } = await boot({ security: { canExport: async () => false, explain: noCreate, getWritableFields } });
286+
const builder = vi.spyOn(rest as any, 'answerImportTemplate');
287+
const out = await get({ template: 'true' });
288+
expect(out.status()).toBe(403);
289+
expect(out.json()).toMatchObject({ success: false, error: { code: 'PERMISSION_DENIED', details: { object: 'deal' } } });
290+
expect(out.body().length).toBe(0);
291+
expect(builder).not.toHaveBeenCalled();
292+
expect(getWritableFields).not.toHaveBeenCalled();
293+
});
294+
295+
it('allowExport and no create → 403 on template=true — the export permission admits no template', async () => {
296+
const canExport = vi.fn(async () => true);
297+
const getWritableFields = vi.fn(async () => ['title']);
298+
const { get, rest } = await boot({ security: { canExport, explain: noCreate, getWritableFields } });
299+
const builder = vi.spyOn(rest as any, 'answerImportTemplate');
300+
const out = await get({ template: 'true' });
301+
expect(out.status()).toBe(403);
302+
expect(out.json()).toMatchObject({ success: false, error: { code: 'PERMISSION_DENIED', details: { object: 'deal' } } });
303+
expect(out.body().length).toBe(0);
304+
expect(builder).not.toHaveBeenCalled();
305+
expect(getWritableFields).not.toHaveBeenCalled();
306+
expect(canExport).not.toHaveBeenCalled();
307+
});
308+
309+
it('a create verdict that throws is a denial, never a grant', async () => {
310+
const { get } = await boot({
311+
security: { explain: async () => { throw new Error('permission sets did not resolve'); }, getWritableFields: async () => ['title'] },
312+
});
259313
const out = await get({ template: 'true' });
260314
expect(out.status()).toBe(403);
261-
expect(out.json()).toMatchObject({ code: 'EXPORT_NOT_PERMITTED' });
315+
expect(out.json()?.error?.code).toBe('PERMISSION_DENIED');
262316
expect(out.body().length).toBe(0);
263317
});
264318

265-
it('an object that does not expose export is refused before the template', async () => {
319+
it('an object exposing create but not export serves the template — its export exposure no longer stands in front', async () => {
266320
const { get, engine } = await boot();
267321
engine.registry.registerObject({
268-
// `export` is derived from `list`, so only a whitelist without `list` closes it.
269-
name: 'locked', label: 'Locked', enable: { apiMethods: ['get'] },
322+
// `export` is derived from `list`; `import` from `create ∨ update`.
323+
name: 'intake', label: 'Intake', enable: { apiMethods: ['create'] },
324+
fields: { title: { name: 'title', type: 'text', label: 'Title' } },
325+
} as any);
326+
const out = await get({ template: 'true' }, 'intake');
327+
expect(out.status()).toBe(200);
328+
expect((await headerRow(out.body())).header).toEqual(['Title']);
329+
});
330+
331+
it('an object exposing export but neither create nor update is refused 405 before the template', async () => {
332+
const { get, engine, rest } = await boot();
333+
engine.registry.registerObject({
334+
name: 'ledger', label: 'Ledger', enable: { apiMethods: ['get', 'list'] },
270335
fields: { title: { name: 'title', type: 'text' } },
271336
} as any);
272-
const out = await get({ template: 'true' }, 'locked');
337+
const builder = vi.spyOn(rest as any, 'answerImportTemplate');
338+
const out = await get({ template: 'true' }, 'ledger');
273339
expect(out.status()).toBe(405);
274340
expect(out.json()).toMatchObject({ code: 'OBJECT_API_METHOD_NOT_ALLOWED' });
275341
expect(out.body().length).toBe(0);
342+
expect(builder).not.toHaveBeenCalled();
276343
});
277344
});
278345

@@ -556,8 +623,11 @@ const PRE_CHANGE: Record<string, {
556623
},
557624
};
558625

559-
/** `export-integration.test.ts`'s fixtures: `systemFields: false`, two rows. */
560-
async function bootExportFixture() {
626+
/**
627+
* `export-integration.test.ts`'s fixtures: `systemFields: false`, two rows. An
628+
* optional security service is composed the way `boot` composes one.
629+
*/
630+
async function bootExportFixture(security?: Record<string, unknown>) {
561631
const engine = new ObjectQL();
562632
liveEngines.push(engine);
563633
engine.registerDriver(makeSqliteDriver(), true);
@@ -586,19 +656,22 @@ async function bootExportFixture() {
586656
await engine.insert('task', { id: '1', title: '写代码', done: true, priority: 'high', due: '2026-06-30T00:00:00.000Z', owner: 'u1' });
587657
await engine.insert('task', { id: '2', title: '写文档', done: false, priority: 'low', due: '2026-07-01T00:00:00.000Z', owner: 'u2' });
588658
const protocol = new ObjectStackProtocolImplementation(engine as any);
659+
const findData = vi.spyOn(protocol as any, 'findData');
589660
const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any);
590661
(rest as any).resolveExecCtx = async () => ({ userId: 'test-user', timezone: 'UTC' });
662+
if (security) (rest as any).resolveSecurityService = async () => security;
591663
rest.registerRoutes();
592-
return rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/data/:object/export')!;
664+
const route = rest.getRoutes().find((r: any) => r.method === 'GET' && r.path === '/api/v1/data/:object/export')!;
665+
return { route, findData };
593666
}
594667

595-
async function exportOnce(query: Record<string, string>) {
668+
async function exportOnce(query: Record<string, string>, security?: Record<string, unknown>) {
596669
vi.useFakeTimers({ now: new Date(FROZEN_NOW), toFake: ['Date'] });
597670
try {
598-
const route: any = await bootExportFixture();
671+
const { route, findData } = await bootExportFixture(security);
599672
const out = makeRes();
600673
await route.handler({ params: { object: 'task' }, query } as any, out.res);
601-
return out;
674+
return { ...out, findData };
602675
} finally {
603676
vi.useRealTimers();
604677
}
@@ -625,4 +698,27 @@ describe('without ?template=true the export is byte-identical to the pre-change
625698
expect(off.body().equals(plain.body())).toBe(true);
626699
expect(off.body().toString('utf8')).toBe(PRE_CHANGE.csvDefault.text);
627700
});
701+
702+
// [#20896] The template's gate swap reaches no export request: the export
703+
// is still judged by `canExport`, and the create verdict is never asked.
704+
it('without allowExport: 403 EXPORT_NOT_PERMITTED before a row is read, whatever the caller may create', async () => {
705+
const explain = vi.fn(MAY_CREATE);
706+
const out = await exportOnce({}, { canExport: async () => false, explain });
707+
expect(out.status()).toBe(403);
708+
expect(out.json()).toMatchObject({ code: 'EXPORT_NOT_PERMITTED', object: 'task' });
709+
expect(out.body().length).toBe(0);
710+
expect(out.findData).not.toHaveBeenCalled();
711+
expect(explain).not.toHaveBeenCalled();
712+
});
713+
714+
it('with allowExport and without create: the pre-change bytes, the create verdict never asked', async () => {
715+
const explain = vi.fn(async () => ({ allowed: false }));
716+
const out = await exportOnce({}, { canExport: async () => true, explain });
717+
const body = out.body();
718+
expect(out.status()).toBe(200);
719+
expect(out.headers).toEqual(PRE_CHANGE.csvDefault.headers);
720+
expect(body.toString('utf8')).toBe(PRE_CHANGE.csvDefault.text);
721+
expect(createHash('sha256').update(body).digest('hex')).toBe(PRE_CHANGE.csvDefault.sha256);
722+
expect(explain).not.toHaveBeenCalled();
723+
});
628724
});

0 commit comments

Comments
 (0)