fix: make Browser RPC and Realtime own the physical work they report on

A server stream's registration was pruned on any settled close receipt.
`waitClosed()` rejecting, throwing synchronously, or not returning a
promise at all was absorbed into a fulfilled `undefined`, so the runtime
opened a second physical stream for the same operation while the first was
still running against the server. Only a fulfilled, contract-shaped
receipt confirms closure now; every negative receipt keeps the operation
DRAINING.

Cleanup also read foreign state outside the result boundary. A throwing
iterator `return` accessor replaced the already selected timeout with a
native `TypeError` and skipped the rest of the teardown, and the exported
lease decoder threw on a hostile `Symbol.asyncIterator`. Both reads move
inside their own boundaries, and the positive-close subscription is
installed before any fallible cleanup.

Composition validated the caller's registries before snapshotting them, so
a hostile accessor ran twice during validation, and rows hiding fields
behind a prototype or a non-enumerable key installed. Transport results
were checked for allowed own keys only, so own `{ok,message,encodedBytes}`
plus a prototype `injected` was a success and a missing `message` reached
a permissive schema as `undefined`.

In Realtime the tracked task was registered after the collaborator
returned. An authority that re-entered `close()` from inside its own
invocation saw an empty registry and got `{ok:true}` while its effect was
pending. The task is now registered first and the collaborator is invoked
a microtask later. A `scheduleTimeout` that threw was worse: the caller's
own catch treated it as an apply failure and started a recovery beside the
still-running effect, and `close()` rejected with a native `TypeError`. An
uninstallable deadline now fails closed as an expired one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
DongHyeonka
2026-08-15 01:25:48 +09:00
co-authored by Claude Opus 5
parent 632b230c82
commit aa8ac35600
6 changed files with 698 additions and 54 deletions
+125 -29
View File
@@ -113,11 +113,11 @@ export function createBrowserRpcRuntime(
const clock = dependencies.clock ?? systemClock;
const generationFence =
dependencies.generationFence ?? stableGenerationFence;
// RPC-RR-03. Snapshot before validating. Reading the caller's transport
// objects first would run their accessors, letting a hostile getter observe
// validation and then return something else to the runtime.
// RPC-RR-03 / RPC-03. Snapshot before validating — every registry, not just
// the transports. Handing the caller's own objects to the join validation
// first ran their accessors, so a hostile getter could observe validation and
// then answer the runtime differently.
const installedTransports = snapshotTransports(dependencies.transports);
validateRuntimeDependencies(dependencies, installedTransports);
// R-04. Install exact immutable snapshots once. Every later `bind()` reads
// the snapshot, never the caller's registry objects, so a post-composition
// mutation cannot change replay policy, deadlines, byte ceilings or
@@ -129,6 +129,7 @@ export function createBrowserRpcRuntime(
mappers: dependencies.mappers,
requestEncoders: dependencies.requestEncoders,
});
validateRuntimeDependencies(installed, installedTransports);
// RPC-RR-01. One entry per physical stream that has been opened and not yet
// confirmed closed. A stream stays here through DRAINING — after cancel,
@@ -215,9 +216,69 @@ export function createBrowserRpcRuntime(
type ActiveStreamLease = Readonly<{
streamId: string;
cancel(reason: string): void;
closed: Promise<void>;
/**
* RPC-01. Fulfils with `true` only when the transport positively confirmed
* the physical stream closed, and with `false` for every negative receipt —
* a rejection, a synchronous throw, or a `waitClosed()` that did not even
* return a promise. A negative receipt keeps the operation DRAINING.
*/
closed: Promise<boolean>;
}>;
/**
* RPC-01. Turns a lease's close receipt into evidence the registry can trust.
* Only a fulfilled promise from a contract-shaped `waitClosed()` counts as
* confirmation that the physical stream ended.
*/
function confirmPhysicalClosure(
lease: Readonly<{ waitClosed(): Promise<void> }>,
): Promise<boolean> {
return Promise.resolve()
.then(() => {
const receipt: unknown = lease.waitClosed();
if (
receipt === null ||
(typeof receipt !== "object" && typeof receipt !== "function") ||
typeof (receipt as PromiseLike<void>).then !== "function"
) {
return false;
}
return Promise.resolve(receipt as PromiseLike<void>).then(
() => true,
() => false,
);
})
.catch(() => false);
}
/**
* RPC-02. Reads an iterator's optional `return` once, through its own data
* descriptor. Testing `iterator.return` directly ran a foreign accessor outside
* any boundary, so a throwing getter replaced the already selected stream
* outcome with a native rejection and skipped the rest of the cleanup.
*/
function safeIteratorReturn(
iterator: AsyncIterator<unknown> | null | undefined,
): (() => unknown) | null {
if (!iterator) return null;
try {
let current: object | null = iterator;
while (current !== null) {
const descriptor = Object.getOwnPropertyDescriptor(current, "return");
if (descriptor) {
if (!("value" in descriptor)) return null;
return typeof descriptor.value === "function"
? (descriptor.value as () => unknown).bind(iterator)
: null;
}
current = Reflect.getPrototypeOf(current);
}
return null;
} catch {
return null;
}
}
async function executeUnary<Output>(
installed: InstalledBrowserRpcContractBindings,
dependencies: BrowserRpcRuntimeDependencies,
@@ -608,9 +669,12 @@ async function* executeServerStream<Event>(
Object.freeze({
streamId: lease.streamId,
cancel: lease.cancel,
closed: Promise.resolve()
.then(() => lease!.waitClosed())
.catch(() => undefined) as Promise<void>,
// RPC-01. Only a fulfilled, contract-shaped receipt is evidence that the
// physical stream closed. Absorbing a rejection, a synchronous throw or
// a non-promise into `undefined` forged that evidence, and the registry
// then admitted a second stream for the same operation while the first
// was still running against the server.
closed: confirmPhysicalClosure(lease),
}),
);
try {
@@ -793,25 +857,34 @@ async function* executeServerStream<Event>(
// A transport that refuses to cancel stays DRAINING below.
}
}
if (iterator?.return) {
if (registered && registered.streamId === lease?.streamId) {
// RPC-02. The positive-close subscription is installed before any
// fallible cleanup. Running cleanup first meant a throwing `return`
// accessor could skip it entirely and strand the registry entry.
// The entry is removed only once the transport confirms the physical
// stream closed. Until then the operation stays DRAINING and admits
// nothing new — a bounded wait here would re-open the very hole this
// registry exists to close.
void registered.closed.then((confirmed) => {
if (
confirmed &&
activeStreams.get(operation.operationId) === registered
) {
activeStreams.delete(operation.operationId);
}
});
}
const returnIterator = safeIteratorReturn(iterator);
if (returnIterator) {
const cleanup = Promise.resolve()
.then(async () => await iterator?.return?.())
.then(async () => await returnIterator())
// Cleanup cannot replace the already selected stream outcome.
.catch(() => undefined);
await boundedStreamCleanup(cleanup, clock, STREAM_CLEANUP_BOUND_MS);
}
if (registered && registered.streamId === lease?.streamId) {
// The entry is removed only once the transport confirms the physical
// stream closed. Until then the operation stays DRAINING and admits
// nothing new — a bounded wait here would re-open the very hole this
// registry exists to close.
void registered.closed.then(() => {
if (activeStreams.get(operation.operationId) === registered) {
activeStreams.delete(operation.operationId);
}
});
await boundedStreamCleanup(
registered.closed,
registered.closed.then(() => undefined),
clock,
STREAM_CLEANUP_BOUND_MS,
);
@@ -903,7 +976,7 @@ function snapshotTransports(
}
function validateRuntimeDependencies(
dependencies: BrowserRpcRuntimeDependencies,
installed: InstalledBrowserRpcContractBindings,
transports: ReadOnlyRegistry<string, BrowserRpcTransport>,
): void {
const runtimeBindings: Record<
@@ -929,15 +1002,16 @@ function validateRuntimeDependencies(
rpcKind: transport.rpcKind,
});
}
// Only the installed snapshot reaches the join validation.
validateBrowserRpcContractBindings({
operations: dependencies.operations,
profiles: dependencies.profiles,
schemaCodecs: dependencies.schemaCodecs,
mappers: dependencies.mappers,
requestEncoders: dependencies.requestEncoders,
operations: Object.fromEntries(installed.operations.entries()),
profiles: Object.fromEntries(installed.profiles.entries()),
schemaCodecs: Object.fromEntries(installed.schemaCodecs.entries()),
mappers: Object.fromEntries(installed.mappers.entries()),
requestEncoders: Object.fromEntries(installed.requestEncoders.entries()),
runtimeBindings: Object.freeze(runtimeBindings),
});
for (const operation of Object.values(dependencies.operations)) {
for (const operation of installed.operations.values()) {
if (!transports.has(operation.runtimeProfileId)) {
throw new TypeError(
`Browser RPC transport is missing: ${operation.operationId}`,
@@ -1248,17 +1322,32 @@ function ownDataValue(source: unknown, key: string): unknown {
}
}
/**
* RPC-04. An exact union means exactly these own data keys, all of them, on a
* plain object. Checking only that each own name was *allowed* let a result
* carry required fields it never declared and let a custom prototype smuggle
* metadata past the trust boundary while the shape still looked valid.
*/
function exactOwnKeys(
source: unknown,
allowed: ReadonlySet<string>,
required: ReadonlySet<string> = allowed,
): boolean {
if (source === null || typeof source !== "object") return false;
try {
if (Object.getOwnPropertySymbols(source).length > 0) return false;
const prototype = Reflect.getPrototypeOf(source);
if (prototype !== Object.prototype && prototype !== null) return false;
const present = new Set<string>();
for (const key of Object.getOwnPropertyNames(source)) {
if (!allowed.has(key)) return false;
const descriptor = Object.getOwnPropertyDescriptor(source, key);
if (!descriptor || !("value" in descriptor)) return false;
if (descriptor.enumerable !== true) return false;
present.add(key);
}
for (const key of required) {
if (!present.has(key)) return false;
}
return true;
} catch {
@@ -1271,6 +1360,9 @@ const UNARY_OK_KEYS: ReadonlySet<string> = new Set([
"message",
"encodedBytes",
]);
/** `message` is a wire field, so an absent one is a protocol breach even when
* a permissive schema would happily accept `undefined`. */
const UNARY_OK_REQUIRED: ReadonlySet<string> = UNARY_OK_KEYS;
const UNARY_FAILED_KEYS: ReadonlySet<string> = new Set(["ok", "failure"]);
const FRAME_MESSAGE_KEYS: ReadonlySet<string> = new Set([
"kind",
@@ -1287,6 +1379,8 @@ const TRANSPORT_FAILURE_KEYS: ReadonlySet<string> = new Set([
"code",
"retryAfterMs",
]);
/** `retryAfterMs` is genuinely optional; `code` is not. */
const TRANSPORT_FAILURE_REQUIRED: ReadonlySet<string> = new Set(["code"]);
function validateUnaryTransportResult(
value: unknown,
@@ -1294,7 +1388,7 @@ function validateUnaryTransportResult(
const ok = ownDataValue(value, "ok");
if (typeof ok !== "boolean") return null;
if (ok) {
if (!exactOwnKeys(value, UNARY_OK_KEYS)) return null;
if (!exactOwnKeys(value, UNARY_OK_KEYS, UNARY_OK_REQUIRED)) return null;
const encodedBytes = ownDataValue(value, "encodedBytes");
if (!validEncodedByteCount(encodedBytes, Number.MAX_SAFE_INTEGER)) {
return null;
@@ -1347,7 +1441,9 @@ function validateStreamFrame(value: unknown): BrowserRpcStreamFrame | null {
function decodeTransportFailure(
value: unknown,
): BrowserRpcTransportFailure | null {
if (!exactOwnKeys(value, TRANSPORT_FAILURE_KEYS)) return null;
if (!exactOwnKeys(value, TRANSPORT_FAILURE_KEYS, TRANSPORT_FAILURE_REQUIRED)) {
return null;
}
const code = ownDataValue(value, "code");
const retryAfterMs = ownDataValue(value, "retryAfterMs");
if (!TRANSPORT_FAILURE_CODES.has(code as BrowserRpcTransportFailureCode)) {
+24 -11
View File
@@ -119,21 +119,34 @@ export function decodeServerStreamLease(
} catch {
return null;
}
if (
typeof streamId !== "string" ||
!STREAM_ID.test(streamId) ||
frames === null ||
typeof frames !== "object" ||
typeof (frames as AsyncIterable<unknown>)[Symbol.asyncIterator] !==
"function" ||
typeof cancel !== "function" ||
typeof waitClosed !== "function"
) {
// RPC-02. The async-iterator lookup is a read of foreign state like any
// other, so it happens inside the decoder's own boundary. Performing it after
// the `try` let a throwing `Symbol.asyncIterator` getter escape this
// function as a native `TypeError`, breaking the decoder's totality.
let openFrames: unknown;
try {
if (
typeof streamId !== "string" ||
!STREAM_ID.test(streamId) ||
frames === null ||
typeof frames !== "object" ||
typeof cancel !== "function" ||
typeof waitClosed !== "function"
) {
return null;
}
openFrames = (frames as AsyncIterable<unknown>)[Symbol.asyncIterator];
if (typeof openFrames !== "function") return null;
} catch {
return null;
}
const iterate = (openFrames as () => AsyncIterator<BrowserRpcStreamFrame>)
.bind(frames);
return Object.freeze({
streamId,
frames: frames as AsyncIterable<BrowserRpcStreamFrame>,
frames: Object.freeze({
[Symbol.asyncIterator]: iterate,
}) as AsyncIterable<BrowserRpcStreamFrame>,
cancel: (cancel as (reason: string) => void).bind(value),
waitClosed: (waitClosed as () => Promise<void>).bind(value),
});
+44 -12
View File
@@ -185,6 +185,15 @@ export function createRealtimeStreamCoordinator(
const states = new Map<StreamRegistrationId, StreamState>();
let closed = false;
/** RT-02. Releasing a timer is best effort and never a public failure. */
const clearTimerSafely = (handle: unknown): void => {
try {
clearScheduledTimeout(handle);
} catch {
// A broken scheduler cannot change an already classified outcome.
}
};
const TASK_TIMED_OUT = Symbol("REALTIME_TASK_TIMED_OUT");
/**
@@ -194,10 +203,16 @@ export function createRealtimeStreamCoordinator(
*/
async function awaitTaskWithinDeadline<Value>(
state: StreamState,
task: Promise<Value>,
invoke: () => Promise<Value> | Value,
timeoutMs: number,
revokeAndAbort: () => void,
): Promise<Value | typeof TASK_TIMED_OUT> {
// RT-01. The collaborator is invoked on the next microtask, after the task
// is already in the registry. Calling it first left a window in which an
// authority that re-entered `close()` from inside its own invocation saw an
// empty registry, so `close()` reported quiescence while its effect was
// still running.
const task = Promise.resolve().then(invoke);
task.catch(() => {});
// RT-RR-01. The task is a physical effect the moment it is created, so it
// is registered here rather than when its public wait happens to expire.
@@ -212,12 +227,23 @@ export function createRealtimeStreamCoordinator(
if (!state.closed) state.freshness = "STALE";
}
});
// RT-02. A scheduler that cannot install the deadline leaves the wait
// unbounded. Letting the exception escape turned a typed realtime result
// into a native rejection and — through the caller's own catch — started a
// recovery that overlapped the effect still running, so an install failure
// fails closed as an expired deadline instead.
let handle: unknown;
let installed = false;
const timeout = new Promise<typeof TASK_TIMED_OUT>((resolve) => {
handle = scheduleTimeout(() => resolve(TASK_TIMED_OUT), timeoutMs);
try {
handle = scheduleTimeout(() => resolve(TASK_TIMED_OUT), timeoutMs);
installed = true;
} catch {
resolve(TASK_TIMED_OUT);
}
});
const outcome = await Promise.race([task, timeout]);
clearScheduledTimeout(handle);
if (installed) clearTimerSafely(handle);
if (outcome !== TASK_TIMED_OUT) return outcome;
// The commit capability is revoked immediately; the work itself is not.
@@ -514,7 +540,7 @@ export function createRealtimeStreamCoordinator(
try {
const applied = await awaitTaskWithinDeadline(
state,
Promise.resolve(
() =>
dependencies.authority.effects.apply(
eventType.effectProfileId,
mapped.value,
@@ -527,7 +553,6 @@ export function createRealtimeStreamCoordinator(
}),
effectAbort.signal,
),
),
limits.effectTimeoutMs,
() => {
// Commit capability is revoked permanently for this attempt.
@@ -701,7 +726,7 @@ export function createRealtimeStreamCoordinator(
try {
const outcome = await awaitTaskWithinDeadline(
state,
Promise.resolve(
() =>
dependencies.authority.recovery.recover(
Object.freeze({
streamId: state.registration.id,
@@ -711,7 +736,6 @@ export function createRealtimeStreamCoordinator(
isCurrent: recoveryIsCurrent,
}),
),
),
limits.recoveryTimeoutMs,
() => {
recoveryLeaseActive = false;
@@ -1012,16 +1036,24 @@ export function createRealtimeStreamCoordinator(
return realtimeSuccess(undefined);
}
let handle: unknown;
let installed = false;
const drained = await Promise.race([
Promise.allSettled(retained).then(() => true),
new Promise<false>((resolve) => {
handle = scheduleTimeout(
() => resolve(false),
limits.drainTimeoutMs,
);
try {
handle = scheduleTimeout(
() => resolve(false),
limits.drainTimeoutMs,
);
installed = true;
} catch {
// RT-02. Without a drain bound this call cannot prove quiescence, so
// it reports the honest failure rather than rejecting natively.
resolve(false);
}
}),
]);
clearScheduledTimeout(handle);
if (installed) clearTimerSafely(handle);
if (!drained) {
// Still DRAINING: the caller must not treat this as quiescence.
return realtimeFailure("IDLE_TIMEOUT", "CLOSE", false);
+18 -2
View File
@@ -363,15 +363,23 @@ function installRegistrySnapshot<Value extends object>(
): ReadOnlyRegistry<string, Value> {
let ownKeys: string[];
let symbols: readonly symbol[];
let prototype: object | null;
try {
ownKeys = Object.keys(source);
// RPC-03. Own *names*, not just enumerable keys: a non-enumerable own entry
// is as much a smuggled row as an inherited one, and `Object.keys` never
// saw either.
ownKeys = Object.getOwnPropertyNames(source);
symbols = Object.getOwnPropertySymbols(source);
prototype = Reflect.getPrototypeOf(source);
} catch {
throw new TypeError(`Browser RPC ${label} registry is unreadable.`);
}
if (symbols.length > 0) {
throw new TypeError(`Browser RPC ${label} registry has symbol keys.`);
}
if (prototype !== Object.prototype && prototype !== null) {
throw new TypeError(`Browser RPC ${label} registry has a custom prototype.`);
}
const installed = new Map<string, Value>();
for (const key of ownKeys) {
const descriptor = Object.getOwnPropertyDescriptor(source, key);
@@ -398,15 +406,23 @@ function installRowSnapshot<Value extends object>(
}
let ownKeys: string[];
let symbols: readonly symbol[];
let prototype: object | null;
try {
ownKeys = Object.keys(row);
ownKeys = Object.getOwnPropertyNames(row);
symbols = Object.getOwnPropertySymbols(row);
prototype = Reflect.getPrototypeOf(row);
} catch {
throw new TypeError(`Browser RPC ${label} row is unreadable.`);
}
if (symbols.length > 0) {
throw new TypeError(`Browser RPC ${label} row has symbol keys.`);
}
// RPC-03. A custom prototype carries fields the name sweep never sees and
// stays live after installation, so the installed row would not be the row
// that was checked.
if (prototype !== Object.prototype && prototype !== null) {
throw new TypeError(`Browser RPC ${label} row has a custom prototype.`);
}
const snapshot = Object.create(null) as Record<string, unknown>;
for (const key of ownKeys) {
if (!allowedKeys.includes(key)) {