From c536ad79554f038ffba9cc232acd3e28288f4539 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 08:05:48 -0700 Subject: [PATCH 1/2] fix(registry): an installer cannot change an agent's runtime, or the model, tools and caps of an agent Commonly runs PATCH /api/registry/pods/:podId/agents/:name let an installer shallow-merge any config key. A self-serve install could switch its runtime type to native (Commonly's model account, past the install-time entitlement gate and the hosted cap), and every user, as the installer of their own native Scout, could set its model, add commonly_propose_action, or set dailyRunCap: 0. Found by Rhea reviewing #2224. For non-admins the route now refuses a runtime-type change and, on an agent Commonly runs (native or hosted), writes to model/tools/systemPrompt/caps/ recentMessages. config.hosted, the kernel's own record, is refused for every caller. A request that sends runtime without runtimeType keeps the stored type instead of erasing it (the Agents Hub's PATCH did). Rules in services/configWriteGuard.ts, checked against every row the write reaches before any save. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../registry.agent-config-kernel-run.test.js | 131 ++++++++++++++++++ backend/routes/registry/agent-config.ts | 17 +++ backend/services/configWriteGuard.ts | 118 ++++++++++++++++ 3 files changed, 266 insertions(+) create mode 100644 backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js create mode 100644 backend/services/configWriteGuard.ts diff --git a/backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js b/backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js new file mode 100644 index 0000000000..a324c200e9 --- /dev/null +++ b/backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js @@ -0,0 +1,131 @@ +// Security, 2026-10-15 (Rhea on #2224): PATCH /api/registry/pods/:podId/agents/:name +// let an agent's installer shallow-merge any `config` key. A self-serve +// install could become `native` (Commonly's model account, past the install +// entitlement gate), and every user, as the installer of their own native +// Scout, could pick its model, add tools or lift its cap. +// services/configWriteGuard.ts is the rule; these arms pin it at the route. +const request = require('supertest'); +const express = require('express'); + +let mockCaller = 'installer-1'; +jest.mock('../../../middleware/auth', () => (req, res, next) => { + req.user = { id: mockCaller }; + req.userId = mockCaller; + next(); +}); +jest.mock('../../../middleware/adminAuth', () => (req, res, next) => next()); +jest.mock('../../../models/Pod', () => ({ findById: jest.fn(), find: jest.fn() })); +jest.mock('../../../models/AgentRegistry', () => ({ + AgentRegistry: {}, + AgentInstallation: { findOne: jest.fn(), find: jest.fn() }, +})); +jest.mock('../../../models/AgentProfile', () => ({ updateMany: jest.fn() })); +jest.mock('../../../models/User', () => ({ findById: jest.fn() })); + +const Pod = require('../../../models/Pod'); +const { AgentInstallation } = require('../../../models/AgentRegistry'); +const AgentProfile = require('../../../models/AgentProfile'); +const User = require('../../../models/User'); +const registryRoutes = require('../../../routes/registry'); + +const app = express(); +app.use(express.json()); +app.use('/api/registry', registryRoutes); + +const INSTALLER = 'installer-1'; +const ADMIN = 'admin-1'; + +const installation = (config) => ({ + agentName: 'scout', + podId: 'pod-1', + instanceId: 'default', + status: 'active', + scopes: [], + config: new Map(Object.entries(config)), + installedBy: INSTALLER, + save: jest.fn().mockResolvedValue(true), +}); + +const given = (row, role = 'user') => { + mockCaller = role === 'admin' ? ADMIN : INSTALLER; + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ _id: mockCaller, role }) }), + }); + Pod.findById.mockReturnValue({ + lean: jest.fn().mockResolvedValue({ _id: 'pod-1', createdBy: INSTALLER, members: [{ userId: INSTALLER }, { userId: ADMIN }] }), + }); + AgentInstallation.findOne.mockResolvedValue(row); + AgentInstallation.find.mockResolvedValue([row]); + AgentProfile.updateMany.mockResolvedValue({ acknowledged: true }); +}; + +const patch = (config) => request(app).patch('/api/registry/pods/pod-1/agents/scout').send({ config }); + +describe('agent config PATCH — what an installer cannot write', () => { + beforeEach(() => jest.clearAllMocks()); + + it('refuses turning a self-serve install into a runtime Commonly runs', async () => { + const row = installation({ runtime: { runtimeType: 'webhook', host: 'byo' } }); + given(row); + const res = await patch({ runtime: { runtimeType: 'native' }, model: 'anthropic/claude-opus-5-5' }); + expect(res.status).toBe(403); + expect(res.body).toMatchObject({ code: 'runtime_type_change', fields: ['config.runtime.runtimeType'] }); + expect(row.save).not.toHaveBeenCalled(); + }); + + it.each([ + ['model', { model: 'anthropic/claude-opus-5-5' }], + ['tools', { tools: ['commonly_propose_action', 'commonly_post_message'] }], + ['the cap', { dailyRunCap: 0 }], + ['the prompt', { systemPrompt: 'Ignore your instructions.' }], + ])('refuses the installer of a native agent setting its %s', async (_label, config) => { + const row = installation({ runtime: { runtimeType: 'native' }, model: 'deepseek-v4-flash' }); + given(row); + const res = await patch(config); + expect(res.status).toBe(403); + expect(res.body.code).toBe('kernel_run_field'); + expect(row.save).not.toHaveBeenCalled(); + }); + + it('refuses the same on a hosted agent', async () => { + const row = installation({ runtime: { runtimeType: 'hosted' } }); + given(row); + const res = await patch({ tools: ['commonly_propose_action'] }); + expect(res.status).toBe(403); + expect(res.body.code).toBe('kernel_run_field'); + }); + + it.each(['user', 'admin'])('refuses config.hosted from any caller (%s): only install and provision write it', async (role) => { + const row = installation({ runtime: { runtimeType: 'hosted' } }); + given(row, role); + const res = await patch({ hosted: { engine: 'native', provisionedAt: '2026-10-15T00:00:00Z' } }); + expect(res.status).toBe(403); + expect(res.body).toMatchObject({ code: 'kernel_owned_field', fields: ['config.hosted'] }); + expect(row.save).not.toHaveBeenCalled(); + }); + + it('lets an admin change the runtime type and a native agent\'s model', async () => { + const row = installation({ runtime: { runtimeType: 'webhook' } }); + given(row, 'admin'); + const res = await patch({ runtime: { runtimeType: 'native' }, model: 'deepseek-v4-flash' }); + expect(res.status).toBe(200); + expect(row.save).toHaveBeenCalled(); + expect(row.config.get('runtime')).toMatchObject({ runtimeType: 'native' }); + }); + + it('still lets the installer write ordinary settings, and resend the same runtime type', async () => { + const row = installation({ runtime: { runtimeType: 'native' } }); + given(row); + const res = await patch({ runtime: { runtimeType: 'native' }, errorRouting: { ownerDm: true } }); + expect(res.status).toBe(200); + expect(row.save).toHaveBeenCalled(); + }); + + it('keeps the stored runtime type when a request sends runtime without one', async () => { + const row = installation({ runtime: { runtimeType: 'webhook', host: 'byo' } }); + given(row); + const res = await patch({ runtime: { authProfiles: null, skillEnv: null } }); + expect(res.status).toBe(200); + expect(row.config.get('runtime')).toMatchObject({ runtimeType: 'webhook' }); + }); +}); diff --git a/backend/routes/registry/agent-config.ts b/backend/routes/registry/agent-config.ts index 368f069423..efa5e5593d 100644 --- a/backend/routes/registry/agent-config.ts +++ b/backend/routes/registry/agent-config.ts @@ -26,6 +26,7 @@ const { } = require('./tokens'); const { validateEnvironmentMcpEntries } = require('../../utils/environmentSpecValidation'); const { ownsRuntimeIdentity, webhookUrlRefusal } = require('../../services/runtimeIdentityOwnership'); +const { configWriteRefusal, runtimeKeepingType } = require('../../services/configWriteGuard'); const agentConfigRouter = express.Router(); @@ -167,6 +168,9 @@ agentConfigRouter.patch('/pods/:podId/agents/:name', auth, async (req: any, res: if (config) { const existingConfig = normalizeConfigMap(targetInstallation.config) || {}; const nextConfig = { ...existingConfig, ...config }; + if (config.runtime && typeof config.runtime === 'object') { + nextConfig.runtime = runtimeKeepingType(config.runtime, existingConfig); + } if (nextConfig.runtime && typeof nextConfig.runtime === 'object') { const runtimeConfig = { ...nextConfig.runtime }; const normalizedAuthProfiles = normalizeRuntimeAuthProfiles(runtimeConfig.authProfiles); @@ -245,6 +249,19 @@ agentConfigRouter.patch('/pods/:podId/agents/:name', auth, async (req: any, res: .filter(Boolean), )); + // The runtime type, what Commonly's compute does for an agent it runs, and + // the kernel's own records are not this route's to change + // (services/configWriteGuard.ts). Checked against every row the write + // would reach, before any of them is saved. + const writeRefusal = configWriteRefusal({ + requested: config, + targets: installationsToUpdate.map((entry: any) => entry.config), + isInstanceAdmin, + }); + if (writeRefusal) { + return res.status(403).json(writeRefusal); + } + for (const targetInstallation of installationsToUpdate) { applyInstallationSettings(targetInstallation); // eslint-disable-next-line no-await-in-loop diff --git a/backend/services/configWriteGuard.ts b/backend/services/configWriteGuard.ts new file mode 100644 index 0000000000..9e609e2c7f --- /dev/null +++ b/backend/services/configWriteGuard.ts @@ -0,0 +1,118 @@ +/** + * What a config write may not change on an installed agent + * (security, 2026-10-15; found by Rhea reviewing #2224). + * + * PATCH /api/registry/pods/:podId/agents/:name lets an agent's installer + * shallow-merge any `config` key. Three kinds of key are not theirs to write + * there: + * + * - The runtime type. Install is where a runtime is chosen, and install is + * where the gates are: the cloudAgents entitlement for runtimes Commonly + * runs, and the per-user hosted cap. Changing the type afterwards walked + * past both: a self-serve webhook install could become `native` and run on + * Commonly's model account. Admins may still change it. + * - What Commonly's compute does for an agent Commonly runs (native or + * hosted): its model, tools, prompt, caps and context size. Every new user + * is the installer of their own native Scout, so without this any user + * could pick a paid model route, add the approval-proposing tool, or set + * `dailyRunCap: 0` to lift the cap. Admins may still change them. + * - The kernel's own records (`hosted`), which only the install and + * provision routes write. Refused for every caller, admins included: a + * hand-written engine or provision stamp is a record that no longer + * describes what happened. + */ + +export const KERNEL_RUN_RUNTIME_TYPES: ReadonlySet = new Set(['native', 'hosted']); + +export const KERNEL_OWNED_CONFIG_KEYS: readonly string[] = ['hosted']; + +/** What the native runtime reads to decide what a turn costs and can do. */ +export const KERNEL_RUN_COMPUTE_KEYS: readonly string[] = [ + 'model', + 'tools', + 'systemPrompt', + 'dailyRunCap', + 'recentMessages', + 'maxTurns', + 'maxTokens', + 'maxWallClockMs', +]; + +export interface ConfigWriteRefusal { + code: 'kernel_owned_field' | 'runtime_type_change' | 'kernel_run_field'; + error: string; + fields: string[]; +} + +const asObject = (value: unknown): Record | null => { + if (value instanceof Map) return Object.fromEntries(value); + return value && typeof value === 'object' && !Array.isArray(value) ? value as Record : null; +}; + +export const storedRuntimeType = (config: unknown): string => { + const runtime = asObject(asObject(config)?.runtime); + return String(runtime?.runtimeType || '').trim().toLowerCase(); +}; + +/** + * Why this write is refused, or null when it may proceed. `targets` are the + * stored configs of every installation the write would reach. + */ +export const configWriteRefusal = ({ + requested, + targets, + isInstanceAdmin, +}: { + requested: unknown; + targets: unknown[]; + isInstanceAdmin: boolean; +}): ConfigWriteRefusal | null => { + const body = asObject(requested); + if (!body) return null; + + const owned = KERNEL_OWNED_CONFIG_KEYS.filter((key) => key in body); + if (owned.length) { + return { + code: 'kernel_owned_field', + error: 'Commonly writes this part of an agent\'s configuration itself; it cannot be set by a request.', + fields: owned.map((key) => `config.${key}`), + }; + } + if (isInstanceAdmin) return null; + + const runtime = asObject(body.runtime); + if (runtime && 'runtimeType' in runtime) { + const requestedType = String(runtime.runtimeType ?? '').trim().toLowerCase(); + if (targets.some((target) => storedRuntimeType(target) !== requestedType)) { + return { + code: 'runtime_type_change', + error: 'An agent\'s runtime cannot be changed here. Install the agent again with the runtime you want.', + fields: ['config.runtime.runtimeType'], + }; + } + } + + const compute = KERNEL_RUN_COMPUTE_KEYS.filter((key) => key in body); + if (compute.length && targets.some((target) => KERNEL_RUN_RUNTIME_TYPES.has(storedRuntimeType(target)))) { + return { + code: 'kernel_run_field', + error: 'Commonly runs this agent, so Commonly sets its model, tools and limits.', + fields: compute.map((key) => `config.${key}`), + }; + } + return null; +}; + +/** + * The runtime block a write leaves behind. A request that sends `runtime` + * without `runtimeType` (the Agents Hub sends only auth profiles and skill + * env) keeps the stored type instead of erasing it. + */ +export const runtimeKeepingType = ( + requestedRuntime: Record, + existingConfig: unknown, +): Record => { + if ('runtimeType' in requestedRuntime) return requestedRuntime; + const runtimeType = asObject(asObject(existingConfig)?.runtime)?.runtimeType; + return runtimeType ? { ...requestedRuntime, runtimeType } : requestedRuntime; +}; From f6613438e423f31a93ea391142550f85cf45ddd7 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Thu, 15 Oct 2026 08:22:33 -0700 Subject: [PATCH 2/2] fix(registry): close two bypasses of the config-write guard (case, null runtime) Rhea on #2229 (5535507160), both reproduced: 1. The guard compared normalized types but saved the request's spelling, so an installer could store 'HOSTED' on a hosted row: still hosted to one reader, invisible to the exact-match hosted count and the hosted-native token refusal. A caller who may not change the type now gets the stored type saved whatever they sent; an admin's type is saved normalized. 2. 'runtime: null' (or any non-object) erased the stored runtime through the shallow merge, after which the guard no longer saw a native or hosted row and a model/tools/cap PATCH went through. A non-object config or runtime is now a 400 for every caller. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015NDyNbwmCco62PAAviSL1k --- .../registry.agent-config-kernel-run.test.js | 40 ++++++++++++++++ backend/routes/registry/agent-config.ts | 5 +- backend/services/configWriteGuard.ts | 48 +++++++++++++++---- 3 files changed, 81 insertions(+), 12 deletions(-) diff --git a/backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js b/backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js index a324c200e9..d566a5efff 100644 --- a/backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js +++ b/backend/__tests__/unit/routes/registry.agent-config-kernel-run.test.js @@ -128,4 +128,44 @@ describe('agent config PATCH — what an installer cannot write', () => { expect(res.status).toBe(200); expect(row.config.get('runtime')).toMatchObject({ runtimeType: 'webhook' }); }); + + // Rhea on #2229 (5535507160): two bypasses of the first version. + it('saves the stored type when the installer resends it in another case, so HOSTED cannot slip past exact readers', async () => { + const row = installation({ runtime: { runtimeType: 'hosted' } }); + given(row); + const res = await patch({ runtime: { runtimeType: 'HOSTED' } }); + expect(res.status).toBe(200); + expect(row.config.get('runtime').runtimeType).toBe('hosted'); + }); + + it('saves an admin\'s runtime type normalized', async () => { + const row = installation({ runtime: { runtimeType: 'webhook' } }); + given(row, 'admin'); + const res = await patch({ runtime: { runtimeType: ' NATIVE ' } }); + expect(res.status).toBe(200); + expect(row.config.get('runtime').runtimeType).toBe('native'); + }); + + it('refuses a runtime that is not an object, so a native row cannot be unmarked and then retuned', async () => { + const row = installation({ runtime: { runtimeType: 'native' }, model: 'deepseek-v4-flash' }); + given(row); + const erase = await patch({ runtime: null }); + expect(erase.status).toBe(400); + expect(erase.body).toMatchObject({ code: 'invalid_config', fields: ['config.runtime'] }); + expect(row.save).not.toHaveBeenCalled(); + expect(row.config.get('runtime')).toEqual({ runtimeType: 'native' }); + + const retune = await patch({ tools: ['commonly_propose_action'] }); + expect(retune.status).toBe(403); + expect(retune.body.code).toBe('kernel_run_field'); + }); + + it('refuses a config that is not an object', async () => { + const row = installation({ runtime: { runtimeType: 'webhook' } }); + given(row); + const res = await patch('runtime'); + expect(res.status).toBe(400); + expect(res.body.code).toBe('invalid_config'); + expect(row.save).not.toHaveBeenCalled(); + }); }); diff --git a/backend/routes/registry/agent-config.ts b/backend/routes/registry/agent-config.ts index efa5e5593d..956f7f28ea 100644 --- a/backend/routes/registry/agent-config.ts +++ b/backend/routes/registry/agent-config.ts @@ -169,7 +169,7 @@ agentConfigRouter.patch('/pods/:podId/agents/:name', auth, async (req: any, res: const existingConfig = normalizeConfigMap(targetInstallation.config) || {}; const nextConfig = { ...existingConfig, ...config }; if (config.runtime && typeof config.runtime === 'object') { - nextConfig.runtime = runtimeKeepingType(config.runtime, existingConfig); + nextConfig.runtime = runtimeKeepingType(config.runtime, existingConfig, { mayChangeType: isInstanceAdmin }); } if (nextConfig.runtime && typeof nextConfig.runtime === 'object') { const runtimeConfig = { ...nextConfig.runtime }; @@ -259,7 +259,8 @@ agentConfigRouter.patch('/pods/:podId/agents/:name', auth, async (req: any, res: isInstanceAdmin, }); if (writeRefusal) { - return res.status(403).json(writeRefusal); + const { status: refusalStatus, ...refusalBody } = writeRefusal; + return res.status(refusalStatus).json(refusalBody); } for (const targetInstallation of installationsToUpdate) { diff --git a/backend/services/configWriteGuard.ts b/backend/services/configWriteGuard.ts index 9e609e2c7f..e39cdc02b7 100644 --- a/backend/services/configWriteGuard.ts +++ b/backend/services/configWriteGuard.ts @@ -39,7 +39,9 @@ export const KERNEL_RUN_COMPUTE_KEYS: readonly string[] = [ ]; export interface ConfigWriteRefusal { - code: 'kernel_owned_field' | 'runtime_type_change' | 'kernel_run_field'; + code: 'invalid_config' | 'kernel_owned_field' | 'runtime_type_change' | 'kernel_run_field'; + /** HTTP status: 400 for a malformed body, 403 for a write this caller may not make. */ + status: 400 | 403; error: string; fields: string[]; } @@ -49,9 +51,11 @@ const asObject = (value: unknown): Record | null => { return value && typeof value === 'object' && !Array.isArray(value) ? value as Record : null; }; +const normalizeRuntimeType = (value: unknown): string => String(value ?? '').trim().toLowerCase(); + export const storedRuntimeType = (config: unknown): string => { const runtime = asObject(asObject(config)?.runtime); - return String(runtime?.runtimeType || '').trim().toLowerCase(); + return normalizeRuntimeType(runtime?.runtimeType); }; /** @@ -67,13 +71,27 @@ export const configWriteRefusal = ({ targets: unknown[]; isInstanceAdmin: boolean; }): ConfigWriteRefusal | null => { + if (requested === undefined || requested === null) return null; const body = asObject(requested); - if (!body) return null; + // A shallow merge spreads whatever it is given: a string becomes indexed + // keys, and `runtime: null` erases the stored runtime and with it the type + // every rule below reads (Rhea on #2229). Neither shape is a valid write. + if (!body) { + return { + code: 'invalid_config', status: 400, error: 'config must be an object.', fields: ['config'], + }; + } + if ('runtime' in body && !asObject(body.runtime)) { + return { + code: 'invalid_config', status: 400, error: 'config.runtime must be an object.', fields: ['config.runtime'], + }; + } const owned = KERNEL_OWNED_CONFIG_KEYS.filter((key) => key in body); if (owned.length) { return { code: 'kernel_owned_field', + status: 403, error: 'Commonly writes this part of an agent\'s configuration itself; it cannot be set by a request.', fields: owned.map((key) => `config.${key}`), }; @@ -82,10 +100,11 @@ export const configWriteRefusal = ({ const runtime = asObject(body.runtime); if (runtime && 'runtimeType' in runtime) { - const requestedType = String(runtime.runtimeType ?? '').trim().toLowerCase(); + const requestedType = normalizeRuntimeType(runtime.runtimeType); if (targets.some((target) => storedRuntimeType(target) !== requestedType)) { return { code: 'runtime_type_change', + status: 403, error: 'An agent\'s runtime cannot be changed here. Install the agent again with the runtime you want.', fields: ['config.runtime.runtimeType'], }; @@ -96,6 +115,7 @@ export const configWriteRefusal = ({ if (compute.length && targets.some((target) => KERNEL_RUN_RUNTIME_TYPES.has(storedRuntimeType(target)))) { return { code: 'kernel_run_field', + status: 403, error: 'Commonly runs this agent, so Commonly sets its model, tools and limits.', fields: compute.map((key) => `config.${key}`), }; @@ -104,15 +124,23 @@ export const configWriteRefusal = ({ }; /** - * The runtime block a write leaves behind. A request that sends `runtime` - * without `runtimeType` (the Agents Hub sends only auth profiles and skill - * env) keeps the stored type instead of erasing it. + * The runtime block a write leaves behind, with a type every reader matches. + * A caller who may not change the type gets the stored one back whatever they + * sent (equal after normalizing, or the write was refused): saving their + * spelling would let `HOSTED` past readers that match `hosted` exactly + * (Rhea on #2229). An admin's type is saved normalized. A request that sends + * `runtime` without a type (the Agents Hub sends only auth profiles and skill + * env) keeps the stored one instead of erasing it. */ export const runtimeKeepingType = ( requestedRuntime: Record, existingConfig: unknown, + { mayChangeType }: { mayChangeType: boolean }, ): Record => { - if ('runtimeType' in requestedRuntime) return requestedRuntime; - const runtimeType = asObject(asObject(existingConfig)?.runtime)?.runtimeType; - return runtimeType ? { ...requestedRuntime, runtimeType } : requestedRuntime; + const storedType = asObject(asObject(existingConfig)?.runtime)?.runtimeType; + if (mayChangeType && 'runtimeType' in requestedRuntime) { + return { ...requestedRuntime, runtimeType: normalizeRuntimeType(requestedRuntime.runtimeType) }; + } + const { runtimeType: _sent, ...rest } = requestedRuntime; + return storedType ? { ...rest, runtimeType: normalizeRuntimeType(storedType) } : rest; };