diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts index c6b32d35bd..c015ead971 100644 --- a/components/mcp/quota.ts +++ b/components/mcp/quota.ts @@ -3,43 +3,40 @@ * * The in-memory buckets in `rateLimit.ts` bound instantaneous rates but are * per-worker and reset on restart — insufficient as a COST control for a - * public unauthenticated tool (an LLM-backed `answer`, say). This hook lets - * the operator implement a durable policy (e.g. a persisted per-IP daily - * counter table) behind config: + * public unauthenticated tool (an LLM-backed `answer`, say). A component + * registers a durable policy (e.g. a persisted per-IP daily counter) as a + * function, so the policy is never itself an exposed Resource: * - * mcp: - * application: - * quota: - * resource: McpQuota # exported Resource path - * method: allowMcpCall # static method on it (this is the default) + * // in a component's resources.js, at module top level + * server.setMcpQuotaHandler(async ({ identity, tool, user, profile, sessionId }) => { + * if (profile !== 'application') return true; // gate per-profile in code + * const used = await bumpCounter(identity); + * return used > DAILY_LIMIT ? { allowed: false, message: 'daily quota reached' } : true; + * }); * - * Before each admitted tools/call, Harper calls - * `QuotaClass.allowMcpCall({ identity, tool, user, profile, sessionId })`. - * Return `true` (or any truthy non-object) to allow; return + * Before each admitted tools/call Harper calls the registered handler. Return + * `true` (or any truthy non-object) to allow; return * `{ allowed: false, message?, retryAfterSeconds? }` to deny — the denial * surfaces to the client as `isError` with `kind: 'quota_exceeded'`. - * Counting is the hook's business: increment on check, or on success via + * Counting is the handler's business: increment on check, or on success via * your own bookkeeping — Harper calls once per attempted tool call. * - * FAIL-CLOSED: a hook that throws (or a configured resource/method that - * doesn't resolve) DENIES the call. Cost protection that silently disables - * itself on a bug is worse than a hard failure (#1422 set this precedent - * for allow* hooks). The raw error goes to the server log only. + * FAIL-CLOSED: a handler that throws DENIES the call. Cost protection that + * silently disables itself on a bug is worse than a hard failure (#1422 set + * this precedent for allow* hooks). The raw error goes to the server log only. * - * RACE-SAFETY: the hook can run concurrently for the SAME identity — within - * a worker (interleaving across the hook's own await boundaries) and across - * workers (separate processes sharing the database). A naive read-then-write - * counter (`get` → `put used+1`) can undercount under that concurrency and - * admit calls past the configured limit. Production implementations should - * make the read-modify-write atomic: run it in a transaction that serializes - * conflicting writers, use a compare-and-set retry loop, or maintain the - * counter in a store with native atomic increments. + * RACE-SAFETY: the handler can run concurrently for the SAME identity — within + * a worker (interleaving across its own await boundaries) and across workers + * (separate processes sharing the database). A naive read-then-write counter + * (`get` → `put used+1`) can undercount under that concurrency and admit calls + * past the limit. Production handlers should make the read-modify-write atomic: + * run it in a transaction that serializes conflicting writers, use a + * compare-and-set retry loop, or maintain the counter in a store with native + * atomic increments. * - * Dispatch uses the LIVE registry class, same as custom tools — an exported - * subclass replacing the entry on reload wins. + * The latest registration wins, so a reloaded component replaces the previous + * handler. */ -import * as env from '../../utility/environment/environmentManager.ts'; -import { CONFIG_PARAMS } from '../../utility/hdbTerms.ts'; import harperLogger from '../../utility/logging/harper_logger.ts'; import type { McpProfile } from './transport.ts'; import type { AuthedUser } from './toolRegistry.ts'; @@ -62,67 +59,34 @@ export interface QuotaDenial { export type QuotaDecision = { allowed: true } | QuotaDenial; -const CONFIG_KEYS: Record = { - operations: { - resource: CONFIG_PARAMS.MCP_OPERATIONS_QUOTA_RESOURCE, - method: CONFIG_PARAMS.MCP_OPERATIONS_QUOTA_METHOD, - }, - application: { - resource: CONFIG_PARAMS.MCP_APPLICATION_QUOTA_RESOURCE, - method: CONFIG_PARAMS.MCP_APPLICATION_QUOTA_METHOD, - }, -}; - -const DEFAULT_METHOD = 'allowMcpCall'; - -type ResourcesLike = Map | undefined; - -// Test seam — mirrors resources.ts: the real registry initializes the whole -// Harper graph at import, which unit tests can't do. -let _resourcesOverride: ResourcesLike; -export function _setQuotaResourcesForTest(r: ResourcesLike): void { - _resourcesOverride = r; -} +/** + * The durable quota policy: return `true` (or any truthy non-object) to allow, + * or `{ allowed: false, message?, retryAfterSeconds? }` to deny. Receives the + * per-call {@link QuotaCheckInfo}, including `profile`, so a single handler can + * gate the operations and application profiles differently. + */ +export type McpQuotaHandler = (info: QuotaCheckInfo) => QuotaDecision | boolean | Promise; -function getResources(): ResourcesLike { - if (_resourcesOverride) return _resourcesOverride; - const { resources } = require('../../resources/Resources'); - return resources as ResourcesLike; -} +// Single, process-wide handler. Registered by a component via +// `server.setMcpQuotaHandler(fn)`; the latest registration wins (so a reloaded +// component replaces the previous one), and `undefined` clears it. +let quotaHandler: McpQuotaHandler | undefined; -/** Warn-once-per-profile state for a misconfigured hook (missing resource/method). */ -const warnedMisconfigured = new Set(); -export function _resetQuotaWarningsForTest(): void { - warnedMisconfigured.clear(); +export function setMcpQuotaHandler(handler: McpQuotaHandler | undefined): void { + quotaHandler = handler; } /** - * Run the configured durable quota hook, if any. Returns `{allowed: true}` - * when no hook is configured (the feature is opt-in). Misconfiguration and - * hook errors DENY (fail-closed) with a sanitized message. + * Run the registered durable quota handler, if any. Returns `{allowed: true}` + * when no handler is registered (the feature is opt-in). A handler that throws + * DENIES (fail-closed) with a sanitized message. */ export async function checkDurableQuota(info: QuotaCheckInfo): Promise { - const keys = CONFIG_KEYS[info.profile]; - const resourcePath = env.get(keys.resource); - if (typeof resourcePath !== 'string' || !resourcePath) { + if (!quotaHandler) { return { allowed: true }; } - const methodName = - typeof env.get(keys.method) === 'string' && env.get(keys.method) ? env.get(keys.method) : DEFAULT_METHOD; - const entry = getResources()?.get(resourcePath); - const QuotaClass = entry?.Resource as Record | undefined; - const method = QuotaClass?.[methodName as string]; - if (typeof method !== 'function') { - if (!warnedMisconfigured.has(info.profile)) { - warnedMisconfigured.add(info.profile); - harperLogger.warn( - `MCP ${info.profile} quota hook misconfigured: no exported resource '${resourcePath}' with static method '${methodName}'; DENYING tool calls (fail-closed)` - ); - } - return { allowed: false, message: 'quota policy unavailable' }; - } try { - const result = await (method as (i: QuotaCheckInfo) => unknown).call(QuotaClass, info); + const result = await quotaHandler(info); if (result && typeof result === 'object') { const decision = result as { allowed?: unknown; message?: unknown; retryAfterSeconds?: unknown }; if (decision.allowed === false) { @@ -136,10 +100,7 @@ export async function checkDurableQuota(info: QuotaCheckInfo): Promise { + // Process-wide `server.*` registrations (registerOperation, setMcpQuotaHandler) are not owned by + // a Scope, so a candidate's top-level registration during this throwaway load would otherwise + // outlive it and pollute the live worker on a failed/rolled-back deploy. The guard makes those + // registration methods no-op for the duration of the load. + const { runWithDeployValidationGuard } = require('../server/serverHelpers/deployValidationState.ts'); + const validation = runWithDeployValidationGuard(async () => { try { await componentLoader.loadComponent(application.dirPath, pseudoResources, undefined, { collectScopes: validationScopes, @@ -541,7 +546,7 @@ async function deployComponent(req) { if (result.status === 'rejected') log.warn('Failed to close a deploy-validation Scope', result.reason); } } - })(); + }); // Track the load+close so a concurrent worker shutdown waits for these scopes to finish // disposing — a plugin may start a native runtime in handleApplication — before realExit. trackScopeClose(validation); diff --git a/integrationTests/fixtures/mcp-quota/config.yaml b/integrationTests/fixtures/mcp-quota/config.yaml index 49bf06d470..4b4a044aa5 100644 --- a/integrationTests/fixtures/mcp-quota/config.yaml +++ b/integrationTests/fixtures/mcp-quota/config.yaml @@ -1,6 +1,7 @@ # Fixture for MCP per-client-identity rate limiting + the durable quota hook -# (#1610). The McpQuota class carries a cost-bearing custom tool and the -# config-named quota policy backed by the QuotaCounter table. +# (#1610/#1809). Answerer exposes a cost-bearing custom tool; the quota policy is +# registered as a function via server.setMcpQuotaHandler and backed by the +# internal (non-exported) QuotaCounter table. graphqlSchema: files: '*.graphql' jsResource: diff --git a/integrationTests/fixtures/mcp-quota/resources.js b/integrationTests/fixtures/mcp-quota/resources.js index 0663a9591d..16bd4e700e 100644 --- a/integrationTests/fixtures/mcp-quota/resources.js +++ b/integrationTests/fixtures/mcp-quota/resources.js @@ -1,16 +1,17 @@ -// #1610 fixture: a cost-bearing custom tool plus the config-named durable -// quota policy. `allowMcpCall` increments a persisted per-identity counter and -// denies past DAILY_LIMIT — the reporter's public-docs `answer` tool shape. +// #1610 / #1809 fixture: a cost-bearing custom MCP tool plus a durable quota +// policy registered as a FUNCTION (server.setMcpQuotaHandler). The policy is +// never an exposed Resource, and the counter it uses is an internal (non-@export) +// table, so no client can read or reset its own counter over any transport. // // NOTE: the get-then-put below is NOT race-safe — concurrent calls for one // identity can interleave between the read and the write and undercount. Fine -// for this single-threaded test instance; production quota implementations -// must make the read-modify-write atomic (see the RACE-SAFETY note in -// components/mcp/quota.ts). +// for this single-threaded test instance; production handlers must make the +// read-modify-write atomic (see the RACE-SAFETY note in components/mcp/quota.ts). const DAILY_LIMIT = 3; -export class McpQuota extends tables.QuotaCounter { +// The cost-bearing tool clients call (exported — this is the public surface). +export class Answerer extends Resource { static mcpTools = [ { name: 'answer', @@ -23,15 +24,17 @@ export class McpQuota extends tables.QuotaCounter { async doAnswer(args) { return { answered: args?.q ?? '' }; } +} - static async allowMcpCall({ identity }) { - const id = identity ?? 'unknown'; - const existing = await McpQuota.get(id); - const used = (existing?.used ?? 0) + 1; - await McpQuota.put({ id, used }); - if (used > DAILY_LIMIT) { - return { allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 }; - } - return true; +// Durable quota policy, registered as a function and backed by the internal +// QuotaCounter table — no config points at a Resource, and nothing is exposed. +server.setMcpQuotaHandler(async ({ identity }) => { + const id = identity ?? 'unknown'; + const existing = await tables.QuotaCounter.get(id); + const used = (existing?.used ?? 0) + 1; + await tables.QuotaCounter.put({ id, used }); + if (used > DAILY_LIMIT) { + return { allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 }; } -} + return true; +}); diff --git a/integrationTests/fixtures/mcp-quota/schema.graphql b/integrationTests/fixtures/mcp-quota/schema.graphql index e32ee6a624..e6bdba9ee4 100644 --- a/integrationTests/fixtures/mcp-quota/schema.graphql +++ b/integrationTests/fixtures/mcp-quota/schema.graphql @@ -1,5 +1,7 @@ -# Persisted per-identity counter behind the durable quota hook (#1610). -type QuotaCounter @table @export { +# Internal per-identity counter behind the durable quota handler (#1610/#1809). +# NOT @export — the handler reaches it via tables.QuotaCounter, and no client can +# read or reset it over any transport. +type QuotaCounter @table { id: ID @primaryKey used: Int } diff --git a/integrationTests/mcp/quota.test.ts b/integrationTests/mcp/quota.test.ts index a950ee0048..a628691d95 100644 --- a/integrationTests/mcp/quota.test.ts +++ b/integrationTests/mcp/quota.test.ts @@ -1,9 +1,10 @@ /** - * #1610 — per-client-identity rate limiting + the durable quota hook. + * #1610/#1809 — per-client-identity rate limiting + the durable quota handler. * * The instance configures identityHeader: x-test-client (so the test controls - * identity per call), a per-client bucket of burst 6 with negligible refill, - * and quota.resource: McpQuota (persisted per-identity counter, limit 3). + * identity per call) and a per-client bucket of burst 6 with negligible refill. + * The fixture registers the durable quota policy via server.setMcpQuotaHandler + * (limit 3), backed by an internal per-identity counter table. * Every tools/call opens a FRESH session — the session-cycling abuse loop the * issue describes — so anything that throttles here is client-scoped, not * session-scoped. Expected ladder for one identity: @@ -90,7 +91,6 @@ suite('MCP per-client rate limit + durable quota (#1610)', (ctx: ContextWithHarp perClientPerSecond: 0.001, perClientBurst: 6, }, - quota: { resource: 'McpQuota' }, }, }, }, @@ -133,15 +133,47 @@ suite('MCP per-client rate limit + durable quota (#1610)', (ctx: ContextWithHarp strictEqual(payload.retryAfterSeconds, 3600); }); - test('a different client identity is unaffected and the counter persists per identity', async () => { + test('a different client identity is unaffected; the internal counter is not client-reachable', async () => { + // A fresh identity starts with a clean quota (per-identity isolation; persistence + // per identity is covered by the ok,ok,ok→quota_exceeded sequence above). const body = await callAnswerFreshSession('client-c'); strictEqual(body.result?.isError ?? false, false, `fresh identity admitted: ${JSON.stringify(body)}`); - // The durable counter is a real table row, visible over REST. + // The counter table is internal (not @export), so — unlike the old exported-Resource + // approach — no client can read or reset it over REST. The route should not resolve. const res = await fetch(new URL('/QuotaCounter/client-c', ctx.harper.httpURL), { headers: { authorization: auth }, }); - strictEqual(res.status, 200); - const record = await res.json(); - strictEqual(record.used, 1); + strictEqual(res.status, 404, `internal counter must not be REST-exposed (got ${res.status})`); + }); + + test('the internal counter exposes no MCP CRUD tools (no way to reset a quota)', async () => { + const baseHeaders = { + 'content-type': 'application/json', + 'accept': 'application/json, text/event-stream', + 'authorization': auth, + }; + const initRes = await fetch(new URL('/mcp', ctx.harper.httpURL), { + method: 'POST', + headers: baseHeaders, + body: JSON.stringify({ + jsonrpc: '2.0', + id: ++rpcId, + method: 'initialize', + params: { protocolVersion: '2025-06-18', capabilities: {}, clientInfo: { name: 'quota-it', version: '0' } }, + }), + }); + const sessionId = initRes.headers.get('mcp-session-id'); + const listRes = await fetch(new URL('/mcp', ctx.harper.httpURL), { + method: 'POST', + headers: { ...baseHeaders, 'mcp-session-id': sessionId as string, 'mcp-protocol-version': '2025-06-18' }, + body: JSON.stringify({ jsonrpc: '2.0', id: ++rpcId, method: 'tools/list', params: {} }), + }); + const body = JSON.parse(await listRes.text()); + const names: string[] = (body.result?.tools ?? []).map((t: { name: string }) => t.name); + ok(names.includes('answer'), `the cost-bearing tool is exposed: ${names.join(', ')}`); + // Under the old exported-Resource design the counter surfaced update_/delete_ tools a client + // could call to reset its quota; the internal table must expose none. + const counterCrud = names.filter((n) => /quotacounter/i.test(n)); + strictEqual(counterCrud.length, 0, `no QuotaCounter CRUD tools should exist, found: ${counterCrud.join(', ')}`); }); }); diff --git a/server/Server.ts b/server/Server.ts index fc2e55ab98..415ad4de75 100644 --- a/server/Server.ts +++ b/server/Server.ts @@ -2,6 +2,7 @@ import { Socket } from 'net'; import { _assignPackageExport } from '../globals.js'; import type { Value } from '../resources/analytics/write.ts'; import type { Resources } from '../resources/Resources.ts'; +import type { McpQuotaHandler } from '../components/mcp/quota.ts'; import { OperationDefinition } from './serverHelpers/serverUtilities.ts'; import { Duplex } from 'stream'; import { Request } from './serverHelpers/Request.ts'; @@ -32,6 +33,8 @@ export interface Server { authenticateUser(username: string, password: string, request: Request): any; operation(operation: any, context: any, authorize?: boolean): Promise; registerOperation(operationDefinition: OperationDefinition): void; + /** Register the durable MCP quota policy (opt-in). The latest registration wins; pass `undefined` to clear. */ + setMcpQuotaHandler(handler: McpQuotaHandler | undefined): void; recordAnalytics(value: Value, metric: string, path?: string, method?: string, type?: string): void; nodes: Node[]; shards: Map; diff --git a/server/serverHelpers/deployValidationState.ts b/server/serverHelpers/deployValidationState.ts new file mode 100644 index 0000000000..759a9220e3 --- /dev/null +++ b/server/serverHelpers/deployValidationState.ts @@ -0,0 +1,25 @@ +// Deploy pre-flight validation loads throwaway candidate component code purely to surface load-time +// errors — its top-level `server.registerOperation` / `server.setMcpQuotaHandler` calls mutate +// process-wide state (the operation map, its cross-thread announcement, the quota handler) that is +// NOT owned by a Scope, so a failed/rolled-back deploy would otherwise leak the candidate's +// registrations onto the live worker. Those registration methods no-op while this guard is active. +// +// Depth-counted so overlapping validations both keep the guard raised until the last one finishes. +// Caveat: while a validation is in flight, a legitimate registration from an interleaving real load +// is also skipped (it re-registers on its next load); that window is narrow, and the alternative — +// leaking a candidate's registration live — is worse. +let validationDepth = 0; + +export function isDeployValidating(): boolean { + return validationDepth > 0; +} + +/** Run `fn` (the throwaway validation load) with the guard raised, lowering it even if `fn` throws. */ +export async function runWithDeployValidationGuard(fn: () => Promise): Promise { + validationDepth++; + try { + return await fn(); + } finally { + validationDepth--; + } +} diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index 02599dbcd6..57c7abe739 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -7,6 +7,8 @@ import readAuditLog from '../../dataLayer/readAuditLog.ts'; import * as user from '../../security/user.ts'; import * as role from '../../security/role.ts'; import customFunctionOperations from '../../components/operations.js'; +import { setMcpQuotaHandler } from '../../components/mcp/quota.ts'; +import { isDeployValidating } from './deployValidationState.ts'; import harperLogger from '../../utility/logging/harper_logger.ts'; import readLog from '../../utility/logging/readLog.ts'; import * as export_ from '../../dataLayer/export.ts'; @@ -157,6 +159,8 @@ export type OperationDefinition = { * @param operationDefinition */ server.registerOperation = (operationDefinition: OperationDefinition) => { + // A throwaway deploy-validation load must not register (or announce) operations onto the live worker. + if (isDeployValidating()) return; const { name, execute, requiresSuperUser } = operationDefinition; let handler = execute; if (requiresSuperUser !== undefined) { @@ -177,6 +181,14 @@ server.registerOperation = (operationDefinition: OperationDefinition) => { if (!isMainThread) announceRegisteredOperation(name); }; +// Register the durable MCP quota policy as a function (see components/mcp/quota.ts). Worker-local, +// like the tool dispatch that consults it, so no cross-thread announcement is needed. Skipped during +// deploy validation so a throwaway candidate load can't replace the live worker's policy. +server.setMcpQuotaHandler = (handler) => { + if (isDeployValidating()) return; + setMcpQuotaHandler(handler); +}; + export function chooseOperation(json: OperationRequestBody) { let getOpResult: OperationFunctionObject; try { diff --git a/unitTests/components/mcp/quota.test.js b/unitTests/components/mcp/quota.test.js index b3fecbac91..dd7e3bc51c 100644 --- a/unitTests/components/mcp/quota.test.js +++ b/unitTests/components/mcp/quota.test.js @@ -1,10 +1,5 @@ const assert = require('node:assert'); -const { - checkDurableQuota, - _setQuotaResourcesForTest, - _resetQuotaWarningsForTest, -} = require('#src/components/mcp/quota'); -const env = require('#src/utility/environment/environmentManager'); +const { checkDurableQuota, setMcpQuotaHandler } = require('#src/components/mcp/quota'); const INFO = { identity: '203.0.113.7', @@ -14,106 +9,61 @@ const INFO = { sessionId: 's1', }; -describe('mcp/quota (#1610)', () => { - let envOverrides; - const originalEnvGet = env.get; - - beforeEach(() => { - envOverrides = {}; - _resetQuotaWarningsForTest(); - env.get = (key) => (key in envOverrides ? envOverrides[key] : originalEnvGet.call(env, key)); - }); - +describe('mcp/quota (#1610, #1809 registration handler)', () => { afterEach(() => { - _setQuotaResourcesForTest(undefined); - env.get = originalEnvGet; + setMcpQuotaHandler(undefined); }); - it('allows when no quota resource is configured (opt-in feature)', async () => { + it('allows when no handler is registered (opt-in feature)', async () => { assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); }); - it('fails closed when the configured resource or method does not resolve', async () => { - envOverrides.mcp_application_quota_resource = 'McpQuota'; - _setQuotaResourcesForTest(new Map()); - const decision = await checkDurableQuota(INFO); - assert.equal(decision.allowed, false); - assert.equal(decision.message, 'quota policy unavailable'); - }); - - it('calls the default-named static with the check info and honors boolean results', async () => { - envOverrides.mcp_application_quota_resource = 'McpQuota'; + it('calls the registered handler with the check info and honors boolean results', async () => { const calls = []; - class McpQuota { - static allowMcpCall(info) { - calls.push(info); - return info.identity !== 'blocked'; - } - } - _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + setMcpQuotaHandler((info) => { + calls.push(info); + return info.identity !== 'blocked'; + }); assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); assert.equal(calls.length, 1); assert.equal(calls[0].tool, 'answer'); assert.equal(calls[0].identity, '203.0.113.7'); + assert.equal(calls[0].profile, 'application'); const denied = await checkDurableQuota({ ...INFO, identity: 'blocked' }); assert.equal(denied.allowed, false); }); - it('honors a configured method name and structured denials', async () => { - envOverrides.mcp_application_quota_resource = 'McpQuota'; - envOverrides.mcp_application_quota_method = 'checkDaily'; - class McpQuota { - static checkDaily() { - return { allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 }; - } - } - _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + it('honors structured denials', async () => { + setMcpQuotaHandler(() => ({ allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 })); const decision = await checkDurableQuota(INFO); assert.deepEqual(decision, { allowed: false, message: 'daily quota reached', retryAfterSeconds: 3600 }); }); - it('treats a non-denial object result as allowed and awaits async hooks', async () => { - envOverrides.mcp_application_quota_resource = 'McpQuota'; - class McpQuota { - static async allowMcpCall() { - return { remaining: 12 }; - } - } - _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + it('treats a non-denial object result as allowed and awaits async handlers', async () => { + setMcpQuotaHandler(async () => ({ remaining: 12 })); assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); }); - it('fails closed with a sanitized message when the hook throws', async () => { - envOverrides.mcp_application_quota_resource = 'McpQuota'; - class McpQuota { - static allowMcpCall() { - throw new Error('table exploded: secret connection string'); - } - } - _setQuotaResourcesForTest(new Map([['McpQuota', { Resource: McpQuota }]])); + it('fails closed with a sanitized message when the handler throws', async () => { + setMcpQuotaHandler(() => { + throw new Error('table exploded: secret connection string'); + }); const decision = await checkDurableQuota(INFO); assert.equal(decision.allowed, false); assert.equal(decision.message, 'quota check failed'); assert.ok(!JSON.stringify(decision).includes('secret'), 'raw error does not leak'); }); - it('dispatches on the live registry class (exported subclass wins)', async () => { - envOverrides.mcp_application_quota_resource = 'McpQuota'; - class Base { - static allowMcpCall() { - return true; - } - } - const registry = new Map([['McpQuota', { Resource: Base }]]); - _setQuotaResourcesForTest(registry); + it('lets the handler gate per profile via info.profile', async () => { + setMcpQuotaHandler((info) => (info.profile === 'application' ? { allowed: false, message: 'app only' } : true)); + assert.equal((await checkDurableQuota(INFO)).allowed, false); + assert.deepEqual(await checkDurableQuota({ ...INFO, profile: 'operations' }), { allowed: true }); + }); + + it('latest registration wins (a reloaded component replaces the handler)', async () => { + setMcpQuotaHandler(() => true); assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); - class Sub { - static allowMcpCall() { - return { allowed: false, message: 'reloaded policy' }; - } - } - registry.get('McpQuota').Resource = Sub; - const decision = await checkDurableQuota(INFO); - assert.deepEqual(decision, { allowed: false, message: 'reloaded policy' }); + setMcpQuotaHandler(() => ({ allowed: false, message: 'reloaded policy' })); + assert.deepEqual(await checkDurableQuota(INFO), { allowed: false, message: 'reloaded policy' }); }); }); diff --git a/unitTests/server/serverHelpers/serverUtilities.test.js b/unitTests/server/serverHelpers/serverUtilities.test.js index e9d7f3c9df..0527a3ebd3 100644 --- a/unitTests/server/serverHelpers/serverUtilities.test.js +++ b/unitTests/server/serverHelpers/serverUtilities.test.js @@ -8,6 +8,8 @@ const sinon = require('sinon'); const sandbox = sinon.createSandbox(); const { TEST_JSON_SUPER_USER, TEST_JSON_NON_SU } = require('../../test_data'); const serverUtilities = require('#src/server/serverHelpers/serverUtilities'); +const { runWithDeployValidationGuard } = require('#src/server/serverHelpers/deployValidationState'); +const quota = require('#src/components/mcp/quota'); const operation_function_caller = require('#src/utility/OperationFunctionCaller'); const logger = require('#src/utility/logging/harper_logger'); const { contextStorage } = require('#src/resources/transaction'); @@ -659,4 +661,51 @@ describe('Test serverUtilities.js module ', () => { assert.equal(op_auth.verifyPerms(nonSuRequest(SU_OP, [SU_OP]), SU_OP), null); }); }); + + // #1809 — process-wide server.* registrations must not leak from a throwaway deploy-validation load. + describe('deploy-validation guard: registrations no-op while validating', () => { + const CAND_OP = 'validation_candidate_op'; + const QUOTA_INFO = { tool: 'answer', user: { username: 'u' }, profile: 'application', sessionId: 's' }; + + afterEach(() => { + serverUtilities.OPERATION_FUNCTION_MAP.delete(CAND_OP); + quota.setMcpQuotaHandler(undefined); + }); + + it('skips server.registerOperation during validation, then resumes after', async () => { + await runWithDeployValidationGuard(async () => { + server.registerOperation({ name: CAND_OP, execute: async () => ({}) }); + assert.equal(serverUtilities.OPERATION_FUNCTION_MAP.has(CAND_OP), false, 'not registered during validation'); + }); + server.registerOperation({ name: CAND_OP, execute: async () => ({}) }); + assert.equal( + serverUtilities.OPERATION_FUNCTION_MAP.has(CAND_OP), + true, + 'registers normally after the guard lowers' + ); + }); + + it('skips server.setMcpQuotaHandler during validation, keeping the live policy', async () => { + server.setMcpQuotaHandler(() => ({ allowed: false, message: 'live policy' })); + await runWithDeployValidationGuard(async () => { + server.setMcpQuotaHandler(() => true); // a candidate trying to disable the live policy + }); + assert.deepEqual(await quota.checkDurableQuota(QUOTA_INFO), { allowed: false, message: 'live policy' }); + }); + + it('lowers the guard even when the validation load throws', async () => { + await assert.rejects( + runWithDeployValidationGuard(async () => { + throw new Error('load failed'); + }), + /load failed/ + ); + server.registerOperation({ name: CAND_OP, execute: async () => ({}) }); + assert.equal( + serverUtilities.OPERATION_FUNCTION_MAP.has(CAND_OP), + true, + 'registration works again after a failure' + ); + }); + }); }); diff --git a/utility/hdbTerms.ts b/utility/hdbTerms.ts index cc708f15f7..9e522ffb3c 100644 --- a/utility/hdbTerms.ts +++ b/utility/hdbTerms.ts @@ -563,8 +563,6 @@ export const CONFIG_PARAMS = { MCP_OPERATIONS_RATELIMIT_PERCLIENTPERSECOND: 'mcp_operations_rateLimit_perClientPerSecond', MCP_OPERATIONS_RATELIMIT_PERCLIENTBURST: 'mcp_operations_rateLimit_perClientBurst', MCP_OPERATIONS_RATELIMIT_IDENTITYHEADER: 'mcp_operations_rateLimit_identityHeader', - MCP_OPERATIONS_QUOTA_RESOURCE: 'mcp_operations_quota_resource', - MCP_OPERATIONS_QUOTA_METHOD: 'mcp_operations_quota_method', MCP_APPLICATION_MOUNTPATH: 'mcp_application_mountPath', MCP_APPLICATION_ALLOW: 'mcp_application_allow', MCP_APPLICATION_DENY: 'mcp_application_deny', @@ -577,8 +575,6 @@ export const CONFIG_PARAMS = { MCP_APPLICATION_RATELIMIT_PERCLIENTPERSECOND: 'mcp_application_rateLimit_perClientPerSecond', MCP_APPLICATION_RATELIMIT_PERCLIENTBURST: 'mcp_application_rateLimit_perClientBurst', MCP_APPLICATION_RATELIMIT_IDENTITYHEADER: 'mcp_application_rateLimit_identityHeader', - MCP_APPLICATION_QUOTA_RESOURCE: 'mcp_application_quota_resource', - MCP_APPLICATION_QUOTA_METHOD: 'mcp_application_quota_method', MCP_SESSION_IDLETIMEOUTSECONDS: 'mcp_session_idleTimeoutSeconds', MCP_SESSION_ALLOWCLIENTDELETE: 'mcp_session_allowClientDelete', AGENT_ENABLED: 'agent_enabled',