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
57 changes: 57 additions & 0 deletions .changeset/serve-tenancy-posture-boot-gate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
---
"@objectstack/cli": patch
---

fix(cli): 非法 `OS_TENANCY_POSTURE` 在 `serve` 最开头被显式拒绝,不再先伪装成「AuthPlugin 加载失败」(#5359)

`resolveTenancyPosture()`(`@objectstack/types`)对无法识别的值抛错,文案自称
「Refusing to boot rather than silently falling back to a posture with no
organization wall」。**拒绝本身一直是对的,错的是这个拒绝怎么传出去。**

`serve` 过去没有单独解析 posture,而是让抛错从「第一处读到它的地方」自然逃逸,而那个
位置恰好在 AuthPlugin 那个很宽的 `try` 里 —— 它的 catch 只打印一句黄字就继续。于是一个
env 拼写错误的第一现场是:

```
⚠ AuthPlugin failed to load: Invalid OS_TENANCY_POSTURE="bogus". Expected one of: …
```

把环境变量拼错报成了插件加载问题。启动随后**带伤继续**:整个 capability slate 照常装载,
本地 crypto 密钥被生成并**持久化到磁盘**,直到下一处没有被 `try` 包住的读取
(ObjectQL `SchemaRegistry` 构造,内核 bootstrap 阶段一)才让 `runtime.start()` 中止,
最终由通用 `printError` 把解析器那句话裸着打出来 —— 退出码对,别的都不对。

**现在的行为。** `serve.run()` 在 `dotenv-flow` 载入之后、读取配置文件之前、任何 `try`
之外解析一次 posture。非法值走 ADR-0093 D5 同款形状的显式拒绝 —— FATAL + 完整修法清单 +
`process.exit(1)`(不是 throw,throw 正是会被下游 catch 降级成 warning 的那种东西):

```
✖ FATAL: OS_TENANCY_POSTURE="bogus" is not a recognized tenancy posture.
Refusing to boot. …

No config has been loaded, no plugin has been mounted, and the HTTP server was
never started — this deployment has not served a single request.

Fix one of:
• set OS_TENANCY_POSTURE=single — …
• set OS_TENANCY_POSTURE=group — …
• set OS_TENANCY_POSTURE=isolated — …
• unset OS_TENANCY_POSTURE entirely — …

cause: Invalid OS_TENANCY_POSTURE="bogus". …
```

修法清单由 `@objectstack/spec/security` 的 `TENANCY_POSTURES` 生成,不是第二份字面量,
所以新增一个 posture 不会让这段建议悄悄过期。文案里点名 `.env` —— 该变量常常来自提交进
仓库的 `.env*` 而不是 shell,这也是把闸门放在 dotenv 载入**之后**的原因。

`serve` 内后续所有 posture 读取(组织插件装载判断、启动横幅的 `Tenancy:` 行)改为复用
闸门解析出的那一个值,不再各自重新解析。横幅那处尤其值得说:它此前是非法 posture 的
**最后一道防线**,等于让一个诊断输出承担安全属性 —— 现在闸门拥有这个拒绝,横幅只负责
汇报闸门的结论。

**一处订正。** #5359 的静态追踪认为进程「在报错前已经 listen 过」。实测并非如此:逃逸的
抛错发生在内核 bootstrap **阶段一**(插件 init),而监听套接字在**阶段四**的
`kernel:listening` 钩子才打开,所以端口从未被绑定过 —— 修复前后都是如此。本次真正改变的
是:拒绝成为第一条也是唯一一条输出、归因正确、带处方,且启动不再留下任何副作用
(修复前那次「被拒绝」的启动会在磁盘上留下持久化的 dev crypto key)。
270 changes: 270 additions & 0 deletions packages/cli/src/commands/serve-tenancy-posture-gate.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* The tenancy-posture boot gate (#5359).
*
* `resolveTenancyPosture()` in `@objectstack/types` refuses an unrecognized
* `OS_TENANCY_POSTURE` and says so in the error text — "Refusing to boot rather
* than silently falling back to a posture with no organization wall". The
* refusal itself was never the problem. HOW it travelled was:
*
* • serve's first read sat inside the broad AuthPlugin `try`, whose catch
* prints `⚠ AuthPlugin failed to load: …` and carries on. So the first —
* and for a long stretch the only — thing an operator saw for a misspelled
* env var was a PLUGIN-LOADING failure.
* • Boot then continued, degraded and without plugin-auth, through the whole
* capability slate (generating and PERSISTING a dev crypto key on the way)
* before the next unguarded read aborted it with a bare `printError`.
*
* `packages/cli` had no test on any of this: before this file,
* `git grep -n "OS_TENANCY_POSTURE" packages/cli/src` matched only the prose in
* serve.ts and the sibling `verify` test's back-compat notes.
*
* ── On what these tests do and do not claim ──────────────────────────────
*
* The issue that prompted the fix (#5359) traced this statically and concluded
* the process had "already listened" before refusing. It has not: the throw the
* banner was blamed for actually escapes far earlier, from ObjectQL's
* `SchemaRegistry` constructor during kernel bootstrap Phase 1, while the HTTP
* socket only opens on the `kernel:listening` hook in Phase 4. So "the port
* never binds" is TRUE BOTH BEFORE AND AFTER this change, and no assertion here
* is written as if it were the fix's evidence — a test that passes because
* nothing was produced proves nothing.
*
* What the change actually moves, and what these tests therefore pin:
* • the refusal is a VERDICT, not a throw — nothing downstream can demote it
* to a warning the way the AuthPlugin catch did;
* • it names OS_TENANCY_POSTURE and prescribes every way out (ADR-0093 D5's
* shape), instead of arriving bare through a generic error printer;
* • the fix list is generated from the posture vocabulary, so it cannot go
* stale when a posture is added.
*/

import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { TENANCY_POSTURES } from '@objectstack/spec/security';

import Serve, { resolveTenancyPostureOrRefusal } from './serve.js';

/** `packages/cli` — the oclif root the command is loaded against below. */
const CLI_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..');

/**
* `chalk` may or may not emit SGR codes depending on TTY detection.
*
* The escape is written as `\x1b`, never as the byte itself: one raw control
* character makes grep treat the whole file as binary, and a test file nobody's
* `git grep` can find is a test file that stops being maintained (#4890/#5157).
*/
const SGR = /\x1b\[[0-9;]*m/g;
const plain = (s: string) => s.replace(SGR, '');

const TOUCHED = ['OS_TENANCY_POSTURE', 'OS_MULTI_ORG_ENABLED'] as const;
let saved: Record<string, string | undefined> = {};

beforeEach(() => {
saved = Object.fromEntries(TOUCHED.map((k) => [k, process.env[k]]));
for (const k of TOUCHED) delete process.env[k];
});

afterEach(() => {
for (const k of TOUCHED) {
if (saved[k] === undefined) delete process.env[k];
else process.env[k] = saved[k];
}
});

describe('resolveTenancyPostureOrRefusal — accepted values', () => {
it('passes every posture the spec vocabulary declares', () => {
for (const posture of TENANCY_POSTURES) {
process.env.OS_TENANCY_POSTURE = posture;
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture });
}
});

it("keeps the legacy 'multi' spelling normalizing to isolated", () => {
process.env.OS_TENANCY_POSTURE = 'multi';
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'isolated' });
});

it('unset falls back to the OS_MULTI_ORG_ENABLED derivation, not to a refusal', () => {
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'single' });

process.env.OS_MULTI_ORG_ENABLED = 'true';
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'isolated' });
});

