diff --git a/src/domain/workflow-transitions.ts b/src/domain/workflow-transitions.ts index 6de0b3b..803c73c 100644 --- a/src/domain/workflow-transitions.ts +++ b/src/domain/workflow-transitions.ts @@ -105,6 +105,7 @@ const objectFreeze = Object.freeze; const objectHasOwn = Object.hasOwn; const arrayIsArray = Array.isArray; const objectDefineProperty = Object.defineProperty; +const objectSetPrototypeOf = Object.setPrototypeOf; const objectIsFrozen = Object.isFrozen; const reflectIsExtensible = Reflect.isExtensible; const reflectOwnKeys = Reflect.ownKeys; @@ -318,20 +319,28 @@ function noteRevisionSpan( const low = lowest[index]; const high = highest[index]; if (low !== undefined && sequence < low) { - objectDefineProperty(lowest, index, { + // Null-prototype the descriptor before `defineProperty` reads it, so a + // poisoned inherited `get`/`set` cannot be observed by + // `ToPropertyDescriptor` and turn this stamp into a thrown `TypeError`. + // Same insulation as `workflow.ts::append`, kept inline here on purpose. + const descriptor: PropertyDescriptor = { value: sequence, writable: true, enumerable: true, configurable: true, - }); + }; + objectSetPrototypeOf(descriptor, null); + objectDefineProperty(lowest, index, descriptor); } if (high !== undefined && sequence > high) { - objectDefineProperty(highest, index, { + const descriptor: PropertyDescriptor = { value: sequence, writable: true, enumerable: true, configurable: true, - }); + }; + objectSetPrototypeOf(descriptor, null); + objectDefineProperty(highest, index, descriptor); } return; } diff --git a/src/domain/workflow.ts b/src/domain/workflow.ts index 107058f..bcc4949 100644 --- a/src/domain/workflow.ts +++ b/src/domain/workflow.ts @@ -60,6 +60,7 @@ import type { InvocationReportResult } from './agent-invocation-report.js'; */ const objectFreeze = Object.freeze; const objectDefineProperty = Object.defineProperty; +const objectSetPrototypeOf = Object.setPrototypeOf; const objectHasOwn = Object.hasOwn; const objectIs = Object.is; const numberIsInteger = Number.isInteger; @@ -90,12 +91,20 @@ function containsValue(list: readonly string[], value: unknown): boolean { /** Append by defining an own element, bypassing inherited index setters. */ export function append(list: T[], value: T): void { - objectDefineProperty(list, list.length, { + // The descriptor is null-prototyped before `defineProperty` consumes it. + // `ToPropertyDescriptor` tests `get`/`set` with `HasProperty`, which walks the + // prototype chain: an ordinary literal inherits from `Object.prototype`, so a + // poisoned inherited `get`/`set` would be observed and throw `TypeError` + // instead of appending. Severing the prototype leaves only the own data + // fields visible, so conversion sees exactly what is written here. + const descriptor: PropertyDescriptor = { value, writable: true, enumerable: true, configurable: true, - }); + }; + objectSetPrototypeOf(descriptor, null); + objectDefineProperty(list, list.length, descriptor); } /** diff --git a/tests/domain/workflow-invariants.test.ts b/tests/domain/workflow-invariants.test.ts index d76c55f..f352f90 100644 --- a/tests/domain/workflow-invariants.test.ts +++ b/tests/domain/workflow-invariants.test.ts @@ -27,6 +27,7 @@ import { type WorkflowEvent, type WorkflowState, } from '../../src/domain/index.js'; +import { append } from '../../src/domain/workflow.js'; import { admitEvidence, admitReview, @@ -127,6 +128,46 @@ function withPoisoned(target: object, key: PropertyKey, value: unknown, body: () } } +/** + * Plant inherited accessor fields on `Object.prototype` and restore them exactly. + * + * A poisoned `get`/`set` is what makes `ToPropertyDescriptor` throw on any + * ordinary descriptor literal: it sees an inherited, callable accessor sitting + * beside the literal's own `value`/`writable`, the one combination the intrinsic + * refuses. The installing descriptor is itself null-prototyped, so poisoning + * `set` while `get` is already poisoned does not disrupt the very + * `defineProperty` call that installs it — the harness stays valid under the + * exact condition it exercises, and cannot mask the defect with its own throw. + */ +function withAccessorPoison(keys: readonly PropertyKey[], body: () => void): void { + const proto = Object.prototype; + const saved = keys.map((key) => Object.getOwnPropertyDescriptor(proto, key)); + const poison: PropertyDescriptor = { + value(): unknown { + return undefined; + }, + writable: true, + enumerable: false, + configurable: true, + }; + Object.setPrototypeOf(poison, null); + try { + for (const key of keys) { + Object.defineProperty(proto, key, poison); + } + body(); + } finally { + keys.forEach((key, index) => { + const original = saved[index]; + if (original === undefined) { + Reflect.deleteProperty(proto, key); + } else { + Object.defineProperty(proto, key, original); + } + }); + } +} + describe('group H — provider, purpose, and reported status are inert', () => { /** Replace the three recorded label fields, so only they may differ. */ function withoutLabels(state: WorkflowState): unknown { @@ -2822,3 +2863,151 @@ describe('group M — forbidden vocabulary', () => { } }); }); + +describe('group N — PR9-WF-F1: descriptor objects survive prototype poisoning', () => { + // `append` builds a PropertyDescriptor and hands it to `Object.defineProperty`. + // `ToPropertyDescriptor` probes `get`/`set` with `HasProperty`, which walks the + // prototype chain, so an inherited poison on `Object.prototype` was observed by + // the conversion and threw `TypeError` — turning the layer's intended + // deterministic result into an unexpected throw. Every case here must complete + // without throwing and must leave the realm exactly as it found it. + + // The realm is always restored before any assertion runs: a matcher such as + // `toEqual` builds descriptor objects of its own, which would themselves throw + // under the poison and mask what is being tested (Section 14). Every case + // captures plain values inside the poisoned block and asserts once outside it. + + it('appends under a poisoned Object.prototype.get without throwing', () => { + const list: number[] = []; + withAccessorPoison(['get'], () => { + append(list, 7); + }); + expect(list).toHaveLength(1); + expect(list[0]).toBe(7); + }); + + it('appends under a poisoned Object.prototype.set without throwing', () => { + const list: string[] = []; + withAccessorPoison(['set'], () => { + append(list, 'x'); + }); + expect(list).toEqual(['x']); + }); + + it('appends under a poisoned get *and* set without throwing', () => { + const list: number[] = []; + withAccessorPoison(['get', 'set'], () => { + append(list, 1); + append(list, 2); + }); + expect(list).toEqual([1, 2]); + }); + + it('preserves append index and flag semantics under poison', () => { + const list: number[] = []; + let descriptor: PropertyDescriptor | undefined; + withAccessorPoison(['get', 'set'], () => { + append(list, 42); + descriptor = Object.getOwnPropertyDescriptor(list, 0); + }); + expect(descriptor).toEqual({ + value: 42, + writable: true, + enumerable: true, + configurable: true, + }); + }); + + it.each([['get'], ['set'], ['get', 'set']] as const)( + 'reaches append through applyWorkflowEvent under %s poison and still applies', + (...keys) => { + // `requested()` carries one invocation, so validating it re-runs the + // snapshot append path before the report is applied. Compare against the + // clean evaluation: same input semantics must yield the same output. + const clean = applyWorkflowEvent(requested(), reportInvocation()); + let poisoned: unknown; + + withAccessorPoison([...keys], () => { + poisoned = applyWorkflowEvent(requested(), reportInvocation()); + }); + + expect(poisoned).toEqual(clean); + expect((poisoned as typeof clean).outcome).toBe('APPLIED'); + expect((poisoned as typeof clean).state.invocations[0]?.state).toBe('REPORTED'); + }, + ); + + it('returns the identical prior state on a rejection reached under poison', () => { + const prior = requested(); + let result: ReturnType | undefined; + + withAccessorPoison(['get', 'set'], () => { + // A duplicate invocation id rejects, and the snapshot of `prior` reaches + // `append` on the way to that rejection. + result = applyWorkflowEvent(prior, requestInvocation()); + }); + + expect(result?.outcome).toBe('REJECTED'); + expect(result?.rejection).toBe('DUPLICATE_INVOCATION_ID'); + expect(result?.state).toBe(prior); + }); + + it.each([['get'], ['set']] as const)( + 'survives an Object.prototype.%s poison installed mid-evaluation before a later append', + (key) => { + const proto = Object.prototype; + const saved = Object.getOwnPropertyDescriptor(proto, key); + const base = requested(); + const hostile = { ...base } as Record; + const poison: PropertyDescriptor = { + value(): unknown { + return undefined; + }, + writable: true, + enumerable: false, + configurable: true, + }; + Object.setPrototypeOf(poison, null); + // The bound commit is read early in the snapshot; arming the poison from + // its getter guarantees the poison is live before the invocation list's + // `append` calls run later in the same evaluation. + Object.defineProperty(hostile, 'boundCommitSha', { + get(): string { + Object.defineProperty(proto, key, poison); + return base.boundCommitSha; + }, + enumerable: true, + configurable: true, + }); + + let outcome: string | undefined; + let invocationState: string | undefined; + try { + const result = applyWorkflowEvent( + hostile as unknown as WorkflowState, + reportInvocation(), + ); + outcome = result.outcome; + invocationState = result.state.invocations[0]?.state; + } finally { + if (saved === undefined) { + Reflect.deleteProperty(proto, key); + } else { + Object.defineProperty(proto, key, saved); + } + } + + // Assert only after the realm is restored, so the matcher itself runs + // against a clean `Object.prototype`. + expect(outcome).toBe('APPLIED'); + expect(invocationState).toBe('REPORTED'); + }, + ); + + it('leaves Object.prototype.get and Object.prototype.set untouched afterwards', () => { + // Every case above restores in `finally`; this pins that the realm is clean + // once the group has run, so no later test inherits a poisoned prototype. + expect(Object.getOwnPropertyDescriptor(Object.prototype, 'get')).toBeUndefined(); + expect(Object.getOwnPropertyDescriptor(Object.prototype, 'set')).toBeUndefined(); + }); +}); diff --git a/tests/domain/workflow-transitions.test.ts b/tests/domain/workflow-transitions.test.ts index 8b47a1e..5a4c9df 100644 --- a/tests/domain/workflow-transitions.test.ts +++ b/tests/domain/workflow-transitions.test.ts @@ -1283,3 +1283,134 @@ describe('applyWorkflowEvent — group N, end-to-end lifecycle replay', () => { expect(label(REVIEW_B)).toContain(REVIEW_B); }); }); + +/** + * Plant inherited accessor fields on `Object.prototype`, restoring them exactly. + * + * The installing descriptor is null-prototyped so that poisoning `set` while + * `get` is already poisoned does not disrupt the very `defineProperty` that + * installs it — the harness stays valid under the same condition it exercises. + */ +function withAccessorPoison(keys: readonly PropertyKey[], body: () => void): void { + const proto = Object.prototype; + const saved = keys.map((key) => Object.getOwnPropertyDescriptor(proto, key)); + const poison: PropertyDescriptor = { + value(): unknown { + return undefined; + }, + writable: true, + enumerable: false, + configurable: true, + }; + Object.setPrototypeOf(poison, null); + try { + for (const key of keys) { + Object.defineProperty(proto, key, poison); + } + body(); + } finally { + keys.forEach((key, index) => { + const original = saved[index]; + if (original === undefined) { + Reflect.deleteProperty(proto, key); + } else { + Object.defineProperty(proto, key, original); + } + }); + } +} + +describe('PR9-WF-F1: noteRevisionSpan inline descriptors survive prototype poisoning', () => { + // `noteRevisionSpan` stamps its lowest/highest slots with `Object.defineProperty` + // over an inline descriptor. Those calls are on the public evaluation path: + // `applyWorkflowEvent` -> `snapshotWorkflow` -> `noteRevisionSpan`. An inherited + // `get`/`set` poison made `ToPropertyDescriptor` throw there, so a hostile realm + // turned an intended apply/rejection into an unexpected `TypeError`. + + /** One invocation requested then reported at the same revision. */ + function reportedInvocation(): WorkflowState { + return applyOrThrow(withRequestedInvocation(), reportInvocation()); + } + + /** + * A state whose two same-revision invocation records are ordered so the + * second-listed carries the lower sequence — driving `noteRevisionSpan` + * through its lowest-slot inline descriptor. Built from real transitions, + * then reordered; lists are refrozen to stay faithful to a produced state. + */ + function reachesLowestSpanSite(): WorkflowState { + let state = openedWorkflow(); + state = applyOrThrow(state, requestInvocation(buildInvocation({ invocationId: INVOCATION_A }))); + state = applyOrThrow(state, requestInvocation(buildInvocation({ invocationId: INVOCATION_B }))); + const invocations = Object.freeze([ + Object.freeze({ ...state.invocations[0], requestedAtSequence: 2 }), + Object.freeze({ ...state.invocations[1], requestedAtSequence: 1 }), + ]); + return Object.freeze({ ...state, invocations }) as WorkflowState; + } + + const deeplyFrozen = (state: WorkflowState): boolean => + Object.isFrozen(state) && + Object.isFrozen(state.invocations) && + Object.isFrozen(state.evidence) && + Object.isFrozen(state.reviews); + + it.each([['get'], ['set'], ['get', 'set']] as const)( + 'reaches the highest-slot inline descriptor under %s poison and applies unchanged', + (...keys) => { + const prior = reportedInvocation(); + const clean = applyWorkflowEvent(prior, admitEvidence()); + let poisoned: ReturnType | undefined; + + withAccessorPoison([...keys], () => { + poisoned = applyWorkflowEvent(prior, admitEvidence()); + }); + + expect(poisoned).toEqual(clean); + expect(poisoned?.outcome).toBe('APPLIED'); + // Chronology, revision, and sequence accounting are all unchanged. + expect(poisoned?.state.revision).toBe(clean.state.revision); + expect(poisoned?.state.sequence).toBe(clean.state.sequence); + expect(poisoned ? deeplyFrozen(poisoned.state) : false).toBe(true); + }, + ); + + it.each([['get'], ['set'], ['get', 'set']] as const)( + 'reaches the lowest-slot inline descriptor under %s poison with identical outcome', + (...keys) => { + const prior = reachesLowestSpanSite(); + const clean = applyWorkflowEvent(prior, admitEvidence()); + let poisoned: ReturnType | undefined; + + withAccessorPoison([...keys], () => { + poisoned = applyWorkflowEvent(prior, admitEvidence()); + }); + + // Whether the reordered state reads as applicable or as a deterministic + // rejection, the poisoned run must reproduce the clean run exactly. + expect(poisoned).toEqual(clean); + expect(poisoned?.outcome).toBe(clean.outcome); + expect(poisoned?.rejection).toBe(clean.rejection); + }, + ); + + it('preserves prior-state identity on a rejection reached through noteRevisionSpan', () => { + const prior = reportedInvocation(); + let poisoned: ReturnType | undefined; + + withAccessorPoison(['get', 'set'], () => { + // A duplicate invocation id rejects, but the snapshot of `prior` reaches + // `noteRevisionSpan` first. + poisoned = applyWorkflowEvent(prior, requestInvocation()); + }); + + expect(poisoned?.outcome).toBe('REJECTED'); + expect(poisoned?.rejection).toBe('DUPLICATE_INVOCATION_ID'); + expect(poisoned?.state).toBe(prior); + }); + + it('leaves the realm clean after exercising the inline descriptors', () => { + expect(Object.getOwnPropertyDescriptor(Object.prototype, 'get')).toBeUndefined(); + expect(Object.getOwnPropertyDescriptor(Object.prototype, 'set')).toBeUndefined(); + }); +});