[Security] MDC context leak across async-storage tenant boundaries

[pathosDev]

Severity / Size

• Severity: MEDIUM
• Size: M
• Threat model: multi-tenant deployment where one actor system serves requests for several tenants. The risk is data confidentiality — tenant B sees tenant A's log fields (userId, correlationId, traceId).

Affected files

• src/LogContext.ts:62-87 — run / with / get over a single shared AsyncLocalStorage.
• src/internal/ActorCell.ts:619-622 — LogContext.run(env.context, dispatch) wraps every message dispatch.
• src/cluster/Cluster.ts:538 — same wrap for cross-node envelopes.
• src/internal/LocalActorRef.ts:21-30, src/cluster/RemoteActorRef.ts:35 — snapshot at tell() time.
• src/Logger.ts:42-45 — merges MDC into every log line.

Background

The MDC implementation is AsyncLocalStorage-based. The model:

1. Caller side: every tell() snapshots LogContext.get() and attaches it to the envelope (env.context).
2. Receiver side: ActorCell._dispatchOne wraps the user's behavior(msg) call in LogContext.run(env.context, dispatch). Inside that scope, any further tell() re-snapshots the propagated context, so the trail stays consistent across hops.

This is correct as long as the message handler stays inside the dispatch's await behavior(msg) chain. The trouble starts when:

1. A handler kicks off fire-and-forget async work: void doSomethingAsync() — the void discards the promise. AsyncLocalStorage does propagate through the awaits inside doSomethingAsync, so for that call chain it works. But the actor cell's _dispatchOne returns immediately after await behavior(msg), and the next message arrives. The next message gets its own MDC. The fire-and-forget chain still carries the previous MDC — which is correct (it logically belongs to the original message), but…
2. …the fire-and-forget chain captures a context-less callback and re-enters the system: setTimeout(() => this.log.info('done'), 1000). setTimeout does propagate ALS in Node ≥14 / Bun. So the log line gets the original MDC — still correct.
3. The genuine break — a worker actor doing batch processing: imagine BatchProcessor that holds a queue and processes items in onReceive:

class BatchProcessor extends Actor {
  private queue: Array<{ tenant: string; item: unknown }> = [];

  onReceive(msg: Msg): void {
    if (msg.kind === 'enqueue') {
      this.queue.push({ tenant: msg.tenant, item: msg.item });
      this.flushIfReady();
    }
  }

  private async flushIfReady(): Promise<void> {
    const batch = this.queue.splice(0);
    for (const { tenant, item } of batch) {
      // process item — log lines here will use whatever MDC is currently
      // active, which is the MDC of the LAST message that triggered flush,
      // not the MDC of the message that enqueued each item.
      this.log.info({ tenant }, 'processing item');
      await processItem(item);
    }
  }
}

The flushIfReady() call happens inside onReceive(msgN), which means it inherits msgN's MDC. But the batch contains items from msg1, msg2, …, msgN — each with potentially different tenants. Items from msg1 get processed with msg-N's MDC attached. Tenant A enqueues, tenant B's enqueue triggers flush, log line for tenant A's item carries tenant B's correlationId/userId. Leak.

The framework can't fix the BatchProcessor pattern for users, but it can:

• Document the boundary explicitly.
• Provide a LogContext.runFresh(fn) helper that drops the current scope (the inverse of with) for explicit "I'm now in a different request context" boundaries.
• Provide a LogContext.runEach(items, fn) helper that takes a list of (ctx, item) pairs and re-establishes scope per item — exactly the BatchProcessor pattern.

Exploit walkthrough

Step 1 — App uses tenant-tagged MDC at HTTP boundary:

get(async (req) => {
  const tenant = getTenantFromSession(req);
  return LogContext.run({ tenant, userId: getUserFromSession(req) }, async () => {
    workerActor.tell({ kind: 'enqueue', payload: req.body });
    return completeJson(202, { queued: true });
  });
});

Two concurrent requests: tenant A's userId=alice, tenant B's userId=bob.

Step 2 — Worker is a batch processor (pseudocode above). Both enqueue messages arrive in the worker's mailbox. Worker processes them in flushIfReady() triggered by the second message.

Step 3 — flushIfReady() inherits the MDC of msg-2 (tenant B / bob). Logs Item-1's processing as tenant=B, userId=bob, item=<tenant A's payload>.

Step 4 — Log aggregator indexes by userId=bob and shows operations on tenant A's data. Bob's session shows operations he never performed. Audit-log integrity broken.

Realistic worst case: SOC analyst investigating a security incident attributes actions to the wrong user, missing the actual perpetrator. Or compliance audit ($GDPR / SOC2) fails because audit-trail entries can't be uniquely tied to a single tenant.

How the 8 already-landed security fixes inform this

• MDC cross-tenant is the same shape as the idempotency body-fingerprint fix: a stale piece of state (cached body / cached context) gets reused across logically distinct requests. The fix in both cases is to bind the state to the request's identity and refuse to reuse it.
• Hello-handshake hijack defence taught: "the framework can validate that the right peer is on the other end". Here the analog: the framework can't tell what context "should" be active for an arbitrary handler invocation, but it can provide primitives (runFresh, runEach) that make the safe pattern syntactically obvious.

The shape: don't try to auto-fix at the framework layer, but ship the primitives + docs so application code can't get this wrong by accident.

Fix design

Track 1 — LogContext.runFresh(fn) (primary). Drop the current scope, run fn with an empty context. The inverse of with(). Use at batch-processor boundaries.

runFresh<T>(fn: () => T): T {
  return storage.run(EMPTY, fn);
}

Track 2 — LogContext.runEach<I>(items, contextOf, fn) helper. Iterate items, re-establishing scope per item from the per-item context.

async runEach<I>(
  items: Iterable<I>,
  contextOf: (i: I) => LogContextData,
  fn: (i: I) => Promise<void> | void,
): Promise<void> {
  for (const i of items) {
    await storage.run(contextOf(i), () => fn(i));
  }
}

Then the BatchProcessor becomes:

private async flushIfReady(): Promise<void> {
  const batch = this.queue.splice(0);
  await LogContext.runEach(
    batch,
    ({ ctx }) => ctx,
    async ({ item }) => { /* per-item processing with its own MDC */ },
  );
}

Track 3 — Snapshot MDC into the queued items, not just at flush time. Documented pattern: when enqueueing, capture LogContext.snapshot() alongside the payload. Then flushIfReady has per-item context available without the framework guessing.

onReceive(msg: Msg): void {
  if (msg.kind === 'enqueue') {
    this.queue.push({ ctx: LogContext.snapshot(), item: msg.item });
    this.flushIfReady();
  }
}

This is what the framework's own tell() already does — the docs should call out that user-level queues need to do the same.

Track 4 — Defence-in-depth: WARN-mode that emits a console.warn on suspected MDC inversion. If a log line emits a correlationId that doesn't match any recently-seen tell() envelope context (best-effort heuristic), warn once per (actor, correlationId) pair. Default off; on via ActorSystem.create({ debugMode: true }). Catches the BatchProcessor footgun in dev/test.

API surface

export const LogContext = {
  // existing:
  run<T>(ctx: LogContextData, fn: () => T): T;
  get(): LogContextData;
  with<T>(extra: LogContextData, fn: () => T): T;
  snapshot(): Record<string, string | number | boolean>;

  // NEW:
  /** Drop the current scope; run fn with no MDC fields. */
  runFresh<T>(fn: () => T): T;

  /** Iterate items, re-establishing per-item MDC. */
  runEach<I>(
    items: Iterable<I>,
    contextOf: (i: I) => LogContextData,
    fn: (i: I) => Promise<void> | void,
  ): Promise<void>;
};

Backward compatibility

Non-breaking. Two new helpers added; existing run / with / get unchanged. The debugMode warn-flag is opt-in.

Test plan

1. Exploit-reproduction test — BatchProcessor actor with two tenants' messages, verify that without the fix log lines for tenant A's item carry tenant B's MDC (proves the leak is real).
2. Fix test (Track 1) — same processor, but wrap each item's processing in LogContext.runFresh(() => …). Verify log lines have no userId field — the explicit reset works.
3. Fix test (Track 2) — same processor using LogContext.runEach. Verify each item's log line carries the original enqueue-time MDC.
4. Snapshot-at-enqueue test (Track 3) — verify LogContext.snapshot() returns a copy that can be passed across await boundaries without sharing mutable state.
5. Debug-mode warn test — enable debugMode: true, run the unfixed BatchProcessor, capture the console.warn.
6. Regression — LogContext.run/with/get unchanged; existing 1 720+ tests pass.

Acceptance criteria

• LogContext.runFresh(fn) added with full coverage.
• LogContext.runEach(items, contextOf, fn) added with full coverage.
• Test that reproduces the BatchProcessor leak (proof of the threat) — included as a describe.skip regression marker or as a documented anti-pattern in tests/log-context-tenant-leak.test.ts.
• README "Known security caveats" updated with the BatchProcessor pattern + the runFresh / runEach mitigation.
• LogContext JSDoc explicitly warns about the BatchProcessor anti-pattern.
• CHANGELOG entry under "MDC primitives: runFresh, runEach for tenant-isolation boundaries".