Skip to content

Commit df54254

Browse files
baozhoutaoclaude
andauthored
fix(cli): 非法 OS_TENANCY_POSTURE 在 serve 最开头被显式拒绝,不再伪装成 AuthPlugin 加载失败 (#5359) (#5381)
* fix(cli): refuse an invalid OS_TENANCY_POSTURE at the top of serve, explicitly (#5359) `resolveTenancyPosture()` refuses an unrecognized `OS_TENANCY_POSTURE` and its message says "Refusing to boot". The refusal was never the problem; how it travelled was. serve had no dedicated read — it let the throw escape from whichever call site reached it first, and that site sat inside the broad AuthPlugin `try`, whose catch only warns. So an env-var typo announced itself as `AuthPlugin failed to load`, boot continued degraded through the whole capability slate (persisting a generated dev crypto key on the way), and the resolver's sentence only reached the operator much later, bare, from a generic printError. Resolve the posture once at the top of `run()` — after dotenv-flow's load so a `.env`-sourced value is seen, outside every `try` so nothing can demote it — and refuse an invalid value with an ADR-0093 D5-shaped FATAL (fix list generated from `TENANCY_POSTURES`, so it cannot go stale) plus `process.exit(1)`. The refusal is returned as a verdict rather than thrown, which is what makes it undemotable. Both later readers in serve (the organizations-plugin gate, the banner's `Tenancy:` row) reuse the resolved value; the banner in particular was the last line of defence against an invalid posture, i.e. a diagnostic surface made load-bearing for a safety property. One correction to the issue's static trace: the port was never bound before the refusal. The escaping throw aborted kernel bootstrap Phase 1 (ObjectQL's SchemaRegistry constructor), while the listening socket only opens in Phase 4, so "never listened" held before this change too and is not claimed as its evidence. What moved is that the refusal is now first, sole, correctly attributed, prescriptive, and side-effect free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VkPSGsX9o17MsGv3Lbxu2w * test(cli): give the in-process serve ordering case a 60s timeout (#5359) The merge queue took PR #5381 out on `Test timed out in 5000ms` at serve-tenancy-posture-gate.test.ts (queue run 30971902650, Test Core 3/3) — a timeout, not a failed assertion. Ten of the eleven cases in that file only inspect the refusal string and run in 0-3ms. The eleventh imports and runs the real `serve` command in-process: the whole serve module graph plus a port-availability probe. It measures 2535ms on an idle runner; on the queue's full-suite shard, sharing with the serve e2e tests (that shard reported import 94.8s / tests 282s), it exceeded the 5s default. The PR shards are lighter, so PR CI stayed green throughout. Same posture as the existing `}, 60_000)` cases in this package (utils/sqlite-occupancy.test.ts, utils/schema-migrate.deferred-ddl.integration .test.ts) and as #4856's package-level testTimeout. Superficially the #4796 5000ms signature but a different cause: that family was the spec template suite, already fixed by #4856. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VkPSGsX9o17MsGv3Lbxu2w --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ada0b84 commit df54254

3 files changed

Lines changed: 469 additions & 2 deletions

File tree

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(cli): 非法 `OS_TENANCY_POSTURE``serve` 最开头被显式拒绝,不再先伪装成「AuthPlugin 加载失败」(#5359)
6+
7+
`resolveTenancyPosture()`(`@objectstack/types`)对无法识别的值抛错,文案自称
8+
「Refusing to boot rather than silently falling back to a posture with no
9+
organization wall」。**拒绝本身一直是对的,错的是这个拒绝怎么传出去。**
10+
11+
`serve` 过去没有单独解析 posture,而是让抛错从「第一处读到它的地方」自然逃逸,而那个
12+
位置恰好在 AuthPlugin 那个很宽的 `try` 里 —— 它的 catch 只打印一句黄字就继续。于是一个
13+
env 拼写错误的第一现场是:
14+
15+
```
16+
⚠ AuthPlugin failed to load: Invalid OS_TENANCY_POSTURE="bogus". Expected one of: …
17+
```
18+
19+
把环境变量拼错报成了插件加载问题。启动随后**带伤继续**:整个 capability slate 照常装载,
20+
本地 crypto 密钥被生成并**持久化到磁盘**,直到下一处没有被 `try` 包住的读取
21+
(ObjectQL `SchemaRegistry` 构造,内核 bootstrap 阶段一)才让 `runtime.start()` 中止,
22+
最终由通用 `printError` 把解析器那句话裸着打出来 —— 退出码对,别的都不对。
23+
24+
**现在的行为。** `serve.run()``dotenv-flow` 载入之后、读取配置文件之前、任何 `try`
25+
之外解析一次 posture。非法值走 ADR-0093 D5 同款形状的显式拒绝 —— FATAL + 完整修法清单 +
26+
`process.exit(1)`(不是 throw,throw 正是会被下游 catch 降级成 warning 的那种东西):
27+
28+
```
29+
✖ FATAL: OS_TENANCY_POSTURE="bogus" is not a recognized tenancy posture.
30+
Refusing to boot. …
31+
32+
No config has been loaded, no plugin has been mounted, and the HTTP server was
33+
never started — this deployment has not served a single request.
34+
35+
Fix one of:
36+
• set OS_TENANCY_POSTURE=single — …
37+
• set OS_TENANCY_POSTURE=group — …
38+
• set OS_TENANCY_POSTURE=isolated — …
39+
• unset OS_TENANCY_POSTURE entirely — …
40+
41+
cause: Invalid OS_TENANCY_POSTURE="bogus". …
42+
```
43+
44+
修法清单由 `@objectstack/spec/security``TENANCY_POSTURES` 生成,不是第二份字面量,
45+
所以新增一个 posture 不会让这段建议悄悄过期。文案里点名 `.env` —— 该变量常常来自提交进
46+
仓库的 `.env*` 而不是 shell,这也是把闸门放在 dotenv 载入**之后**的原因。
47+
48+
`serve` 内后续所有 posture 读取(组织插件装载判断、启动横幅的 `Tenancy:` 行)改为复用
49+
闸门解析出的那一个值,不再各自重新解析。横幅那处尤其值得说:它此前是非法 posture 的
50+
**最后一道防线**,等于让一个诊断输出承担安全属性 —— 现在闸门拥有这个拒绝,横幅只负责
51+
汇报闸门的结论。
52+
53+
**一处订正。** #5359 的静态追踪认为进程「在报错前已经 listen 过」。实测并非如此:逃逸的
54+
抛错发生在内核 bootstrap **阶段一**(插件 init),而监听套接字在**阶段四**
55+
`kernel:listening` 钩子才打开,所以端口从未被绑定过 —— 修复前后都是如此。本次真正改变的
56+
是:拒绝成为第一条也是唯一一条输出、归因正确、带处方,且启动不再留下任何副作用
57+
(修复前那次「被拒绝」的启动会在磁盘上留下持久化的 dev crypto key)。
Lines changed: 270 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,270 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The tenancy-posture boot gate (#5359).
5+
*
6+
* `resolveTenancyPosture()` in `@objectstack/types` refuses an unrecognized
7+
* `OS_TENANCY_POSTURE` and says so in the error text — "Refusing to boot rather
8+
* than silently falling back to a posture with no organization wall". The
9+
* refusal itself was never the problem. HOW it travelled was:
10+
*
11+
* • serve's first read sat inside the broad AuthPlugin `try`, whose catch
12+
* prints `⚠ AuthPlugin failed to load: …` and carries on. So the first —
13+
* and for a long stretch the only — thing an operator saw for a misspelled
14+
* env var was a PLUGIN-LOADING failure.
15+
* • Boot then continued, degraded and without plugin-auth, through the whole
16+
* capability slate (generating and PERSISTING a dev crypto key on the way)
17+
* before the next unguarded read aborted it with a bare `printError`.
18+
*
19+
* `packages/cli` had no test on any of this: before this file,
20+
* `git grep -n "OS_TENANCY_POSTURE" packages/cli/src` matched only the prose in
21+
* serve.ts and the sibling `verify` test's back-compat notes.
22+
*
23+
* ── On what these tests do and do not claim ──────────────────────────────
24+
*
25+
* The issue that prompted the fix (#5359) traced this statically and concluded
26+
* the process had "already listened" before refusing. It has not: the throw the
27+
* banner was blamed for actually escapes far earlier, from ObjectQL's
28+
* `SchemaRegistry` constructor during kernel bootstrap Phase 1, while the HTTP
29+
* socket only opens on the `kernel:listening` hook in Phase 4. So "the port
30+
* never binds" is TRUE BOTH BEFORE AND AFTER this change, and no assertion here
31+
* is written as if it were the fix's evidence — a test that passes because
32+
* nothing was produced proves nothing.
33+
*
34+
* What the change actually moves, and what these tests therefore pin:
35+
* • the refusal is a VERDICT, not a throw — nothing downstream can demote it
36+
* to a warning the way the AuthPlugin catch did;
37+
* • it names OS_TENANCY_POSTURE and prescribes every way out (ADR-0093 D5's
38+
* shape), instead of arriving bare through a generic error printer;
39+
* • the fix list is generated from the posture vocabulary, so it cannot go
40+
* stale when a posture is added.
41+
*/
42+
43+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
44+
import path from 'node:path';
45+
import { fileURLToPath } from 'node:url';
46+
import { TENANCY_POSTURES } from '@objectstack/spec/security';
47+
48+
import Serve, { resolveTenancyPostureOrRefusal } from './serve.js';
49+
50+
/** `packages/cli` — the oclif root the command is loaded against below. */
51+
const CLI_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');
52+
53+
/**
54+
* `chalk` may or may not emit SGR codes depending on TTY detection.
55+
*
56+
* The escape is written as `\x1b`, never as the byte itself: one raw control
57+
* character makes grep treat the whole file as binary, and a test file nobody's
58+
* `git grep` can find is a test file that stops being maintained (#4890/#5157).
59+
*/
60+
const SGR = /\x1b\[[0-9;]*m/g;
61+
const plain = (s: string) => s.replace(SGR, '');
62+
63+
const TOUCHED = ['OS_TENANCY_POSTURE', 'OS_MULTI_ORG_ENABLED'] as const;
64+
let saved: Record<string, string | undefined> = {};
65+
66+
beforeEach(() => {
67+
saved = Object.fromEntries(TOUCHED.map((k) => [k, process.env[k]]));
68+
for (const k of TOUCHED) delete process.env[k];
69+
});
70+
71+
afterEach(() => {
72+
for (const k of TOUCHED) {
73+
if (saved[k] === undefined) delete process.env[k];
74+
else process.env[k] = saved[k];
75+
}
76+
});
77+
78+
describe('resolveTenancyPostureOrRefusal — accepted values', () => {
79+
it('passes every posture the spec vocabulary declares', () => {
80+
for (const posture of TENANCY_POSTURES) {
81+
process.env.OS_TENANCY_POSTURE = posture;
82+
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture });
83+
}
84+
});
85+
86+
it("keeps the legacy 'multi' spelling normalizing to isolated", () => {
87+
process.env.OS_TENANCY_POSTURE = 'multi';
88+
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'isolated' });
89+
});
90+
91+
it('unset falls back to the OS_MULTI_ORG_ENABLED derivation, not to a refusal', () => {
92+
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'single' });
93+
94+
process.env.OS_MULTI_ORG_ENABLED = 'true';
95+
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'isolated' });
96+
});
97+
98+
it('treats a blank value as unset — a gate that refused it would break `OS_TENANCY_POSTURE=` in a .env', () => {
99+
process.env.OS_TENANCY_POSTURE = ' ';
100+
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'single' });
101+
});
102+
});
103+
104+
describe('resolveTenancyPostureOrRefusal — the refusal', () => {
105+
it('REFUSES AS A VALUE, never as a throw — the property the AuthPlugin catch destroyed', () => {
106+
process.env.OS_TENANCY_POSTURE = 'bogus';
107+
108+
// The point of the whole change. `resolveTenancyPosture()` throws here; if
109+
// this wrapper let that escape, any enclosing `try` — and serve has a broad
110+
// one — could turn "refuse to boot" back into a yellow warning, which is
111+
// precisely what shipped. A verdict cannot be caught.
112+
expect(() => resolveTenancyPostureOrRefusal()).not.toThrow();
113+
114+
const verdict = resolveTenancyPostureOrRefusal();
115+
expect(verdict.ok).toBe(false);
116+
});
117+
118+
it('names the fact: FATAL, the variable, and the value the operator actually typed', () => {
119+
process.env.OS_TENANCY_POSTURE = 'islolated'; // a real transposition typo
120+
const verdict = resolveTenancyPostureOrRefusal();
121+
if (verdict.ok) throw new Error('expected a refusal');
122+
123+
const text = plain(verdict.fatal);
124+
expect(text).toContain('FATAL');
125+
expect(text).toContain('OS_TENANCY_POSTURE="islolated"');
126+
expect(text).toContain('Refusing to boot');
127+
128+
// Not an AuthPlugin problem, not a plugin problem at all. The misattribution
129+
// is the defect; the word must not reappear in the refusal.
130+
expect(text).not.toContain('AuthPlugin');
131+
});
132+
133+
it('prescribes a way out for EVERY posture the vocabulary declares (drift guard)', () => {
134+
process.env.OS_TENANCY_POSTURE = 'bogus';
135+
const verdict = resolveTenancyPostureOrRefusal();
136+
if (verdict.ok) throw new Error('expected a refusal');
137+
138+
const text = plain(verdict.fatal);
139+
// Generated from TENANCY_POSTURES rather than restated, so a posture added
140+
// to the spec cannot leave this advice quietly incomplete.
141+
for (const posture of TENANCY_POSTURES) {
142+
expect(text).toContain(`set OS_TENANCY_POSTURE=${posture}`);
143+
}
144+
// …plus the escape the enumeration cannot express.
145+
expect(text).toContain('unset OS_TENANCY_POSTURE');
146+
expect(text).toContain('OS_MULTI_ORG_ENABLED');
147+
});
148+
149+
it('points at .env files, the source a shell-only search misses', () => {
150+
process.env.OS_TENANCY_POSTURE = 'bogus';
151+
const verdict = resolveTenancyPostureOrRefusal();
152+
if (verdict.ok) throw new Error('expected a refusal');
153+
154+
// The gate is deliberately placed AFTER dotenv-flow's load, so a value from
155+
// a committed `.env*` reaches it. Saying so is what stops the next operator
156+
// grepping only their shell profile.
157+
expect(plain(verdict.fatal)).toContain('.env');
158+
});
159+
160+
it('carries the resolver\'s own sentence as `cause` rather than paraphrasing it', () => {
161+
process.env.OS_TENANCY_POSTURE = 'bogus';
162+
const verdict = resolveTenancyPostureOrRefusal();
163+
if (verdict.ok) throw new Error('expected a refusal');
164+
165+
// `@objectstack/types` owns the vocabulary and its wording; serve must not
166+
// maintain a second copy that can disagree with it.
167+
expect(plain(verdict.fatal)).toContain('cause: Invalid OS_TENANCY_POSTURE="bogus"');
168+
});
169+
170+
it('states that nothing was loaded and nothing was served', () => {
171+
process.env.OS_TENANCY_POSTURE = 'bogus';
172+
const verdict = resolveTenancyPostureOrRefusal();
173+
if (verdict.ok) throw new Error('expected a refusal');
174+
175+
const text = plain(verdict.fatal);
176+
// Only honest because the gate runs at the top of `run()` — the ordering
177+
// test below is what keeps these two sentences true.
178+
expect(text).toContain('No config has been loaded');
179+
expect(text).toContain('the HTTP server was\n never started');
180+
181+
// And deliberately NOT the stronger "no port has been bound": serve probes
182+
// port availability (bind + close) just above the gate. Overclaiming by one
183+
// word is how a diagnostic stops being trustworthy.
184+
expect(text).not.toContain('no port has been bound');
185+
});
186+
});
187+
188+
describe('the gate runs before serve does ANY boot work', () => {
189+
/**
190+
* The ordering assertion, run against the real `serve` command in-process.
191+
*
192+
* This is the one that would have caught #5359. Before the fix, an invalid
193+
* posture got as far as `Loading objectstack.config.ts…`, the whole plugin
194+
* slate, a persisted dev crypto key and a degraded kernel bootstrap before
195+
* anything refused. After it, `run()` reaches the gate and stops: the FATAL
196+
* is the only thing written, and `console.log` — which is where every
197+
* subsequent boot step reports — is never touched at all.
198+
*
199+
* Note what is deliberately NOT asserted: "the port never listened". That was
200+
* true before the fix too (the escaping throw aborted kernel Phase 1, while
201+
* the socket only opens in Phase 4), so asserting it would pass for reasons
202+
* having nothing to do with this change. "No boot work happened at all" is
203+
* strictly stronger and actually moved.
204+
*/
205+
it('refuses before the config file is even read, and writes nothing else', async () => {
206+
const savedNodeEnv = process.env.NODE_ENV;
207+
process.env.OS_TENANCY_POSTURE = 'bogus';
208+
209+
const errors: string[] = [];
210+
const logs: string[] = [];
211+
const errSpy = vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => {
212+
errors.push(a.join(' '));
213+
});
214+
const logSpy = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => {
215+
logs.push(a.join(' '));
216+
});
217+
// The gate exits the PROCESS on purpose (a throw is what the broad
218+
// AuthPlugin catch used to swallow). Convert it to something catchable so
219+
// the test runner survives, and assert it was reached.
220+
const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
221+
throw new Error(`__PROCESS_EXIT__:${code}`);
222+
}) as never);
223+
224+
let raised: unknown;
225+
try {
226+
// `--dev` keeps the port-availability probe on the auto-shift path so a
227+
// busy port in CI cannot pre-empt the gate we are measuring.
228+
await Serve.run(['--dev', '--port', '39871'], { root: CLI_ROOT });
229+
} catch (err) {
230+
raised = err;
231+
} finally {
232+
errSpy.mockRestore();
233+
logSpy.mockRestore();
234+
exitSpy.mockRestore();
235+
if (savedNodeEnv === undefined) delete process.env.NODE_ENV;
236+
else process.env.NODE_ENV = savedNodeEnv;
237+
}
238+
239+
// Refused via process.exit(1), not by throwing into a catchable boot path.
240+
expect((raised as Error | undefined)?.message).toBe('__PROCESS_EXIT__:1');
241+
242+
const stderr = plain(errors.join('\n'));
243+
expect(stderr).toContain('FATAL');
244+
expect(stderr).toContain('OS_TENANCY_POSTURE="bogus"');
245+
246+
// ── The ordering facts ────────────────────────────────────────────────
247+
// serve announces the config load on stdout as its first boot step. It is
248+
// absent, so the gate preceded it — and therefore preceded every plugin
249+
// load, the kernel bootstrap and the listening socket that follow it.
250+
expect(logs.join('\n')).not.toContain('Loading');
251+
// Nothing at all reached stdout, in fact: the refusal is the whole output.
252+
expect(logs).toEqual([]);
253+
254+
// The misattribution that made this issue expensive to diagnose is gone:
255+
// no warning blames a plugin for an environment-variable typo.
256+
expect(stderr).not.toContain('AuthPlugin failed to load');
257+
// 60s, not the 5s default: unlike the ten message-only cases above, this one
258+
// imports and runs the REAL serve command in-process — the whole serve
259+
// module graph plus a port-availability probe. On a lightly-loaded PR shard
260+
// that costs a moment; on the merge queue's full-suite runner, sharing a
261+
// shard with the serve e2e tests (vitest reported import 94.8s / tests 282s
262+
// for that shard), it blew the 5s default and this case timed out — queue
263+
// run 30971902650, which is what took the PR out of the queue. Same posture
264+
// as the existing `}, 60_000)` cases in this package
265+
// (`utils/sqlite-occupancy.test.ts`, `utils/schema-migrate.deferred-ddl.
266+
// integration.test.ts`) and as #4856's package-level `testTimeout`.
267+
// Superficially the #4796 5000ms signature, but a different cause: that
268+
// family was the spec template suite, already fixed.
269+
}, 60_000);
270+
});

0 commit comments

Comments
 (0)