fix(spec,objectql)!: an empty hook target is refused, not widened to the wildcard (#4281)

HookSchema.object had no emptiness constraint, so '', [] and [''] all
parsed. normalizeObjects then mapped the first two to ['*'] — the
engine's match-everything sentinel — so a hook whose target was left
blank registered on EVERY object, on every event it listed, silently.
That is #4001 pointed the wrong way: the usual silent strip narrows what
was written, this one widened blank intent into the broadest possible
blast radius. [''] failed the other way, registering on an object name
nothing matches (ADR-0078 silently-inert).

Both are refused now, in two places: HookSchema gains a refinement whose
error names the spellings that work, and the binder — which accepts
unparsed Hook input, so the schema alone would not cover it — skips and
records an untargetable hook instead of widening it. A wildcard hook
stays legitimate; it has to be spelled '*' so a reviewer sees it.

Also fixes bindHooksToEngine's strict option: documented as fail-fast,
it never threw, because the per-hook try/catch swallowed the throw its
own strict branch raised — the failure was recorded twice and binding
carried on. Proven by probe before changing it.

Docs: hooks.mdx gains a Targeting objects section covering the three
forms, the refusal, and the review bar a wildcard earns.

First-party scan: zero assets use an empty target; the wildcards in
plugin.ts / permission sets are code-registered and unaffected.


Claude-Session: https://claude.ai/code/session_0147tNF4Snk7Ry1KGt4a5PY4

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/hook-empty-target-not-wildcard.md

@@ -0,0 +1,10 @@
+---
+'@objectstack/objectql': minor
+'@objectstack/spec': minor
+---
+
+A hook with an empty `object` target is refused instead of silently widened to the wildcard.
+
+`HookSchema.object` had no emptiness constraint, so `''`, `[]` and `['']` all parsed. The binder's `normalizeObjects` then mapped the first two to `['*']` — the engine's match-everything sentinel — so a hook whose target was left blank registered on **every** object in the tenant, on every event it listed, with no diagnostic anywhere. `['']` failed the other way, registering on an object name nothing matches: a hook that could never fire (ADR-0078). Both shapes are now refused, at parse time and again in the binder (which accepts unparsed input, so the guard has to hold in both places). The error names the two spellings that work and the wildcard the blank silently became. A wildcard hook stays legitimate — it just has to be spelled `'*'`, so it is a choice visible in a diff.
+
+Also fixes `bindHooksToEngine`'s `strict` option, which is documented as "fail fast on misconfiguration" but never threw: the per-hook `try`/`catch` swallowed the throw its own strict branch raised, recording the failure twice and carrying on. Under `strict` a bind failure is now fatal, as advertised.

content/docs/automation/hooks.mdx

@@ -48,6 +48,49 @@ Logic that genuinely needs code still doesn't have to be a hook: a flow
 keeps the orchestration in checked metadata and the code in one named,
 testable unit.
 
+## Targeting objects
+
+`object` takes three forms, and the third one carries a review cost the other
+two do not:
+
+```typescript
+object: 'account'                    // one object
+object: ['account', 'contact']       // a named set
+object: '*'                          // every object in the tenant
+```
+
+<Callout type="warn">
+**An empty target is refused, not treated as "no target".** `''`, `[]` and
+`['']` used to parse; the binder then widened `''` and `[]` to the wildcard, so
+a blank target registered the hook on **every** object, on every event it
+listed, with no diagnostic. (`['']` failed the other way — an object name
+nothing matches, a hook that could never fire.) All three are now a parse
+error naming the two spellings that work. A wildcard hook stays entirely
+legitimate; it just has to be **spelled** `'*'`, so it is a choice a reviewer
+sees in the diff rather than a default someone fell into ([#4001](https://github.com/objectstack-ai/objectstack/issues/4001)).
+</Callout>
+
+### Wildcard hooks earn a higher review bar
+
+`object: '*'` is the right tool for genuinely cross-cutting concerns — audit
+trails, provenance stamping, tenant-wide guards. It is also the broadest thing
+a hook can be, so review it as such:
+
+- **Blast radius is every object, including ones added later.** A wildcard hook
+  written today runs against objects that do not exist yet, whose fields it was
+  never checked against. Prefer a named set when the list is actually known.
+- **Cost multiplies by object count and write volume.** A `beforeUpdate`
+  wildcard hook runs on every write in the tenant; anything non-trivial in it
+  belongs behind a `condition` (evaluated before the handler) rather than an
+  early `return` inside the body.
+- **Reads and writes both widen.** A wildcard hook holding an L2 body with
+  `api.write` can write anywhere. Whether the fields it writes exist is
+  [not statically checked](/docs/automation/hook-bodies#not-statically-checked-the-write-set),
+  so a wildcard write set is the least verifiable shape in the system.
+
+Where a wildcard is the honest answer, say so in `description` — it is the one
+place a reviewer can find out why the broad target was chosen.
+
 ## Before Hook
 
 Mutate the incoming record before it is saved. The engine exposes the pending

packages/objectql/src/hook-binder.test.ts

@@ -77,6 +77,109 @@ describe('bindHooksToEngine', () => {
     expect(result.errors[0]?.reason).toMatch(/unknown function/);
   });
 
+  // #4001: `normalizeObjects` used to widen a blank target to `['*']`, the
+  // engine's match-everything sentinel — so a hook whose target was empty
+  // registered on EVERY object, on every event it listed, silently. The binder
+  // takes unparsed `Hook` input, so this guard has to hold here independently
+  // of the `HookSchema` refinement.
+  describe('an empty object target is skipped, never widened to the wildcard', () => {
+    it.each([
+      ['empty string', ''],
+      ['empty array', [] as string[]],
+      ['array of blanks', ['', '  '] as string[]],
+      ['undefined', undefined],
+    ])('skips a hook targeting %s', async (_label, object) => {
+      const engine = makeEngine();
+      const calls: string[] = [];
+      const hook = {
+        name: 'blank_target', object, events: ['beforeInsert'], priority: 100,
+        handler: () => { calls.push('ran'); },
+      } as unknown as Hook;
+
+      const result = bindHooksToEngine(engine, [hook], { packageId: 'p' });
+
+      expect(result.registered).toBe(0);
+      expect(result.skipped).toBe(1);
+      expect(result.errors[0]?.reason).toMatch(/names no object/);
+
+      // The point of the guard: it must not have become a wildcard.
+      await engine.triggerHooks('beforeInsert', makeCtx({ object: 'anything' }));
+      expect(calls).toEqual([]);
+    });
+
+    it('drops only the blank members of an otherwise valid list', async () => {
+      const engine = makeEngine();
+      const calls: string[] = [];
+      const hook = {
+        name: 'mixed_target', object: ['account', ''], events: ['beforeInsert'], priority: 100,
+        handler: (ctx: HookContext) => { calls.push(ctx.object); },
+      } as unknown as Hook;
+
+      const result = bindHooksToEngine(engine, [hook], { packageId: 'p' });
+      expect(result.registered).toBe(1);
+
+      await engine.triggerHooks('beforeInsert', makeCtx({ object: 'account' }));
+      await engine.triggerHooks('beforeInsert', makeCtx({ object: 'lead' }));
+      expect(calls).toEqual(['account']);
+    });
+
+    it('still binds an explicitly spelled wildcard', async () => {
+      const engine = makeEngine();
+      const calls: string[] = [];
+      const hook: Hook = {
+        name: 'explicit_wildcard', object: '*', events: ['beforeInsert'], priority: 100,
+        handler: (ctx) => { calls.push(ctx.object); },
+      };
+
+      const result = bindHooksToEngine(engine, [hook], { packageId: 'p' });
+      expect(result.registered).toBe(1);
+
+      await engine.triggerHooks('beforeInsert', makeCtx({ object: 'account' }));
+      await engine.triggerHooks('beforeInsert', makeCtx({ object: 'lead' }));
+      expect(calls).toEqual(['account', 'lead']);
+    });
+
+    it('is fatal under strict', () => {
+      const engine = makeEngine();
+      const hook = {
+        name: 'blank_strict', object: [], events: ['beforeInsert'], priority: 100,
+        handler: () => {},
+      } as unknown as Hook;
+
+      expect(() => bindHooksToEngine(engine, [hook], { packageId: 'p', strict: true }))
+        .toThrow(/names no object/);
+    });
+  });
+
+  // `strict` is documented as "fail fast on misconfiguration", but the per-hook
+  // try/catch swallowed the throw its own strict branch raised — the failure was
+  // recorded twice (once by the skip path, once by the caught throw) and binding
+  // carried on. A runtime that opted into fail-fast never got it.
+  describe('strict really is fatal (the catch no longer swallows its own throw)', () => {
+    it('throws on an unresolvable handler ref instead of recording it twice', () => {
+      const engine = makeEngine();
+      const hook: Hook = {
+        name: 'unresolvable', object: 'account', events: ['beforeInsert'], priority: 100,
+        handler: 'no_such_fn',
+      };
+
+      expect(() => bindHooksToEngine(engine, [hook], { strict: true }))
+        .toThrow(/unknown function 'no_such_fn'/);
+    });
+
+    it('still records-and-continues when strict is off', () => {
+      const engine = makeEngine();
+      const hook: Hook = {
+        name: 'unresolvable', object: 'account', events: ['beforeInsert'], priority: 100,
+        handler: 'no_such_fn',
+      };
+
+      const result = bindHooksToEngine(engine, [hook], {});
+      expect(result.skipped).toBe(1);
+      expect(result.errors).toHaveLength(1);
+    });
+  });
+
   it('replaces existing hooks under the same packageId on re-bind', async () => {
     const engine = makeEngine();
     const calls: string[] = [];

packages/objectql/src/hook-binder.ts

@@ -166,8 +166,25 @@ export function bindHooksToEngine(
         });
       }
 
-      const wrapped = wrapDeclarativeHook(hook, resolved, { logger, metrics: opts.metrics });
       const objects = normalizeObjects(hook.object);
+      if (objects.length === 0) {
+        result.skipped += 1;
+        const reason =
+          'hook target names no object — an empty `object` is refused rather than widened to '
+          + "the wildcard '*' (#4001). Name the object(s), or write `object: '*'` if firing on "
+          + 'every object is the intent.';
+        result.errors.push({ hook: hook.name, reason });
+        if (opts.strict) {
+          throw new Error(`[hook-binder] strict: cannot bind hook '${hook.name}': ${reason}`);
+        }
+        logger.warn('[hook-binder] skipping hook with an empty object target', {
+          hook: hook.name,
+          object: hook.object,
+        });
+        continue;
+      }
+
+      const wrapped = wrapDeclarativeHook(hook, resolved, { logger, metrics: opts.metrics });
       const events = Array.isArray(hook.events) ? hook.events : [];
 
       for (const event of events) {
@@ -185,6 +202,12 @@ export function bindHooksToEngine(
         }
       }
     } catch (err: any) {
+      // `strict` is documented as "fail fast on misconfiguration", but this
+      // catch swallowed the throw the strict branches raise — including its
+      // own — so the option only ever recorded the failure twice and carried
+      // on. A production runtime that opted into fail-fast never got it.
+      // Under strict, every bind failure is fatal, as advertised.
+      if (opts.strict) throw err;
       result.errors.push({ hook: hook.name, reason: err?.message ?? String(err) });
       logger.error('[hook-binder] failed to bind hook', {
         hook: hook.name,
@@ -204,10 +227,23 @@ export function bindHooksToEngine(
   return result;
 }
 
+/**
+ * Resolve a hook's declared target to the object names it registers on.
+ *
+ * Returns `[]` when the target names nothing. This used to fall back to
+ * `['*']`, which is the engine's match-everything sentinel — so a hook whose
+ * target was blank (`''`, `[]`, or a list of blanks) silently registered on
+ * EVERY object, on every event it listed. Blank intent became the broadest
+ * possible blast radius with no diagnostic (#4001).
+ *
+ * `HookSchema` now refuses those shapes at parse time, but this binder accepts
+ * unparsed `Hook` input, so the guard has to hold here too. An empty result is
+ * skipped and recorded by the caller rather than widened: a wildcard hook is
+ * legitimate, it just has to be spelled `'*'` so a reviewer can see it.
+ */
 function normalizeObjects(target: Hook['object']): string[] {
-  if (Array.isArray(target)) return target.length > 0 ? target : ['*'];
-  if (typeof target === 'string' && target.length > 0) return [target];
-  return ['*'];
+  const names = Array.isArray(target) ? target : [target];
+  return names.filter((name): name is string => typeof name === 'string' && name.trim().length > 0);
 }
 
 function resolveHandler(

packages/spec/src/data/hook.test.ts

@@ -140,6 +140,41 @@ describe('HookSchema', () => {
 
       expect(hook.object).toBe('*');
     });
+
+    // #4001: an empty target used to parse, and the binder widened `''` / `[]`
+    // to the wildcard `'*'` — so a blank target registered the hook on EVERY
+    // object. `['']` failed the other way: an object name nothing matches, a
+    // hook that could never fire. Both are refused; a wildcard must be spelled.
+    describe('an empty target is refused, not widened to the wildcard', () => {
+      it.each([
+        ['empty string', ''],
+        ['whitespace-only string', '   '],
+        ['empty array', [] as string[]],
+        ['array of one blank', [''] as string[]],
+        ['array with a blank member', ['account', ''] as string[]],
+      ])('rejects %s', (_label, object) => {
+        const result = HookSchema.safeParse({
+          name: 'blank_target',
+          object,
+          events: ['beforeInsert'],
+        });
+
+        expect(result.success).toBe(false);
+        const message = result.error!.issues.map((i) => i.message).join('\n');
+        // The error has to be fixable, not merely loud (#4001): it names both
+        // spellings that work and the wildcard that the blank silently became.
+        expect(message).toMatch(/must name at least one object/);
+        expect(message).toMatch(/object: '\*'/);
+      });
+
+      it('still accepts a wildcard inside an array', () => {
+        expect(HookSchema.parse({
+          name: 'array_wildcard',
+          object: ['*'],
+          events: ['afterUpdate'],
+        }).object).toEqual(['*']);
+      });
+    });
   });
 
   describe('Event Subscription', () => {

packages/spec/src/data/hook.zod.ts

@@ -33,6 +33,35 @@ import { HookBodySchema } from './hook-body.zod';
  *   - hook `retryPolicy.backoffMs` vs datasource `retryPolicy.baseDelayMs`
  */
 
+/*
+ * ── An empty target is not "no target" ──────────────────────────────────────
+ *
+ * `object` had no emptiness constraint, so `''`, `[]` and `['']` all parsed.
+ * The binder (`normalizeObjects` in `packages/objectql/src/hook-binder.ts`)
+ * then mapped the first two to `['*']` — and `'*'` is the match-everything
+ * sentinel in the engine's dispatch (`targets.includes('*')`). An author who
+ * left the target blank therefore got a hook registered on EVERY object in the
+ * tenant, on every event listed, with no diagnostic anywhere.
+ *
+ * That is the #4001 failure mode pointed the wrong way: the usual silent strip
+ * narrows what was written, this one WIDENS it — blank intent became the
+ * broadest possible blast radius. `['']` failed the other way, registering on
+ * an object named `''` that nothing matches — a hook that can never fire
+ * (ADR-0078 "no silently inert metadata").
+ *
+ * Both are refused here, and the binder no longer escalates: a target it cannot
+ * make sense of is skipped and recorded, never widened. A wildcard hook remains
+ * entirely legitimate — it just has to be SPELLED `'*'`, so that it is a choice
+ * a reviewer can see in the diff rather than a default someone fell into.
+ */
+const hookTargetError =
+  'A hook `object` target must name at least one object. An empty target is not '
+  + '"no target": until #4001 `\'\'` and `[]` were widened to the wildcard `\'*\'`, '
+  + 'registering the hook on EVERY object, and `[\'\']` registered it on an object '
+  + 'name nothing matches, so it could never fire. Name the object(s) — '
+  + "`object: 'account'` or `object: ['account', 'contact']` — or, if firing on "
+  + "every object really is the intent, write the wildcard explicitly: `object: '*'`.";
+
 /** Keys {@link HookSchema} declares (drift-guarded by hook.test.ts). */
 const HOOK_KEYS = [
   'name', 'label', 'object', 'events', 'handler', 'body', 'priority',
@@ -142,8 +171,19 @@ export const HookSchema = lazySchema(() => z.object({
    * - Single object: "account"
    * - List of objects: ["account", "contact"]
    * - Wildcard: "*" (All objects)
+   *
+   * Must name at least one object. An empty target (`''`, `[]`, `['']`) is
+   * refused rather than widened to the wildcard — see the note above
+   * {@link HOOK_KEYS}.
    */
-  object: z.union([z.string(), z.array(z.string())]).describe('Target object(s)'),
+  object: z.union([z.string(), z.array(z.string())])
+    .refine(
+      (v) => (Array.isArray(v)
+        ? v.length > 0 && v.every((name) => name.trim().length > 0)
+        : v.trim().length > 0),
+      { error: hookTargetError },
+    )
+    .describe('Target object(s)'),
 
   /**
    * Events to subscribe to