Skip to content

Commit 039de63

Browse files
os-zhuangclaude
andcommitted
feat(security): enforce live-but-ungated access gaps — agent access & object API exposure (ADR-0049)
PR B of the #1878 P0 cluster: the items where the feature already exists but the gate was missing or bypassed (vs PR A, which neutralised dangling spec promises). #1884 — agent access control was a no-op. The chat route only enforced the static route-level permissions; each agent's own `access`/`permissions` were never consulted, so "who can chat with this agent" enforced nothing. Adds a pure `evaluateAgentAccess` (required-permissions ALL, allow-list match by userId/role, fail-closed on missing user) wired into the chat route after the active check. `visibility` (organization/private) is intentionally not enforced here — the request context carries no tenant/owner — and that gap is documented rather than half-enforced. #1889 — object `apiEnabled`/`apiMethods` were ignored by the HTTP/MCP data dispatch; an object could not be hidden from the API nor its methods restricted. Adds a pure `checkApiExposure` gate in `callData`: `apiEnabled:false` → 404, non-whitelisted op → 405. System/internal contexts bypass; an unresolvable definition falls open to the schema defaults (apiEnabled defaults true), so the gate is a pure additive restriction with no regression. Both helpers are unit-tested (agent-access 9, api-exposure 8). The action `disabled`-CEL gap (#1885) lives in the objectui renderer and is fixed there in a separate commit. Refs #1878 #1884 #1889 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 47caef3 commit 039de63

6 files changed

Lines changed: 308 additions & 0 deletions

