[Security] Global ask-counter overflow after 2^31 calls

[pathosDev]

Severity / Size

• Severity: LOW — the global askCounter is just used to build the ActorPath name (askResp-N). The path string is what hits the deadletter log and the actor-tree. Collisions are operationally noisy (two different asks share a name in logs) but don't break correctness — each PromiseActorRef is a fresh object and the routing happens by reference, not by path-string match.
• Size: S (~1d).
• Threat model: none. Counter overflow is a JS-number precision issue after ~2^53 calls; at 10K asks/sec that's ~28k years. In practice it would matter on a single process running >100K asks/sec for >3 years. Mostly a code-hygiene finding.

Affected files

• src/Ask.ts:4 — let askCounter = 0;
• src/Ask.ts:20 — const name = \askResp-${++askCounter}`;`

Background

Every ask() call increments a module-level counter to build a unique sender-path: askResp-1, askResp-2, askResp-3, … The name shows up in:

• Deadletter log entries when a tell is unable to deliver to a settled PromiseActorRef.
• Trace span attributes when tracing is enabled.
• Debug logs from ActorCell when it processes the synthetic sender.

The counter is not used for correlation — replies are routed via the captured PromiseActorRef instance, not by path-string lookup. So a collision is harmless to message routing, but is operationally confusing: two unrelated asks share a path name in logs.

At JS-number precision, ++askCounter past 2^53 starts losing precision — 2^53 + 1 === 2^53 returns true, so the increment is no longer monotonic. Below 2^53 it's mathematically exact. In practice, processes don't run long enough to hit that.

The bigger concern is the bit-wide concern: the counter is module-global, so any code that imports ask.ts shares the counter. If two ActorSystems coexist in the same process, their askResp names share the global counter — operationally fine, but the path names are not scoped to the system.

Exploit walkthrough (none, this is a hygiene item)

There is no exploit. Documenting why this is tracked at LOW:

• The counter overflow is mathematical; no attacker triggers it.
• A collision in the path-string doesn't affect routing.
• The only observable effect is log-noise if two log lines say askResp-X referring to different ask events.

The reason it's worth fixing: a small refactor brings the counter under per-system scope and switches to crypto.randomUUID() for unguessable identifiers. Same hygiene the nextAskId() fix (#120) is applying to ClusterClient.

How the 8 already-landed security fixes inform this

• Idempotency body-fingerprint (4cac92a): used a strong hash for correlation IDs. Apply the same principle here: random-UUID for the ask-name eliminates the counter entirely.
• The pending askId-predictability fix ([Security] ClusterClient askId predictability via Date.now()+counter #120, batch 1): replacing the millisecond + counter with crypto.randomUUID() is the exact same change shape.

Fix design

Two small changes.

Track 1 — UUID-based ask names.

Replace the global counter with a UUID:

import { PromiseActorRef } from './internal/PromiseActorRef.js';

export function ask<TReq, TRes = unknown>(
  target: ActorRef<TReq>,
  message: TReq,
  timeoutMs: number = 5_000,
): Promise<TRes> {
  const name = `askResp-${nextAskName()}`;
  const systemName = target.path.systemName;
  const ref = new PromiseActorRef<TRes>(systemName, name, timeoutMs, target.path.toString());
  target.tell(message, ref);
  return ref.promise;
}

function nextAskName(): string {
  if (typeof globalThis.crypto?.randomUUID === 'function') {
    return globalThis.crypto.randomUUID();
  }
  // Fallback for runtimes without crypto.randomUUID (very old Node).
  const bytes = new Uint8Array(8);
  globalThis.crypto.getRandomValues(bytes);
  return Array.from(bytes, (b) => b.toString(16).padStart(2, '0')).join('');
}

128 bits of entropy → no overflow concerns, no collisions, no shared global state.

Track 2 — Drop the global counter.

Once UUIDs are in, delete let askCounter = 0 and the ++ increment. No state. No precision concerns.

Optional Track 3 — log-friendly short form.

UUIDs are 36 chars (xxxxxxxx-xxxx-...). Logs of asks become noisy. For readability:

function nextAskName(): string {
  const uuid = globalThis.crypto.randomUUID();
  return uuid.slice(0, 8);  // first 8 hex chars — 32 bits, still collision-resistant for log purposes
}

32 bits collide birthday-wise at 2^16 = 65k asks. For a single process that's a few seconds at load. Hmm — actually that's not enough. Use first 12 chars instead → 48 bits → birthday collision at 2^24 = ~16M asks, fine for any realistic single-process log window.

API surface

No public-API change. ask() signature unchanged.

Backward compatibility

Strictly improvement. The path-name format changes from askResp-N to askResp-<hex> but no caller depends on the format (it's a log-only string).

Test plan

1. Uniqueness test: 100K ask() calls; collect every ref.path.name; assert pairwise uniqueness.

2. No-counter regression: ensure no module-level askCounter variable exists after the refactor (grep / lint).

3. Multi-system isolation: two ActorSystem instances each issuing ask calls; verify names are independently random (not from a shared counter).

4. Format-readability test: assert names match askResp-<12-hex-chars> for log-grep compatibility.

5. Regression: existing Ask.test.ts passes; deadletter/trace tests passing show no log-format breakage.

Acceptance criteria

• askCounter deleted; UUIDs (or 12-char hex slice) used instead.
• nextAskName() exported as a test-friendly helper (for [Security] ClusterClient askId predictability via Date.now()+counter #120's tests to reuse).
• Five new tests pass; existing tests green.
• Plan-doc + README "Known security caveats" mark this resolved (it's a code-quality item more than security; can be folded into the same PR as [Security] ClusterClient askId predictability via Date.now()+counter #120).