Skip to content

Commit b070943

Browse files
authored
test(spec): 把「有单条写就必须有批量」做成全仓棘轮,防 #3745 复发 (#3026) (#3755)
#3745 的根因不是判断错,而是审计范围:#3391 P1 把 bulk 门禁改成 `bulk ∧ derived(child)` 后,配套的「补 bulk 原语」被写成了 platform-objects 包审计,而缺口在另外三个包。 本 PR 把人工排查换成扫描 packages/** 下每个 *.object.ts 的 CI 检查,断言三条不变量:有单条写就必须有批量(否则登记豁免并写理由,今天为空)、只能声明 6 个原语(legacy 动词声明了是静默死元数据)、豁免清单不许留陈旧条目;外加扫描量下限断言,防止扫不到文件时真空通过。 扫源码而非 import 全部对象包,避免反转 spec → * 依赖方向,且新包新对象自动被覆盖 —— 这次缺口的形状正是「新包没人扫」。 防假绿:拿掉 sys_permission_set 的 'bulk' 后棘轮如实变红并打印对象名、白名单与文件路径。spec 259 files / 6721 tests 全过;对最新 main 扫描 58 处声明 0 违规。
1 parent f31cc8d commit b070943

2 files changed

Lines changed: 164 additions & 0 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
---
3+
4+
test(spec): ratchet the "single-record writes imply batch" invariant across every `*.object.ts` (#3026)
5+
6+
Test-only — releases nothing. Adds the monorepo-wide scan that would have
7+
caught #3745 before it shipped: the eight objects that kept 405-ing `/batch`
8+
and the `*Many` routes after the #3391 P1 bulk gate became `bulk ∧ child`,
9+
because the sweep that added the `bulk` primitive was scoped to one package
10+
while the gap lived in three others.
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// Conformance ratchet: a tightened `apiMethods` whitelist that grants
4+
// single-record writes must also grant the `bulk` primitive (#3026 follow-up).
5+
//
6+
// The #3391 P1 contract made the bulk gate `bulk ∧ derived(child)`: a batch
7+
// request is admitted only when the object grants the `bulk` PRIMITIVE and the
8+
// batched child operation is itself allowed. Before that, the `*Many` routes
9+
// checked only the child verb, so a boilerplate CRUD-five whitelist batched
10+
// fine. Every object carrying an explicit whitelist therefore needed `bulk`
11+
// added — and the sweep that did it was scoped to `platform-objects`, so eight
12+
// objects in `plugin-security`, `plugin-approvals` and `metadata-core` silently
13+
// kept 405-ing `/batch`, `createMany`, `updateMany` and `deleteMany` while
14+
// their single-record writes stayed wide open (fixed in #3745).
15+
//
16+
// The bug was not a wrong judgement — it was an audit whose SCOPE was a package
17+
// name while the gap lived in packages nobody thought to open. This test
18+
// replaces that manual sweep with a scan of every `*.object.ts` in the
19+
// monorepo, so a new declaration anywhere is covered by construction. Scanning
20+
// source (rather than importing the packages) keeps the check next to the
21+
// derivation table without inverting the spec → * dependency direction — the
22+
// same technique as `system/constants/platform-object-names.test.ts`.
23+
//
24+
// If this fails, pick one deliberately:
25+
// - the object should batch → add `'bulk'` to its `apiMethods`;
26+
// - it genuinely must not → register it in SINGLE_RECORD_WRITE_ONLY below
27+
// with the reason, so the choice is on the record.
28+
29+
import { describe, it, expect } from 'vitest';
30+
import { readFileSync, readdirSync, statSync } from 'node:fs';
31+
import { dirname, join, resolve } from 'node:path';
32+
import { fileURLToPath } from 'node:url';
33+
import { API_PRIMITIVES } from './api-derivation';
34+
35+
const HERE = dirname(fileURLToPath(import.meta.url));
36+
/** …/packages/spec/src/data → repo root */
37+
const REPO_ROOT = resolve(HERE, '../../../..');
38+
const PACKAGES_DIR = join(REPO_ROOT, 'packages');
39+
40+
/** The single-record write primitives whose batch shape `bulk` unlocks. */
41+
const WRITE_PRIMITIVES = ['create', 'update', 'delete'] as const;
42+
43+
/**
44+
* Objects that deliberately expose single-record writes but NO batch route,
45+
* keyed by object name with the reason. Empty today: every tightened whitelist
46+
* in the monorepo either grants `bulk` or grants no write verb at all.
47+
*
48+
* Adding an entry is a real decision — batch denial is invisible until a user
49+
* multi-selects rows and `data-objectstack` rethrows the 405 without falling
50+
* back to per-row writes. Write down why the object is worth that.
51+
*/
52+
const SINGLE_RECORD_WRITE_ONLY: Record<string, string> = {};
53+
54+
/** Every `*.object.ts` under `packages/`, skipping build output and deps. */
55+
function walkObjectFiles(dir: string, out: string[] = []): string[] {
56+
let entries: string[];
57+
try {
58+
entries = readdirSync(dir);
59+
} catch {
60+
return out;
61+
}
62+
for (const entry of entries) {
63+
if (entry === 'node_modules' || entry === 'dist' || entry.startsWith('.')) continue;
64+
const full = join(dir, entry);
65+
let isDir: boolean;
66+
try {
67+
isDir = statSync(full).isDirectory();
68+
} catch {
69+
continue;
70+
}
71+
if (isDir) walkObjectFiles(full, out);
72+
else if (entry.endsWith('.object.ts')) out.push(full);
73+
}
74+
return out;
75+
}
76+
77+
/** The object name declared by `ObjectSchema.create({ name: '…' })`, if any. */
78+
function declaredObjectName(source: string): string | undefined {
79+
return source.match(/ObjectSchema\.create\(\{[\s\S]{0,600}?name:\s*'([a-z0-9_]+)'/)?.[1];
80+
}
81+
82+
interface Whitelist {
83+
object: string;
84+
file: string;
85+
methods: string[];
86+
}
87+
88+
/**
89+
* Every authored `apiMethods: [...]` array literal under `packages/`. Legacy /
90+
* unknown strings are kept as authored — the resolver ignores them, and this
91+
* check only reasons about primitives.
92+
*/
93+
function collectWhitelists(): Whitelist[] {
94+
const out: Whitelist[] = [];
95+
for (const file of walkObjectFiles(PACKAGES_DIR)) {
96+
const source = readFileSync(file, 'utf8');
97+
const object = declaredObjectName(source) ?? '(unnamed)';
98+
for (const match of source.matchAll(/apiMethods:\s*\[([^\]]*)\]/g)) {
99+
out.push({
100+
object,
101+
file: file.slice(REPO_ROOT.length + 1),
102+
methods: [...match[1].matchAll(/'([a-z]+)'/g)].map((m) => m[1]),
103+
});
104+
}
105+
}
106+
return out;
107+
}
108+
109+
const WHITELISTS = collectWhitelists();
110+
111+
describe('apiMethods conformance — single-record writes imply batch (#3026)', () => {
112+
it('scans a plausible number of declarations (guards a silently empty sweep)', () => {
113+
// A scan that matches nothing passes every assertion below vacuously — the
114+
// exact failure mode this file exists to prevent. Pin a floor instead.
115+
expect(WHITELISTS.length).toBeGreaterThan(40);
116+
});
117+
118+
it('only authors the six primitives (legacy verbs are derived, never declared)', () => {
119+
// Since the #3543 enum shrink a legacy verb in a whitelist is stripped at
120+
// parse and ignored by the resolver, so declaring one is silently dead
121+
// metadata — catch it at the source instead.
122+
const primitives = new Set<string>(API_PRIMITIVES);
123+
const offenders = WHITELISTS.flatMap(({ object, file, methods }) =>
124+
methods.filter((m) => !primitives.has(m)).map((m) => `${object} declares '${m}' (${file})`),
125+
);
126+
expect(offenders).toEqual([]);
127+
});
128+
129+
it('grants bulk wherever it grants create / update / delete', () => {
130+
const offenders = WHITELISTS.filter(({ object, methods }) => {
131+
if (SINGLE_RECORD_WRITE_ONLY[object] !== undefined) return false;
132+
const grantsWrite = WRITE_PRIMITIVES.some((verb) => methods.includes(verb));
133+
return grantsWrite && !methods.includes('bulk');
134+
}).map(
135+
({ object, file, methods }) =>
136+
`${object}: [${methods.join(', ')}] grants single-record writes but not 'bulk' — ` +
137+
`/batch and the *Many routes will 405 (${file})`,
138+
);
139+
expect(offenders).toEqual([]);
140+
});
141+
142+
it('keeps the exemption list free of stale entries', () => {
143+
// An exemption that no longer describes a real single-record-write-only
144+
// object reads as a documented decision while documenting nothing.
145+
const stale = Object.keys(SINGLE_RECORD_WRITE_ONLY).filter((object) => {
146+
const declared = WHITELISTS.filter((w) => w.object === object);
147+
if (declared.length === 0) return true;
148+
return declared.every(
149+
(w) => w.methods.includes('bulk') || !WRITE_PRIMITIVES.some((v) => w.methods.includes(v)),
150+
);
151+
});
152+
expect(stale).toEqual([]);
153+
});
154+
});

0 commit comments

Comments
 (0)