File tree

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { checkApiExposure } from './api-exposure.js';
5+
6+
describe('checkApiExposure (#1889)', () => {
7+
it('falls open when the definition is unresolvable', () => {
8+
expect(checkApiExposure(undefined, 'get').allowed).toBe(true);
9+
expect(checkApiExposure(null, 'create').allowed).toBe(true);
10+
});
11+
12+
it('allows by default (apiEnabled defaults true, no whitelist)', () => {
13+
expect(checkApiExposure({}, 'query').allowed).toBe(true);
14+
expect(checkApiExposure({ apiEnabled: true }, 'delete').allowed).toBe(true);
15+
});
16+
17+
it('hides the object (404) when apiEnabled is false', () => {
18+
const d = checkApiExposure({ apiEnabled: false }, 'get');
19+
expect(d.allowed).toBe(false);
20+
expect(d.status).toBe(404);
21+
});
22+
23+
describe('apiMethods whitelist', () => {
24+
it('allows a whitelisted operation', () => {
25+
// query maps to ApiMethod 'list'
26+
expect(checkApiExposure({ apiMethods: ['list', 'get'] }, 'query').allowed).toBe(true);
27+
expect(checkApiExposure({ apiMethods: ['list', 'get'] }, 'get').allowed).toBe(true);
28+
});
29+
30+
it('blocks a non-whitelisted operation (405)', () => {
31+
const d = checkApiExposure({ apiMethods: ['list', 'get'] }, 'create');
32+
expect(d.allowed).toBe(false);
33+
expect(d.status).toBe(405);
34+
expect(d.reason).toContain('create');
35+
});
36+
37+
it('maps delete/update/create/find correctly', () => {
38+
const ro = { apiMethods: ['list', 'get'] };
39+
expect(checkApiExposure(ro, 'delete').allowed).toBe(false);
40+
expect(checkApiExposure(ro, 'update').allowed).toBe(false);
41+
expect(checkApiExposure(ro, 'find').allowed).toBe(true); // find → list
42+
});
43+
44+
it('an empty whitelist is treated as no restriction', () => {
45+
expect(checkApiExposure({ apiMethods: [] }, 'create').allowed).toBe(true);
46+
});
47+
48+
it('does not gate actions with no ApiMethod mapping', () => {
49+
expect(checkApiExposure({ apiMethods: ['list'] }, 'somethingCustom').allowed).toBe(true);
50+
});
51+
});
52+
});
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Object-level API exposure gate (ADR-0049, #1889).
5+
*
6+
* Objects declare `apiEnabled` (default true) and an optional `apiMethods`
7+
* whitelist, but the HTTP/MCP data dispatch previously ignored both — an object
8+
* could not actually be hidden from the API, nor could its allowed operations
9+
* be restricted. This module decides, for a given data action, whether the
10+
* object's declared exposure permits it.
11+
*
12+
* Both fields are *additive restrictions* over a default-allow surface
13+
* (`apiEnabled` defaults true; absent `apiMethods` means "all operations").
14+
* Therefore an unresolvable object definition fails OPEN here — that matches
15+
* the schema defaults and avoids breaking traffic when metadata is briefly
16+
* unavailable. The gate is a no-op for system/internal contexts (callers pass
17+
* `isSystem` and skip this check entirely).
18+
*/
19+
20+
/** The exposure-relevant slice of an object definition. */
21+
export interface ObjectApiDef {
22+
apiEnabled?: boolean;
23+
apiMethods?: string[] | null;
24+
}
25+
26+
export interface ApiExposureDecision {
27+
allowed: boolean;
28+
/** HTTP status to return when denied (404 hides, 405 = method not allowed). */
29+
status?: number;
30+
reason?: string;
31+
}
32+
33+
/**
34+
* Map an internal `callData` action onto the spec `ApiMethod` vocabulary
35+
* (`object.zod.ts` → `ApiMethod`). Actions with no mapping are not gated by
36+
* `apiMethods` (they still respect `apiEnabled`).
37+
*/
38+
const ACTION_TO_API_METHOD: Record<string, string> = {
39+
create: 'create',
40+
get: 'get',
41+
update: 'update',
42+
delete: 'delete',
43+
query: 'list',
44+
find: 'list',
45+
batch: 'bulk',
46+
};
47+
48+
export function checkApiExposure(def: ObjectApiDef | null | undefined, action: string): ApiExposureDecision {
49+
// Unresolvable definition → fall open to the schema defaults.
50+
if (!def) return { allowed: true };
51+
52+
// `apiEnabled: false` hides the object from the API entirely → 404.
53+
if (def.apiEnabled === false) {
54+
return { allowed: false, status: 404, reason: 'object is not exposed via the API' };
55+
}
56+
57+
// `apiMethods` whitelist (when present and non-empty) restricts operations.
58+
const whitelist = def.apiMethods;
59+
if (Array.isArray(whitelist) && whitelist.length > 0) {
60+
const method = ACTION_TO_API_METHOD[action];
61+
// Only gate actions that map to a known ApiMethod; unmapped actions pass.
62+
if (method && !whitelist.includes(method)) {
63+
return {
64+
allowed: false,
65+
status: 405,
66+
reason: `API operation '${method}' is not allowed for this object`,
67+
};
68+
}
69+
}
70+
71+
return { allowed: true };
72+
}

packages/runtime/src/http-dispatcher.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { CoreServiceName } from '@objectstack/spec/system';
55
import { pluralToSingular, PLURAL_TO_SINGULAR } from '@objectstack/spec/shared';
66
import type { ExecutionContext } from '@objectstack/spec/kernel';
77
import { setPackageDisabled } from './package-state-store.js';
8+
import { checkApiExposure } from './api-exposure.js';
89

910
/** Minimal local interface — full EnvironmentScopeManager was removed in Phase R. */
1011
interface EnvironmentScopeManager {
@@ -258,6 +259,24 @@ export class HttpDispatcher {
258259
scopeId?: string,
259260
executionContext?: ExecutionContext,
260261
): Promise<any> {
262+
// ── Object-level API exposure gate (ADR-0049, #1889) ─────
263+
// Honour the object's `apiEnabled` / `apiMethods` declarations for
264+
// external traffic. System/internal contexts bypass — these flags
265+
// govern API *exposure*, not internal engine self-writes.
266+
if (!executionContext?.isSystem && params?.object) {
267+
let def: any;
268+
try {
269+
const meta = await this.resolveService('metadata', scopeId);
270+
def = await (meta as any)?.getObject?.(params.object);
271+
} catch {
272+
def = undefined; // fall open to schema defaults (apiEnabled=true)
273+
}
274+
const gate = checkApiExposure(def, action);
275+
if (!gate.allowed) {
276+
throw { statusCode: gate.status ?? 403, message: gate.reason ?? 'API access denied' };
277+
}
278+
}
279+
261280
const protocol = await this.resolveService('protocol', scopeId);
262281
const qlService = dataDriver ?? await this.getObjectQLService(scopeId);
263282
const ql = qlService ?? await this.resolveService('objectql', scopeId);
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { evaluateAgentAccess } from './agent-access.js';
5+
6+
describe('evaluateAgentAccess (#1884)', () => {
7+
const user = (over: Partial<{ userId: string; roles: string[]; permissions: string[] }> = {}) => ({
8+
userId: 'u1',
9+
roles: [] as string[],
10+
permissions: [] as string[],
11+
...over,
12+
});
13+
14+
it('allows when the agent declares no access/permissions', () => {
15+
expect(evaluateAgentAccess({}, user()).allowed).toBe(true);
16+
expect(evaluateAgentAccess({ visibility: 'private' }, user()).allowed).toBe(true);
17+
});
18+
19+
it('fails closed when the user is missing', () => {
20+
const d = evaluateAgentAccess({ access: ['u1'] }, undefined);
21+
expect(d.allowed).toBe(false);
22+
});
23+
24+
describe('required permissions (must hold ALL)', () => {
25+
it('denies when a required permission is missing', () => {
26+
const d = evaluateAgentAccess({ permissions: ['hr:read', 'hr:write'] }, user({ permissions: ['hr:read'] }));
27+
expect(d.allowed).toBe(false);
28+
expect(d.reason).toContain('hr:write');
29+
});
30+
31+
it('allows when all are held via permissions', () => {
32+
expect(evaluateAgentAccess({ permissions: ['hr:read'] }, user({ permissions: ['hr:read'] })).allowed).toBe(true);
33+
});
34+
35+
it('satisfies a required entry via roles too (permissions OR roles)', () => {
36+
expect(evaluateAgentAccess({ permissions: ['manager'] }, user({ roles: ['manager'] })).allowed).toBe(true);
37+
});
38+
});
39+
40+
describe('access allow-list (must match one)', () => {
41+
it('allows a listed user id', () => {
42+
expect(evaluateAgentAccess({ access: ['u1', 'u2'] }, user()).allowed).toBe(true);
43+
});
44+
45+
it('allows a listed role', () => {
46+
expect(evaluateAgentAccess({ access: ['support'] }, user({ roles: ['support'] })).allowed).toBe(true);
47+
});
48+
49+
it('denies a caller not on the list', () => {
50+
const d = evaluateAgentAccess({ access: ['u2', 'admins'] }, user({ userId: 'u1', roles: ['sales'] }));
51+
expect(d.allowed).toBe(false);
52+
expect(d.reason).toMatch(/access list/);
53+
});
54+
});
55+
56+
it('enforces permissions AND allow-list together', () => {
57+
const agent = { permissions: ['ai:beta'], access: ['vip'] };
58+
// has permission but not on allow-list
59+
expect(evaluateAgentAccess(agent, user({ permissions: ['ai:beta'], roles: [] })).allowed).toBe(false);
60+
// on allow-list but missing permission
61+
expect(evaluateAgentAccess(agent, user({ roles: ['vip'], permissions: [] })).allowed).toBe(false);
62+
// both satisfied
63+
expect(evaluateAgentAccess(agent, user({ roles: ['vip'], permissions: ['ai:beta'] })).allowed).toBe(true);
64+
});
65+
});
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import type { RouteUserContext } from './ai-routes.js';
4+
5+
/**
6+
* The subset of an agent definition relevant to "who can chat with it".
7+
* Kept structural (not the full Agent type) so the check is decoupled from the
8+
* runtime's loaded-agent shape and trivially unit-testable.
9+
*/
10+
export interface AgentAccessSpec {
11+
/** Allow-list of user IDs or role names permitted to chat. */
12+
access?: string[];
13+
/** Permissions or roles the caller must ALL hold to use the agent. */
14+
permissions?: string[];
15+
/** Declared scope. Only `private` is gate-able at the route layer today. */
16+
visibility?: 'global' | 'organization' | 'private';
17+
}
18+
19+
export interface AgentAccessDecision {
20+
allowed: boolean;
21+
/** Human-readable reason when denied (safe to surface in a 403). */
22+
reason?: string;
23+
}
24+
25+
/**
26+
* Enforce per-agent access control (ADR-0049, #1884).
27+
*
28+
* Before this check the chat route only enforced the coarse, static route-level
29+
* permissions (`['ai:chat','ai:agents']`) — every agent's `access`/`permissions`
30+
* were a no-op, so "who can chat with this agent" enforced nothing. This closes
31+
* that gap for the two fields that concretely express it:
32+
*
33+
* - `permissions` (required): the caller must hold EVERY listed entry. An
34+
* entry is satisfied if it appears in the caller's `permissions` OR `roles`
35+
* (the spec field is "Required permissions or roles"). Empty → no extra
36+
* requirement beyond the route-level gate.
37+
* - `access` (allow-list): when present and non-empty, the caller must match
38+
* at least one entry, by `userId` or by membership in `roles`. Empty/absent
39+
* → not restricted by allow-list.
40+
*
41+
* `visibility` (`organization`/`private`) is intentionally NOT enforced here:
42+
* the request context carries no tenant id or agent-owner, so a correct
43+
* organization/private gate needs auth-middleware changes (tracked separately).
44+
* Enforcing a partial/guessed version would risk both lock-out and false
45+
* security, so we enforce only what the context can decide.
46+
*
47+
* Fails CLOSED on a malformed user (no userId) — an unauthenticated caller that
48+
* slipped past the route gate is denied rather than defaulted-open.
49+
*/
50+
export function evaluateAgentAccess(
51+
agent: AgentAccessSpec,
52+
user: RouteUserContext | undefined,
53+
): AgentAccessDecision {
54+
const required = agent.permissions ?? [];
55+
const allowList = agent.access ?? [];
56+
57+
// No per-agent restriction declared → allowed (route-level gate already passed).
58+
if (required.length === 0 && allowList.length === 0) {
59+
return { allowed: true };
60+
}
61+
62+
if (!user || !user.userId) {
63+
return { allowed: false, reason: 'authentication required to chat with this agent' };
64+
}
65+
66+
const held = new Set<string>([...(user.permissions ?? []), ...(user.roles ?? [])]);
67+
68+
// Required permissions: caller must hold ALL.
69+
const missing = required.filter((p) => !held.has(p));
70+
if (missing.length > 0) {
71+
return {
72+
allowed: false,
73+
reason: `missing required permission(s): ${missing.join(', ')}`,
74+
};
75+
}
76+
77+
// Allow-list: caller must match by userId or role.
78+
if (allowList.length > 0) {
79+
const roles = new Set<string>(user.roles ?? []);
80+
const matched = allowList.some((entry) => entry === user.userId || roles.has(entry));
81+
if (!matched) {
82+
return { allowed: false, reason: 'not in this agent’s access list' };
83+
}
84+
}
85+
86+
return { allowed: true };
87+
}

packages/services/service-ai/src/routes/agent-routes.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type { RouteDefinition } from './ai-routes.js';
99
import type { AgentChatQuota } from '../quota/agent-chat-quota.js';
1010
import { normalizeMessage, validateMessageContent } from './message-utils.js';
1111
import { encodeVercelDataStream } from '../stream/vercel-stream-encoder.js';
12+
import { evaluateAgentAccess } from './agent-access.js';
1213

1314
/**
1415
* Allowed message roles for the agent chat endpoint.
@@ -143,6 +144,18 @@ export function buildAgentRoutes(
143144
return { status: 403, body: { error: `Agent "${agentName}" is not active` } };
144145
}
145146

147+
// ── Per-agent access control (ADR-0049, #1884) ───────────
148+
// The route-level `permissions` gate above is the same for every
149+
// agent; it does NOT honour this agent's own `access`/`permissions`.
150+
// Enforce those here so "who can chat with this agent" is real.
151+
const access = evaluateAgentAccess(agent, req.user);
152+
if (!access.allowed) {
153+
return {
154+
status: 403,
155+
body: { error: `Access to agent "${agentName}" denied: ${access.reason}` },
156+
};
157+
}
158+
146159
// ── Per-turn quota gate (optional) ───────────────────────
147160
// Refusal is HONEST at the moment of impact (ADR-0040 §5): why, when
148161
// it recovers, and the way out. Streaming clients get the copy as a

0 commit comments

Comments
 (0)