Skip to content

Commit 91eddca

Browse files
os-helpclaude
andauthored
refactor(runtime): PermissionDeniedError has ONE declaration again (#7270) (#7462)
`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. Both fields of the ADR-0112 denial envelope are load-bearing — `statusCode` is what the dispatcher answers with, `code` is what a matcher keys on — so editing one copy's `403` left every test in the repo green while one dispatch path answered a denial with the wrong status. `@objectstack/plugin-security` throws these (23 call sites); the runtime only catches them. The plugin now owns the single declaration and the runtime module 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 — the bundle gained an `import "@objectstack/plugin-security"` and lost the duplicated class (ESM 428.21 KB -> 428.02 KB). The symbols stay exported from the runtime module rather than being deleted, because `http-dispatcher.ts` imports `isPermissionDeniedError` from that path. Nothing outside the package is affected: `security/index.ts` never re-exported either symbol, so neither was reachable from the public barrel. The matcher 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. The new `permission-denied-error-parity.test.ts` pins both halves — that the two import paths reach the same declaration (this assertion fails against the old copy), and that an instance built from a deliberately foreign class of the same shape is still matched. No behaviour change: `name`, `code: 'PERMISSION_DENIED'` and `statusCode: 403` are byte-identical to what the runtime copy produced. Claude-Session: https://claude.ai/code/session_015TbH9juzW7PvJzbsdpUnEp Co-authored-by: Claude <noreply@anthropic.com>
1 parent bf4ebe2 commit 91eddca

3 files changed

Lines changed: 180 additions & 26 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
refactor(runtime): `PermissionDeniedError` has ONE declaration again (#7270)
6+
7+
`security/resolve-execution-context.ts` re-declared `PermissionDeniedError` and
8+
`isPermissionDeniedError` character-for-character from
9+
`@objectstack/plugin-security`'s `errors.ts`, with a doc comment asking the next
10+
editor to keep them "structurally identical" and **nothing enforcing it**:
11+
12+
```ts
13+
// runtime/src/security/resolve-execution-context.ts ← the copy
14+
export class PermissionDeniedError extends Error {
15+
readonly code = 'PERMISSION_DENIED';
16+
readonly statusCode = 403;
17+
18+
```
19+
20+
Two hand-maintained declarations of an ADR-0112 denial envelope, where both
21+
fields are load-bearing. `statusCode` is what the dispatcher answers with, and
22+
`code` is what a matcher keys on — edit one copy's `403` and every test in the
23+
repo still passes while one dispatch path starts answering a denial with the
24+
wrong status. A comment is not a constraint.
25+
26+
`@objectstack/plugin-security` is the package that *throws* these (23 call sites
27+
across `security-plugin.ts`, `delegated-admin-gate.ts`, `predicate-guard.ts`,
28+
`system-write-guard.ts`, `suggested-audience-bindings.ts`); the runtime only ever
29+
*catches* them. So the plugin owns the declaration and the runtime module now
30+
re-exports it. `@objectstack/plugin-security` was already a plain `dependencies`
31+
entry of `@objectstack/runtime`, so this adds no dependency — and `tsup`
32+
externalizes workspace dependencies, so the built bundle gained an
33+
`import "@objectstack/plugin-security"` and lost the duplicated class (ESM
34+
428.21 KB → 428.02 KB).
35+
36+
The symbols stay exported from `security/resolve-execution-context.ts` rather
37+
than being deleted outright, because `http-dispatcher.ts` imports
38+
`isPermissionDeniedError` from that module path. Nothing outside the package is
39+
affected either way: `runtime/src/security/index.ts` never re-exported either
40+
symbol, so neither was reachable from `@objectstack/runtime`'s public barrel.
41+
42+
The matcher itself is unchanged and stays **duck-typed** (`name` / `code` /
43+
message-prefix, never `instanceof`), which is what makes the re-export safe: dual
44+
CJS/ESM output and bundling can still hand the two sides distinct class objects,
45+
and a denial crossing that boundary is recognized regardless. A new
46+
`security/permission-denied-error-parity.test.ts` pins both halves — that the two
47+
import paths reach the same declaration (the assertion that fails against the old
48+
copy), and that an instance built from a *deliberately foreign* class of the same
49+
shape is still matched, so the duck-typed property is held independently of
50+
whether the two ever collapse to one class object.
51+
52+
No behaviour change: `name`, `code: 'PERMISSION_DENIED'` and `statusCode: 403`
53+
are byte-identical to what the runtime copy produced.
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#7270] `PermissionDeniedError` / `isPermissionDeniedError` — ONE declaration.
5+
*
6+
* `security/resolve-execution-context.ts` used to re-declare both symbols
7+
* character-for-character from `@objectstack/plugin-security`, with nothing
8+
* enforcing the identity. Two hand-maintained copies of an ADR-0112 denial
9+
* envelope (`code: 'PERMISSION_DENIED'`, `statusCode: 403`) are free to drift:
10+
* edit one `statusCode` and every test still passes while the dispatcher starts
11+
* answering a denial with the wrong HTTP status on one path only.
12+
*
13+
* The duplicate is gone — the runtime module re-exports the plugin's
14+
* declaration. These tests pin BOTH halves of why that is safe:
15+
*
16+
* 1. the two import paths reach the SAME declaration (the assertion that would
17+
* have failed while the copy existed), and
18+
* 2. the matcher is duck-typed, NOT `instanceof`-based — so an instance built
19+
* from a *distinct* class object (what dual CJS/ESM output or a bundler
20+
* duplicating the module actually produces at runtime) is still recognized.
21+
*
22+
* (2) is what makes the re-export sound: it holds whether or not the two sides
23+
* ever collapse to one class object in a given deployment's module graph.
24+
*/
25+
26+
import { describe, it, expect } from 'vitest';
27+
28+
import {
29+
PermissionDeniedError as PluginPermissionDeniedError,
30+
isPermissionDeniedError as pluginIsPermissionDeniedError,
31+
} from '@objectstack/plugin-security';
32+
33+
import {
34+
PermissionDeniedError as RuntimePermissionDeniedError,
35+
isPermissionDeniedError as runtimeIsPermissionDeniedError,
36+
} from './resolve-execution-context.js';
37+
38+
/**
39+
* A stand-in for "the same class, loaded twice" — a second class object with the
40+
* identical shape, exactly what a CJS/ESM dual load or a bundled second copy of
41+
* `plugin-security` hands the other side of the boundary. Declared locally on
42+
* purpose: it must NOT be either package's class, or it proves nothing.
43+
*/
44+
class ForeignPermissionDeniedError extends Error {
45+
readonly code = 'PERMISSION_DENIED';
46+
readonly statusCode = 403;
47+
readonly details?: Record<string, unknown>;
48+
constructor(message: string, details?: Record<string, unknown>) {
49+
super(message);
50+
this.name = 'PermissionDeniedError';
51+
this.details = details;
52+
}
53+
}
54+
55+
describe('PermissionDeniedError — single declaration across packages', () => {
56+
it('runtime re-exports the plugin declaration rather than re-declaring it', () => {
57+
expect(RuntimePermissionDeniedError).toBe(PluginPermissionDeniedError);
58+
expect(runtimeIsPermissionDeniedError).toBe(pluginIsPermissionDeniedError);
59+
});
60+
61+
it('carries the ADR-0112 denial envelope', () => {
62+
const e = new PluginPermissionDeniedError('[Security] Access denied: nope', { reason: 'rls' });
63+
64+
expect(e).toBeInstanceOf(Error);
65+
expect(e.name).toBe('PermissionDeniedError');
66+
expect(e.code).toBe('PERMISSION_DENIED');
67+
expect(e.statusCode).toBe(403);
68+
expect(e.message).toBe('[Security] Access denied: nope');
69+
expect(e.details).toEqual({ reason: 'rls' });
70+
});
71+
72+
it('omits `details` when none is supplied', () => {
73+
expect(new PluginPermissionDeniedError('denied').details).toBeUndefined();
74+
});
75+
});
76+
77+
describe('isPermissionDeniedError — cross-package recognition', () => {
78+
const matchers: Array<[string, (e: unknown) => boolean]> = [
79+
['@objectstack/plugin-security', pluginIsPermissionDeniedError],
80+
['@objectstack/runtime', runtimeIsPermissionDeniedError],
81+
];
82+
83+
for (const [owner, isPermissionDenied] of matchers) {
84+
describe(`matcher from ${owner}`, () => {
85+
it("matches an instance of the plugin's own class", () => {
86+
expect(isPermissionDenied(new PluginPermissionDeniedError('denied'))).toBe(true);
87+
});
88+
89+
it('matches an instance of a DISTINCT class object of the same shape', () => {
90+
const foreign = new ForeignPermissionDeniedError('denied');
91+
92+
// The point of the assertion: not the same class, still recognized.
93+
expect(foreign).not.toBeInstanceOf(PluginPermissionDeniedError);
94+
expect(isPermissionDenied(foreign)).toBe(true);
95+
});
96+
97+
it('matches on `name` alone', () => {
98+
expect(isPermissionDenied({ name: 'PermissionDeniedError' })).toBe(true);
99+
});
100+
101+
it('matches on `code` alone', () => {
102+
expect(isPermissionDenied({ code: 'PERMISSION_DENIED' })).toBe(true);
103+
});
104+
105+
it('rejects unrelated errors and non-objects', () => {
106+
expect(isPermissionDenied(new Error('boom'))).toBe(false);
107+
expect(isPermissionDenied({ name: 'NotFoundError', code: 'NOT_FOUND' })).toBe(false);
108+
expect(isPermissionDenied(null)).toBe(false);
109+
expect(isPermissionDenied(undefined)).toBe(false);
110+
expect(isPermissionDenied('PermissionDeniedError')).toBe(false);
111+
});
112+
});
113+
}
114+
});

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

Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -234,31 +234,18 @@ export async function resolveExecutionContext(opts: ResolveOptions): Promise<Exe
234234
}
235235

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

0 commit comments

Comments
 (0)