Skip to content

Commit c044f08

Browse files
os-zhuangclaude
andauthored
fix(settings): verified authz on settings routes — close unauthenticated write (Critical) (#2848)
* fix(settings): verified authz on settings routes — close unauth write (Critical) The settings HTTP routes trusted spoofable identity headers (x-user-id / x-tenant-id / x-permissions) and the write path (setMany) had NO permission gate. On a standard `os serve --server` (settings + HTTP server composed by default, routes on the raw app with no auth middleware) an UNAUTHENTICATED remote client could write tenant- or platform-scoped settings — including the auth security-policy manifest — and enumerate every namespace. - Verified identity: SettingsServicePlugin derives the caller's identity and capabilities from the platform's verified resolution (resolveAuthzContext: session cookie / API key / OAuth), never from request headers. The route default is now SECURE (anonymous, denied). - Capability gates: manifest readPermission/writePermission enforced for HTTP callers (reads of a protected namespace, writes, and actions require the declared capability; writes default to at least the read capability, never ungated). Gated behind an `enforced` flag set only at the HTTP boundary — in-process/boot callers (kernel.getService, seed) are unchanged and keep trusted access. - Unauthenticated HTTP → 403 SETTINGS_FORBIDDEN. Found by an adversarial review of the request→ExecutionContext trust boundary. Tests: service-level enforced-mode gates + route-level deny (anonymous read hidden, write 403, spoofed headers grant nothing, read-only cannot write). service-settings suite 144 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(security): declare + grant setup.write so admins can write Setup settings Enforcing the settings manifests' declared writePermission (previous commit) surfaced a modeling gap: `setup.write` — the write counterpart to `setup.access`, used by the branding/company/localization/feature-flag manifests — was referenced but NEVER declared as a capability or granted to any permission set. Harmless while settings writes went ungated; under enforcement it meant NOBODY (not even an admin) could write those namespaces (403). - Declare `setup.write` in PLATFORM_CAPABILITIES (scope: org). - Grant it to admin_full_access and organization_admin (the sets that already hold setup.access / manage tenant settings). Verified: the analytics-timezone dogfood (which PUTs /api/settings/ localization as the signed-in admin, then buckets) now passes — proving the admin bearer resolves through the settings plugin's verified resolution and the write gate admits it. plugin-security 298, service-settings 144, spec api-surface unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b97af7e commit c044f08

9 files changed

Lines changed: 314 additions & 38 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/service-settings': minor
3+
'@objectstack/spec': minor
4+
'@objectstack/plugin-security': patch
5+
---
6+
7+
**Security fix (Critical): the settings HTTP routes no longer trust spoofable identity headers, and writes are now capability-gated.**
8+
9+
Previously `GET/PUT/POST /api/settings/*` derived the caller's identity from `x-user-id` / `x-tenant-id` / `x-permissions` request headers (the route default), and `setMany` performed **no permission check** — so on a standard `os serve --server` deployment (settings + HTTP server composed by default, routes registered on the raw app with no auth middleware) an **unauthenticated** remote client could write tenant- or platform-scoped settings (including the auth security-policy, localization, and company manifests) and enumerate every namespace.
10+
11+
Fixes:
12+
13+
- **Verified identity.** `SettingsServicePlugin` now derives the caller's identity and capabilities from the platform's verified resolution (`resolveAuthzContext` — session cookie / API key / OAuth), never from request headers. The route default is now SECURE: it trusts no identity header and yields an anonymous, denied context.
14+
- **Capability gates.** Manifest `readPermission` / `writePermission` are enforced for HTTP callers: reads of a protected namespace, writes, and actions require the declared capability (writes default to at least the read capability, never ungated). Enforced via a new `enforced` flag set only at the HTTP boundary — **in-process/boot callers (`kernel.getService('settings')`, seed) are unchanged** and keep full trusted access.
15+
- Unauthenticated HTTP callers can no longer enumerate protected manifests or write; a `403 SETTINGS_FORBIDDEN` is returned when the capability is missing.
16+
17+
**`setup.write` capability now real.** Enforcing the manifests' declared `writePermission` surfaced a modeling gap: `setup.write` (the write counterpart to `setup.access`, used by the branding / company / localization / feature-flag manifests) was referenced but never declared or granted — so under enforcement *nobody*, not even an admin, could write those namespaces. It is now a declared platform capability (`PLATFORM_CAPABILITIES`) held by `admin_full_access` and `organization_admin`, alongside `setup.access`.
18+
19+
**Behaviour change:** a deployment that relied on the old header-trusted default must present a real verified session/API-key/OAuth credential (which the console already does). A custom integration may still inject its own `contextFromRequest`.
20+
21+
Found by an adversarial security review of the request→ExecutionContext trust boundary.

packages/plugins/plugin-security/src/objects/default-permission-sets.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ export const defaultPermissionSets: PermissionSet[] = [
102102
'manage_metadata',
103103
'manage_platform_settings',
104104
'setup.access',
105+
'setup.write',
105106
'studio.access',
106107
],
107108
}),
@@ -154,7 +155,7 @@ export const defaultPermissionSets: PermissionSet[] = [
154155
sys_user_permission_set: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false },
155156
sys_user_position: { allowRead: true, allowCreate: false, allowEdit: false, allowDelete: false },
156157
},
157-
systemPermissions: ['manage_org_users', 'setup.access'],
158+
systemPermissions: ['manage_org_users', 'setup.access', 'setup.write'],
158159
rowLevelSecurity: [
159160
{
160161
name: 'tenant_isolation',

packages/services/service-settings/src/settings-routes.test.ts

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,17 @@ function makeReqRes(opts: { params?: Record<string, string>; body?: any; headers
4141
return { req, res, state };
4242
}
4343

44+
// [Finding-1] An authorized admin context (verified, holds the branding
45+
// manifest's setup.access/setup.write capabilities). The production plugin
46+
// derives this from the verified session/API-key; here we inject it directly.
47+
const adminProvider = () => ({ enforced: true, permissions: ['setup.access', 'setup.write'] });
48+
4449
describe('settings-routes', () => {
4550
it('GET /api/settings → manifests', async () => {
4651
const http = new MockHttp();
4752
const svc = new SettingsService();
4853
svc.registerManifest(brandingSettingsManifest);
49-
registerSettingsRoutes(http, svc);
54+
registerSettingsRoutes(http, svc, { contextFromRequest: adminProvider });
5055

5156
const h = http.routes.get('GET /api/settings')!;
5257
const { req, res, state } = makeReqRes();
@@ -58,7 +63,7 @@ describe('settings-routes', () => {
5863
const http = new MockHttp();
5964
const svc = new SettingsService({ env: {} });
6065
svc.registerManifest(brandingSettingsManifest);
61-
registerSettingsRoutes(http, svc);
66+
registerSettingsRoutes(http, svc, { contextFromRequest: adminProvider });
6267

6368
const h = http.routes.get('GET /api/settings/:namespace')!;
6469
const { req, res, state } = makeReqRes({ params: { namespace: 'branding' } });
@@ -71,7 +76,7 @@ describe('settings-routes', () => {
7176
const http = new MockHttp();
7277
const svc = new SettingsService({ env: { OS_BRANDING_WORKSPACE_NAME: 'X' } });
7378
svc.registerManifest(brandingSettingsManifest);
74-
registerSettingsRoutes(http, svc);
79+
registerSettingsRoutes(http, svc, { contextFromRequest: adminProvider });
7580

7681
const h = http.routes.get('PUT /api/settings/:namespace')!;
7782
const { req, res, state } = makeReqRes({ params: { namespace: 'branding' }, body: { workspace_name: 'Y' } });
@@ -94,7 +99,7 @@ describe('settings-routes', () => {
9499
const http = new MockHttp();
95100
const svc = new SettingsService({ env: {} });
96101
svc.registerManifest(brandingSettingsManifest);
97-
registerSettingsRoutes(http, svc);
102+
registerSettingsRoutes(http, svc, { contextFromRequest: adminProvider });
98103

99104
const h = http.routes.get('PUT /api/settings/:namespace')!;
100105
const { req, res, state } = makeReqRes({ params: { namespace: 'branding' }, body: { values: { workspace_name: 'My Co' } } });
@@ -108,7 +113,7 @@ describe('settings-routes', () => {
108113
const http = new MockHttp();
109114
const svc = new SettingsService({ env: {} });
110115
svc.registerManifest(brandingSettingsManifest);
111-
registerSettingsRoutes(http, svc);
116+
registerSettingsRoutes(http, svc, { contextFromRequest: adminProvider });
112117

113118
const h = http.routes.get('PUT /api/settings/:namespace')!;
114119
// Exactly what GET returns: { values: { key: { value, source, ... } } }
@@ -126,12 +131,86 @@ describe('settings-routes', () => {
126131
const svc = new SettingsService({ env: {} });
127132
svc.registerManifest(brandingSettingsManifest);
128133
svc.registerAction('branding', 'ping', () => ({ ok: true, message: 'pong' }));
129-
registerSettingsRoutes(http, svc);
134+
registerSettingsRoutes(http, svc, { contextFromRequest: adminProvider });
130135

131136
const h = http.routes.get('POST /api/settings/:namespace/:actionId')!;
132137
const { req, res, state } = makeReqRes({ params: { namespace: 'branding', actionId: 'ping' }, body: null });
133138
await h(req, res);
134139
expect(state.status).toBe(200);
135140
expect(state.body.ok).toBe(true);
136141
});
142+
143+
// ── [Finding-1] the DEFAULT (no verified provider) is SECURE ──────────────
144+
// Header-trusted identity is gone; an unauthenticated request can neither
145+
// enumerate protected namespaces nor write them.
146+
it('anonymous GET /api/settings hides manifests that require a read capability', async () => {
147+
const http = new MockHttp();
148+
const svc = new SettingsService();
149+
svc.registerManifest(brandingSettingsManifest); // requires setup.access
150+
registerSettingsRoutes(http, svc); // secure default → anonymous + enforced
151+
152+
const h = http.routes.get('GET /api/settings')!;
153+
const { req, res, state } = makeReqRes();
154+
await h(req, res);
155+
expect(state.body.manifests.length).toBe(0);
156+
});
157+
158+
it('anonymous GET /api/settings/:ns is DENIED (403), not served', async () => {
159+
const http = new MockHttp();
160+
const svc = new SettingsService({ env: {} });
161+
svc.registerManifest(brandingSettingsManifest);
162+
registerSettingsRoutes(http, svc);
163+
164+
const h = http.routes.get('GET /api/settings/:namespace')!;
165+
const { req, res, state } = makeReqRes({ params: { namespace: 'branding' } });
166+
await h(req, res);
167+
expect(state.status).toBe(403);
168+
expect(state.body.error.code).toBe('SETTINGS_FORBIDDEN');
169+
});
170+
171+
it('anonymous PUT /api/settings/:ns is DENIED (403) — the unauthenticated write hole is closed', async () => {
172+
const http = new MockHttp();
173+
const svc = new SettingsService({ env: {} });
174+
svc.registerManifest(brandingSettingsManifest);
175+
registerSettingsRoutes(http, svc);
176+
177+
const h = http.routes.get('PUT /api/settings/:namespace')!;
178+
const { req, res, state } = makeReqRes({ params: { namespace: 'branding' }, body: { workspace_name: 'pwn' } });
179+
await h(req, res);
180+
expect(state.status).toBe(403);
181+
expect(state.body.error.code).toBe('SETTINGS_FORBIDDEN');
182+
});
183+
184+
it('a spoofed x-user-id / x-permissions header grants NOTHING (default ignores identity headers)', async () => {
185+
const http = new MockHttp();
186+
const svc = new SettingsService({ env: {} });
187+
svc.registerManifest(brandingSettingsManifest);
188+
registerSettingsRoutes(http, svc);
189+
190+
const h = http.routes.get('PUT /api/settings/:namespace')!;
191+
const { req, res, state } = makeReqRes({
192+
params: { namespace: 'branding' },
193+
body: { workspace_name: 'pwn' },
194+
headers: { 'x-user-id': 'attacker', 'x-permissions': 'setup.write,setup.access' },
195+
});
196+
await h(req, res);
197+
expect(state.status).toBe(403);
198+
});
199+
200+
it('a caller holding only read (setup.access) may read but NOT write', async () => {
201+
const http = new MockHttp();
202+
const svc = new SettingsService({ env: {} });
203+
svc.registerManifest(brandingSettingsManifest);
204+
registerSettingsRoutes(http, svc, { contextFromRequest: () => ({ enforced: true, permissions: ['setup.access'] }) });
205+
206+
const read = http.routes.get('GET /api/settings/:namespace')!;
207+
const r1 = makeReqRes({ params: { namespace: 'branding' } });
208+
await read(r1.req, r1.res);
209+
expect(r1.state.body.manifest.namespace).toBe('branding');
210+
211+
const write = http.routes.get('PUT /api/settings/:namespace')!;
212+
const r2 = makeReqRes({ params: { namespace: 'branding' }, body: { workspace_name: 'X' } });
213+
await write(r2.req, r2.res);
214+
expect(r2.state.status).toBe(403); // has setup.access, lacks setup.write
215+
});
137216
});

packages/services/service-settings/src/settings-routes.ts

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
1717
import { SettingsService } from './settings-service.js';
1818
import {
19+
SettingsForbiddenError,
1920
SettingsLockedError,
2021
SettingsValidationError,
2122
UnknownKeyError,
@@ -27,27 +28,23 @@ export interface SettingsRoutesOptions {
2728
/** Base path. Default `/api/settings`. */
2829
basePath?: string;
2930
/**
30-
* Extract caller identity from the request. The default reads
31-
* `x-user-id` / `x-tenant-id` headers and parses
32-
* `x-permissions` as a comma-separated list — fine for dev and
33-
* straightforward to override in production wiring.
31+
* Derive the VERIFIED caller identity from the request. Production wiring
32+
* (`SettingsServicePlugin`) passes a resolver backed by the platform's
33+
* verified session / API-key / OAuth resolution (`resolveAuthzContext`), so
34+
* `permissions` reflect real capabilities and are never spoofable.
35+
*
36+
* [Finding-1] The default is SECURE: it trusts NO identity header and yields
37+
* an anonymous, `enforced` context (deny protected reads + all writes). The
38+
* old default trusted `x-user-id` / `x-permissions` headers, which let an
39+
* unauthenticated client forge any identity and write platform settings.
3440
*/
35-
contextFromRequest?: (req: IHttpRequest) => SettingsContext;
41+
contextFromRequest?: (req: IHttpRequest) => SettingsContext | Promise<SettingsContext>;
3642
}
3743

38-
const defaultContext = (req: IHttpRequest): SettingsContext => {
39-
const header = (name: string): string | undefined => {
40-
const v = req.headers?.[name];
41-
return Array.isArray(v) ? v[0] : v;
42-
};
43-
const perms = header('x-permissions');
44-
return {
45-
userId: header('x-user-id'),
46-
tenantId: header('x-tenant-id'),
47-
permissions: perms ? perms.split(',').map((s) => s.trim()).filter(Boolean) : undefined,
48-
requestId: header('x-request-id'),
49-
};
50-
};
44+
// [Finding-1] Secure default: anonymous + enforced. No identity is read from
45+
// request headers — a deployment that wants authenticated settings access must
46+
// wire a verified `contextFromRequest` (the plugin does).
47+
const defaultContext = (_req: IHttpRequest): SettingsContext => ({ enforced: true });
5148

5249
function sendError(res: IHttpResponse, status: number, code: string, message: string, extra?: Record<string, unknown>) {
5350
res.status(status).json({ error: { code, message, ...extra } });
@@ -63,22 +60,28 @@ export function registerSettingsRoutes(
6360

6461
http.get(base, (async (req, res) => {
6562
try {
66-
const ctx = ctxOf(req);
63+
const ctx = await ctxOf(req);
6764
const manifests = service.listManifests(ctx);
6865
await res.json({ manifests });
6966
} catch (err: any) {
70-
sendError(res, 500, 'INTERNAL', err?.message ?? 'Failed to list manifests');
67+
if (err instanceof SettingsForbiddenError) {
68+
sendError(res, 403, 'SETTINGS_FORBIDDEN', err.message, { namespace: err.namespace });
69+
} else {
70+
sendError(res, 500, 'INTERNAL', err?.message ?? 'Failed to list manifests');
71+
}
7172
}
7273
}) satisfies RouteHandler);
7374

7475
http.get(`${base}/:namespace`, (async (req, res) => {
7576
const ns = req.params.namespace;
7677
try {
77-
const ctx = ctxOf(req);
78+
const ctx = await ctxOf(req);
7879
const payload = await service.getNamespace(ns, ctx);
7980
await res.json(payload);
8081
} catch (err: any) {
81-
if (err instanceof UnknownNamespaceError) {
82+
if (err instanceof SettingsForbiddenError) {
83+
sendError(res, 403, 'SETTINGS_FORBIDDEN', err.message, { namespace: err.namespace });
84+
} else if (err instanceof UnknownNamespaceError) {
8285
sendError(res, 404, 'UNKNOWN_NAMESPACE', err.message);
8386
} else {
8487
sendError(res, 500, 'INTERNAL', err?.message ?? 'Failed to read namespace');
@@ -110,11 +113,13 @@ export function registerSettingsRoutes(
110113
);
111114
}
112115
try {
113-
const ctx = ctxOf(req);
116+
const ctx = await ctxOf(req);
114117
const result = await service.setMany(ns, body, ctx);
115118
await res.json({ values: result });
116119
} catch (err: any) {
117-
if (err instanceof SettingsLockedError) {
120+
if (err instanceof SettingsForbiddenError) {
121+
sendError(res, 403, 'SETTINGS_FORBIDDEN', err.message, { namespace: err.namespace });
122+
} else if (err instanceof SettingsLockedError) {
118123
sendError(res, 409, 'SETTINGS_LOCKED', err.message, {
119124
namespace: err.namespace,
120125
key: err.key,
@@ -138,12 +143,14 @@ export function registerSettingsRoutes(
138143
http.post(`${base}/:namespace/:actionId`, (async (req, res) => {
139144
const { namespace, actionId } = req.params;
140145
try {
141-
const ctx = ctxOf(req);
146+
const ctx = await ctxOf(req);
142147
const result = await service.runAction(namespace, actionId, req.body, ctx);
143148
const status = result.ok ? 200 : 400;
144149
await res.status(status).json(result);
145150
} catch (err: any) {
146-
if (err instanceof UnknownNamespaceError) {
151+
if (err instanceof SettingsForbiddenError) {
152+
sendError(res, 403, 'SETTINGS_FORBIDDEN', err.message, { namespace: err.namespace });
153+
} else if (err instanceof UnknownNamespaceError) {
147154
sendError(res, 404, 'UNKNOWN_NAMESPACE', err.message);
148155
} else {
149156
sendError(res, 500, 'INTERNAL', err?.message ?? 'Action failed');

packages/services/service-settings/src/settings-service-plugin.ts

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import type { Plugin, PluginContext } from '@objectstack/core';
4-
import type { IHttpServer, IDataEngine } from '@objectstack/spec/contracts';
4+
import { resolveAuthzContext } from '@objectstack/core';
5+
import type { IHttpServer, IDataEngine, IHttpRequest } from '@objectstack/spec/contracts';
6+
import type { SettingsContext } from './settings-service.types.js';
57
import type { SettingsManifest } from '@objectstack/spec/system';
68
import { SettingsService } from './settings-service.js';
79
import type { ICryptoProvider } from '@objectstack/spec/contracts';
@@ -200,7 +202,49 @@ export class SettingsServicePlugin implements Plugin {
200202
);
201203
return;
202204
}
203-
registerSettingsRoutes(http, this.service!, { basePath: this.opts.basePath });
205+
// [Finding-1] Derive the caller's identity + capabilities from the
206+
// platform's VERIFIED resolution (session cookie / API key / OAuth), NOT
207+
// from spoofable `x-user-id` / `x-permissions` headers. `resolveAuthzContext`
208+
// returns real, server-verified capabilities; `enforced: true` makes the
209+
// service apply the manifest read/write permission gates. An unresolvable
210+
// request fails CLOSED (anonymous + enforced → denied).
211+
const verifiedContextFromRequest = async (req: IHttpRequest): Promise<SettingsContext> => {
212+
try {
213+
const ql: any = ctx.getService('objectql');
214+
const headers = new Headers();
215+
for (const [k, v] of Object.entries(req.headers ?? {})) {
216+
if (v == null) continue;
217+
headers.set(String(k), Array.isArray(v) ? v.join(',') : String(v));
218+
}
219+
const getSession = async (h: any) => {
220+
try {
221+
const authService: any = ctx.getService('auth');
222+
let api: any = authService?.api;
223+
if (!api && typeof authService?.getApi === 'function') api = await authService.getApi();
224+
return await api?.getSession?.({ headers: h });
225+
} catch {
226+
return undefined;
227+
}
228+
};
229+
const authz = await resolveAuthzContext({ ql, headers, getSession });
230+
return {
231+
userId: authz.userId,
232+
tenantId: authz.tenantId,
233+
// Manifest read/write permissions are system CAPABILITIES
234+
// (setup.access / setup.write / manage_platform_settings) — union
235+
// both aggregates so either kind of declared gate resolves.
236+
permissions: [...(authz.systemPermissions ?? []), ...(authz.permissions ?? [])],
237+
enforced: true,
238+
};
239+
} catch {
240+
return { enforced: true }; // fail closed
241+
}
242+
};
243+
244+
registerSettingsRoutes(http, this.service!, {
245+
basePath: this.opts.basePath,
246+
contextFromRequest: verifiedContextFromRequest,
247+
});
204248
ctx.logger?.info?.(
205249
'SettingsServicePlugin: REST routes registered at ' + (this.opts.basePath ?? '/api/settings'),
206250
);

0 commit comments

Comments
 (0)