From a702f9843ac9c91394bf227b280813a45b5e2b5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=A8=E5=98=89=E4=BC=9F?= <202383014@uibe.edu.cn> Date: Wed, 7 Oct 2026 23:25:19 +0800 Subject: [PATCH] fix: validate persisted turn change records --- ...URN_CHANGE_RECORD_VALIDATION_2026-10-07.md | 18 +++ tests/web/turn-change-decoder.test.ts | 110 ++++++++++++++++++ web/protocol/turn-changes.ts | 9 +- 3 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 docs/research/TURN_CHANGE_RECORD_VALIDATION_2026-10-07.md create mode 100644 tests/web/turn-change-decoder.test.ts diff --git a/docs/research/TURN_CHANGE_RECORD_VALIDATION_2026-10-07.md b/docs/research/TURN_CHANGE_RECORD_VALIDATION_2026-10-07.md new file mode 100644 index 00000000..8b4ad613 --- /dev/null +++ b/docs/research/TURN_CHANGE_RECORD_VALIDATION_2026-10-07.md @@ -0,0 +1,18 @@ +# Persisted turn change validation + +- Status: validated source and regression evidence +- Created / verified: 2026-10-07 +- Source: origin/main at 3cb2ecfe1bfbb98252651885311d309f83749428 +- Issue: [#698](https://github.com/openpi-dev/openpi/issues/698) +- Supersedes: none + +## Verified facts + +String coercion accepted array-valued persisted state and unavailable-reason enums. previousPath admitted empty or NUL-containing paths and escaped the total byte budget. Decoder tests reject these malformed records while preserving valid v1/v2 records. + +## Verification boundary + +Four pure decoder regressions pass after failing on the source baseline. No recovery mutation or installed UI acceptance is claimed. + +Full repository validation results and CI are recorded on the linked PR. Local +Windows failures are retained separately and are not reported as passing. diff --git a/tests/web/turn-change-decoder.test.ts b/tests/web/turn-change-decoder.test.ts new file mode 100644 index 00000000..4c9afbc7 --- /dev/null +++ b/tests/web/turn-change-decoder.test.ts @@ -0,0 +1,110 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { readTurnChangesDetail } from "../../web/protocol/turn-changes.ts"; + +function record() { + return { + version: 2, + source: "file-tools", + sessionId: "session", + promptEntryId: "prompt", + state: "complete", + fileCount: 1, + files: [ + { + path: "after.txt", + previousPath: "before.txt", + status: "renamed", + diff: "", + diffTruncated: false, + additions: 0, + deletions: 0, + }, + ], + additions: 0, + deletions: 0, + }; +} + +test("persisted turn-change enums require actual strings", () => { + for (const state of [ + ["complete"], + ["partial"], + ["unavailable"], + null, + {}, + 1, + ]) { + assert.equal(readTurnChangesDetail({ ...record(), state }), undefined); + } + for (const statsUnavailable of [ + ["content_limit"], + ["before_unavailable"], + ["concurrent_change"], + null, + {}, + 1, + ]) { + const value = record(); + assert.equal( + readTurnChangesDetail({ + ...value, + state: "partial", + files: [{ ...value.files[0], statsUnavailable }], + }), + undefined, + ); + } +}); + +test("persisted previous paths obey the same nonempty and NUL boundary as paths", () => { + for (const previousPath of ["", "before\0.txt", "x".repeat(2001)]) { + const value = record(); + assert.equal( + readTurnChangesDetail({ + ...value, + files: [{ ...value.files[0], previousPath }], + }), + undefined, + ); + } +}); + +test("retained previous-path UTF-8 bytes count toward the record budget", () => { + const value = record(); + const files = Array.from({ length: 50 }, (_, index) => ({ + ...value.files[0], + path: `${index}.txt`, + previousPath: "δΈ­".repeat(1999), + })); + assert.equal( + readTurnChangesDetail({ + ...value, + state: "partial", + fileCount: files.length, + files, + }), + undefined, + ); +}); + +test("valid legacy and native turn-change records retain their fields", () => { + const value = record(); + assert.ok(readTurnChangesDetail(value)); + assert.ok(readTurnChangesDetail({ ...value, version: 1 })); + for (const state of ["partial", "unavailable"]) { + for (const statsUnavailable of [ + "before_unavailable", + "content_limit", + "concurrent_change", + ]) { + const result = readTurnChangesDetail({ + ...value, + state, + files: [{ ...value.files[0], statsUnavailable }], + }); + assert.equal(result?.state, state); + assert.equal(result?.files[0]?.statsUnavailable, statsUnavailable); + } + } +}); diff --git a/web/protocol/turn-changes.ts b/web/protocol/turn-changes.ts index 2c028022..904656d0 100644 --- a/web/protocol/turn-changes.ts +++ b/web/protocol/turn-changes.ts @@ -51,7 +51,7 @@ export function readTurnChangesDetail(value: unknown): WebTurnChangesDetail | un (record.version === 2 && record.source !== "file-tools") || !boundedId(record.sessionId) || !boundedId(record.promptEntryId) || - !["complete", "partial", "unavailable"].includes(String(record.state)) || + typeof record.state !== "string" || !["complete", "partial", "unavailable"].includes(record.state) || !count(record.additions) || !count(record.deletions) || !Array.isArray(record.files) || record.files.length > WEB_MAX_TURN_CHANGE_FILES || !(record.fileCount === null || count(record.fileCount)) @@ -64,15 +64,16 @@ export function readTurnChangesDetail(value: unknown): WebTurnChangesDetail | un const file = item as Record; if ( typeof file.path !== "string" || !file.path || file.path.length > 2_000 || file.path.includes("\0") || - !(file.previousPath === undefined || (typeof file.previousPath === "string" && file.previousPath.length <= 2_000)) || + !(file.previousPath === undefined || (typeof file.previousPath === "string" && file.previousPath.length > 0 && file.previousPath.length <= 2_000 && !file.previousPath.includes("\0"))) || !fileStatuses.has(file.status as WebGitReviewFileStatus) || typeof file.diff !== "string" || typeof file.diffTruncated !== "boolean" || !(file.binary === undefined || typeof file.binary === "boolean") || - !(file.statsUnavailable === undefined || ["before_unavailable", "content_limit", "concurrent_change"].includes(String(file.statsUnavailable))) || + !(file.statsUnavailable === undefined || (typeof file.statsUnavailable === "string" && ["before_unavailable", "content_limit", "concurrent_change"].includes(file.statsUnavailable))) || !count(file.additions) || !count(file.deletions) ) return undefined; - bytes += new TextEncoder().encode(file.path).byteLength + new TextEncoder().encode(file.diff).byteLength; + bytes += new TextEncoder().encode(file.path).byteLength + new TextEncoder().encode(file.diff).byteLength + + (typeof file.previousPath === "string" ? new TextEncoder().encode(file.previousPath).byteLength : 0); if (bytes > WEB_MAX_TURN_CHANGE_RECORD_BYTES) return undefined; files.push({ path: file.path,