it('treats a blank value as unset — a gate that refused it would break `OS_TENANCY_POSTURE=` in a .env', () => {
process.env.OS_TENANCY_POSTURE = ' ';
expect(resolveTenancyPostureOrRefusal()).toEqual({ ok: true, posture: 'single' });
});
});

describe('resolveTenancyPostureOrRefusal — the refusal', () => {
it('REFUSES AS A VALUE, never as a throw — the property the AuthPlugin catch destroyed', () => {
process.env.OS_TENANCY_POSTURE = 'bogus';

// The point of the whole change. `resolveTenancyPosture()` throws here; if
// this wrapper let that escape, any enclosing `try` — and serve has a broad
// one — could turn "refuse to boot" back into a yellow warning, which is
// precisely what shipped. A verdict cannot be caught.
expect(() => resolveTenancyPostureOrRefusal()).not.toThrow();

const verdict = resolveTenancyPostureOrRefusal();
expect(verdict.ok).toBe(false);
});

it('names the fact: FATAL, the variable, and the value the operator actually typed', () => {
process.env.OS_TENANCY_POSTURE = 'islolated'; // a real transposition typo
const verdict = resolveTenancyPostureOrRefusal();
if (verdict.ok) throw new Error('expected a refusal');

const text = plain(verdict.fatal);
expect(text).toContain('FATAL');
expect(text).toContain('OS_TENANCY_POSTURE="islolated"');
expect(text).toContain('Refusing to boot');

// Not an AuthPlugin problem, not a plugin problem at all. The misattribution
// is the defect; the word must not reappear in the refusal.
expect(text).not.toContain('AuthPlugin');
});

it('prescribes a way out for EVERY posture the vocabulary declares (drift guard)', () => {
process.env.OS_TENANCY_POSTURE = 'bogus';
const verdict = resolveTenancyPostureOrRefusal();
if (verdict.ok) throw new Error('expected a refusal');

const text = plain(verdict.fatal);
// Generated from TENANCY_POSTURES rather than restated, so a posture added
// to the spec cannot leave this advice quietly incomplete.
for (const posture of TENANCY_POSTURES) {
expect(text).toContain(`set OS_TENANCY_POSTURE=${posture}`);
}
// …plus the escape the enumeration cannot express.
expect(text).toContain('unset OS_TENANCY_POSTURE');
expect(text).toContain('OS_MULTI_ORG_ENABLED');
});

it('points at .env files, the source a shell-only search misses', () => {
process.env.OS_TENANCY_POSTURE = 'bogus';
const verdict = resolveTenancyPostureOrRefusal();
if (verdict.ok) throw new Error('expected a refusal');

// The gate is deliberately placed AFTER dotenv-flow's load, so a value from
// a committed `.env*` reaches it. Saying so is what stops the next operator
// grepping only their shell profile.
expect(plain(verdict.fatal)).toContain('.env');
});

it('carries the resolver\'s own sentence as `cause` rather than paraphrasing it', () => {
process.env.OS_TENANCY_POSTURE = 'bogus';
const verdict = resolveTenancyPostureOrRefusal();
if (verdict.ok) throw new Error('expected a refusal');

// `@objectstack/types` owns the vocabulary and its wording; serve must not
// maintain a second copy that can disagree with it.
expect(plain(verdict.fatal)).toContain('cause: Invalid OS_TENANCY_POSTURE="bogus"');
});

it('states that nothing was loaded and nothing was served', () => {
process.env.OS_TENANCY_POSTURE = 'bogus';
const verdict = resolveTenancyPostureOrRefusal();
if (verdict.ok) throw new Error('expected a refusal');

const text = plain(verdict.fatal);
// Only honest because the gate runs at the top of `run()` — the ordering
// test below is what keeps these two sentences true.
expect(text).toContain('No config has been loaded');
expect(text).toContain('the HTTP server was\n never started');

// And deliberately NOT the stronger "no port has been bound": serve probes
// port availability (bind + close) just above the gate. Overclaiming by one
// word is how a diagnostic stops being trustworthy.
expect(text).not.toContain('no port has been bound');
});
});

