diff --git a/.changeset/hook-body-write-set-lint.md b/.changeset/hook-body-write-set-lint.md new file mode 100644 index 0000000000..dc689c7ee2 --- /dev/null +++ b/.changeset/hook-body-write-set-lint.md @@ -0,0 +1,51 @@ +--- +"@objectstack/lint": minor +"@objectstack/spec": patch +--- + +feat(lint): L2 hook-body writes to undeclared fields warn at author time (#4271) + +An L2 (`language:'js'`) hook body that writes a field the target object never +declares — `ctx.input.amout = 0`, `ctx.api.object('deal').update({ stag: … })` +— runs clean in the QuickJS sandbox, reports success, and the unknown column +simply never lands in the stored record. No diagnostic anywhere: the #4001 +"silent no-op manufactures false completion" failure mode at the +runtime-expression layer. The read side (`hook.condition`) and the capability +surface were already statically checked; the write side was the one blind face, +and `hook-body.zod.ts` carried it as an **accepted gap**. + +**New rule — `hook-body-write-unknown-field` (advisory).** `@objectstack/lint` +now parses each L2 body (TypeScript parser; parsed, never executed, never +type-checked) and resolves its literal writes against the target object's +declared + system fields. An unknown field warns with a did-you-mean. Wired +into `REFERENCE_INTEGRITY_RULES`, so `os validate`, `os lint` and `os compile` +all report it; it never blocks a build. + +The recognized write shapes are declared as data — `HOOK_BODY_WRITE_PATTERNS`, +each entry carrying a canonical example that a reconciliation test round-trips +through the real extractor, so a pattern cannot be declared-but-unverified +(#3528's death). v1 ships three: + +- `ctx.input. = …` / `ctx.input[''] ⟨op⟩= …` → the hook's own + target object(s); flat-input envelope keys (`id`/`options`/`ast`/`data`) are + never treated as record fields. +- `Object.assign(ctx.input, { : … })` → same target. +- `ctx.api.object('').insert|create|update({…})` / `.updateById(id, {…})` + → the named object, at the **real** `ObjectRepository` payload positions + (`update(data)` — the payload is argument 0, not `update(id, data)`). + +Everything statically unknowable is skipped silently, favouring missed findings +over false ones: computed keys, spreads, non-literal payloads, dynamic object +names, wildcard-target (`object:'*'`) input writes, cross-package targets, +aliased input (`const doc = ctx.input`), and multi-target hooks where the field +exists on *some* target (the body may branch per object — only an +everywhere-miss warns). + +The lint stays off the kernel boot path: the TypeScript compiler loads lazily, +only when a hook actually carries a JS body (same contract as the react-page +gates, guarded by `lazy-deps.test.ts`). + +`@objectstack/spec`: the `ScriptBodySchema` header's "write-set opacity — +accepted static-analysis gap" note now points at the lint instead, and spells +out what remains opaque so the warning's absence is not read as proof of +correctness. diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 1d0685ec8c..210be51a35 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -249,6 +249,19 @@ export type { AiAgentAuthoringSeverity, } from './validate-ai-agent-authoring.js'; +export { + validateHookBodyWrites, + extractHookBodyWrites, + HOOK_BODY_WRITE_PATTERNS, + HOOK_BODY_WRITE_UNKNOWN_FIELD, +} from './validate-hook-body-writes.js'; +export type { + HookBodyWriteFinding, + HookBodyWriteSeverity, + HookBodyWritePattern, + ExtractedHookBodyWrite, +} from './validate-hook-body-writes.js'; + // One entry point for the reference-resolution rules above (#3583 §5 D5). // Adding a rule to `REFERENCE_INTEGRITY_RULES` runs it on `validate`, `lint` // and `compile` at once — the CLI call sites do not change. diff --git a/packages/lint/src/lazy-deps.test.ts b/packages/lint/src/lazy-deps.test.ts index 702eacd260..ad817b1dcb 100644 --- a/packages/lint/src/lazy-deps.test.ts +++ b/packages/lint/src/lazy-deps.test.ts @@ -73,10 +73,16 @@ describe('lazy dependency loading (kernel boot-path contract)', () => { for (const dep of ${JSON.stringify(LAZY_DEPS)}) { if (loaded(dep)) fail(dep + ' was loaded eagerly, at import time'); } + const jsHook = (source, language) => ({ objects: [{ name: 'a', fields: { amount: {} } }], hooks: [{ name: 'h', object: 'a', events: ['beforeInsert'], body: { language: language ?? 'js', source } }] }); + mod.validateHookBodyWrites(jsHook('input.x > 0', 'expression')); + if (loaded('typescript')) fail('the hook-body write gate on an L1-only stack must not load typescript'); const syntax = mod.validateReactPages(${reactStack('function Page(){ return
oops; }')}); if (!loaded('sucrase')) fail('sucrase was not loaded by a react-page syntax validation'); if (loaded('typescript')) fail('the syntax gate must not load typescript'); if (!syntax.some((f) => f.rule === 'react-page-syntax')) fail('syntax gate produced no finding'); + const hookWrites = mod.validateHookBodyWrites(jsHook('ctx.input.amout = 1;')); + if (!loaded('typescript')) fail('typescript was not loaded by an L2 hook-body write validation'); + if (!hookWrites.some((f) => f.rule === 'hook-body-write-unknown-field')) fail('hook-body write gate produced no finding'); const props = mod.validateReactPageProps(${reactStack('function Page(){ return ; }')}); if (!loaded('typescript')) fail('typescript was not loaded by a react-page props validation'); if (!props.some((f) => f.rule === 'react-prop-missing-required')) fail('props gate produced no finding'); @@ -111,14 +117,20 @@ describe('lazy dependency loading (kernel boot-path contract)', () => { it('loads each dep lazily in-process and the gates still work', async () => { const req = createRequire(import.meta.url); - const { validateReactPages, validateReactPageProps } = await import('./index.js'); + const { validateReactPages, validateReactPageProps, validateHookBodyWrites } = await import('./index.js'); // Stacks without a react-source page never touch either dep. expect(validateReactPages({ pages: [{ name: 'p', kind: 'object' }] })).toEqual([]); expect(validateReactPageProps({ pages: [{ name: 'p', kind: 'object' }] })).toEqual([]); expect(validateReactPageProps({ pages: [{ name: 'r', kind: 'react', source: ' ' }] })).toEqual([]); + // Nor do stacks whose hooks carry no L2 JS body — including a JS body that + // never mentions `ctx`/`Object` (the prefilter skips the parse entirely). + const hook = (body: unknown) => ({ name: 'h', object: 'a', events: ['beforeInsert'], body }); + expect(validateHookBodyWrites({ hooks: [hook(undefined)] })).toEqual([]); + expect(validateHookBodyWrites({ hooks: [hook({ language: 'expression', source: 'input.x > 0' })] })).toEqual([]); + expect(validateHookBodyWrites({ hooks: [hook({ language: 'js', source: 'return 1;' })] })).toEqual([]); for (const dep of LAZY_DEPS) { - expect(depLoaded(req.cache, dep), `${dep} loaded before any react-source validation`).toBe(false); + expect(depLoaded(req.cache, dep), `${dep} loaded before any react-source or L2-body validation`).toBe(false); } // The first react page with source pays the cost of exactly its own gate's @@ -130,6 +142,15 @@ describe('lazy dependency loading (kernel boot-path contract)', () => { expect(depLoaded(req.cache, 'typescript'), 'the syntax gate must not load typescript').toBe(false); expect(syntax.some((f) => f.rule === 'react-page-syntax' && f.severity === 'error')).toBe(true); + // The first hook with an L2 JS body pays the typescript load — and the + // write-set gate works (#4271). + const hookWrites = validateHookBodyWrites({ + objects: [{ name: 'a', fields: { amount: {} } }], + hooks: [hook({ language: 'js', source: 'ctx.input.amout = 1;' })], + }); + expect(depLoaded(req.cache, 'typescript')).toBe(true); + expect(hookWrites.some((f) => f.rule === 'hook-body-write-unknown-field' && f.severity === 'warning')).toBe(true); + const props = validateReactPageProps({ pages: [{ name: 'r', kind: 'react', source: 'function Page(){ return ; }' }], }); diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts index 8939c703a5..2d1548343c 100644 --- a/packages/lint/src/reference-integrity-suite.test.ts +++ b/packages/lint/src/reference-integrity-suite.test.ts @@ -25,6 +25,7 @@ describe('reference-integrity suite — membership', () => { 'validateAiSurfaceAffinity', 'validateAiToolReferences', 'validateAiAgentAuthoring', + 'validateHookBodyWrites', ]); }); @@ -118,6 +119,16 @@ describe('reference-integrity suite — every member actually runs', () => { // validateAiToolReferences: a tool name nothing declares, registers, or // materialises (the HotCRM fictional-tool class). skills: [{ name: 'metadata_authoring', surface: 'build', tools: ['forecast_revenue'] }], + hooks: [ + // validateHookBodyWrites: the L2 body writes a field crm_lead does not + // declare — runs clean in the sandbox, never lands in the record (#4271). + { + name: 'score_lead', + object: 'crm_lead', + events: ['beforeInsert'], + body: { language: 'js', source: "ctx.input.lead_score = 100;" }, + }, + ], flows: [ { name: 'lead_followup', @@ -153,6 +164,7 @@ describe('reference-integrity suite — every member actually runs', () => { expect(rules).toContain('ai-skill-surface-mismatch'); expect(rules).toContain('ai-skill-tool-unresolved'); expect(rules).toContain('agent-authoring-withdrawn'); + expect(rules).toContain('hook-body-write-unknown-field'); }); it('carries a gating flow-template finding through the suite (#3810)', () => { @@ -174,9 +186,9 @@ describe('reference-integrity suite — every member actually runs', () => { expect(typeof f.message).toBe('string'); expect(typeof f.hint).toBe('string'); } - // Object references run first, agent-authoring last. + // Object references run first, hook-body writes last. expect(findings[0].rule).toBe('object-reference-unknown'); - expect(findings[findings.length - 1].rule).toBe('agent-authoring-withdrawn'); + expect(findings[findings.length - 1].rule).toBe('hook-body-write-unknown-field'); }); it('returns nothing for an empty stack', () => { diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts index 79840b9aad..04be6ddb48 100644 --- a/packages/lint/src/reference-integrity-suite.ts +++ b/packages/lint/src/reference-integrity-suite.ts @@ -55,6 +55,7 @@ import { validateFlowTemplatePaths } from './validate-flow-template-paths.js'; import { validateAiSurfaceAffinity } from './validate-ai-surface-affinity.js'; import { validateAiToolReferences } from './validate-ai-tool-references.js'; import { validateAiAgentAuthoring } from './validate-ai-agent-authoring.js'; +import { validateHookBodyWrites } from './validate-hook-body-writes.js'; export type ReferenceIntegritySeverity = 'error' | 'warning'; @@ -100,6 +101,12 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ { name: 'validateAiSurfaceAffinity', run: validateAiSurfaceAffinity }, { name: 'validateAiToolReferences', run: validateAiToolReferences }, { name: 'validateAiAgentAuthoring', run: validateAiAgentAuthoring }, + // Field names WRITTEN by an L2 hook body (`ctx.input.x = …`, + // `ctx.api.object('y').update({ x })`), resolved against the target object's + // declared fields — the write-side counterpart of validateFlowTemplatePaths' + // read-side membership (#4271). Lazy: only a hook that actually carries a + // `language:'js'` body loads the TypeScript parser. + { name: 'validateHookBodyWrites', run: validateHookBodyWrites }, ]; /** diff --git a/packages/lint/src/validate-hook-body-writes.test.ts b/packages/lint/src/validate-hook-body-writes.test.ts new file mode 100644 index 0000000000..5c6f3b8164 --- /dev/null +++ b/packages/lint/src/validate-hook-body-writes.test.ts @@ -0,0 +1,241 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + validateHookBodyWrites, + extractHookBodyWrites, + HOOK_BODY_WRITE_PATTERNS, + HOOK_BODY_WRITE_UNKNOWN_FIELD, +} from './validate-hook-body-writes.js'; + +// Target objects: array-shaped and map-shaped `fields`, plus a second object +// for cross-object `ctx.api` writes and multi-target hooks. +const dealObject = { + name: 'crm_deal', + fields: [ + { name: 'stage', type: 'text' }, + { name: 'amount', type: 'currency' }, + { name: 'discount_total', type: 'currency' }, + ], +}; +const contactObject = { + name: 'crm_contact', + fields: { + email: { type: 'text' }, + amount: { type: 'currency' }, // shared with crm_deal, for partial-miss cases + }, +}; + +/** A stack with a single JS-body hook (hooks[0]) over `source`. */ +function stackWith(source: string, hookOverrides: Record = {}) { + return { + objects: [dealObject, contactObject], + hooks: [ + { + name: 'normalize_deal', + object: 'crm_deal', + events: ['beforeInsert'], + body: { language: 'js', source }, + ...hookOverrides, + }, + ], + }; +} + +describe('HOOK_BODY_WRITE_PATTERNS — ledger ⇄ extractor reconciliation', () => { + // The ledger is the published answer to "which writes does the lint see?". + // Each entry's example must round-trip through the real extractor — a + // declared-but-unextracted pattern (#3528's death) fails here. + it('declares unique pattern ids', () => { + const ids = HOOK_BODY_WRITE_PATTERNS.map((p) => p.id); + expect(new Set(ids).size).toBe(ids.length); + }); + + for (const pattern of HOOK_BODY_WRITE_PATTERNS) { + it(`extracts exactly the declared writes for '${pattern.id}'`, () => { + const extracted = extractHookBodyWrites(pattern.example.source); + // Every extraction from the canonical example carries this entry's id — + // examples stay pure per-pattern. + expect(extracted.map((w) => w.patternId)).toEqual(extracted.map(() => pattern.id)); + expect(extracted.map(({ field, object }) => (object ? { field, object } : { field }))) + .toEqual(pattern.example.writes); + }); + } +}); + +describe('validateHookBodyWrites — ctx.input writes', () => { + it('warns on a field the target object never declares, with a did-you-mean', () => { + const findings = validateHookBodyWrites(stackWith("ctx.input.discont_total = 0;")); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].rule).toBe(HOOK_BODY_WRITE_UNKNOWN_FIELD); + expect(findings[0].where).toBe('hook "normalize_deal" › body'); + expect(findings[0].path).toBe('hooks[0].body.source'); + expect(findings[0].message).toContain("discont_total"); + expect(findings[0].message).toContain('crm_deal'); + expect(findings[0].message).toContain('#4271'); + expect(findings[0].hint).toContain("'discount_total'"); + }); + + it('accepts declared fields, system columns, and envelope keys', () => { + const findings = validateHookBodyWrites( + stackWith( + "ctx.input.stage = 'won'; ctx.input.updated_by = ctx.user.id; " + + "ctx.input.options = { skip: true }; ctx.input.data = {}; ctx.input.ast = null;", + ), + ); + expect(findings).toEqual([]); + }); + + it("catches bracket-literal and compound/logical assignments", () => { + const findings = validateHookBodyWrites( + stackWith("ctx.input['stge'] = 'won'; ctx.input.amout += 1; ctx.input.emial ??= 'x';"), + ); + expect(findings.map((f) => f.message.match(/'(\w+)'/)?.[1])).toEqual(['stge', 'amout', 'emial']); + }); + + it('catches Object.assign literal keys (shorthand included) and skips its opaque members', () => { + const findings = validateHookBodyWrites( + stackWith("const extra = {}; Object.assign(ctx.input, extra, { amount: 1, totl: 2, ...extra });"), + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain("'totl'"); + }); + + it('stays silent on statically-opaque writes: computed keys, nested paths, aliased input', () => { + const findings = validateHookBodyWrites( + stackWith( + // computed key; nested sub-object write; the documented v1 known miss + // (one-level alias) — all bail silently rather than guess. + "const k = 'x'; ctx.input[k] = 1; ctx.input.address.city = 'SF'; " + + 'const doc = ctx.input; doc.not_a_field = 1;', + ), + ); + expect(findings).toEqual([]); + }); + + it("skips ctx.input writes on wildcard hooks — but still checks their ctx.api writes", () => { + const findings = validateHookBodyWrites( + stackWith( + "ctx.input.anything = 1; await ctx.api.object('crm_contact').update({ emial: 'x' });", + { object: '*' }, + ), + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('crm_contact'); + expect(findings[0].message).toContain("'emial'"); + }); + + it('skips hooks targeting an object this stack does not declare', () => { + const findings = validateHookBodyWrites(stackWith('ctx.input.whatever = 1;', { object: 'pkg_external' })); + expect(findings).toEqual([]); + }); + + it('multi-target: a field on SOME target is a legitimate per-object branch — only an everywhere-miss warns', () => { + const partial = validateHookBodyWrites( + // `email` exists on crm_contact only; the body may guard on ctx.object. + stackWith("ctx.input.email = 'x';", { object: ['crm_deal', 'crm_contact'] }), + ); + expect(partial).toEqual([]); + + const everywhere = validateHookBodyWrites( + stackWith("ctx.input.nowhere_field = 'x';", { object: ['crm_deal', 'crm_contact'] }), + ); + expect(everywhere).toHaveLength(1); + expect(everywhere[0].message).toContain('crm_deal, crm_contact'); + }); + + it('multi-target with any cross-package member is unjudgeable — silent', () => { + const findings = validateHookBodyWrites( + stackWith("ctx.input.nowhere_field = 'x';", { object: ['crm_deal', 'pkg_external'] }), + ); + expect(findings).toEqual([]); + }); +}); + +describe('validateHookBodyWrites — ctx.api writes', () => { + it('checks insert/create/update payloads (argument 0) against the named object', () => { + const findings = validateHookBodyWrites( + stackWith( + "await ctx.api.object('crm_contact').insert({ emial: 'a' }); " + + "await ctx.api.object('crm_contact').create({ email: 'b' }); " + + "await ctx.api.object('crm_contact').update({ id, email: 'c' });", + ), + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain("ctx.api.object('crm_contact').insert"); + expect(findings[0].hint).toContain("'email'"); + }); + + it('checks updateById payloads at argument 1, not 0', () => { + const findings = validateHookBodyWrites( + stackWith("await ctx.api.object('crm_deal').updateById(ctx.input.id, { stag: 'won' });"), + ); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('updateById'); + expect(findings[0].message).toContain("'stag'"); + }); + + it('stays silent on dynamic object names, unknown objects, and non-literal payloads', () => { + const findings = validateHookBodyWrites( + stackWith( + "await ctx.api.object(target).update({ nope: 1 }); " + // dynamic name + "await ctx.api.object('pkg_external').insert({ nope: 1 }); " + // other package + "await ctx.api.object('crm_deal').update(payload); " + // opaque payload + "await ctx.api.object('crm_deal').delete({ where: { id } });", // not a write-payload method + ), + ); + expect(findings).toEqual([]); + }); +}); + +describe('validateHookBodyWrites — scope and shape tolerance', () => { + it('ignores hooks with no body, L1 expression bodies, and empty sources', () => { + expect( + validateHookBodyWrites({ + objects: [dealObject], + hooks: [ + { name: 'a', object: 'crm_deal', events: ['beforeInsert'], handler: 'legacy_fn' }, + { + name: 'b', + object: 'crm_deal', + events: ['beforeInsert'], + body: { language: 'expression', source: 'input.amount > 0' }, + }, + { name: 'c', object: 'crm_deal', events: ['beforeInsert'], body: { language: 'js', source: ' ' } }, + ], + }), + ).toEqual([]); + }); + + it('returns [] for stacks with no hooks at all', () => { + expect(validateHookBodyWrites({})).toEqual([]); + expect(validateHookBodyWrites({ hooks: [] })).toEqual([]); + }); + + it('reports each unknown field once per hook, even when written repeatedly', () => { + const findings = validateHookBodyWrites( + stackWith('ctx.input.totl = 1; ctx.input.totl = 2; Object.assign(ctx.input, { totl: 3 });'), + ); + expect(findings).toHaveLength(1); + }); + + it('accepts map-shaped hooks collections (name injected)', () => { + const findings = validateHookBodyWrites({ + objects: [dealObject], + hooks: { + normalize_deal: { + object: 'crm_deal', + events: ['beforeInsert'], + body: { language: 'js', source: 'ctx.input.totl = 1;' }, + }, + }, + }); + expect(findings).toHaveLength(1); + expect(findings[0].where).toBe('hook "normalize_deal" › body'); + }); + + it('does not throw on source with syntax errors — fewer matches, never a crash', () => { + expect(() => validateHookBodyWrites(stackWith('ctx.input.x = ; if ('))).not.toThrow(); + }); +}); diff --git a/packages/lint/src/validate-hook-body-writes.ts b/packages/lint/src/validate-hook-body-writes.ts new file mode 100644 index 0000000000..8a8f571d68 --- /dev/null +++ b/packages/lint/src/validate-hook-body-writes.ts @@ -0,0 +1,478 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Author-time write-set check for L2 (`language:'js'`) hook bodies (#4271). +// +// An L2 body that writes a field the target object never declares — +// `ctx.input.amout = 0`, `ctx.api.object('deal').update({ stag: 'won' })` — +// runs clean in the QuickJS sandbox, reports success, and the unknown column +// simply never lands in the stored record. No diagnostic anywhere: the exact +// "silent no-op manufactures false completion" failure mode of #4001, at the +// runtime-expression layer. The read side (`hook.condition`, ADR-0032) and the +// capability surface are statically checked; until this rule, the write side +// was the one blind face (the gap `hook-body.zod.ts` used to carry as +// "accepted"). +// +// Scope — the literal write patterns in {@link HOOK_BODY_WRITE_PATTERNS}, and +// nothing else. The body is PARSED (TypeScript parser, never executed, never +// type-checked); each declared pattern is reconciliation-tested against the +// extractor, so a pattern cannot be declared-but-unverified (#3528's death). +// Everything statically unknowable is skipped SILENTLY, asymmetrically +// favouring missed findings over false ones — a false positive kills an +// advisory lint, a miss just leaves the gap open a little longer: +// +// • computed keys (`ctx.input[k] = …`), spreads, non-literal payloads; +// • dynamic object names (`ctx.api.object(name)`); +// • `object:'*'` wildcard hooks' `ctx.input` writes (no single target); +// • multi-target hooks where the field exists on SOME target — the body may +// legitimately branch per object (`if (ctx.object === '…')`), so only a +// field missing on EVERY named target is flagged; +// • targets declared by another package (not in this stack); +// • one-level aliasing (`const doc = ctx.input; doc.x = 1`) — known miss, +// deliberately: v1 does no data-flow analysis. +// +// Severity: always `warning` (advisory, never gates). Same posture as +// `lintUnknownAuthoringKeys` (#3786) — ratchet only with field data. +// +// Wired via REFERENCE_INTEGRITY_RULES (it resolves field NAMES written in +// metadata against what the stack declares — the suite's exact membership +// test), so it runs on `os validate`, `os lint` and `os compile` at once. +// Deliberately NOT in the `defineStack` runtime path: the TypeScript parser +// has no place on kernel boot (see the lazy-load contract below). + +import { createRequire } from 'node:module'; +import type ts from 'typescript'; +import { findClosestMatches, formatSuggestion } from '@objectstack/spec/shared'; + +// The TypeScript compiler must NOT be imported at module top level: it is +// ~9 MB of CJS, and @objectstack/lint sits on the kernel boot path — while +// this gate only parses when a hook actually carries an L2 JS body. Same +// lazy-load contract as validate-react-page-props.ts (which see for the +// history, including production images pruning the package). Guarded by +// lazy-deps.test.ts. +// +// `node:module` is a Node builtin, untouched by esbuild/tsup, so the static +// `createRequire` import survives bundling; the `createRequire(...)` call is +// deferred because `import.meta.url` is rewritten to an empty stub in the CJS +// build (same pattern as driver-sqlite-wasm's knex-wasm-dialect). +let cachedTs: typeof ts | null = null; +function loadTypeScript(): typeof ts { + if (cachedTs) return cachedTs; + const anchor = + typeof import.meta !== 'undefined' && import.meta.url + ? import.meta.url + : typeof __filename !== 'undefined' + ? __filename + : process.cwd() + '/'; + try { + cachedTs = createRequire(anchor)('typescript') as typeof ts; + } catch (err) { + throw new Error( + `@objectstack/lint: checking an L2 (language:'js') hook body requires the "typescript" package, which could not ` + + `be loaded (${err instanceof Error ? err.message : String(err)}). It is a declared dependency of ` + + `@objectstack/lint — if this deployment prunes packages, keep "typescript" in the image; it is only loaded ` + + `when a hook with a JS body is validated.`, + ); + } + return cachedTs; +} + +export type HookBodyWriteSeverity = 'warning'; + +export interface HookBodyWriteFinding { + /** v1 is advisory-only by contract — the type says so. */ + severity: HookBodyWriteSeverity; + rule: string; + /** Human-readable location, e.g. `hook "normalize_lead" › body`. */ + where: string; + /** Config path, e.g. `hooks[0].body.source`. */ + path: string; + message: string; + hint: string; +} + +// Rule id (registry entry). +export const HOOK_BODY_WRITE_UNKNOWN_FIELD = 'hook-body-write-unknown-field'; + +// ─── The write-pattern ledger ─────────────────────────────────────────────── +// +// Every syntactic write shape the extractor recognizes, declared as data. The +// reconciliation test (validate-hook-body-writes.test.ts) runs the extractor +// over each entry's example and asserts it yields EXACTLY the declared writes, +// each tagged with this entry's id — so "the docs say this pattern is covered +// but nothing extracts it" cannot happen (#3528), and the answer to "which +// writes does the lint see?" is this list, not the extractor's code. + +/** One syntactic write shape the extractor recognizes. */ +export interface HookBodyWritePattern { + /** Stable pattern id, carried on every extracted write. */ + readonly id: string; + /** Author-facing syntax summary (for docs/diagnostics, not matching). */ + readonly syntax: string; + /** Reconciliation fixture: extracting `source` must yield exactly `writes`. */ + readonly example: { + readonly source: string; + readonly writes: ReadonlyArray<{ field: string; object?: string }>; + }; +} + +export const HOOK_BODY_WRITE_PATTERNS: readonly HookBodyWritePattern[] = [ + { + id: 'input-property-assign', + syntax: "ctx.input. = … | ctx.input[''] ⟨op⟩= …", + example: { + // Compound (`+=`) and logical (`??=`) assignment operators write their + // LHS exactly like `=` does — the example pins the whole operator range. + source: "ctx.input.total = 0; ctx.input['status'] ??= 'open'; ctx.input.retries += 1;", + writes: [{ field: 'total' }, { field: 'status' }, { field: 'retries' }], + }, + }, + { + id: 'input-object-assign', + syntax: 'Object.assign(ctx.input, { : … })', + example: { + source: "Object.assign(ctx.input, { total: 5, 'status': 'open', discount });", + writes: [{ field: 'total' }, { field: 'status' }, { field: 'discount' }], + }, + }, + { + id: 'api-crud-literal', + syntax: + "ctx.api.object('').insert({…}) | .create({…}) | .update({…}) | .updateById(id, {…})", + example: { + // Real ObjectRepository signatures: the record payload is argument 0 for + // insert/create/update and argument 1 for updateById. (`update(data)` — + // NOT `update(id, data)`; the id travels inside the payload/options.) + source: + "await ctx.api.object('audit_log').insert({ event: 'won' }); " + + "await ctx.api.object('crm_deal').updateById(id, { stage: 'won' });", + writes: [ + { field: 'event', object: 'audit_log' }, + { field: 'stage', object: 'crm_deal' }, + ], + }, + }, +]; + +/** + * `ctx.api.object(name)` write methods → index of the record-payload argument. + * Mirrors `ObjectRepository` in packages/objectql (the surface hooks actually + * receive): `upsert` exists only on the last-resort engine facade actions may + * fall back to, never on the hook path, so it is deliberately absent. + */ +const API_WRITE_METHODS: ReadonlyMap = new Map([ + ['insert', 0], + ['create', 0], + ['update', 0], + ['updateById', 1], +]); + +/** + * Wrapper keys of the flat-input proxy (`installFlatInput` in + * packages/objectql/src/hook-wrappers.ts). `ctx.input.data = …` replaces the + * whole record payload and `id`/`options`/`ast` address the operation + * envelope — none is a record-FIELD write, so none is ever flagged. + */ +const INPUT_ENVELOPE_KEYS: ReadonlySet = new Set(['id', 'options', 'ast', 'data']); + +/** + * Registry-injected columns present on (almost) every object but absent from + * `object.fields` — always legitimately writable by automation. The UNION of + * the sets the other field-resolving rules carry, because the cost asymmetry + * is the same everywhere: over-inclusion is at worst a missed finding, + * under-inclusion is a false one. + */ +const SYSTEM_FIELDS: ReadonlySet = new Set([ + 'id', '_id', 'name', 'space', + 'owner', 'owner_id', 'user_id', + 'created_at', 'created_by', 'updated_at', 'updated_by', + 'organization_id', 'tenant_id', + 'is_deleted', 'deleted_at', 'record_type', +]); + +type AnyRec = Record; + +const isRec = (v: unknown): v is AnyRec => !!v && typeof v === 'object' && !Array.isArray(v); + +/** Coerce an array-or-name-keyed-map collection to an array (name injected). */ +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v.filter((x): x is AnyRec => isRec(x)); + if (isRec(v)) { + return Object.entries(v).map(([name, def]) => ({ + name, + ...(isRec(def) ? def : {}), + })); + } + return []; +} + +/** object name → its declared field names (both `fields` authoring shapes). */ +function indexObjectFields(stack: AnyRec): Map> { + const out = new Map>(); + for (const obj of asArray(stack.objects)) { + const name = typeof obj.name === 'string' ? obj.name : undefined; + if (!name) continue; + const names = new Set(); + for (const f of asArray(obj.fields)) { + if (typeof f.name === 'string' && f.name) names.add(f.name); + } + out.set(name, names); + } + return out; +} + +/** One statically-extracted field write found in an L2 body. */ +export interface ExtractedHookBodyWrite { + /** Which {@link HOOK_BODY_WRITE_PATTERNS} entry matched. */ + patternId: string; + /** Target object name; `undefined` = the hook's own target object(s). */ + object?: string; + /** The `ctx.api` method for diagnostics (`insert`/`create`/`update`/`updateById`). */ + method?: string; + field: string; +} + +/** + * Extract every literal field write the pattern ledger declares from an L2 + * body's source. Parse-only (the source is never executed), error-tolerant + * (a body with syntax errors simply yields fewer matches), and lazy: the + * TypeScript compiler is not loaded when no pattern can possibly match. + */ +export function extractHookBodyWrites(source: string): ExtractedHookBodyWrite[] { + // Every recognizable pattern begins at a `ctx` or `Object` identifier — a + // body containing neither cannot match, and must not pay the compiler load. + if (!/\bctx\b/.test(source) && !/\bObject\b/.test(source)) return []; + + const tsc = loadTypeScript(); + // The runtime wraps a hook body as `new AsyncFunction('ctx', source)` — a + // FUNCTION BODY, not a module. Parse it in the same context so bare + // `return` / `await` mean what they mean at run time. + const sf = tsc.createSourceFile( + 'hook-body.ts', + `async function __body(ctx) {\n${source}\n}`, + tsc.ScriptTarget.Latest, + /* setParentNodes */ false, + tsc.ScriptKind.TS, + ); + + const writes: ExtractedHookBodyWrite[] = []; + + /** `node` is exactly `ctx.`. */ + const isCtxDot = (node: ts.Node, prop: string): boolean => + tsc.isPropertyAccessExpression(node) && + tsc.isIdentifier(node.expression) && + node.expression.text === 'ctx' && + node.name.text === prop; + + /** The literal record-field name of an LHS rooted at `ctx.input`, if any. */ + const fieldOfInputLhs = (lhs: ts.Expression): string | undefined => { + if (tsc.isPropertyAccessExpression(lhs) && tsc.isIdentifier(lhs.name) && isCtxDot(lhs.expression, 'input')) { + return lhs.name.text; + } + if (tsc.isElementAccessExpression(lhs) && isCtxDot(lhs.expression, 'input')) { + const arg = lhs.argumentExpression; + if (tsc.isStringLiteral(arg) || tsc.isNoSubstitutionTemplateLiteral(arg)) return arg.text; + } + return undefined; // computed key / nested path — statically opaque + }; + + /** Literal keys of an object-literal expression (spreads/computed skipped). */ + const literalObjectKeys = (node: ts.Expression): string[] => { + if (!tsc.isObjectLiteralExpression(node)) return []; + const keys: string[] = []; + for (const p of node.properties) { + if (tsc.isPropertyAssignment(p)) { + if (tsc.isIdentifier(p.name) || tsc.isStringLiteral(p.name)) keys.push(p.name.text); + } else if (tsc.isShorthandPropertyAssignment(p)) { + keys.push(p.name.text); + } + // spread / computed / method members are statically opaque — skipped + } + return keys; + }; + + const visit = (node: ts.Node): void => { + // Pattern: input-property-assign. FirstAssignment..LastAssignment spans + // `=` and every compound/logical assignment operator (`+=`, `??=`, …) — + // each writes its LHS. + if ( + tsc.isBinaryExpression(node) && + node.operatorToken.kind >= tsc.SyntaxKind.FirstAssignment && + node.operatorToken.kind <= tsc.SyntaxKind.LastAssignment + ) { + const field = fieldOfInputLhs(node.left); + if (field !== undefined && !INPUT_ENVELOPE_KEYS.has(field)) { + writes.push({ patternId: 'input-property-assign', field }); + } + } + + if (tsc.isCallExpression(node)) { + const callee = node.expression; + + // Pattern: input-object-assign. + if ( + tsc.isPropertyAccessExpression(callee) && + tsc.isIdentifier(callee.expression) && + callee.expression.text === 'Object' && + callee.name.text === 'assign' && + node.arguments.length >= 2 && + isCtxDot(node.arguments[0], 'input') + ) { + // Later Object.assign sources overwrite earlier ones but never remove + // a key, so every literal key is genuinely written regardless of the + // non-literal arguments around it. + for (const arg of node.arguments.slice(1)) { + for (const field of literalObjectKeys(arg)) { + if (!INPUT_ENVELOPE_KEYS.has(field)) { + writes.push({ patternId: 'input-object-assign', field }); + } + } + } + } + + // Pattern: api-crud-literal — ctx.api.object('').(payload…). + if (tsc.isPropertyAccessExpression(callee) && tsc.isIdentifier(callee.name)) { + const payloadIndex = API_WRITE_METHODS.get(callee.name.text); + const recv = callee.expression; + if ( + payloadIndex !== undefined && + tsc.isCallExpression(recv) && + tsc.isPropertyAccessExpression(recv.expression) && + recv.expression.name.text === 'object' && + isCtxDot(recv.expression.expression, 'api') && + recv.arguments.length === 1 + ) { + const objArg = recv.arguments[0]; + const objectName = + tsc.isStringLiteral(objArg) || tsc.isNoSubstitutionTemplateLiteral(objArg) + ? objArg.text + : undefined; // dynamic object name — statically opaque + const payload = node.arguments[payloadIndex]; + if (objectName && payload !== undefined) { + for (const field of literalObjectKeys(payload)) { + writes.push({ + patternId: 'api-crud-literal', + object: objectName, + method: callee.name.text, + field, + }); + } + } + } + } + } + + tsc.forEachChild(node, visit); + }; + visit(sf); + return writes; +} + +/** + * Validate L2 hook-body writes against target-object field declarations. + * Pure `(stack) => Finding[]` (ADR-0019); safe on pre- or post-parse stacks. + */ +export function validateHookBodyWrites(stack: AnyRec): HookBodyWriteFinding[] { + const findings: HookBodyWriteFinding[] = []; + const hooks = asArray(stack.hooks); + if (hooks.length === 0) return findings; + + // Built lazily: a stack whose hooks are all L1/handler-based never pays it. + let objectFields: Map> | null = null; + + hooks.forEach((hook, hookIndex) => { + const body = hook.body; + if (!isRec(body) || body.language !== 'js') return; + const source = body.source; + if (typeof source !== 'string' || source.trim() === '') return; + + const writes = extractHookBodyWrites(source); + if (writes.length === 0) return; + + objectFields ??= indexObjectFields(stack); + const hookName = typeof hook.name === 'string' && hook.name ? hook.name : `#${hookIndex}`; + + // The hook's own target set, for `ctx.input` writes. A wildcard target has + // no single object to check against; an unknown (cross-package) target + // cannot be judged — either way `ctx.input` writes are skipped, not guessed. + const targets = (Array.isArray(hook.object) ? hook.object : [hook.object]).filter( + (o): o is string => typeof o === 'string' && o.trim() !== '', + ); + const targetSets = targets.map((t) => objectFields!.get(t)); + const inputJudgeable = + targets.length > 0 && !targets.includes('*') && targetSets.every((s) => s !== undefined); + + const where = `hook "${hookName}" › body`; + const path = `hooks[${hookIndex}].body.source`; + const reported = new Set(); + + for (const w of writes) { + const dedupeKey = `${w.object ?? ''}\u0000${w.field}`; + if (reported.has(dedupeKey)) continue; + + if (w.object === undefined) { + // ctx.input write → the hook's own object(s). Flag only a field + // missing on EVERY named target (a multi-target body may branch per + // object, so a partial miss is not statically wrong). + if (!inputJudgeable) continue; + if (SYSTEM_FIELDS.has(w.field)) continue; + if (targetSets.some((s) => s!.has(w.field))) continue; + + reported.add(dedupeKey); + const objDesc = + targets.length === 1 + ? `object '${targets[0]}'` + : `none of its target objects (${targets.join(', ')})`; + const declares = targets.length === 1 ? 'declares no such field' : 'declare that field'; + findings.push({ + severity: 'warning', + rule: HOOK_BODY_WRITE_UNKNOWN_FIELD, + where, + path, + message: + `body writes '${w.field}' to its input, but ${objDesc} ${declares}. The sandboxed script runs ` + + `clean and the value is copied back onto the record payload — then the unknown column silently ` + + `never lands in the stored record (#4271).`, + hint: fixHint(w.field, unionCandidates(targetSets)), + }); + } else { + // ctx.api write → the named object. + const known = objectFields!.get(w.object); + if (!known) continue; // object declared by another package — cannot judge + if (SYSTEM_FIELDS.has(w.field) || known.has(w.field)) continue; + + reported.add(dedupeKey); + findings.push({ + severity: 'warning', + rule: HOOK_BODY_WRITE_UNKNOWN_FIELD, + where, + path, + message: + `body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', but ` + + `object '${w.object}' declares no such field. The call succeeds while the unknown column silently ` + + `never lands (#4271).`, + hint: fixHint(w.field, [...known]), + }); + } + } + }); + + return findings; +} + +/** Every field name declared across the (all-known) target sets, deduplicated. */ +function unionCandidates(targetSets: ReadonlyArray | undefined>): string[] { + const out = new Set(); + for (const s of targetSets) for (const f of s ?? []) out.add(f); + return [...out]; +} + +/** Did-you-mean (declared + system columns as candidates) plus the fix. */ +function fixHint(field: string, declared: string[]): string { + const suggestion = formatSuggestion(findClosestMatches(field, [...declared, ...SYSTEM_FIELDS])); + return ( + (suggestion ? `${suggestion} ` : '') + + `Fix the field name, or declare '${field}' on the object. Only the literal write patterns in ` + + `HOOK_BODY_WRITE_PATTERNS are checked — computed keys, spreads and aliased input are not — and this ` + + `warning never blocks a build.` + ); +} diff --git a/packages/spec/src/data/hook-body.zod.ts b/packages/spec/src/data/hook-body.zod.ts index 37a1d3bab4..133d5cff44 100644 --- a/packages/spec/src/data/hook-body.zod.ts +++ b/packages/spec/src/data/hook-body.zod.ts @@ -131,13 +131,19 @@ export type ExpressionBody = z.infer; * - `process`, `globalThis`, `eval`, `new Function` * - any identifier resolved from a value-only top-level import * - * **Write-set opacity — accepted static-analysis gap.** - * `source` is opaque to static analysis: no lint verifies that the fields the - * body writes (`ctx.input.x = …`, `ctx.api.object('y').update(id, { x })`) - * exist on the target object(s). Only the read side (`hook.condition`) and - * the capability surface are statically checked. Write-target field existence - * is the author's responsibility; when the write set is fixed, prefer a flow - * `update_record` node, whose structural `fields` config IS lint-checked. + * **Write-set lint — author-time, advisory (#4271).** + * The fields the body writes are statically checked by + * `validateHookBodyWrites` in `@objectstack/lint` (run by `os validate` / + * `os lint` / `os build`): the literal write patterns its + * `HOOK_BODY_WRITE_PATTERNS` ledger declares (`ctx.input.x = …`, + * `Object.assign(ctx.input, { x })`, `ctx.api.object('y').update({ x })`) + * are resolved against the target object's declared + system fields, and an + * unknown field warns with a did-you-mean. Statically unknowable writes — + * computed keys, spreads, aliased `ctx.input`, dynamic object names, + * wildcard-target hooks — remain opaque and are skipped silently, so the + * warning's absence is not proof of correctness. When the write set is fixed, + * still prefer a flow `update_record` node, whose structural `fields` config + * is error-checked rather than advisory. * * @example * ```json