From 6c91bfb911b8ba7b1c1c1d44fa30d9840c948a71 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 15 Jul 2026 09:45:04 -0600 Subject: [PATCH 1/6] Register the MCP durable quota policy as a function, not a config-referenced Resource (#1809) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The quota hook was configured by `mcp..quota.resource` pointing at an exported Resource, whose inherited CRUD then surfaced on every transport (update_/delete_ MCP tools + REST/SSE/WS/GraphQL/MQTT) — a permitted client could reset its own counter. The docs example worked around it by turning six exportTypes flags off, which made the safe path the easy-to-forget one. Replace it with a registration function: `server.setMcpQuotaHandler(fn)`. The policy is a plain function (never an exposed Resource), enabled by registering it (no config). checkDurableQuota invokes the registered handler; no handler => allowed (opt-in), throw => fail-closed deny (unchanged). The handler receives `profile` so one handler can gate operations vs application. - Remove the `mcp..quota.resource`/`.method` config params. - Wire `server.setMcpQuotaHandler` next to `server.registerOperation`. - Migrate the mcp-quota fixture off `tables.QuotaCounter`: an exported tool (Answerer) + an internal (non-@export) counter table + a registered handler, so nothing exposes the counter. Integration test asserts the counter is not REST-reachable. Supersedes the docs#576 six-`false` example; docs follow-up separately. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/quota.ts | 127 ++++++------------ .../fixtures/mcp-quota/config.yaml | 5 +- .../fixtures/mcp-quota/resources.js | 37 ++--- .../fixtures/mcp-quota/schema.graphql | 6 +- integrationTests/mcp/quota.test.ts | 21 +-- server/Server.ts | 3 + server/serverHelpers/serverUtilities.ts | 7 + unitTests/components/mcp/quota.test.js | 106 ++++----------- utility/hdbTerms.ts | 4 - 9 files changed, 120 insertions(+), 196 deletions(-) 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 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..598ec25126 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: @@ -13,7 +14,7 @@ * npm run test:integration -- "integrationTests/mcp/quota.test.ts" */ import { suite, test, before, after } from 'node:test'; -import { ok, strictEqual } from 'node:assert'; +import { ok, strictEqual, notStrictEqual } from 'node:assert'; import { resolve } from 'node:path'; import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; @@ -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,16 @@ 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); + notStrictEqual(res.status, 200, `internal counter must not be REST-exposed (got ${res.status})`); }); }); diff --git a/server/Server.ts b/server/Server.ts index fc2e55ab98..85e7d0bf87 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. */ + setMcpQuotaHandler(handler: McpQuotaHandler): void; recordAnalytics(value: Value, metric: string, path?: string, method?: string, type?: string): void; nodes: Node[]; shards: Map; diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index 02599dbcd6..f1e8990e17 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -7,6 +7,7 @@ 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, type McpQuotaHandler } from '../../components/mcp/quota.ts'; import harperLogger from '../../utility/logging/harper_logger.ts'; import readLog from '../../utility/logging/readLog.ts'; import * as export_ from '../../dataLayer/export.ts'; @@ -177,6 +178,12 @@ 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. +server.setMcpQuotaHandler = (handler: McpQuotaHandler) => { + 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/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', From 60a51f4b619965ee9503bf621926750daf9ff706 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 15 Jul 2026 09:51:22 -0600 Subject: [PATCH 2/6] Isolate the quota handler from deploy pre-flight validation; allow clearing (review) Codex review: - P1: the quota handler is a process-wide singleton, so a candidate component's top-level server.setMcpQuotaHandler(...) during deploy pre-flight validation would outlive the throwaway load and alter live enforcement on a failed deploy. Snapshot the handler before the validation load and restore it in the finally (added getMcpQuotaHandler()). - P2: the Server interface rejected undefined though the setter supports clearing; widen the public type to McpQuotaHandler | undefined. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/quota.ts | 9 +++++++++ components/operations.js | 6 ++++++ server/Server.ts | 4 ++-- unitTests/components/mcp/quota.test.js | 16 +++++++++++++++- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts index c015ead971..75440792f8 100644 --- a/components/mcp/quota.ts +++ b/components/mcp/quota.ts @@ -76,6 +76,15 @@ export function setMcpQuotaHandler(handler: McpQuotaHandler | undefined): void { quotaHandler = handler; } +/** + * The currently-registered handler. Used by deploy pre-flight validation to snapshot + * and restore around a throwaway candidate load, so a candidate's `setMcpQuotaHandler` + * (or a failed deploy) can't leave its policy — or `undefined` — active on the live worker. + */ +export function getMcpQuotaHandler(): McpQuotaHandler | undefined { + return quotaHandler; +} + /** * 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 diff --git a/components/operations.js b/components/operations.js index 6a147943ea..db5cc57c1d 100644 --- a/components/operations.js +++ b/components/operations.js @@ -530,12 +530,18 @@ async function deployComponent(req) { // deploy-lifecycle listeners on this worker, eventually tripping MaxListenersExceededWarning // (#1462). const validationScopes = new Set(); + // The MCP quota handler is a process-wide singleton, not owned by a Scope, so a candidate's + // top-level `server.setMcpQuotaHandler(...)` would otherwise outlive this throwaway load and + // alter live quota enforcement on a failed/rolled-back deploy. Snapshot and restore it. + const { getMcpQuotaHandler, setMcpQuotaHandler } = require('./mcp/quota.ts'); + const savedQuotaHandler = getMcpQuotaHandler(); const validation = (async () => { try { await componentLoader.loadComponent(application.dirPath, pseudoResources, undefined, { collectScopes: validationScopes, }); } finally { + setMcpQuotaHandler(savedQuotaHandler); const closeResults = await Promise.allSettled(Array.from(validationScopes, (scope) => scope.close())); for (const result of closeResults) { if (result.status === 'rejected') log.warn('Failed to close a deploy-validation Scope', result.reason); diff --git a/server/Server.ts b/server/Server.ts index 85e7d0bf87..415ad4de75 100644 --- a/server/Server.ts +++ b/server/Server.ts @@ -33,8 +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. */ - setMcpQuotaHandler(handler: McpQuotaHandler): 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/unitTests/components/mcp/quota.test.js b/unitTests/components/mcp/quota.test.js index dd7e3bc51c..bec2e851a0 100644 --- a/unitTests/components/mcp/quota.test.js +++ b/unitTests/components/mcp/quota.test.js @@ -1,5 +1,5 @@ const assert = require('node:assert'); -const { checkDurableQuota, setMcpQuotaHandler } = require('#src/components/mcp/quota'); +const { checkDurableQuota, setMcpQuotaHandler, getMcpQuotaHandler } = require('#src/components/mcp/quota'); const INFO = { identity: '203.0.113.7', @@ -66,4 +66,18 @@ describe('mcp/quota (#1610, #1809 registration handler)', () => { setMcpQuotaHandler(() => ({ allowed: false, message: 'reloaded policy' })); assert.deepEqual(await checkDurableQuota(INFO), { allowed: false, message: 'reloaded policy' }); }); + + // Supports the deploy pre-flight snapshot/restore that keeps a throwaway candidate load from + // leaving its handler active on the live worker. + it('getMcpQuotaHandler round-trips the registered handler for snapshot/restore', async () => { + assert.equal(getMcpQuotaHandler(), undefined); + const live = () => true; + setMcpQuotaHandler(live); + const snapshot = getMcpQuotaHandler(); + assert.equal(snapshot, live); + // a candidate load overwrites it, then we restore the snapshot + setMcpQuotaHandler(() => ({ allowed: false, message: 'candidate' })); + setMcpQuotaHandler(snapshot); + assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + }); }); From 94a85f34cfe8ea6ec3a27108921b91bac774a770 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 15 Jul 2026 10:42:03 -0600 Subject: [PATCH 3/6] Test: assert the internal counter exposes no REST route and no MCP CRUD tools (#1809) Codify the security property the redesign delivers (and that /verify checked by hand): the counter table is internal, so GET /QuotaCounter 404s and tools/list carries no QuotaCounter update_/delete_ tools a client could call to reset its quota. Tightened the counter-not-exposed assertion from !=200 to ==404. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- integrationTests/mcp/quota.test.ts | 35 ++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/integrationTests/mcp/quota.test.ts b/integrationTests/mcp/quota.test.ts index 598ec25126..a628691d95 100644 --- a/integrationTests/mcp/quota.test.ts +++ b/integrationTests/mcp/quota.test.ts @@ -14,7 +14,7 @@ * npm run test:integration -- "integrationTests/mcp/quota.test.ts" */ import { suite, test, before, after } from 'node:test'; -import { ok, strictEqual, notStrictEqual } from 'node:assert'; +import { ok, strictEqual } from 'node:assert'; import { resolve } from 'node:path'; import { setupHarperWithFixture, teardownHarper, type ContextWithHarper } from '@harperfast/integration-testing'; @@ -143,6 +143,37 @@ suite('MCP per-client rate limit + durable quota (#1610)', (ctx: ContextWithHarp const res = await fetch(new URL('/QuotaCounter/client-c', ctx.harper.httpURL), { headers: { authorization: auth }, }); - notStrictEqual(res.status, 200, `internal counter must not be REST-exposed (got ${res.status})`); + 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(', ')}`); }); }); From 33ce1dfb671e33eb7d7960f4c55706e48e7a83a8 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 15 Jul 2026 10:46:51 -0600 Subject: [PATCH 4/6] Extract withMcpQuotaHandlerPreserved and unit-test the deploy-validation isolation (#1809) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The P1 snapshot/restore was inline in the deploy op and only its get/set primitive was covered. Extract it into withMcpQuotaHandlerPreserved(fn) — operations.js wraps the throwaway validation load in it — and unit-test the isolation directly: a candidate that registers a different handler, one that clears it, and a load that throws all leave the live worker's handler intact. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/quota.ts | 16 +++++--- components/operations.js | 10 ++--- unitTests/components/mcp/quota.test.js | 53 +++++++++++++++++++------- 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts index 75440792f8..5fa53b6ac9 100644 --- a/components/mcp/quota.ts +++ b/components/mcp/quota.ts @@ -77,12 +77,18 @@ export function setMcpQuotaHandler(handler: McpQuotaHandler | undefined): void { } /** - * The currently-registered handler. Used by deploy pre-flight validation to snapshot - * and restore around a throwaway candidate load, so a candidate's `setMcpQuotaHandler` - * (or a failed deploy) can't leave its policy — or `undefined` — active on the live worker. + * Run `fn` with the registered handler snapshotted and restored afterward (even if `fn` throws). + * Deploy pre-flight validation loads throwaway candidate component code that may call + * `setMcpQuotaHandler` (or clear it); wrapping that load in this keeps a candidate's policy — or a + * failed/rolled-back deploy — from altering live quota enforcement on this worker. */ -export function getMcpQuotaHandler(): McpQuotaHandler | undefined { - return quotaHandler; +export async function withMcpQuotaHandlerPreserved(fn: () => Promise): Promise { + const saved = quotaHandler; + try { + return await fn(); + } finally { + quotaHandler = saved; + } } /** diff --git a/components/operations.js b/components/operations.js index db5cc57c1d..0429049193 100644 --- a/components/operations.js +++ b/components/operations.js @@ -532,22 +532,20 @@ async function deployComponent(req) { const validationScopes = new Set(); // The MCP quota handler is a process-wide singleton, not owned by a Scope, so a candidate's // top-level `server.setMcpQuotaHandler(...)` would otherwise outlive this throwaway load and - // alter live quota enforcement on a failed/rolled-back deploy. Snapshot and restore it. - const { getMcpQuotaHandler, setMcpQuotaHandler } = require('./mcp/quota.ts'); - const savedQuotaHandler = getMcpQuotaHandler(); - const validation = (async () => { + // alter live quota enforcement on a failed/rolled-back deploy. Preserve it across the load. + const { withMcpQuotaHandlerPreserved } = require('./mcp/quota.ts'); + const validation = withMcpQuotaHandlerPreserved(async () => { try { await componentLoader.loadComponent(application.dirPath, pseudoResources, undefined, { collectScopes: validationScopes, }); } finally { - setMcpQuotaHandler(savedQuotaHandler); const closeResults = await Promise.allSettled(Array.from(validationScopes, (scope) => scope.close())); for (const result of closeResults) { 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/unitTests/components/mcp/quota.test.js b/unitTests/components/mcp/quota.test.js index bec2e851a0..e1305c7dbf 100644 --- a/unitTests/components/mcp/quota.test.js +++ b/unitTests/components/mcp/quota.test.js @@ -1,5 +1,5 @@ const assert = require('node:assert'); -const { checkDurableQuota, setMcpQuotaHandler, getMcpQuotaHandler } = require('#src/components/mcp/quota'); +const { checkDurableQuota, setMcpQuotaHandler, withMcpQuotaHandlerPreserved } = require('#src/components/mcp/quota'); const INFO = { identity: '203.0.113.7', @@ -67,17 +67,44 @@ describe('mcp/quota (#1610, #1809 registration handler)', () => { assert.deepEqual(await checkDurableQuota(INFO), { allowed: false, message: 'reloaded policy' }); }); - // Supports the deploy pre-flight snapshot/restore that keeps a throwaway candidate load from - // leaving its handler active on the live worker. - it('getMcpQuotaHandler round-trips the registered handler for snapshot/restore', async () => { - assert.equal(getMcpQuotaHandler(), undefined); - const live = () => true; - setMcpQuotaHandler(live); - const snapshot = getMcpQuotaHandler(); - assert.equal(snapshot, live); - // a candidate load overwrites it, then we restore the snapshot - setMcpQuotaHandler(() => ({ allowed: false, message: 'candidate' })); - setMcpQuotaHandler(snapshot); - assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + // withMcpQuotaHandlerPreserved wraps deploy pre-flight validation so a throwaway candidate load + // can't leave its policy — or a cleared/failed one — active on the live worker. + describe('withMcpQuotaHandlerPreserved (deploy-validation isolation)', () => { + it('restores the live handler after a candidate registers a different one', async () => { + const live = () => true; + setMcpQuotaHandler(live); + await withMcpQuotaHandlerPreserved(async () => { + setMcpQuotaHandler(() => ({ allowed: false, message: 'candidate policy' })); + }); + // the candidate's deny handler must not survive the validation load + assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + }); + + it('restores the live handler after a candidate clears it', async () => { + setMcpQuotaHandler(() => ({ allowed: false, message: 'live policy' })); + await withMcpQuotaHandlerPreserved(async () => { + setMcpQuotaHandler(undefined); // a candidate that disables its policy + }); + // clearing must not leave the live worker with quota off + assert.deepEqual(await checkDurableQuota(INFO), { allowed: false, message: 'live policy' }); + }); + + it('restores the live handler even when the load throws', async () => { + const live = () => true; + setMcpQuotaHandler(live); + await assert.rejects( + withMcpQuotaHandlerPreserved(async () => { + setMcpQuotaHandler(() => ({ allowed: false, message: 'candidate' })); + throw new Error('validation failed'); + }), + /validation failed/ + ); + assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); + }); + + it('returns the wrapped function result', async () => { + const result = await withMcpQuotaHandlerPreserved(async () => 'loaded'); + assert.equal(result, 'loaded'); + }); }); }); From fc0f995088fb118881ca4a1ff9c16668c7b45e6e Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 15 Jul 2026 10:51:44 -0600 Subject: [PATCH 5/6] Simplify quota-handler wiring; note the snapshot-restore tradeoff (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review: assign server.setMcpQuotaHandler directly instead of a redundant wrapper (also keeps the impl param type in sync with the Server interface's McpQuotaHandler | undefined). Document that withMcpQuotaHandlerPreserved restores unconditionally, so a legitimate interleaving registration would be reverted — a narrow window, and the lesser evil vs leaking a candidate policy live. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/quota.ts | 4 ++++ server/serverHelpers/serverUtilities.ts | 6 ++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts index 5fa53b6ac9..dfcf0a15ce 100644 --- a/components/mcp/quota.ts +++ b/components/mcp/quota.ts @@ -81,6 +81,10 @@ export function setMcpQuotaHandler(handler: McpQuotaHandler | undefined): void { * Deploy pre-flight validation loads throwaway candidate component code that may call * `setMcpQuotaHandler` (or clear it); wrapping that load in this keeps a candidate's policy — or a * failed/rolled-back deploy — from altering live quota enforcement on this worker. + * + * Note: this restores the snapshot unconditionally, so a *legitimate* registration made during `fn` + * by an interleaving load would be reverted. That window is narrow (a validation load overlapping a + * real one) and the alternative — leaking a candidate's policy onto the live worker — is worse. */ export async function withMcpQuotaHandlerPreserved(fn: () => Promise): Promise { const saved = quotaHandler; diff --git a/server/serverHelpers/serverUtilities.ts b/server/serverHelpers/serverUtilities.ts index f1e8990e17..ddd53d1ea2 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -7,7 +7,7 @@ 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, type McpQuotaHandler } from '../../components/mcp/quota.ts'; +import { setMcpQuotaHandler } from '../../components/mcp/quota.ts'; import harperLogger from '../../utility/logging/harper_logger.ts'; import readLog from '../../utility/logging/readLog.ts'; import * as export_ from '../../dataLayer/export.ts'; @@ -180,9 +180,7 @@ server.registerOperation = (operationDefinition: OperationDefinition) => { // 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. -server.setMcpQuotaHandler = (handler: McpQuotaHandler) => { - setMcpQuotaHandler(handler); -}; +server.setMcpQuotaHandler = setMcpQuotaHandler; export function chooseOperation(json: OperationRequestBody) { let getOpResult: OperationFunctionObject; From 8da8768e16739108079ee25a159f9d9addf2ac32 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Wed, 15 Jul 2026 11:02:42 -0600 Subject: [PATCH 6/6] Generalize deploy-validation isolation to all process-wide server.* registrations (#1809) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review #3: the deploy pre-flight snapshot/restore only isolated the quota handler; a candidate's server.registerOperation during the throwaway validation load still mutated the process-wide operation map AND announced to the main thread, leaking onto the live worker on a failed deploy. Replace the quota-specific withMcpQuotaHandlerPreserved with a general guard (deployValidationState.ts): server.registerOperation and server.setMcpQuotaHandler both no-op while a validation load is in flight (validation only needs to surface load-time errors, not register anything). operations.js wraps the validation load in runWithDeployValidationGuard. Skipping is cleaner than snapshot/restore here — it also suppresses the cross-thread operation announce, which a local restore can't undo. Depth-counted; the narrow interleaving caveat is documented. Tests: registerOperation + setMcpQuotaHandler are skipped during validation and resume after (incl. after a thrown load), in serverUtilities.test.js. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/quota.ts | 19 ------- components/operations.js | 11 +++-- server/serverHelpers/deployValidationState.ts | 25 ++++++++++ server/serverHelpers/serverUtilities.ts | 11 ++++- unitTests/components/mcp/quota.test.js | 43 +--------------- .../serverHelpers/serverUtilities.test.js | 49 +++++++++++++++++++ 6 files changed, 90 insertions(+), 68 deletions(-) create mode 100644 server/serverHelpers/deployValidationState.ts diff --git a/components/mcp/quota.ts b/components/mcp/quota.ts index dfcf0a15ce..c015ead971 100644 --- a/components/mcp/quota.ts +++ b/components/mcp/quota.ts @@ -76,25 +76,6 @@ export function setMcpQuotaHandler(handler: McpQuotaHandler | undefined): void { quotaHandler = handler; } -/** - * Run `fn` with the registered handler snapshotted and restored afterward (even if `fn` throws). - * Deploy pre-flight validation loads throwaway candidate component code that may call - * `setMcpQuotaHandler` (or clear it); wrapping that load in this keeps a candidate's policy — or a - * failed/rolled-back deploy — from altering live quota enforcement on this worker. - * - * Note: this restores the snapshot unconditionally, so a *legitimate* registration made during `fn` - * by an interleaving load would be reverted. That window is narrow (a validation load overlapping a - * real one) and the alternative — leaking a candidate's policy onto the live worker — is worse. - */ -export async function withMcpQuotaHandlerPreserved(fn: () => Promise): Promise { - const saved = quotaHandler; - try { - return await fn(); - } finally { - quotaHandler = saved; - } -} - /** * 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 diff --git a/components/operations.js b/components/operations.js index 0429049193..264f766a90 100644 --- a/components/operations.js +++ b/components/operations.js @@ -530,11 +530,12 @@ async function deployComponent(req) { // deploy-lifecycle listeners on this worker, eventually tripping MaxListenersExceededWarning // (#1462). const validationScopes = new Set(); - // The MCP quota handler is a process-wide singleton, not owned by a Scope, so a candidate's - // top-level `server.setMcpQuotaHandler(...)` would otherwise outlive this throwaway load and - // alter live quota enforcement on a failed/rolled-back deploy. Preserve it across the load. - const { withMcpQuotaHandlerPreserved } = require('./mcp/quota.ts'); - const validation = withMcpQuotaHandlerPreserved(async () => { + // 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, 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 ddd53d1ea2..57c7abe739 100644 --- a/server/serverHelpers/serverUtilities.ts +++ b/server/serverHelpers/serverUtilities.ts @@ -8,6 +8,7 @@ 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'; @@ -158,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) { @@ -179,8 +182,12 @@ server.registerOperation = (operationDefinition: OperationDefinition) => { }; // 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. -server.setMcpQuotaHandler = setMcpQuotaHandler; +// 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; diff --git a/unitTests/components/mcp/quota.test.js b/unitTests/components/mcp/quota.test.js index e1305c7dbf..dd7e3bc51c 100644 --- a/unitTests/components/mcp/quota.test.js +++ b/unitTests/components/mcp/quota.test.js @@ -1,5 +1,5 @@ const assert = require('node:assert'); -const { checkDurableQuota, setMcpQuotaHandler, withMcpQuotaHandlerPreserved } = require('#src/components/mcp/quota'); +const { checkDurableQuota, setMcpQuotaHandler } = require('#src/components/mcp/quota'); const INFO = { identity: '203.0.113.7', @@ -66,45 +66,4 @@ describe('mcp/quota (#1610, #1809 registration handler)', () => { setMcpQuotaHandler(() => ({ allowed: false, message: 'reloaded policy' })); assert.deepEqual(await checkDurableQuota(INFO), { allowed: false, message: 'reloaded policy' }); }); - - // withMcpQuotaHandlerPreserved wraps deploy pre-flight validation so a throwaway candidate load - // can't leave its policy — or a cleared/failed one — active on the live worker. - describe('withMcpQuotaHandlerPreserved (deploy-validation isolation)', () => { - it('restores the live handler after a candidate registers a different one', async () => { - const live = () => true; - setMcpQuotaHandler(live); - await withMcpQuotaHandlerPreserved(async () => { - setMcpQuotaHandler(() => ({ allowed: false, message: 'candidate policy' })); - }); - // the candidate's deny handler must not survive the validation load - assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); - }); - - it('restores the live handler after a candidate clears it', async () => { - setMcpQuotaHandler(() => ({ allowed: false, message: 'live policy' })); - await withMcpQuotaHandlerPreserved(async () => { - setMcpQuotaHandler(undefined); // a candidate that disables its policy - }); - // clearing must not leave the live worker with quota off - assert.deepEqual(await checkDurableQuota(INFO), { allowed: false, message: 'live policy' }); - }); - - it('restores the live handler even when the load throws', async () => { - const live = () => true; - setMcpQuotaHandler(live); - await assert.rejects( - withMcpQuotaHandlerPreserved(async () => { - setMcpQuotaHandler(() => ({ allowed: false, message: 'candidate' })); - throw new Error('validation failed'); - }), - /validation failed/ - ); - assert.deepEqual(await checkDurableQuota(INFO), { allowed: true }); - }); - - it('returns the wrapped function result', async () => { - const result = await withMcpQuotaHandlerPreserved(async () => 'loaded'); - assert.equal(result, 'loaded'); - }); - }); }); 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' + ); + }); + }); });