refactor(runtime): PermissionDeniedError has ONE declaration again (#7270)

.changeset/permission-denied-error-single-declaration.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/runtime": patch
+---
+
+refactor(runtime): `PermissionDeniedError` has ONE declaration again (#7270)
+
+`security/resolve-execution-context.ts` re-declared `PermissionDeniedError` and
+`isPermissionDeniedError` character-for-character from
+`@objectstack/plugin-security`'s `errors.ts`, with a doc comment asking the next
+editor to keep them "structurally identical" and **nothing enforcing it**:
+
+```ts
+// runtime/src/security/resolve-execution-context.ts   ← the copy
+export class PermissionDeniedError extends Error {
+  readonly code = 'PERMISSION_DENIED';
+  readonly statusCode = 403;
+  …
+```
+
+Two hand-maintained declarations of an ADR-0112 denial envelope, where both
+fields are load-bearing. `statusCode` is what the dispatcher answers with, and
+`code` is what a matcher keys on — edit one copy's `403` and every test in the
+repo still passes while one dispatch path starts answering a denial with the
+wrong status. A comment is not a constraint.
+
+`@objectstack/plugin-security` is the package that *throws* these (23 call sites
+across `security-plugin.ts`, `delegated-admin-gate.ts`, `predicate-guard.ts`,
+`system-write-guard.ts`, `suggested-audience-bindings.ts`); the runtime only ever
+*catches* them. So the plugin owns the declaration and the runtime module now
+re-exports it. `@objectstack/plugin-security` was already a plain `dependencies`
+entry of `@objectstack/runtime`, so this adds no dependency — and `tsup`
+externalizes workspace dependencies, so the built bundle gained an
+`import "@objectstack/plugin-security"` and lost the duplicated class (ESM
+428.21 KB → 428.02 KB).
+
+The symbols stay exported from `security/resolve-execution-context.ts` rather
+than being deleted outright, because `http-dispatcher.ts` imports
+`isPermissionDeniedError` from that module path. Nothing outside the package is
+affected either way: `runtime/src/security/index.ts` never re-exported either
+symbol, so neither was reachable from `@objectstack/runtime`'s public barrel.
+
+The matcher itself is unchanged and stays **duck-typed** (`name` / `code` /
+message-prefix, never `instanceof`), which is what makes the re-export safe: dual
+CJS/ESM output and bundling can still hand the two sides distinct class objects,
+and a denial crossing that boundary is recognized regardless. A new
+`security/permission-denied-error-parity.test.ts` pins both halves — that the two
+import paths reach the same declaration (the assertion that fails against the old
+copy), and that an instance built from a *deliberately foreign* class of the same
+shape is still matched, so the duck-typed property is held independently of
+whether the two ever collapse to one class object.
+
+No behaviour change: `name`, `code: 'PERMISSION_DENIED'` and `statusCode: 403`
+are byte-identical to what the runtime copy produced.

packages/runtime/src/security/permission-denied-error-parity.test.ts

@@ -0,0 +1,114 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#7270] `PermissionDeniedError` / `isPermissionDeniedError` — ONE declaration.
+ *
+ * `security/resolve-execution-context.ts` used to re-declare both symbols
+ * character-for-character from `@objectstack/plugin-security`, with nothing
+ * enforcing the identity. Two hand-maintained copies of an ADR-0112 denial
+ * envelope (`code: 'PERMISSION_DENIED'`, `statusCode: 403`) are free to drift:
+ * edit one `statusCode` and every test still passes while the dispatcher starts
+ * answering a denial with the wrong HTTP status on one path only.
+ *
+ * The duplicate is gone — the runtime module re-exports the plugin's
+ * declaration. These tests pin BOTH halves of why that is safe:
+ *
+ *  1. the two import paths reach the SAME declaration (the assertion that would
+ *     have failed while the copy existed), and
+ *  2. the matcher is duck-typed, NOT `instanceof`-based — so an instance built
+ *     from a *distinct* class object (what dual CJS/ESM output or a bundler
+ *     duplicating the module actually produces at runtime) is still recognized.
+ *
+ * (2) is what makes the re-export sound: it holds whether or not the two sides
+ * ever collapse to one class object in a given deployment's module graph.
+ */
+
+import { describe, it, expect } from 'vitest';
+
+import {
+  PermissionDeniedError as PluginPermissionDeniedError,
+  isPermissionDeniedError as pluginIsPermissionDeniedError,
+} from '@objectstack/plugin-security';
+
+import {
+  PermissionDeniedError as RuntimePermissionDeniedError,
+  isPermissionDeniedError as runtimeIsPermissionDeniedError,
+} from './resolve-execution-context.js';
+
+/**
+ * A stand-in for "the same class, loaded twice" — a second class object with the
+ * identical shape, exactly what a CJS/ESM dual load or a bundled second copy of
+ * `plugin-security` hands the other side of the boundary. Declared locally on
+ * purpose: it must NOT be either package's class, or it proves nothing.
+ */
+class ForeignPermissionDeniedError extends Error {
+  readonly code = 'PERMISSION_DENIED';
+  readonly statusCode = 403;
+  readonly details?: Record<string, unknown>;
+  constructor(message: string, details?: Record<string, unknown>) {
+    super(message);
+    this.name = 'PermissionDeniedError';
+    this.details = details;
+  }
+}
+
+describe('PermissionDeniedError — single declaration across packages', () => {
+  it('runtime re-exports the plugin declaration rather than re-declaring it', () => {
+    expect(RuntimePermissionDeniedError).toBe(PluginPermissionDeniedError);
+    expect(runtimeIsPermissionDeniedError).toBe(pluginIsPermissionDeniedError);
+  });
+
+  it('carries the ADR-0112 denial envelope', () => {
+    const e = new PluginPermissionDeniedError('[Security] Access denied: nope', { reason: 'rls' });
+
+    expect(e).toBeInstanceOf(Error);
+    expect(e.name).toBe('PermissionDeniedError');
+    expect(e.code).toBe('PERMISSION_DENIED');
+    expect(e.statusCode).toBe(403);
+    expect(e.message).toBe('[Security] Access denied: nope');
+    expect(e.details).toEqual({ reason: 'rls' });
+  });
+
+  it('omits `details` when none is supplied', () => {
+    expect(new PluginPermissionDeniedError('denied').details).toBeUndefined();
+  });
+});
+
+describe('isPermissionDeniedError — cross-package recognition', () => {
+  const matchers: Array<[string, (e: unknown) => boolean]> = [
+    ['@objectstack/plugin-security', pluginIsPermissionDeniedError],
+    ['@objectstack/runtime', runtimeIsPermissionDeniedError],
+  ];
+
+  for (const [owner, isPermissionDenied] of matchers) {
+    describe(`matcher from ${owner}`, () => {
+      it("matches an instance of the plugin's own class", () => {
+        expect(isPermissionDenied(new PluginPermissionDeniedError('denied'))).toBe(true);
+      });
+
+      it('matches an instance of a DISTINCT class object of the same shape', () => {
+        const foreign = new ForeignPermissionDeniedError('denied');
+
+        // The point of the assertion: not the same class, still recognized.
+        expect(foreign).not.toBeInstanceOf(PluginPermissionDeniedError);
+        expect(isPermissionDenied(foreign)).toBe(true);
+      });
+
+      it('matches on `name` alone', () => {
+        expect(isPermissionDenied({ name: 'PermissionDeniedError' })).toBe(true);
+      });
+
+      it('matches on `code` alone', () => {
+        expect(isPermissionDenied({ code: 'PERMISSION_DENIED' })).toBe(true);
+      });
+
+      it('rejects unrelated errors and non-objects', () => {
+        expect(isPermissionDenied(new Error('boom'))).toBe(false);
+        expect(isPermissionDenied({ name: 'NotFoundError', code: 'NOT_FOUND' })).toBe(false);
+        expect(isPermissionDenied(null)).toBe(false);
+        expect(isPermissionDenied(undefined)).toBe(false);
+        expect(isPermissionDenied('PermissionDeniedError')).toBe(false);
+      });
+    });
+  }
+});

packages/runtime/src/security/resolve-execution-context.ts

@@ -234,31 +234,18 @@ export async function resolveExecutionContext(opts: ResolveOptions): Promise<Exe
 }
 
 /**
- * Typed sentinel error thrown by SecurityPlugin (and re-thrown here) when an
- * operation is denied. The dispatcher catches it and translates to HTTP 403.
+ * Typed sentinel error thrown by SecurityPlugin when an operation is denied.
+ * The dispatcher catches it and translates to HTTP 403.
  *
- * Kept structurally identical to `@objectstack/plugin-security`'s
- * `PermissionDeniedError` so `isPermissionDeniedError` matches whichever class
- * instance crosses the boundary, regardless of which package owns the actual
- * class identity at runtime.
+ * This module used to re-declare the class and its matcher character-for-character
+ * from `@objectstack/plugin-security`, with nothing enforcing the identity — two
+ * declarations of an ADR-0112 envelope (`code`, `statusCode`) free to drift apart
+ * silently. `@objectstack/plugin-security` is the package that THROWS these, so it
+ * owns the single declaration; this is a re-export, not a copy (#7270).
+ *
+ * Re-exported (rather than dropped) because `http-dispatcher.ts` already imports
+ * `isPermissionDeniedError` from this module path. The matcher stays duck-typed
+ * upstream, so an instance crossing a package boundary is still recognized when
+ * dual CJS/ESM output or bundling hands the two sides distinct class objects.
  */
-export class PermissionDeniedError extends Error {
-  readonly code = 'PERMISSION_DENIED';
-  readonly statusCode = 403;
-  readonly details?: Record<string, unknown>;
-  constructor(message: string, details?: Record<string, unknown>) {
-    super(message);
-    this.name = 'PermissionDeniedError';
-    this.details = details;
-  }
-}
-
-export function isPermissionDeniedError(e: unknown): e is PermissionDeniedError {
-  if (!e || typeof e !== 'object') return false;
-  const anyE = e as any;
-  return (
-    anyE.name === 'PermissionDeniedError' ||
-    anyE.code === 'PERMISSION_DENIED' ||
-    (typeof anyE.message === 'string' && anyE.message.startsWith('[Security] Access denied'))
-  );
-}
+export { PermissionDeniedError, isPermissionDeniedError } from '@objectstack/plugin-security';