Skip to content

Potential post-execution metering issue: MCP handler runs before credits are deducted #7

@chenshj73

Description

@chenshj73

Hi, I noticed a possible post-execution metering issue in the current MCP middleware pipeline.

The file header describes the intended billing pipeline as validate, check credits, execute, then meter:

4:  * Key extraction, validation, credit check, and metering for MCP tool calls.
5:  * This module implements the full billing pipeline: validate key -> check credits -> execute -> meter.

The implementation validates the key and checks that the current balance is sufficient before running the handler:

448: /** Full middleware pipeline: validate -> check credits -> execute -> meter */
449: async function execute<T>(
450:   apiKey: string,
451:   method: string,
452:   handler: () => Promise<T> | T,
504: // 1. Validate key
505: const validation = await validateKey(apiKey)
506: if (!validation.valid) {
507:   throw new InvalidKeyError()
510: // 2. Check credits
511: const { sufficient, costCents } = checkCredits(
512:   validation.balanceCents,
513:   method,
514:   units,
516: if (!sufficient) {
517:   throw new InsufficientCreditsError(costCents, validation.balanceCents)

The paid MCP handler runs before metering:

529: // 3. Execute the handler
530: const result = await handler()
532: // 4. Meter the invocation (fire and forget in non-debug mode)
533: const context: InvocationContext = {
534:   consumerId: validation.consumerId,
535:   toolId: validation.toolId,
536:   keyId: validation.keyId,
537:   method,
538:   costCents,
539:   startTime,
540: }
542: if (config.debug) {
543:   await meter(context)
544: } else {
545:   // Fire and forget -- don't block the response
546:   // Errors are silently swallowed; debug mode (above) awaits for diagnostics
547:   meter(context).catch(() => {})
550: return result

The actual meter call deducts credits / records usage via the SettleGrid API:

412: /** Meter an invocation (deduct credits, record usage) */
413: async function meter(context: InvocationContext): Promise<MeterResponse> {
416:   const result = await apiCall<MeterResponse>(config, '/meter', {
417:     toolSlug: config.toolSlug,
418:     consumerId: context.consumerId,
419:     toolId: context.toolId,
420:     keyId: context.keyId,
421:     method: context.method,
422:     costCents: context.costCents,

This creates a possible accounting gap:

source: API key + current cached/validated balance
transform: checkCredits(balanceCents >= costCents)
sink: handler() executes paid work
later sink: /meter deducts credits, fire-and-forget unless debug=true

If /meter fails, times out, is rate limited, or the process exits after returning the handler result, the paid work may already have been released while the credit deduction is missing or delayed. Concurrent requests can also observe the same pre-meter balance and all pass the local checkCredits step before any deduction is committed.

Possible hardening directions:

  • Add an optional reserve-before-execute mode for expensive or non-idempotent MCP tools.
  • In non-debug mode, surface metering failure at least for tools that require strict billing.
  • Make /meter idempotent and support a reservation id created before handler().
  • Add a concurrency regression test where multiple calls pass checkCredits before any meter call updates balance.

I am reporting this as a potential issue rather than a confirmed exploit, since post-execution metering may be an intentional tradeoff for latency.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions