[Security] PromiseActorRef.tell timer leak on duplicate reply

[pathosDev]

Severity / Size

• Severity: MEDIUM — re-examination of the code shows the original auditor's "timer-leak" claim was inaccurate (the first tell() does clear the timer correctly). The real issue is narrower: duplicate tell() calls are silently ignored and there's no way for a caller to cancel an in-flight ask before the timer fires. Under load, an abandoned-but-still-armed timer keeps the PromiseActorRef from being GC'd until the timer fires.
• Size: S (~1d).
• Threat model: not an attacker. This is a robustness / DX issue: programmer error (duplicate reply) is invisible, and abandoned asks (caller threw mid-Promise.all) sit in the timer heap until their timeout elapses.

Affected files

• src/internal/PromiseActorRef.ts:35-41 — tell(). The early-return at line 36 silently drops duplicates.
• src/internal/PromiseActorRef.ts:25-32 — timer is armed in the constructor; never cancellable from outside.
• src/Ask.ts:15-26 — ask() builds the PromiseActorRef but doesn't expose a cancellation token.

Background

PromiseActorRef is the short-lived ActorRef used by ask. Lifecycle:

1. Constructor — arm setTimeout(timeoutMs).
2. tell(reply) — settle the promise, clear the timer.
3. Timeout fires before tell — reject with AskTimeoutError, set settled = true.

The current code clears the timer correctly on first-tell. The cases that look like leaks but aren't:

• Duplicate tell: second tell() returns early because settled is true. The timer is already cleared from the first call (timer set to null). No actual heap leak.
• Caller-abandoned ask: caller writes const p = ask(...), then catches an exception elsewhere and never awaits p. Promise GC'd by V8 once unreferenced, but the timer keeps this alive until it fires. Timer pressure = number of in-flight asks × average ask-lifetime.

Two genuine concerns:

1. No cancellation path: a caller that learns the request is no longer needed (user closed the page, parent operation cancelled) cannot tell the framework to drop the ask. The timer fires anyway, the reply (if it ever arrives) is dropped, the PromiseActorRef stays in memory.

2. Duplicate-tell silently swallowed: helpful in adversarial cases (replay attacks) but hides programmer bugs (e.g. an actor replies via two code paths). No diagnostic.

"Exploit" walkthrough (DX failure, not adversarial)

Setup: a service that runs 10K asks per second, each with a 30s timeout.

Scenario A — abandoned asks:

• User cancels their request mid-flight (HTTP socket dropped).
• The Promise chain throws somewhere upstream; the ask's promise becomes unreachable.
• The timer is still in Node's timer heap for the full 30s.
• 10K asks/s × 30s = 300K timers in the heap at steady state.

Each timer is ~200B in Node's heap. 300K × 200B = ~60 MB of timer-related memory that could be released immediately if the caller had a way to say "I don't care about this ask anymore".

Scenario B — duplicate-tell:

• Actor replies via the wrong code path (e.g. both this.sender.tell(success) and a watchdog that does sender.tell(fallback)).
• Caller receives the first reply; doesn't notice the duplicate.
• Subtle bugs (which reply wins?) become hard to diagnose because there's no visible signal.

How the 8 already-landed security fixes inform this

• Idempotency body-fingerprint (4cac92a): made explicit what was implicit — same-key-different-body → loud error. Same pattern here: duplicate tell() deserves at least a debug log, ideally an opt-in error.
• WebSocket frame-size cap (368ed81): added a knob. Here: add a cancel-token knob.

Fix design

Two complementary additions.

Track 1 — AbortSignal support on ask().

Extend the public ask() signature:

export function ask<TReq, TRes = unknown>(
  target: ActorRef<TReq>,
  message: TReq,
  timeoutMs: number = 5_000,
  options: { signal?: AbortSignal } = {},
): Promise<TRes>;

When signal is provided, the PromiseActorRef listens for signal.aborted:

constructor(..., signal?: AbortSignal) {
  // ... existing
  if (signal) {
    if (signal.aborted) {
      this.settled = true;
      this.rejectFn(new AbortError('ask cancelled before send'));
      return;
    }
    const onAbort = (): void => {
      if (this.settled) return;
      this.settled = true;
      if (this.timer) { clearTimeout(this.timer); this.timer = null; }
      this.rejectFn(new AbortError('ask cancelled by AbortSignal'));
      signal.removeEventListener('abort', onAbort);
    };
    signal.addEventListener('abort', onAbort, { once: true });
  }
}

Callers can pass AbortController().signal and call controller.abort() to release the ask immediately.

Track 2 — duplicate-tell diagnostics.

Make the silent-swallow visible:

tell(message: unknown): void {
  if (this.settled) {
    // Optional diagnostic — opt-in via settings to avoid noise.
    PromiseActorRef.onDuplicateTell?.(this, message);
    return;
  }
  // ... existing
}

// Settable via:
PromiseActorRef.onDuplicateTell = (ref, msg) => {
  log.warn(`duplicate tell on settled ask ${ref.path}: ${describe(msg)}`);
};

Default: no-op. Test code (and production with debug-logging) can opt in.

Track 3 — metric counter.

ask_duplicate_reply_total{targetLabel} — operators see when duplicate replies happen in production.

API surface

// src/Ask.ts
export function ask<TReq, TRes = unknown>(
  target: ActorRef<TReq>,
  message: TReq,
  timeoutMs?: number,
  options?: { signal?: AbortSignal },
): Promise<TRes>;

// src/SystemMessages.ts
export class AbortError extends Error {
  constructor(message: string) { super(message); this.name = 'AbortError'; }
}

// src/internal/PromiseActorRef.ts
export class PromiseActorRef<T> extends ActorRef<unknown> {
  static onDuplicateTell?: (ref: PromiseActorRef<unknown>, msg: unknown) => void;
  // ... constructor takes optional signal
}

Backward compatibility

ask() adds an optional 4th parameter; existing callers unaffected. onDuplicateTell defaults to undefined (silent, current behaviour).

Test plan

1. AbortSignal happy path: ask with signal; before timeout, controller.abort(); promise rejects with AbortError; timer cleared (verify via unref-style assertion or by counting in-flight timers in a test harness).

2. AbortSignal already-aborted: pass a pre-aborted signal; promise rejects immediately, no timer armed.

3. Cancel during in-flight target: target actor is slow; caller aborts after 10ms; verify the actor's eventual tell() is a duplicate (silently ignored).

4. Duplicate-tell diagnostic: install a counter on PromiseActorRef.onDuplicateTell; replicate Scenario B; counter increments.

5. Regression: existing Ask.test.ts still passes; no behaviour change without options.signal.

Acceptance criteria

• ask() accepts optional { signal: AbortSignal }.
• PromiseActorRef honours signal abort (rejects + clears timer + removes listener).
• onDuplicateTell static hook for diagnostics.
• ask_duplicate_reply_total metric exposed.
• Five new tests pass; existing ask + ask-pattern tests green.
• Plan-doc + README "Known security caveats" updated on land.