diff --git a/.changeset/hook-body-gate-reporting-honesty.md b/.changeset/hook-body-gate-reporting-honesty.md new file mode 100644 index 0000000000..5c8e1c6dd5 --- /dev/null +++ b/.changeset/hook-body-gate-reporting-honesty.md @@ -0,0 +1,54 @@ +--- +"@objectstack/cli": patch +--- + +Make the hook-body build gates report only what they establish (#10678). Three +defects, one shape — a gate reporting something it never established. The +enforcement net was never the gap and is unchanged: no forbidden body ever +shipped as `body.source`, and every forbidden or free-identifier hook is still +refused under `--strict-body`, at the same exit codes as before. + +**The default build no longer warn-and-bundles in silence.** A hook body +containing a forbidden pattern made `os build` exit 0 with no output at all: the +extraction failure was recorded in `bodyExtractionWarnings` and then printed +nowhere, so the only way to learn a handler had *not* become a metadata body was +to diff the artifact. The recorded warnings now reach a human — on stdout, +naming the hook and the pattern, with a pointer at `--strict-body` — and in +`--json` under a new `bodyExtractionWarnings` key. That key is separate from +`warnings` on purpose: `warnings` carries author-time rule advisories in the +shape `os validate --json` also reports, and these are a different record +(`{origin, reason}`). It is an empty array on a clean build, so a CI consumer can +read it unconditionally. + +The build still exits 0 in this case. Making a forbidden pattern fatal by default +would change what `os build` accepts and is not part of this change. + +**The `require()` refusal reason now fires on the real authoring path.** A +TypeScript config is loaded through `bundle-require` → esbuild, whose ESM interop +shim rewrites `require('node:os')` to `__require("node:os")` before `String(fn)` +runs — so the `require()`-specific reason could never match, and the refusal +arrived instead as the generic free-identifier message naming `__require`, an +identifier the author never typed. Both spellings now carry the one reason, which +also explains the rewrite. Accept behaviour is unchanged: the body was already +refused, already bundled, at the same exit code; only the wording moved. + +**The `// @capabilities` directive is documented at its real reach.** It is read +off `String(fn)`, and esbuild strips `//` line comments before the handler is ever +a runtime function — so through `os build` it reaches the extractor from no +ordinary authoring shape. Measured on all four: `objectstack.config.ts`, `.js`, +`.mjs`, and a handler imported from a local `./handlers.js` all silently drop it +and ship the inferred capabilities alone. `hook-bodies.mdx` and the extractor +header now say so, and point at `body.capabilities` — data rather than a comment — +as the escape hatch that does survive. Whether the directive should gain a real +authorable surface or be retired is left open. + +The extractor header claimed a forbidden pattern "makes the build **fail** … +no silent fallback"; docs described warn-and-bundle. The code agreed with the +docs, so the header was the outlier and has been rewritten to describe both +outcomes. + +A new `os build`-level test (`hook-body-build-reach.e2e.test.ts`) spawns the real +CLI and pins all three behaviours against the artifact and the shell's exit code. +The existing extractor unit tests could not have caught any of this: they feed raw +JS function literals, which keep their comments and their `require(` spelling +because nothing transformed them. diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index a7e56edbef..557efd6be7 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -253,9 +253,11 @@ If you have a body that genuinely cannot be expressed in L1+L2 (typically: it ne 2. For each inline handler, take its source via `String(fn)` (the callable is already loaded by tsx/esbuild). 3. Run a regex allow-list over the stringified body (see "What the sandbox forbids" above). 4. **Pass:** emit `body: { language: 'js', source: , capabilities: }`. -5. **Forbidden token (default):** extraction fails, a `bodyExtractionWarning` is recorded, and the callable still ships via the back-compat handler-ref bundle — the build does **not** abort. Pass `objectstack compile --strict-body` to turn extraction warnings into a hard build failure (exit 1) with a per-callable diagnostic, e.g. `hook 'normalize_account': fetch() is not allowed in hook/action bodies — declare a Connector recipe instead.` +5. **Forbidden token (default):** extraction fails, a `bodyExtractionWarning` is recorded, and the callable still ships via the back-compat handler-ref bundle — the build does **not** abort. The warning is **printed** (and carried in `--json` under `bodyExtractionWarnings`), so a forbidden pattern is a visible warn-and-bundle rather than a silent success; before #10678 it was recorded and shown to nobody. Pass `objectstack compile --strict-body` to turn extraction warnings into a hard build failure (exit 1) with a per-callable diagnostic, e.g. `hook 'normalize_account': fetch() is not allowed in hook/action bodies — declare a Connector recipe instead.` -Capabilities are inferred by matching known patterns in the body source (e.g. `ctx.api.object(...).insert(...)` ⇒ `api.write`). A fuller AST-based analysis is planned for a later version. You can override with a directive comment when the inference is wrong. + Note that a CommonJS `require('node:os')` in a TypeScript config reaches the extractor as esbuild's `__require("node:os")`. Both spellings are refused under the same `require()` reason. + +Capabilities are inferred by matching known patterns in the body source (e.g. `ctx.api.object(...).insert(...)` ⇒ `api.write`). A fuller AST-based analysis is planned for a later version. When the inference is wrong, supply `body` yourself with an explicit `capabilities` array — the `// @capabilities` directive comment [does not reach the build](#capability-inference). ## Migration @@ -326,6 +328,39 @@ handler: async (ctx) => { } ``` + + The override is read off the handler's **stringified source** (`String(fn)`), so it + only works if the function the CLI holds still carries the comment. Through + `objectstack build`, it never does. + + `loadConfig` runs your config through `bundle-require` → esbuild, and esbuild strips + `//` line comments before the handler is ever a runtime function. The directive is + gone by the time the extractor looks at it. Measured on all four authoring shapes: + + | Authoring shape | `// @capabilities api.write log` reaches the extractor? | + |---|---| + | `objectstack.config.ts` | **No** — comment stripped by esbuild | + | `objectstack.config.js` | **No** — esbuild runs on `.js` too | + | `objectstack.config.mjs` | **No** — same path | + | handler imported from a local `./handlers.js` | **No** — esbuild bundles it as well | + + In every case the build exits 0, prints nothing, and emits the **inferred** + capabilities only. A handler asking for `api.write log` whose body reads + `ctx.api.object('x').find({})` ships `"capabilities": ["api.read"]` — inference won, + silently, and the directive had no effect at all. + + **So do not rely on this directive.** Write the body so the + [inference table](#capability-inference) above derives what you need, or supply + `body` yourself on the hook with an explicit `capabilities` array — that path is + data, not a comment, and survives the build. + + Inference is unaffected: it matches the *code*, which esbuild keeps. Only the + comment-borne override is lost. Tracked in #10678, where the question of whether the + directive should get a real authorable surface (or be retired) is open — this page + documents the reach as measured, and an `objectstack build`-level test pins it so + this page and the extractor cannot drift apart again. + + ### Build pipeline at a glance ``` diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index f930d68cc4..8273b8211e 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -148,6 +148,35 @@ export default class Compile extends Command { } } + // 2c. [#10678] SURFACE the warn-and-bundle. The default (non-`--strict-body`) + // build catches every extraction failure in `lowerCallables`, records it + // in `bodyExtractionWarnings`, ships the callable through the .mjs bundle + // and exits 0. That last part is correct and stays correct — flipping the + // default to a hard failure would change what `os build` ACCEPTS, which is + // not this change. What was wrong is that the recorded warnings reached + // nobody: a hook body containing `fetch()` produced a completely silent + // success, and the only way to learn the handler had NOT become a metadata + // body was to diff the artifact. The data already existed; it just never + // got printed. Advisory only — nothing below may touch the exit code. + // + // The reason strings carry a multi-line `--- offending body source ---` + // dump for `--strict-body`'s per-callable diagnostic; on the default path + // we print the first line and point at the flag for the rest, so the + // default build stays readable while staying honest. + if (lowering.bodyExtractionWarnings.length > 0 && !flags.json) { + const n = lowering.bodyExtractionWarnings.length; + console.log(''); + printWarning( + `${n} handler${n === 1 ? '' : 's'} could not be lowered to a metadata body — ` + + `bundled via the legacy runtime module instead (build still succeeds)`, + ); + for (const w of lowering.bodyExtractionWarnings.slice(0, 20)) { + console.log(` • ${w.origin}: ${String(w.reason).split('\n')[0]}`); + } + if (n > 20) console.log(chalk.dim(` … and ${n - 20} more`)); + console.log(chalk.dim(' → run `os build --strict-body` for the full diagnostic, or to make this fatal')); + } + // 3. Validate the lowered (JSON-safe) stack against the Protocol. if (!flags.json) printStep('Validating protocol compliance...'); const result = ObjectStackDefinitionSchema.safeParse(lowering.lowered); @@ -435,6 +464,15 @@ export default class Compile extends Command { // reports. This key used to carry the widget rule's warnings alone — // one gate out of the twenty-odd that raise them. warnings: ruleAdvisories, + // [#10678] Body-extraction failures that made a callable fall back to + // the legacy .mjs bundle. A SEPARATE key on purpose: `warnings` above + // is author-time RULE advisories (`{where,message,rule,path,hint}`) + // and is the shape `os validate --json` shares — folding a different + // record shape (`{origin,reason}`) into it would break that contract + // for every consumer that reads one shape from either command. Empty + // array when every callable lowered cleanly, so a CI consumer can read + // the key unconditionally. + bodyExtractionWarnings: lowering.bodyExtractionWarnings, // Same key `os validate --json` uses, so a CI consumer reads one shape // from either command rather than learning two. conversions: conversionNotices, @@ -451,6 +489,13 @@ export default class Compile extends Command { if (ruleAdvisories.length > 0) { printWarning(`${ruleAdvisories.length} author-time warning(s) — see above`); } + if (lowering.bodyExtractionWarnings.length > 0) { + // [#10678] Repeat the tally in the summary: the detail printed before the + // parse, and a long build scrolls it away. + printWarning( + `${lowering.bodyExtractionWarnings.length} handler(s) bundled instead of lowered to a metadata body — see above`, + ); + } console.log(''); printMetadataStats(stats); console.log(''); diff --git a/packages/cli/src/utils/extract-hook-body.ts b/packages/cli/src/utils/extract-hook-body.ts index 1e05dd68ec..7f10ce951f 100644 --- a/packages/cli/src/utils/extract-hook-body.ts +++ b/packages/cli/src/utils/extract-hook-body.ts @@ -13,14 +13,43 @@ * * For v1 we apply a deliberately simple **regex allow-list** over the * extracted body — full TypeScript AST analysis is deferred to v2. Anything - * the regex rejects (top-level `import`, `require(`, `fetch(`, `process.*`, - * `globalThis.*`, `eval`, `new Function`) makes the build **fail**. There is - * no silent fallback to the L3 .mjs path because that path is being closed. + * the regex rejects (top-level `import`, `require(` / esbuild's `__require(`, + * `fetch(`, `process.*`, `globalThis.*`, `eval`, `new Function`) makes + * extraction **throw**. + * + * ⚠️ What that throw costs the BUILD depends on the flag, and the two outcomes + * are not the same one. This header used to claim only the second (#10678): + * + * default `os build` {@link lowerCallables} catches the throw, records it in + * `bodyExtractionWarnings`, and ships the callable through + * the back-compat `.mjs` bundle instead. No forbidden body + * is ever emitted as `body.source` — but the build exits + * **0**. `compile.ts` prints the recorded warnings, so + * warn-and-bundle is at least not silent; it was silent + * until #10678, which is the whole defect that card names. + * `--strict-body` the same recorded warnings become a hard failure (exit 1) + * with a per-callable diagnostic, and nothing is bundled. + * + * So the allow-list gates what may become `body.source`; it does not (yet) gate + * what may build. Closing the L3 `.mjs` path is `--strict-body`'s job today and + * Phase 3's later — not this list's. Docs `hook-bodies.mdx` describe the same + * two outcomes; when this header and that page disagree, they are both wrong + * until one of them is measured over a real `os build`. * * Capability inference: we scan the body for known `ctx.api.*`, `ctx.log.*`, * `ctx.crypto.*` access patterns and add the matching capability tokens to - * `body.capabilities` automatically. Authors can still override by setting - * `// @capabilities api.read api.write` as the first line of the function. + * `body.capabilities` automatically. + * + * ⚠️ REACH of the `// @capabilities api.read api.write` override (#10678): it is + * read off `String(fn)`, so it survives only when the LOADED config still has + * the comment. A TypeScript `objectstack.config.ts` does not — `loadConfig` + * runs it through `bundle-require` -> esbuild, which strips `//` line comments + * before the handler is ever a runtime function — so through `os build` the + * directive reaches this code from **pre-bundled JS that preserved its comments + * only**. Every unit test below feeds a raw JS function literal and therefore + * cannot see that: they are why the override read as working for so long. The + * real reach is measured over a spawned `os build` in + * `test/hook-body-build-reach.e2e.test.ts` — change the reach, change that test. * * Self-containment (#1876): a handler that references a module-scope identifier * (helper, import, top-level const) cannot be shipped body-only — the reference @@ -32,7 +61,16 @@ import { detectFreeIdentifiers } from './detect-free-identifiers.js'; const FORBIDDEN_PATTERNS: Array<{ rx: RegExp; reason: string }> = [ { rx: /\bimport\s*[\(\*\{]/, reason: 'dynamic `import()` and ES imports are not allowed in hook/action bodies — declare a Connector recipe instead' }, - { rx: /\brequire\s*\(/, reason: '`require()` is not allowed in hook/action bodies' }, + // Both spellings, one reason (#10678). A TypeScript config is loaded through + // `bundle-require` -> esbuild, whose ESM interop shim rewrites a CommonJS + // `require('node:os')` into `__require("node:os")` BEFORE `String(fn)` ever + // runs. Matching only the source spelling made this reason UNREACHABLE from + // the real authoring path: the refusal still fired, but through the #1876 + // free-identifier gate, naming `__require` — an identifier the author never + // typed and cannot act on. Accept behaviour is unchanged either way (the body + // was already refused); what changes is that the reason now names what was + // written. `\b(?:__)?` cannot widen to `myrequire(` — no word boundary there. + { rx: /\b(?:__)?require\s*\(/, reason: '`require()` is not allowed in hook/action bodies (esbuild rewrites it to `__require()` when the config is TypeScript; both spellings are refused)' }, { rx: /\bfetch\s*\(/, reason: '`fetch()` is not allowed in hook/action bodies — declare a Connector recipe instead' }, { rx: /\bprocess\s*\./, reason: '`process` access is not allowed in hook/action bodies' }, { rx: /\bglobalThis\s*\./, reason: '`globalThis` access is not allowed in hook/action bodies' }, @@ -59,7 +97,11 @@ const CAPABILITY_PATTERNS: Array<{ rx: RegExp; cap: 'api.read' | 'api.write' | ' export interface ExtractedBody { /** Pure function-body source (without the surrounding `(ctx) => {...}`). */ source: string; - /** Inferred capability tokens — may be merged with explicit `// @capabilities` line. */ + /** + * Inferred capability tokens — merged with an explicit `// @capabilities` + * line when one survives into `String(fn)`. See the REACH note in this + * file's header: through `os build` on a TS config, it does not. + */ capabilities: Array<'api.read' | 'api.write' | 'crypto.uuid' | 'log'>; /** True when source is a single expression (arrow with implicit return). */ isExpression: boolean; @@ -115,6 +157,8 @@ export function extractHookBody(fn: (...a: unknown[]) => unknown, originLabel: s } // Honour an explicit override: `// @capabilities api.read api.write`. + // Reachable only when the caller handed us a function whose source still + // carries `//` comments — see the REACH note in this file's header (#10678). const overrideMatch = block.source.match(/^[\s\n]*\/\/\s*@capabilities\s+([a-z.\s]+)/m); if (overrideMatch) { const tokens = overrideMatch[1].split(/\s+/).filter(Boolean); diff --git a/packages/cli/test/extract-hook-body.test.ts b/packages/cli/test/extract-hook-body.test.ts index 1693703749..82097d2dcc 100644 --- a/packages/cli/test/extract-hook-body.test.ts +++ b/packages/cli/test/extract-hook-body.test.ts @@ -69,6 +69,34 @@ describe('extractHookBody', () => { expect(() => extractHookBody(fn, 'hook bad')).toThrow(/fetch/); }); + // #10678 — BOTH spellings, one reason. A TS config never reaches the + // extractor spelling `require(`: `loadConfig` runs it through bundle-require + // -> esbuild, whose ESM interop shim rewrites it to `__require("node:os")` + // first. Matching only the source spelling left the promised require()-reason + // unreachable from the real authoring path — the refusal still fired, but as + // the generic #1876 free-identifier message naming an identifier the author + // never typed. These two cases are written with the call built at runtime so + // the test file itself is not rewritten by its own bundler. + it('rejects require() — the spelling the author writes', () => { + const fn = new Function('ctx', "const os = require('node:os'); return os;") as (...a: unknown[]) => unknown; + expect(() => extractHookBody(fn, 'hook bad')).toThrow(/`require\(\)` is not allowed/); + }); + + it('rejects __require() — the spelling esbuild leaves behind (#10678)', () => { + const fn = new Function('ctx', 'const os = __require("node:os"); return os;') as (...a: unknown[]) => unknown; + // The require()-specific reason, NOT the free-identifier fallback. If this + // ever reads "not in scope at runtime" again, the reason went unreachable. + expect(() => extractHookBody(fn, 'hook bad')).toThrow(/`require\(\)` is not allowed/); + expect(() => extractHookBody(fn, 'hook bad')).not.toThrow(/not in scope at runtime/); + }); + + it('does NOT widen to an identifier merely ending in `require` (#10678)', () => { + // `\b(?:__)?require\s*\(` has no word boundary inside `myrequire`, so the + // pattern cannot swallow an author's own helper. Guards the widening. + const fn = new Function('ctx', 'return ctx.myrequire ? 1 : 0;') as (...a: unknown[]) => unknown; + expect(() => extractHookBody(fn, 'hook ok')).not.toThrow(); + }); + it('rejects process access', () => { const fn = (_ctx: any) => { const env = (process as any).env.X; diff --git a/packages/cli/test/hook-body-build-reach.e2e.test.ts b/packages/cli/test/hook-body-build-reach.e2e.test.ts new file mode 100644 index 0000000000..94817794cd --- /dev/null +++ b/packages/cli/test/hook-body-build-reach.e2e.test.ts @@ -0,0 +1,280 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #10678 — the hook-body build gates, measured over a REAL `os build`. + * + * All three defects this file pins are one shape: **a gate reporting something + * it never established.** The enforcement net held throughout — no forbidden + * body ever shipped as `body.source`, and every forbidden/free-identifier hook + * was still refused under `--strict-body`. What was wrong was the reporting and + * the reachability. + * + * ⚠️ WHY THIS FILE SPAWNS THE CLI INSTEAD OF CALLING THE EXTRACTOR + * + * `test/extract-hook-body.test.ts` feeds `extractHookBody` raw JS function + * literals. Those literals keep their `//` comments, because nothing + * transformed them — so the `@capabilities` override tests there pass, and have + * always passed, while the override has never once worked through `os build`. + * A unit test over the extractor alone would restate exactly the false + * confidence this card is about. The only way to establish what the real + * authoring path does is to run it, so every test here spawns the actual CLI + * (`bin/run-dev.js` + tsx against a `mkdtemp` project — the + * `validate-top-level-strict.e2e.test.ts` pattern) and reads the artifact the + * shell was left holding. + * + * ⛔ These tests pin REACH, not endorsement. `@capabilities` being dead through + * `os build` is the measured state, not a decision that it should stay dead — + * whether the directive gets a real authorable surface or is retired is a + * maintainer call on the published surface (#10678). If that call lands, the + * capability assertions below must be REWRITTEN, not deleted, and + * `content/docs/automation/hook-bodies.mdx` must move with them. That coupling + * is the point of the file: docs and extractor cannot drift apart silently + * again. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], cwd: string): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...args], + { cwd, maxBuffer: 16 * 1024 * 1024, env: { ...process.env, NO_COLOR: '1' } }, + (err, stdout, stderr) => { + resolvePromise({ + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +function artifact(dir: string): any { + return JSON.parse(readFileSync(join(dir, 'dist', 'objectstack.json'), 'utf8')); +} + +const OBJECT = `{ + name: 'hb_ticket', + label: 'Ticket', + sharingModel: 'private', + fields: { title: { type: 'text', label: 'Title' } }, + }`; + +/** + * DEFECT 1 fixture. The handler asks for `api.write log` via the directive AND + * contains a `.find(...)` call that inference reads as `api.read`. Both halves + * matter: the inferred token proves the extractor really ran on this body (an + * assertion of `capabilities: []` alone would also pass if the hook had never + * got a body at all), and the absent directive tokens are the defect. + */ +const CONFIG_CAPABILITIES_DIRECTIVE = ` +export default { + manifest: { id: 'com.example.hbcaps', name: 'hbcaps', version: '1.0.0', type: 'app' }, + objects: [${OBJECT}], + hooks: [{ + name: 'hb_directive', + object: 'hb_ticket', + events: ['beforeInsert'], + handler: async (ctx: any) => { + // @capabilities api.write log + const rows = await ctx.api.object('hb_ticket').find({}); + return rows; + }, + }], +}; +`; + +/** The escape hatch the docs now point at: `body.capabilities` is DATA, not a comment. */ +const CONFIG_EXPLICIT_BODY = ` +export default { + manifest: { id: 'com.example.hbbody', name: 'hbbody', version: '1.0.0', type: 'app' }, + objects: [${OBJECT}], + hooks: [{ + name: 'hb_explicit', + object: 'hb_ticket', + events: ['beforeInsert'], + body: { language: 'js', source: 'return ctx;', capabilities: ['api.write', 'log'] }, + }], +}; +`; + +/** DEFECT 2 fixture: a CommonJS `require()` esbuild rewrites to `__require`. */ +const CONFIG_REQUIRE = ` +export default { + manifest: { id: 'com.example.hbreq', name: 'hbreq', version: '1.0.0', type: 'app' }, + objects: [${OBJECT}], + hooks: [{ + name: 'hb_require', + object: 'hb_ticket', + events: ['beforeInsert'], + handler: async (ctx: any) => { + const os = require('node:os'); + return os.platform(); + }, + }], +}; +`; + +/** DEFECT 3 fixture: a forbidden pattern on the DEFAULT (warn-and-bundle) path. */ +const CONFIG_FORBIDDEN = ` +export default { + manifest: { id: 'com.example.hbforbid', name: 'hbforbid', version: '1.0.0', type: 'app' }, + objects: [${OBJECT}], + hooks: [{ + name: 'hb_forbidden', + object: 'hb_ticket', + events: ['beforeInsert'], + handler: async (ctx: any) => { + await fetch('https://example.com/x'); + return ctx; + }, + }], +}; +`; + +const dirs: Record = {}; + +function project(key: string, source: string): string { + const dir = mkdtempSync(join(tmpdir(), `os-hookbody-${key}-`)); + writeFileSync(join(dir, 'objectstack.config.ts'), source); + dirs[key] = dir; + return dir; +} + +beforeAll(() => { + project('caps', CONFIG_CAPABILITIES_DIRECTIVE); + project('body', CONFIG_EXPLICIT_BODY); + project('req', CONFIG_REQUIRE); + project('forbid', CONFIG_FORBIDDEN); +}); + +afterAll(() => { + for (const dir of Object.values(dirs)) rmSync(dir, { recursive: true, force: true }); +}); + +describe('#10678 defect 1 — `// @capabilities` reach through `os build`', () => { + it('the directive does NOT reach the extractor; only inference lands', async () => { + const run = await runCli(['build'], dirs.caps); + expect(run.code, `stdout:\n${run.stdout}\nstderr:\n${run.stderr}`).toBe(0); + + const hook = artifact(dirs.caps).hooks[0]; + + // The extractor DID run on this body — proves the test is not vacuous. + expect(hook.body).toBeTruthy(); + expect(hook.body.language).toBe('js'); + expect(hook.body.source).toContain('.find('); + + // esbuild stripped the `//` line before `String(fn)` ever saw it. + expect(hook.body.source).not.toContain('@capabilities'); + + // Inference won; the directive contributed nothing. `api.read` comes from + // `.object(...).find(...)`; `api.write` and `log` are what the directive + // asked for and did not get. + expect(hook.body.capabilities).toEqual(['api.read']); + expect(hook.body.capabilities).not.toContain('api.write'); + expect(hook.body.capabilities).not.toContain('log'); + }, 120_000); + + it('and says nothing about it — no warning, no error, exit 0', async () => { + const run = await runCli(['build', '--json'], dirs.caps); + expect(run.code).toBe(0); + const json = JSON.parse(run.stdout); + expect(json.success).toBe(true); + // Control for defect 3's key: a cleanly-lowered build reports an EMPTY + // array, not a missing key, so a CI consumer can read it unconditionally. + expect(json.bodyExtractionWarnings).toEqual([]); + }, 120_000); + + it('the documented escape hatch works: an explicit `body.capabilities` survives', async () => { + const run = await runCli(['build'], dirs.body); + expect(run.code, `stdout:\n${run.stdout}\nstderr:\n${run.stderr}`).toBe(0); + const hook = artifact(dirs.body).hooks[0]; + expect(hook.body.capabilities).toEqual(['api.write', 'log']); + }, 120_000); +}); + +describe('#10678 defect 2 — the `require()` reason fires on the real authoring path', () => { + it('--strict-body names require(), not the free identifier `__require`', async () => { + const run = await runCli(['build', '--strict-body'], dirs.req); + const out = run.stdout + run.stderr; + expect(run.code, `expected exit 1; out:\n${out}`).toBe(1); + + // The promised require()-specific reason, on a TS config — the spelling the + // author actually wrote. + expect(out).toContain('`require()` is not allowed in hook/action bodies'); + // And it explains the rewrite, so `__require` in the dumped source is not a + // mystery identifier the author never typed. + expect(out).toContain('__require'); + }, 120_000); + + it('accept behaviour is UNCHANGED: the default build still warn-and-bundles at exit 0', async () => { + // The reason string is all that moved. Before the fix this body was already + // refused (via the #1876 free-identifier gate) and already bundled; it must + // still be refused and still bundled, at the same exit code. A gate that got + // STRICTER here would be a change to what `os build` accepts, which this + // card explicitly does not authorise. + const run = await runCli(['build'], dirs.req); + expect(run.code, `stdout:\n${run.stdout}\nstderr:\n${run.stderr}`).toBe(0); + const hook = artifact(dirs.req).hooks[0]; + expect(hook.body).toBeUndefined(); + expect(typeof hook.handler).toBe('string'); + expect(artifact(dirs.req).runtimeModule).toMatch(/^\.\/objectstack-runtime\./); + }, 120_000); +}); + +describe('#10678 defect 3 — the default build no longer warn-and-bundles in silence', () => { + it('prints the recorded warning, naming the hook and the forbidden pattern', async () => { + const run = await runCli(['build'], dirs.forbid); + const out = run.stdout + run.stderr; + + // Still exit 0 — surfacing the warning must not change what `os build` + // accepts. Flipping this to a hard failure is a maintainer decision. + expect(run.code, `expected exit 0; out:\n${out}`).toBe(0); + + expect(out).toContain("hook 'hb_forbidden'"); + expect(out).toContain('`fetch()` is not allowed in hook/action bodies'); + expect(out).toContain('could not be lowered to a metadata body'); + // The pointer at the flag that makes it fatal. + expect(out).toContain('--strict-body'); + + // And the body genuinely did not ship as metadata — the enforcement half, + // which was never broken, still holds. + const hook = artifact(dirs.forbid).hooks[0]; + expect(hook.body).toBeUndefined(); + }, 120_000); + + it('carries the warnings in --json under `bodyExtractionWarnings`', async () => { + const run = await runCli(['build', '--json'], dirs.forbid); + expect(run.code).toBe(0); + const json = JSON.parse(run.stdout); + expect(json.success).toBe(true); + expect(json.bodyExtractionWarnings).toHaveLength(1); + expect(json.bodyExtractionWarnings[0].origin).toBe("hook 'hb_forbidden'"); + expect(json.bodyExtractionWarnings[0].reason).toContain('`fetch()` is not allowed'); + + // A SEPARATE key from `warnings`, which is the author-time rule advisory + // set `os validate --json` also reports. Folding these in would have broken + // that shared shape for every consumer. + expect(Array.isArray(json.warnings)).toBe(true); + expect(json.warnings).not.toContainEqual( + expect.objectContaining({ origin: "hook 'hb_forbidden'" }), + ); + }, 120_000); +});