From 462038e61a4709b51337382ffff4da388f151a69 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 15:41:08 +0000 Subject: [PATCH 1/2] refactor(security): converge anonymous-deny into one shared function + source-enumerating ratchet (#2567 Phase 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 gated every HTTP surface against the requireAuth posture, but each seam hand-rolled the same anonymous check. Phase 2 removes the duplication and pins the surfaces so a new ungated entry point fails CI. - New shouldDenyAnonymous in @objectstack/core (security/anonymous-deny.ts): the single anonymous-deny decision + shared 401 body/constants, mirroring the auth-gate.ts pattern. All five seams delegate to it — REST enforceAuth, dispatcher handleGraphQL/handleMetadata/handleAI, hono denyAnonymous. Pure refactor, no runtime behavior change; identity resolution and the dynamic exemptions (public-form grants, share-link tokens) are untouched. - discover() ratchet on the authz-conformance matrix: statically enumerates the data/meta/graphql HTTP entry points from source (curated per-file probes, control-plane excluded) and asserts each is classified by a covers key. A new route or a removed/stale covers now fails CI as UNCLASSIFIED/STALE. A negative test proves the ratchet bites. Guards the isAuthGateAllowlisted(undefined)===true trap: body-routed seams (GraphQL) pass no path, so the non-empty-path guard denies anonymous unconditionally rather than falling through to the control-plane allowlist. Verified: core security 114/114 (9 new), runtime requireauth gate 13/13, rest auth-gate 4/4, hono anonymous-deny 5/5, dogfood conformance 5/5 (ratchet bites) + anonymous-deny surfaces e2e 10/10, single-authz-resolver guard intact. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01ShknNUYoSTspJLBiqorD8V --- .../authz-2567-shared-anonymous-deny.md | 34 +++++ .../core/src/security/anonymous-deny.test.ts | 55 ++++++++ packages/core/src/security/anonymous-deny.ts | 73 +++++++++++ packages/core/src/security/index.ts | 10 ++ .../dogfood/test/authz-conformance.matrix.ts | 18 ++- .../dogfood/test/authz-conformance.test.ts | 123 ++++++++++++++++-- .../plugin-hono-server/src/hono-plugin.ts | 26 ++-- packages/rest/src/rest-server.ts | 28 ++-- packages/runtime/src/http-dispatcher.ts | 46 ++++--- 9 files changed, 357 insertions(+), 56 deletions(-) create mode 100644 .changeset/authz-2567-shared-anonymous-deny.md create mode 100644 packages/core/src/security/anonymous-deny.test.ts create mode 100644 packages/core/src/security/anonymous-deny.ts diff --git a/.changeset/authz-2567-shared-anonymous-deny.md b/.changeset/authz-2567-shared-anonymous-deny.md new file mode 100644 index 0000000000..d4c05181b1 --- /dev/null +++ b/.changeset/authz-2567-shared-anonymous-deny.md @@ -0,0 +1,34 @@ +--- +'@objectstack/core': minor +'@objectstack/runtime': patch +'@objectstack/rest': patch +'@objectstack/plugin-hono-server': patch +--- + +refactor(security): converge the anonymous-deny decision into one shared function + a source-enumerating ratchet (#2567 Phase 2) + +Phase 1 gated every HTTP surface (REST `/data`, dispatcher `/graphql` + `/meta`, +raw-hono `/data`) against the secure-by-default `requireAuth` posture, but each +seam hand-rolled the same `!userId && !isSystem → 401` check. Phase 2 removes +that duplication and pins the surfaces so a new ungated entry point fails CI. + +- **New `shouldDenyAnonymous` in `@objectstack/core`** (`security/anonymous-deny.ts`) + — the single anonymous-deny decision + shared 401 body/constants, mirroring the + `auth-gate.ts` pattern (pure function so the seams can never drift). All five + seams — REST `enforceAuth`, dispatcher `handleGraphQL` / `handleMetadata` / + `handleAI`, hono `denyAnonymous` — now delegate to it. **Pure refactor: no + runtime behavior change** (verified by the unchanged Phase-1 handler + e2e + proofs). Identity resolution and the dynamic exemptions (public-form grants, + share-link tokens) are untouched — they run upstream and only ever hand the + seam an already-resolved context. +- **A `discover()` ratchet on the authz-conformance matrix** — it statically + enumerates the data/meta/graphql HTTP entry points from source (curated + per-file probes, control-plane routes excluded) and asserts each is classified + by a matrix `covers` key. A new `/data`/`/meta`/`/graphql` route (or a + removed/stale `covers`) now fails CI as UNCLASSIFIED / STALE, not in review. A + companion negative test proves the ratchet bites. + +A design trap is guarded: `isAuthGateAllowlisted(undefined)` returns `true`, so a +body-routed seam (GraphQL, which has no request path) must pass no path — the +shared function's non-empty-path guard denies anonymous unconditionally there, +never falling through to the control-plane allowlist. diff --git a/packages/core/src/security/anonymous-deny.test.ts b/packages/core/src/security/anonymous-deny.test.ts new file mode 100644 index 0000000000..90c59d953d --- /dev/null +++ b/packages/core/src/security/anonymous-deny.test.ts @@ -0,0 +1,55 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #2567 Phase 2 — the shared anonymous-deny decision. These lock the exact +// contract every HTTP seam now delegates to, including the load-bearing +// `undefined`-path trap (a naive allowlist call would reopen GraphQL). + +import { describe, it, expect } from 'vitest'; +import { shouldDenyAnonymous, ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS } from './anonymous-deny.js'; + +describe('shouldDenyAnonymous — the shared HTTP anonymous-deny decision (#2567)', () => { + it('no-op when requireAuth is off (demo / single-tenant)', () => { + expect(shouldDenyAnonymous({ requireAuth: false })).toBe(false); + expect(shouldDenyAnonymous({ requireAuth: undefined })).toBe(false); + }); + + it('denies an anonymous caller under requireAuth', () => { + expect(shouldDenyAnonymous({ requireAuth: true })).toBe(true); + }); + + it('passes an authenticated caller', () => { + expect(shouldDenyAnonymous({ requireAuth: true, userId: 'u1' })).toBe(false); + }); + + it('passes an internal system context', () => { + expect(shouldDenyAnonymous({ requireAuth: true, isSystem: true })).toBe(false); + }); + + it('passes an OPTIONS preflight even when anonymous', () => { + expect(shouldDenyAnonymous({ requireAuth: true, method: 'OPTIONS' })).toBe(false); + expect(shouldDenyAnonymous({ requireAuth: true, method: 'options' })).toBe(false); + }); + + it('exempts a real control-plane path (auth / health)', () => { + expect(shouldDenyAnonymous({ requireAuth: true, path: '/api/v1/auth/login' })).toBe(false); + expect(shouldDenyAnonymous({ requireAuth: true, path: '/api/v1/health' })).toBe(false); + }); + + it('denies a real data path', () => { + expect(shouldDenyAnonymous({ requireAuth: true, path: '/api/v1/data/sys_user' })).toBe(true); + }); + + // The trap: isAuthGateAllowlisted(undefined) === true. A body-routed seam + // (GraphQL) passes no path; it MUST still deny anonymous, not fall through to + // the allowlist. Guards against silently reopening #2567. + it('denies when path is undefined/empty (body-routed seam — GraphQL trap guard)', () => { + expect(shouldDenyAnonymous({ requireAuth: true, path: undefined })).toBe(true); + expect(shouldDenyAnonymous({ requireAuth: true, path: null })).toBe(true); + expect(shouldDenyAnonymous({ requireAuth: true, path: '' })).toBe(true); + }); + + it('exposes a stable 401 body + status for seams to return', () => { + expect(ANONYMOUS_DENY_STATUS).toBe(401); + expect(ANONYMOUS_DENY_BODY).toEqual({ error: 'unauthenticated', message: expect.any(String) }); + }); +}); diff --git a/packages/core/src/security/anonymous-deny.ts b/packages/core/src/security/anonymous-deny.ts new file mode 100644 index 0000000000..ed4b959f90 --- /dev/null +++ b/packages/core/src/security/anonymous-deny.ts @@ -0,0 +1,73 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #2567 — the single anonymous-deny decision, shared by every HTTP seam. + * + * ADR-0056 D2 made the platform deny anonymous callers by default + * (`requireAuth`). Phase 1 gated each surface (REST `/data`, dispatcher + * `/graphql` + `/meta`, raw-hono `/data`) but every seam hand-rolled the same + * `!userId && !isSystem → 401` check. This centralises that DECISION into one + * pure, tested function — the exact pattern {@link ./auth-gate.ts} established + * for the ADR-0069 auth-policy gate: keeping the decision in one function means + * the seams can never drift on who is denied. + * + * It deliberately does NOT own identity resolution or the dynamic exemptions + * (public-form submission, share-link tokens): those run UPSTREAM and set the + * execution context (a `userId`, or `isSystem`) before a seam calls this, so + * this function only ever inspects the already-resolved context. + */ + +import { isAuthGateAllowlisted } from './auth-gate.js'; + +/** HTTP status every seam returns for an anonymous-denied request. */ +export const ANONYMOUS_DENY_STATUS = 401 as const; +/** Stable machine code (mirrors the REST `enforceAuth` seam). */ +export const ANONYMOUS_DENY_CODE = 'unauthenticated' as const; +/** Human-facing message. */ +export const ANONYMOUS_DENY_MESSAGE = 'Authentication is required to access this endpoint.'; +/** The single 401 body shape every seam returns: `{ error, message }`. */ +export const ANONYMOUS_DENY_BODY = { + error: ANONYMOUS_DENY_CODE, + message: ANONYMOUS_DENY_MESSAGE, +} as const; + +export interface AnonymousDenyInput { + /** The `requireAuth` posture. Falsy ⇒ no-op (demo / single-tenant). */ + requireAuth: boolean | undefined; + /** Resolved caller id, if any. */ + userId?: string | null; + /** Internal system context (never set on inbound HTTP; cannot be forged). */ + isSystem?: boolean; + /** HTTP method — `OPTIONS` (CORS preflight) always passes. */ + method?: string | null; + /** + * OPTIONAL request path. When a NON-EMPTY string, a control-plane path + * (auth / health / ready / discovery — see {@link isAuthGateAllowlisted}) is + * exempt. Body-routed seams (GraphQL) have no meaningful path and pass + * `undefined`; see the guard below for why that is load-bearing. + */ + path?: string | null; +} + +/** + * True when the request MUST be rejected with 401. The one decision every HTTP + * seam shares. + */ +export function shouldDenyAnonymous(input: AnonymousDenyInput): boolean { + if (!input.requireAuth) return false; // posture off + if (typeof input.method === 'string' && input.method.toUpperCase() === 'OPTIONS') { + return false; // CORS preflight + } + if (input.userId || input.isSystem) return false; // authenticated / system + // Control-plane exemption — ONLY for a real, non-empty path. + // + // ⚠️ `isAuthGateAllowlisted(undefined)` returns `true` (it treats "no path" + // as allow-listed for the auth-gate's purposes). A body-routed seam such as + // GraphQL has no meaningful request path; if it passed `undefined` straight + // through, the allowlist would exempt EVERY anonymous query and silently + // reopen exactly the hole #2567 closes. The non-empty guard is mandatory. + if (typeof input.path === 'string' && input.path.length > 0 && isAuthGateAllowlisted(input.path)) { + return false; + } + return true; +} diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index a503fef5c3..e7da5a3528 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -102,5 +102,15 @@ export { } from './posture-ladder.js'; export { isAuthGateAllowlisted, evaluateAuthGate, type AuthGate } from './auth-gate.js'; +// #2567 — the single anonymous-deny decision shared by every HTTP seam. +export { + shouldDenyAnonymous, + ANONYMOUS_DENY_BODY, + ANONYMOUS_DENY_STATUS, + ANONYMOUS_DENY_CODE, + ANONYMOUS_DENY_MESSAGE, + type AnonymousDenyInput, +} from './anonymous-deny.js'; + // ADR-0091 D1/D2 — grant validity windows, the shared resolution-time predicate. export { isGrantActive, isGrantExpired, type GrantValidityWindow } from './grant-validity.js'; diff --git a/packages/dogfood/test/authz-conformance.matrix.ts b/packages/dogfood/test/authz-conformance.matrix.ts index a191b03eb2..d12b0a4dfb 100644 --- a/packages/dogfood/test/authz-conformance.matrix.ts +++ b/packages/dogfood/test/authz-conformance.matrix.ts @@ -21,6 +21,13 @@ export interface AuthzPrimitive { enforcement?: string; /** Dogfood proof filename in this directory (required for high-risk enforced). */ proof?: string; + /** + * Ratchet keys this row accounts for (ADR-0060), matched against the test's + * `discover()`. A discovered HTTP entry point with no covering row fails CI as + * UNCLASSIFIED; a `covers` key no longer in source fails as STALE. See + * authz-conformance.test.ts (#2567 anonymous-deny surface enumeration). + */ + covers?: string[]; /** Why it is experimental/removed, or a roadmap pointer. */ note?: string; } @@ -55,15 +62,18 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ // the same `requireAuth` gate; these rows pin every entry point so a new // ungated surface (or a silent regression) fails CI, not review. { id: 'anonymous-deny-meta', summary: 'anonymous-deny on the metadata endpoints (#2567 surface 1)', state: 'enforced', - enforcement: 'rest/rest-server.ts registerMetadataEndpoints guarded registrar (enforceAuth) — every /meta route inherits the gate; runtime/http-dispatcher.ts handleMetadata mirrors it for the dispatcher metadata catch-all', - proof: 'showcase-anonymous-deny-surfaces.dogfood.test.ts' }, + enforcement: 'rest/rest-server.ts registerMetadataEndpoints guarded registrar (enforceAuth → shouldDenyAnonymous) — every /meta route inherits the gate; runtime/http-dispatcher.ts handleMetadata mirrors it for the dispatcher metadata catch-all', + proof: 'showcase-anonymous-deny-surfaces.dogfood.test.ts', + covers: ['meta:rest-server.ts:registerMetadataEndpoints', 'meta:http-dispatcher.ts:handleMetadata'] }, { id: 'anonymous-deny-graphql', summary: 'anonymous-deny on the dispatcher GraphQL endpoint (#2567 surface 2)', state: 'enforced', - enforcement: 'runtime/http-dispatcher.ts handleGraphQL (requireAuth gate, resolves identity for the direct /graphql route) + runtime/dispatcher-plugin.ts requireAuth default(true), mirroring rest-server.ts', + enforcement: 'runtime/http-dispatcher.ts handleGraphQL (shouldDenyAnonymous, resolves identity for the direct /graphql route) + runtime/dispatcher-plugin.ts requireAuth default(true), mirroring rest-server.ts', proof: 'showcase-anonymous-deny-surfaces.dogfood.test.ts', + covers: ['graphql:http-dispatcher.ts:handleGraphQL', 'graphql:dispatcher-plugin.ts:POST /api/v1/graphql'], note: 'GraphQL reaches the same object data as /data through kernel.graphql, whose security middleware falls OPEN for an anonymous context. Unit-proven in runtime/http-dispatcher.requireauth.test.ts (GraphQL block); e2e on the platform default in the surfaces proof.' }, { id: 'anonymous-deny-hono-data', summary: 'anonymous-deny on the raw-hono standard /data routes (#2567 surface 3)', state: 'enforced', - enforcement: 'plugin-hono-server/hono-plugin.ts denyAnonymous gate on the standard /data routes (requireAuth ?? true, mirroring rest-server.ts)', + enforcement: 'plugin-hono-server/hono-plugin.ts denyAnonymous gate (shouldDenyAnonymous) on the standard /data routes (requireAuth ?? true, mirroring rest-server.ts)', proof: 'showcase-anonymous-deny-surfaces.dogfood.test.ts', + covers: ['data:hono-plugin.ts:POST /data/:object', 'data:hono-plugin.ts:GET /data/:object/:id', 'data:hono-plugin.ts:GET /data/:object'], note: 'These routes delegate straight to ObjectQL and were only shadowed when the REST plugin registered the same paths FIRST — so the posture depended on plugin registration order (a load-order change silently reopened it, no test failing). Gating each route makes the deny decision a property of this entry point too. Handler-level proof in plugin-hono-server/hono-anonymous-deny.test.ts.' }, { id: 'default-profile', summary: 'app-declared default profile (isDefault)', state: 'enforced', enforcement: 'plugin-security/security-plugin.ts fallback resolution', proof: 'showcase-default-profile.dogfood.test.ts' }, diff --git a/packages/dogfood/test/authz-conformance.test.ts b/packages/dogfood/test/authz-conformance.test.ts index 4b4a2c6e9a..d79545b15f 100644 --- a/packages/dogfood/test/authz-conformance.test.ts +++ b/packages/dogfood/test/authz-conformance.test.ts @@ -5,26 +5,131 @@ // every shared invariant (valid state, enforced-has-site, experimental/removed- // has-note, proof-file-exists, high-risk-has-proof). A new fail-open or a deleted // proof breaks the build. +// +// #2567 Phase 2 — the anonymous-deny SURFACES are additionally pinned by the +// `discover()` ratchet: this test STATICALLY enumerates the data/meta/graphql +// HTTP entry points from source and asserts each is classified by a matrix row. +// A new ungated `/data` route (or a removed/stale `covers` key) then fails CI as +// UNCLASSIFIED / STALE — the surface can't silently regress. import { describe, expect, it } from 'vitest'; import { fileURLToPath } from 'node:url'; -import { dirname } from 'node:path'; +import { dirname, join } from 'node:path'; +import { readFileSync } from 'node:fs'; import { checkLedger } from '@objectstack/verify'; -import { AUTHZ_CONFORMANCE } from './authz-conformance.matrix.js'; +import { AUTHZ_CONFORMANCE, type AuthzPrimitive } from './authz-conformance.matrix.js'; const HERE = dirname(fileURLToPath(import.meta.url)); +// packages/dogfood/test → repo root. +const REPO_ROOT = join(HERE, '../../..'); + +// ── #2567 ratchet — static enumeration of anonymous-deny HTTP entry points ── +// +// A CURATED per-file probe table (not a blind repo grep): scoped to the four +// source files and to data/meta/graphql segments only, so control-plane routes +// (/health, /auth, /ready, /discovery) are never enumerated as data surfaces. +// But each probe is pattern-based WITHIN its file, so a genuinely new `/data` +// route (or a new graphql/meta handler) is auto-discovered → new key → a +// missing `covers` fails CI. Keys are derived from source CONTENT (route +// literals / handler names), never line numbers, so they don't churn on edits. +const PROBES: ReadonlyArray<{ file: string; re: RegExp; key: (m: RegExpExecArray) => string }> = [ + // REST /meta umbrella registrar — one guarded registrar covers all ~17 routes. + { + file: 'packages/rest/src/rest-server.ts', + re: /private\s+registerMetadataEndpoints\s*\(/g, + key: () => 'meta:rest-server.ts:registerMetadataEndpoints', + }, + // Dispatcher meta + graphql handlers — curated NAMES only (NOT handleAI / + // handleData / handleSecurity, which are separate surfaces/rows). + { + file: 'packages/runtime/src/http-dispatcher.ts', + re: /async\s+(handleMetadata|handleGraphQL)\s*\(/g, + key: (m) => `${m[1] === 'handleGraphQL' ? 'graphql' : 'meta'}:http-dispatcher.ts:${m[1]}`, + }, + // Dispatcher-plugin direct GraphQL route (other server.post routes are + // control-plane / feature endpoints, deliberately not enumerated here). + { + file: 'packages/runtime/src/dispatcher-plugin.ts', + re: /server\.post\(\s*`\$\{prefix\}\/graphql`/g, + key: () => 'graphql:dispatcher-plugin.ts:POST /api/v1/graphql', + }, + // Raw-hono standard /data routes — genuinely pattern-based: ANY new + // `rawApp.(`${prefix}/data...`)` → a new key → CI fails until a row covers it. + { + file: 'packages/plugins/plugin-hono-server/src/hono-plugin.ts', + re: /rawApp\.(get|post|put|patch|delete)\(\s*`\$\{prefix\}(\/data[^`]*)`/g, + key: (m) => `data:hono-plugin.ts:${m[1].toUpperCase()} ${m[2]}`, + }, +]; + +/** Statically enumerate the anonymous-deny HTTP entry points from source. */ +function discoverAnonymousDenySurfaces(): Set { + const found = new Set(); + for (const probe of PROBES) { + const src = readFileSync(join(REPO_ROOT, probe.file), 'utf8'); + // Fresh lastIndex per file (the RegExp is shared, `g`-flagged). + probe.re.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = probe.re.exec(src)) !== null) found.add(probe.key(m)); + } + return found; +} + +const HIGH_RISK = [ + 'owd-private', 'owd-public-read', 'controlled-by-parent', 'anonymous-deny', 'default-profile', + // #2567 — every anonymous-deny HTTP surface is high-risk: it guards the + // same object data as REST `/data` through a sibling entry point. + 'anonymous-deny-meta', 'anonymous-deny-graphql', 'anonymous-deny-hono-data', +]; describe('ADR-0056 D10 — authorization conformance matrix', () => { - it('is a sound conformance ledger (ADR-0060 checkLedger)', () => { + it('is a sound conformance ledger (ADR-0060 checkLedger) + the #2567 surface ratchet holds', () => { const problems = checkLedger(AUTHZ_CONFORMANCE, { proofRoot: HERE, // proofs are dogfood test files alongside this one - highRisk: [ - 'owd-private', 'owd-public-read', 'controlled-by-parent', 'anonymous-deny', 'default-profile', - // #2567 — every anonymous-deny HTTP surface is high-risk: it guards the - // same object data as REST `/data` through a sibling entry point. - 'anonymous-deny-meta', 'anonymous-deny-graphql', 'anonymous-deny-hono-data', - ], + highRisk: HIGH_RISK, + // The ratchet: every discovered data/meta/graphql entry point must be + // classified by exactly one row's `covers`, and no `covers` key may be + // stale (no longer in source). + discover: () => discoverAnonymousDenySurfaces(), }); expect(problems, problems.join('\n')).toEqual([]); }); }); + +// #2567 — prove the ratchet actually BITES. Drives `checkLedger` with controlled +// inputs (deep-cloned matrix / synthetic discover) so it's deterministic and +// needs no source edits. If these ever pass vacuously, the ratchet is asleep. +describe('#2567 — anonymous-deny surface ratchet bites', () => { + const clone = (): AuthzPrimitive[] => JSON.parse(JSON.stringify(AUTHZ_CONFORMANCE)); + const opts = (discover: () => Iterable) => ({ proofRoot: HERE, highRisk: HIGH_RISK, discover }); + + it('the real matrix + real discover is sound (baseline lock)', () => { + const problems = checkLedger(AUTHZ_CONFORMANCE, opts(() => discoverAnonymousDenySurfaces())); + expect(problems).toEqual([]); + }); + + it('(a) a row that DROPS its covers → UNCLASSIFIED surface failure', () => { + const m = clone(); + const row = m.find((r) => r.id === 'anonymous-deny-hono-data')!; + row.covers = []; + const problems = checkLedger(m, opts(() => discoverAnonymousDenySurfaces())); + expect(problems.some((p) => /UNCLASSIFIED surface/.test(p) && /data:hono-plugin\.ts/.test(p))).toBe(true); + }); + + it('(b) a NEW ungated route appearing in source → UNCLASSIFIED surface failure', () => { + const fake = 'data:hono-plugin.ts:DELETE /data/fake'; + const problems = checkLedger( + AUTHZ_CONFORMANCE, + opts(() => new Set([...discoverAnonymousDenySurfaces(), fake])), + ); + expect(problems.some((p) => p.includes('UNCLASSIFIED surface') && p.includes(fake))).toBe(true); + }); + + it('(c) a covers key no longer in source → STALE covers failure', () => { + const m = clone(); + const row = m.find((r) => r.id === 'anonymous-deny-graphql')!; + row.covers = [...(row.covers ?? []), 'graphql:http-dispatcher.ts:handleRemovedThing']; + const problems = checkLedger(m, opts(() => discoverAnonymousDenySurfaces())); + expect(problems.some((p) => /STALE covers/.test(p) && /handleRemovedThing/.test(p))).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-hono-server/src/hono-plugin.ts b/packages/plugins/plugin-hono-server/src/hono-plugin.ts index e1e1736091..49a30429d6 100644 --- a/packages/plugins/plugin-hono-server/src/hono-plugin.ts +++ b/packages/plugins/plugin-hono-server/src/hono-plugin.ts @@ -1,6 +1,9 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { Plugin, PluginContext, IHttpServer, IDataEngine } from '@objectstack/core'; +import { + Plugin, PluginContext, IHttpServer, IDataEngine, + shouldDenyAnonymous, ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS, +} from '@objectstack/core'; import { RestServerConfig, } from '@objectstack/spec/api'; @@ -555,19 +558,14 @@ export class HonoServerPlugin implements Plugin { ); } // Returns a 401 Response when the caller is anonymous under the deny - // posture, else null (caller proceeds). `isSystem` is never set on - // inbound HTTP (internal-only), so it cannot be forged to bypass this. - const denyAnonymous = (c: any, execCtx: any): Response | null => { - if (!requireAuth) return null; - if (execCtx?.userId || execCtx?.isSystem) return null; - return c.json( - { - error: 'unauthenticated', - message: 'Authentication is required to access this endpoint.', - }, - 401, - ); - }; + // posture, else null (caller proceeds). Delegates the decision to the + // shared `shouldDenyAnonymous` (#2567) so every HTTP seam stays in + // lockstep. `isSystem` is never set on inbound HTTP (internal-only), so + // it cannot be forged to bypass this. + const denyAnonymous = (c: any, execCtx: any): Response | null => + shouldDenyAnonymous({ requireAuth, userId: execCtx?.userId, isSystem: execCtx?.isSystem }) + ? c.json(ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS) + : null; // Basic CRUD data endpoints — delegate to ObjectQL service directly const getObjectQL = () => ctx.getService('objectql'); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 350522f32f..77ee1c10ca 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -1,6 +1,9 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { IHttpServer, resolveAuthzContext, resolveLocalizationContext, isAuthGateAllowlisted } from '@objectstack/core'; +import { + IHttpServer, resolveAuthzContext, resolveLocalizationContext, isAuthGateAllowlisted, + shouldDenyAnonymous, ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS, +} from '@objectstack/core'; import { isMcpServerEnabled } from '@objectstack/types'; import { RouteManager } from './route-manager.js'; import { RestServerConfig, RestApiConfig, CrudEndpointsConfig, MetadataEndpointsConfig, BatchEndpointsConfig, RouteGenerationConfig } from '@objectstack/spec/api'; @@ -936,14 +939,21 @@ export class RestServer { res.status(403).json({ error: { code: gate.code, message: gate.message } }); return true; } - if (!this.config.api.requireAuth) return false; - if (context?.userId) return false; - if (req?.method === 'OPTIONS') return false; - res.status(401).json({ - error: 'unauthenticated', - message: 'Authentication is required to access this endpoint.', - }); - return true; + // Shared anonymous-deny decision (#2567). Pass no `path`: the REST + // control-plane routes are registered WITHOUT `enforceAuth`, so this + // seam only ever guards data/meta — deny unconditionally when anonymous, + // exactly as before (the allowlist is reserved for a future umbrella + // seam). `isSystem` is never set on inbound HTTP, so it cannot bypass. + if (shouldDenyAnonymous({ + requireAuth: this.config.api.requireAuth, + userId: context?.userId, + isSystem: context?.isSystem, + method: req?.method, + })) { + res.status(ANONYMOUS_DENY_STATUS).json(ANONYMOUS_DENY_BODY); + return true; + } + return false; } /** diff --git a/packages/runtime/src/http-dispatcher.ts b/packages/runtime/src/http-dispatcher.ts index 9bb4a4912a..7a2e9865d8 100644 --- a/packages/runtime/src/http-dispatcher.ts +++ b/packages/runtime/src/http-dispatcher.ts @@ -1,6 +1,9 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { ObjectKernel, getEnv, resolveLocale, evaluateAuthGate, isAuthGateAllowlisted } from '@objectstack/core'; +import { + ObjectKernel, getEnv, resolveLocale, evaluateAuthGate, isAuthGateAllowlisted, + shouldDenyAnonymous, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE, +} from '@objectstack/core'; import { isMcpServerEnabled } from '@objectstack/types'; import { CoreServiceName } from '@objectstack/spec/system'; import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai'; @@ -1629,19 +1632,20 @@ export class HttpDispatcher { // The dispatcher-plugin's direct `/graphql` route calls us WITHOUT // resolving identity first (unlike `dispatch()`, which populates // `context.executionContext`), so resolve it here when absent. - if (this.requireAuth) { - let ec: any = context.executionContext; - if (!ec) { - ec = await this.resolveRequestExecutionContext(context); - if (ec) context.executionContext = ec; - } - if (!ec?.userId && !ec?.isSystem) { - throw { - statusCode: 401, - message: 'Authentication is required to access this endpoint.', - code: 'unauthenticated', - }; - } + let ec: any = context.executionContext; + if (this.requireAuth && !ec) { + ec = await this.resolveRequestExecutionContext(context); + if (ec) context.executionContext = ec; + } + // Body-routed seam: no meaningful request path, so pass none — the + // shared decision then denies anonymous unconditionally (see the + // `undefined`-path trap guard in `shouldDenyAnonymous`). + if (shouldDenyAnonymous({ requireAuth: this.requireAuth, userId: ec?.userId, isSystem: ec?.isSystem })) { + throw { + statusCode: ANONYMOUS_DENY_STATUS, + message: ANONYMOUS_DENY_MESSAGE, + code: ANONYMOUS_DENY_CODE, + }; } if (typeof this.kernel.graphql !== 'function') { @@ -1772,12 +1776,12 @@ export class HttpDispatcher { // the cloud runtime). Object/field schemas — SYSTEM-object schemas on a // tenant-less host — must not be readable by anonymous callers when the // deployment requires auth. No-op when `requireAuth` is off. - if (this.requireAuth) { + { const ec: any = _context.executionContext; - if (!ec?.userId && !ec?.isSystem) { + if (shouldDenyAnonymous({ requireAuth: this.requireAuth, userId: ec?.userId, isSystem: ec?.isSystem })) { return { handled: true, - response: this.error('Authentication is required to access this endpoint.', 401, { code: 'unauthenticated' }), + response: this.error(ANONYMOUS_DENY_MESSAGE, ANONYMOUS_DENY_STATUS, { code: ANONYMOUS_DENY_CODE }), }; } } @@ -3864,12 +3868,14 @@ export class HttpDispatcher { // adapter/model config back. Gate when the deployment requires // auth; an authenticated user (or an internal system context) // passes, matching the REST `enforceAuth` seam. Off → unchanged. - if (this.requireAuth && route.auth !== false) { + if (route.auth !== false) { const gec: any = context.executionContext; - if (!gec?.userId && !gec?.isSystem) { + // `requireAuth && route.auth !== false` is the AI-route contract; + // the shared function owns the anonymous decision itself. + if (shouldDenyAnonymous({ requireAuth: this.requireAuth, userId: gec?.userId, isSystem: gec?.isSystem })) { return { handled: true, - response: this.error('Authentication is required to access this endpoint.', 401, { code: 'unauthenticated' }), + response: this.error(ANONYMOUS_DENY_MESSAGE, ANONYMOUS_DENY_STATUS, { code: ANONYMOUS_DENY_CODE }), }; } } From 4fe57b1086b0b0ac1584e3a677f098307d1f3ba8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 16:03:46 +0000 Subject: [PATCH 2/2] fix(hono): drop now-unused IHttpServer import (CodeQL) The anonymous-deny import edit touched the import line, surfacing a pre-existing unused `IHttpServer` import (referenced only in comments). Remove it to clear the CodeQL finding. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01ShknNUYoSTspJLBiqorD8V --- packages/plugins/plugin-hono-server/src/hono-plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/plugin-hono-server/src/hono-plugin.ts b/packages/plugins/plugin-hono-server/src/hono-plugin.ts index 49a30429d6..126a812882 100644 --- a/packages/plugins/plugin-hono-server/src/hono-plugin.ts +++ b/packages/plugins/plugin-hono-server/src/hono-plugin.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { - Plugin, PluginContext, IHttpServer, IDataEngine, + Plugin, PluginContext, IDataEngine, shouldDenyAnonymous, ANONYMOUS_DENY_BODY, ANONYMOUS_DENY_STATUS, } from '@objectstack/core'; import {