describe('the gate runs before serve does ANY boot work', () => {
/**
* The ordering assertion, run against the real `serve` command in-process.
*
* This is the one that would have caught #5359. Before the fix, an invalid
* posture got as far as `Loading objectstack.config.ts…`, the whole plugin
* slate, a persisted dev crypto key and a degraded kernel bootstrap before
* anything refused. After it, `run()` reaches the gate and stops: the FATAL
* is the only thing written, and `console.log` — which is where every
* subsequent boot step reports — is never touched at all.
*
* Note what is deliberately NOT asserted: "the port never listened". That was
* true before the fix too (the escaping throw aborted kernel Phase 1, while
* the socket only opens in Phase 4), so asserting it would pass for reasons
* having nothing to do with this change. "No boot work happened at all" is
* strictly stronger and actually moved.
*/
it('refuses before the config file is even read, and writes nothing else', async () => {
const savedNodeEnv = process.env.NODE_ENV;
process.env.OS_TENANCY_POSTURE = 'bogus';

const errors: string[] = [];
const logs: string[] = [];
const errSpy = vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => {
errors.push(a.join(' '));
});
const logSpy = vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => {
logs.push(a.join(' '));
});
// The gate exits the PROCESS on purpose (a throw is what the broad
// AuthPlugin catch used to swallow). Convert it to something catchable so
// the test runner survives, and assert it was reached.
const exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => {
throw new Error(`__PROCESS_EXIT__:${code}`);
}) as never);

let raised: unknown;
try {
// `--dev` keeps the port-availability probe on the auto-shift path so a
// busy port in CI cannot pre-empt the gate we are measuring.
await Serve.run(['--dev', '--port', '39871'], { root: CLI_ROOT });
} catch (err) {
raised = err;
} finally {
errSpy.mockRestore();
logSpy.mockRestore();
exitSpy.mockRestore();
if (savedNodeEnv === undefined) delete process.env.NODE_ENV;
else process.env.NODE_ENV = savedNodeEnv;
}

// Refused via process.exit(1), not by throwing into a catchable boot path.
expect((raised as Error | undefined)?.message).toBe('__PROCESS_EXIT__:1');

const stderr = plain(errors.join('\n'));
expect(stderr).toContain('FATAL');
expect(stderr).toContain('OS_TENANCY_POSTURE="bogus"');

// ── The ordering facts ────────────────────────────────────────────────
// serve announces the config load on stdout as its first boot step. It is
// absent, so the gate preceded it — and therefore preceded every plugin
// load, the kernel bootstrap and the listening socket that follow it.
expect(logs.join('\n')).not.toContain('Loading');
// Nothing at all reached stdout, in fact: the refusal is the whole output.
expect(logs).toEqual([]);

// The misattribution that made this issue expensive to diagnose is gone:
// no warning blames a plugin for an environment-variable typo.
expect(stderr).not.toContain('AuthPlugin failed to load');
// 60s, not the 5s default: unlike the ten message-only cases above, this one
// imports and runs the REAL serve command in-process — the whole serve
// module graph plus a port-availability probe. On a lightly-loaded PR shard
// that costs a moment; on the merge queue's full-suite runner, sharing a
// shard with the serve e2e tests (vitest reported import 94.8s / tests 282s
// for that shard), it blew the 5s default and this case timed out — queue
// run 30971902650, which is what took the PR out of the queue. 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.
}, 60_000);
});
Loading
Loading