From 83e97192ce287bd31f44f1bb37ef565a90c16af5 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 22 Sep 2026 15:34:16 +0100 Subject: [PATCH 1/4] fix(offline-transactions): harden value serialization --- .../fix-offline-serializer-hardening.md | 5 + .../src/outbox/TransactionSerializer.ts | 94 +++++++++--- .../transaction-serializer.property.test.ts | 144 ++++++++++++++++-- 3 files changed, 209 insertions(+), 34 deletions(-) create mode 100644 .changeset/fix-offline-serializer-hardening.md diff --git a/.changeset/fix-offline-serializer-hardening.md b/.changeset/fix-offline-serializer-hardening.md new file mode 100644 index 000000000..bfefed61c --- /dev/null +++ b/.changeset/fix-offline-serializer-hardening.md @@ -0,0 +1,5 @@ +--- +'@tanstack/offline-transactions': patch +--- + +Reject cyclic offline transaction values with a bounded error, preserve user objects that only imitate Temporal tags, and avoid allocating an index list for every serialized array. diff --git a/packages/offline-transactions/src/outbox/TransactionSerializer.ts b/packages/offline-transactions/src/outbox/TransactionSerializer.ts index 9344d9aba..b5ef60849 100644 --- a/packages/offline-transactions/src/outbox/TransactionSerializer.ts +++ b/packages/offline-transactions/src/outbox/TransactionSerializer.ts @@ -18,7 +18,10 @@ const temporalConstructorNames = [ ] as const type TemporalConstructorName = (typeof temporalConstructorNames)[number] -type TemporalConstructor = { from: (value: string) => unknown } +type TemporalConstructor = { + from: (value: string) => unknown + prototype?: { toString?: () => string } +} function getTemporalConstructorName( type: unknown, @@ -47,6 +50,24 @@ function requireTemporalConstructor( return constructor } +function serializeTemporalValue( + value: object, + name: TemporalConstructorName, +): string | undefined { + const constructor = requireTemporalConstructor(name) + const toString = constructor.prototype?.toString + if (typeof toString !== `function` || toString === Object.prototype.toString) + return + try { + const serialized = toString.call(value) + return typeof serialized === `string` ? serialized : undefined + } catch { + // Temporal prototype methods brand-check their receiver. A matching + // Symbol.toStringTag without the corresponding internal slots is user data. + return + } +} + export class MissingTemporalConstructorError extends Error {} function setDataProperty( @@ -184,7 +205,11 @@ export class TransactionSerializer { } as PendingMutation } - private serializeValue(value: any, jsonKey?: string | false): any { + private serializeValue( + value: any, + jsonKey?: string | false, + ancestors = new WeakSet(), + ): any { if (value === null || typeof value !== `object`) return value if (jsonKey !== false && value instanceof Date) { @@ -196,17 +221,21 @@ export class TransactionSerializer { ? getTemporalConstructorName(value[Symbol.toStringTag]) : undefined if (temporalConstructorName) { - requireTemporalConstructor(temporalConstructorName) - return { - __type: `Temporal`, - type: `Temporal.${temporalConstructorName}`, - value: value.toString(), - } + const temporalValue = serializeTemporalValue( + value, + temporalConstructorName, + ) + if (temporalValue !== undefined) + return { + __type: `Temporal`, + type: `Temporal.${temporalConstructorName}`, + value: temporalValue, + } } const toJSON = typeof jsonKey === `string` && value.toJSON if (typeof toJSON === `function`) - return this.serializeValue(toJSON.call(value, jsonKey), false) + return this.serializeValue(toJSON.call(value, jsonKey), false, ancestors) if ( jsonKey !== undefined && (value instanceof Boolean || @@ -216,20 +245,43 @@ export class TransactionSerializer { ) { return value.valueOf() } + + if (ancestors.has(value)) + throw new TypeError(`Converting circular structure to JSON`) + ancestors.add(value) + const isArray = Array.isArray(value) const result: any = isArray ? [] : {} - const keys = isArray - ? Array.from({ length: value.length }, (_, index) => String(index)) - : Object.keys(value) - for (const key of keys) { - setDataProperty( - result, - key, - this.serializeValue( - value[key], - jsonKey === undefined ? undefined : key, - ), - ) + try { + if (isArray) { + const length = value.length + for (let index = 0; index < length; index++) { + const key = String(index) + setDataProperty( + result, + key, + this.serializeValue( + value[index], + jsonKey === undefined ? undefined : key, + ancestors, + ), + ) + } + } else { + for (const key of Object.keys(value)) { + setDataProperty( + result, + key, + this.serializeValue( + value[key], + jsonKey === undefined ? undefined : key, + ancestors, + ), + ) + } + } + } finally { + ancestors.delete(value) } if (jsonKey === false && typeof result.toJSON === `function`) delete result.toJSON diff --git a/packages/offline-transactions/tests/transaction-serializer.property.test.ts b/packages/offline-transactions/tests/transaction-serializer.property.test.ts index 1a3115b16..74a6fc90d 100644 --- a/packages/offline-transactions/tests/transaction-serializer.property.test.ts +++ b/packages/offline-transactions/tests/transaction-serializer.property.test.ts @@ -43,9 +43,11 @@ import type { PendingMutation } from '@tanstack/db' * versions. Malformed markers and missing Temporal constructors test failure * paths before durable data can be replaced. * - * Limits: cycles, undefined, non-finite numbers, arbitrary native objects, and - * cross-realm boxed values are outside current evidence. This oracle does not - * claim byte stability for object key order beyond JSON's established rules. + * Limits: undefined, non-finite numbers, arbitrary native objects, and + * cross-realm boxed values are outside current evidence. Cycles must fail + * visibly while repeated non-cyclic references retain their values. This + * oracle does not claim byte stability for object key order beyond JSON's + * established rules. */ type Value = @@ -375,6 +377,18 @@ class TemporalStub { } } +function temporalStubConstructor(name: TemporalName) { + return class extends TemporalStub { + constructor(value: string) { + super(name, value) + } + + static from(value: string): TemporalStub { + return new TemporalStub(name, value) + } + } +} + function metadataTransaction( metadata: Record, ): OfflineTransaction { @@ -392,6 +406,101 @@ function metadataTransaction( } } +it(`rejects cyclic metadata with a bounded JSON-style error`, () => { + const metadata: Record = {} + metadata.self = metadata + + expect(() => + new TransactionSerializer({}).serialize(metadataTransaction(metadata)), + ).toThrowError(new TypeError(`Converting circular structure to JSON`)) +}) + +it(`rejects cyclic mutation values with a bounded JSON-style error`, () => { + const collection = { id: `cycle-writer` } as any + const modified: Record = { id: `one` } + modified.self = modified + const transaction: OfflineTransaction = { + ...metadataTransaction({}), + mutations: [ + { + globalKey: `cycle-writer:one`, + type: `insert`, + modified, + original: {}, + changes: modified, + collection, + } as PendingMutation, + ], + keys: [`cycle-writer:one`], + } + + expect(() => + new TransactionSerializer({ rows: collection }).serialize(transaction), + ).toThrowError(new TypeError(`Converting circular structure to JSON`)) +}) + +it(`preserves repeated references that do not form a cycle`, () => { + const shared = { nested: [`value`] } + const transaction = metadataTransaction({ left: shared, right: shared }) + + const wire = JSON.parse(new TransactionSerializer({}).serialize(transaction)) + + expect(wire.metadata).toEqual({ + left: { nested: [`value`] }, + right: { nested: [`value`] }, + }) +}) + +it(`preserves spoofed Temporal tags as data while restoring branded values`, async () => { + class PlainDateStub { + readonly #value: string + + constructor(value: string) { + this.#value = value + } + + static from(value: string): PlainDateStub { + return new PlainDateStub(value) + } + + get [Symbol.toStringTag](): `Temporal.PlainDate` { + return `Temporal.PlainDate` + } + + toString(): string { + return this.#value + } + } + + const temporalGlobal = globalThis as { Temporal?: Record } + const previousTemporal = temporalGlobal.Temporal + temporalGlobal.Temporal = { PlainDate: PlainDateStub } + const storage = new FakeStorageAdapter() + const outbox = new OutboxManager(storage, {}) + const transaction = metadataTransaction({ + spoofed: { + keep: `user data`, + [Symbol.toStringTag]: `Temporal.PlainDate`, + toString: () => `not-a-date`, + }, + genuine: new PlainDateStub(`2026-09-22`), + }) + + try { + await outbox.add(transaction) + const restored = await outbox.get(transaction.id) + + expect(restored?.metadata).toMatchObject({ + spoofed: { keep: `user data` }, + genuine: expect.any(PlainDateStub), + }) + expect(String((restored?.metadata as any).genuine)).toBe(`2026-09-22`) + } finally { + if (previousTemporal === undefined) delete temporalGlobal.Temporal + else temporalGlobal.Temporal = previousTemporal + } +}) + it(`rejects native scalars before storage when global restoration is unavailable`, async () => { const temporalGlobal = globalThis as { Temporal?: Record } const previousTemporal = temporalGlobal.Temporal @@ -428,16 +537,28 @@ it(`uses one validated Temporal tag when writing a marker`, () => { const temporalGlobal = globalThis as { Temporal?: Record } const previousTemporal = temporalGlobal.Temporal let reads = 0 - temporalGlobal.Temporal = { - PlainDate: { from: (value: string) => value }, - } - const value = { + class PlainDateStub { + readonly #value: string + + constructor(value: string) { + this.#value = value + } + + static from(value: string): PlainDateStub { + return new PlainDateStub(value) + } + get [Symbol.toStringTag]() { reads++ return reads === 1 ? `Temporal.PlainDate` : `Temporal.Invalid` - }, - toString: () => `2026-09-16`, + } + + toString(): string { + return this.#value + } } + temporalGlobal.Temporal = { PlainDate: PlainDateStub } + const value = new PlainDateStub(`2026-09-16`) try { const wire = JSON.parse( @@ -656,10 +777,7 @@ it(`preserves native scalar identity across storage restart`, async () => { ).Temporal ;(globalThis as { Temporal?: Record }).Temporal = Object.fromEntries( - temporalCases.map(([name]) => [ - name, - { from: (value: string) => new TemporalStub(name, value) }, - ]), + temporalCases.map(([name]) => [name, temporalStubConstructor(name)]), ) const values = Object.fromEntries( From a4fbe130b74c116a8daa114fb4f6849cf9d76979 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 22 Sep 2026 16:51:19 +0100 Subject: [PATCH 2/4] fix(offline-transactions): preserve plain tagged values --- .../src/outbox/TransactionSerializer.ts | 11 +++- .../transaction-serializer.property.test.ts | 59 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/packages/offline-transactions/src/outbox/TransactionSerializer.ts b/packages/offline-transactions/src/outbox/TransactionSerializer.ts index b5ef60849..67c4d1d3e 100644 --- a/packages/offline-transactions/src/outbox/TransactionSerializer.ts +++ b/packages/offline-transactions/src/outbox/TransactionSerializer.ts @@ -68,6 +68,15 @@ function serializeTemporalValue( } } +function mayBeNativeScalar(value: object): boolean { + const prototype = Object.getPrototypeOf(value) + return ( + prototype !== null && + prototype !== Object.prototype && + !Array.isArray(value) + ) +} + export class MissingTemporalConstructorError extends Error {} function setDataProperty( @@ -217,7 +226,7 @@ export class TransactionSerializer { } const temporalConstructorName = - jsonKey !== false + jsonKey !== false && mayBeNativeScalar(value) ? getTemporalConstructorName(value[Symbol.toStringTag]) : undefined if (temporalConstructorName) { diff --git a/packages/offline-transactions/tests/transaction-serializer.property.test.ts b/packages/offline-transactions/tests/transaction-serializer.property.test.ts index 74a6fc90d..f79c048a2 100644 --- a/packages/offline-transactions/tests/transaction-serializer.property.test.ts +++ b/packages/offline-transactions/tests/transaction-serializer.property.test.ts @@ -501,6 +501,65 @@ it(`preserves spoofed Temporal tags as data while restoring branded values`, asy } }) +it(`preserves plain user data with a Temporal tag when Temporal is unavailable`, () => { + const temporalGlobal = globalThis as { Temporal?: Record } + const previousTemporal = temporalGlobal.Temporal + delete temporalGlobal.Temporal + const transaction = metadataTransaction({ + spoofed: { + keep: `user data`, + [Symbol.toStringTag]: `Temporal.Instant`, + }, + }) + + try { + const wire = JSON.parse( + new TransactionSerializer({}).serialize(transaction), + ) + + expect(wire.metadata).toEqual({ spoofed: { keep: `user data` } }) + } finally { + if (previousTemporal === undefined) delete temporalGlobal.Temporal + else temporalGlobal.Temporal = previousTemporal + } +}) + +it(`does not invoke a mutation value's plain Symbol.toStringTag getter`, () => { + let reads = 0 + const value = { keep: `user data` } + Object.defineProperty(value, Symbol.toStringTag, { + get() { + reads++ + throw new Error(`do not inspect user getters`) + }, + }) + const collection = { id: `tag-getter-writer` } as any + const modified = { id: `one`, value } + const transaction: OfflineTransaction = { + ...metadataTransaction({}), + mutations: [ + { + globalKey: `tag-getter-writer:one`, + type: `insert`, + modified, + original: {}, + changes: modified, + collection, + } as PendingMutation, + ], + keys: [`tag-getter-writer:one`], + } + + const wire = JSON.parse( + new TransactionSerializer({ rows: collection }).serialize(transaction), + ) + + expect({ reads, modified: wire.mutations[0].modified }).toEqual({ + reads: 0, + modified: { id: `one`, value: { keep: `user data` } }, + }) +}) + it(`rejects native scalars before storage when global restoration is unavailable`, async () => { const temporalGlobal = globalThis as { Temporal?: Record } const previousTemporal = temporalGlobal.Temporal From 384b9db4a5bb15d9ec812024edc6c1587e2d0a31 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 22 Sep 2026 16:59:02 +0100 Subject: [PATCH 3/4] test(offline-transactions): mark partial mutation fixture --- .../tests/transaction-serializer.property.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/offline-transactions/tests/transaction-serializer.property.test.ts b/packages/offline-transactions/tests/transaction-serializer.property.test.ts index f79c048a2..ff9e6e9eb 100644 --- a/packages/offline-transactions/tests/transaction-serializer.property.test.ts +++ b/packages/offline-transactions/tests/transaction-serializer.property.test.ts @@ -545,7 +545,7 @@ it(`does not invoke a mutation value's plain Symbol.toStringTag getter`, () => { original: {}, changes: modified, collection, - } as PendingMutation, + } as unknown as PendingMutation, ], keys: [`tag-getter-writer:one`], } From 6ca4dc6e09b2f2815b50aa54dc8a6ba73678a203 Mon Sep 17 00:00:00 2001 From: Kyle Mathews Date: Tue, 22 Sep 2026 19:24:34 +0100 Subject: [PATCH 4/4] fix(offline-transactions): bound toJSON replacement cycles --- .../src/outbox/TransactionSerializer.ts | 20 +++++++++++++++---- .../transaction-serializer.property.test.ts | 14 +++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/offline-transactions/src/outbox/TransactionSerializer.ts b/packages/offline-transactions/src/outbox/TransactionSerializer.ts index 67c4d1d3e..8dae5f06b 100644 --- a/packages/offline-transactions/src/outbox/TransactionSerializer.ts +++ b/packages/offline-transactions/src/outbox/TransactionSerializer.ts @@ -242,9 +242,23 @@ export class TransactionSerializer { } } + if (ancestors.has(value)) + throw new TypeError(`Converting circular structure to JSON`) + const toJSON = typeof jsonKey === `string` && value.toJSON - if (typeof toJSON === `function`) - return this.serializeValue(toJSON.call(value, jsonKey), false, ancestors) + if (typeof toJSON === `function`) { + const replacement = toJSON.call(value, jsonKey) + if (replacement === value) { + return this.serializeValue(replacement, false, ancestors) + } + + ancestors.add(value) + try { + return this.serializeValue(replacement, false, ancestors) + } finally { + ancestors.delete(value) + } + } if ( jsonKey !== undefined && (value instanceof Boolean || @@ -255,8 +269,6 @@ export class TransactionSerializer { return value.valueOf() } - if (ancestors.has(value)) - throw new TypeError(`Converting circular structure to JSON`) ancestors.add(value) const isArray = Array.isArray(value) diff --git a/packages/offline-transactions/tests/transaction-serializer.property.test.ts b/packages/offline-transactions/tests/transaction-serializer.property.test.ts index ff9e6e9eb..4e0cb5953 100644 --- a/packages/offline-transactions/tests/transaction-serializer.property.test.ts +++ b/packages/offline-transactions/tests/transaction-serializer.property.test.ts @@ -724,6 +724,20 @@ it(`does not invoke toJSON again on its immediate replacement`, () => { expect(wire.metadata).toEqual({ value: { nested: `nested:nested` } }) }) +it(`rejects a toJSON replacement that references its receiver`, () => { + const source = { + toJSON() { + return { back: source } + }, + } + + expect(() => + new TransactionSerializer({}).serialize( + metadataTransaction({ value: source }), + ), + ).toThrowError(new TypeError(`Converting circular structure to JSON`)) +}) + it(`treats immediate native-scalar replacements as ordinary JSON values`, () => { const temporalReplacement = { [Symbol.toStringTag]: `Temporal.PlainDate`,