Skip to content

Commit ef4580b

Browse files
NathanFlurryclaude
andcommitted
feat: US-069 - Fix IPC binary frame length guards and fnv1aHash (TypeScript)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1626788 commit ef4580b

4 files changed

Lines changed: 222 additions & 15 deletions

File tree

packages/secure-exec-v8/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// V8 runtime process manager.
2-
export { createV8Runtime } from "./runtime.js";
2+
export { createV8Runtime, fnv1aHash } from "./runtime.js";
33
export type { V8Runtime, V8RuntimeOptions } from "./runtime.js";
44

55
// V8 session types.

packages/secure-exec-v8/src/ipc-binary.ts

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -222,14 +222,19 @@ export function decodeFrame(buf: Buffer): BinaryFrame {
222222
}
223223
let error: ExecutionErrorBin | null = null;
224224
if (flags & FLAG_HAS_ERROR) {
225-
const errorType = readLenPrefixedU16(buf, pos);
226-
pos += 2 + Buffer.byteLength(errorType, "utf8");
227-
const message = readLenPrefixedU16(buf, pos);
228-
pos += 2 + Buffer.byteLength(message, "utf8");
229-
const stack = readLenPrefixedU16(buf, pos);
230-
pos += 2 + Buffer.byteLength(stack, "utf8");
231-
const code = readLenPrefixedU16(buf, pos);
232-
error = { errorType, message, stack, code };
225+
const et = readLenPrefixedU16(buf, pos);
226+
pos += et.bytesRead;
227+
const msg = readLenPrefixedU16(buf, pos);
228+
pos += msg.bytesRead;
229+
const st = readLenPrefixedU16(buf, pos);
230+
pos += st.bytesRead;
231+
const cd = readLenPrefixedU16(buf, pos);
232+
error = {
233+
errorType: et.value,
234+
message: msg.value,
235+
stack: st.value,
236+
code: cd.value,
237+
};
233238
}
234239
return { type: "ExecutionResult", sessionId, exitCode, exports, error };
235240
}
@@ -422,6 +427,11 @@ function encodeBody(frame: BinaryFrame): Buffer {
422427

423428
function encodeSessionId(sid: string): Buffer {
424429
const bytes = Buffer.from(sid, "utf8");
430+
if (bytes.length > 255) {
431+
throw new Error(
432+
`Session ID byte length ${bytes.length} exceeds maximum 255`,
433+
);
434+
}
425435
const out = Buffer.alloc(1 + bytes.length);
426436
out[0] = bytes.length;
427437
bytes.copy(out, 1);
@@ -430,15 +440,24 @@ function encodeSessionId(sid: string): Buffer {
430440

431441
function writeLenPrefixedU16(s: string): Buffer {
432442
const bytes = Buffer.from(s, "utf8");
443+
if (bytes.length > 0xffff) {
444+
throw new Error(
445+
`String byte length ${bytes.length} exceeds maximum 65535`,
446+
);
447+
}
433448
const out = Buffer.alloc(2 + bytes.length);
434449
out.writeUInt16BE(bytes.length, 0);
435450
bytes.copy(out, 2);
436451
return out;
437452
}
438453

439-
function readLenPrefixedU16(buf: Buffer, pos: number): string {
454+
function readLenPrefixedU16(
455+
buf: Buffer,
456+
pos: number,
457+
): { value: string; bytesRead: number } {
440458
const len = buf.readUInt16BE(pos);
441-
return buf.toString("utf8", pos + 2, pos + 2 + len);
459+
const value = buf.toString("utf8", pos + 2, pos + 2 + len);
460+
return { value, bytesRead: 2 + len };
442461
}
443462

444463
// Re-export v8 serialize/deserialize for convenience

packages/secure-exec-v8/src/runtime.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -499,11 +499,12 @@ function readSocketPath(child: ChildProcess): Promise<string> {
499499
}
500500

501501
/** FNV-1a hash of a string, returning a 32-bit integer.
502-
* Matches the hash algorithm used on the Rust side for bridge code comparison. */
503-
function fnv1aHash(str: string): number {
502+
* Hashes over UTF-8 bytes to match the Rust side. */
503+
export function fnv1aHash(str: string): number {
504+
const bytes = Buffer.from(str, "utf8");
504505
let hash = 0x811c9dc5;
505-
for (let i = 0; i < str.length; i++) {
506-
hash ^= str.charCodeAt(i);
506+
for (let i = 0; i < bytes.length; i++) {
507+
hash ^= bytes[i];
507508
hash = Math.imul(hash, 0x01000193);
508509
}
509510
return hash >>> 0;

packages/secure-exec-v8/test/ipc-binary.test.ts

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
deserializePayload,
1717
type BinaryFrame,
1818
} from "../src/ipc-binary.js";
19+
import { fnv1aHash } from "../src/runtime.js";
1920

2021
function roundtrip(frame: BinaryFrame): void {
2122
const encoded = encodeFrame(frame);
@@ -751,3 +752,189 @@ describe("V8 serialize/deserialize payload integration", () => {
751752
}
752753
});
753754
});
755+
756+
// -- Overflow guards --
757+
758+
describe("overflow guards", () => {
759+
it("encodeSessionId throws on >255 byte session ID", () => {
760+
// 256 ASCII chars → 256 bytes UTF-8
761+
const longSid = "x".repeat(256);
762+
expect(() =>
763+
encodeFrame({
764+
type: "DestroySession",
765+
sessionId: longSid,
766+
}),
767+
).toThrow("Session ID byte length 256 exceeds maximum 255");
768+
});
769+
770+
it("encodeSessionId allows exactly 255 byte session ID", () => {
771+
const sid255 = "a".repeat(255);
772+
expect(() =>
773+
encodeFrame({ type: "DestroySession", sessionId: sid255 }),
774+
).not.toThrow();
775+
});
776+
777+
it("encodeSessionId counts UTF-8 bytes not characters", () => {
778+
// Each emoji is 4 bytes in UTF-8 — 64 emojis = 256 bytes → should throw
779+
const emojiSid = "\u{1F600}".repeat(64);
780+
expect(Buffer.byteLength(emojiSid, "utf8")).toBe(256);
781+
expect(() =>
782+
encodeFrame({ type: "DestroySession", sessionId: emojiSid }),
783+
).toThrow("exceeds maximum 255");
784+
});
785+
786+
it("writeLenPrefixedU16 throws on >65535 byte string", () => {
787+
const longStr = "x".repeat(65536);
788+
expect(() =>
789+
encodeFrame({
790+
type: "ExecutionResult",
791+
sessionId: "s",
792+
exitCode: 1,
793+
exports: null,
794+
error: {
795+
errorType: longStr,
796+
message: "",
797+
stack: "",
798+
code: "",
799+
},
800+
}),
801+
).toThrow("String byte length 65536 exceeds maximum 65535");
802+
});
803+
804+
it("writeLenPrefixedU16 allows exactly 65535 byte string", () => {
805+
const str65535 = "a".repeat(65535);
806+
expect(() =>
807+
encodeFrame({
808+
type: "ExecutionResult",
809+
sessionId: "s",
810+
exitCode: 1,
811+
exports: null,
812+
error: {
813+
errorType: str65535,
814+
message: "",
815+
stack: "",
816+
code: "",
817+
},
818+
}),
819+
).not.toThrow();
820+
});
821+
});
822+
823+
// -- readLenPrefixedU16 position advance --
824+
825+
describe("readLenPrefixedU16 position advance", () => {
826+
it("round-trips ExecutionResult with multi-byte UTF-8 error strings", () => {
827+
// Multi-byte UTF-8: each char is 3 bytes in UTF-8 but 1 char in JS
828+
const frame: BinaryFrame = {
829+
type: "ExecutionResult",
830+
sessionId: "s",
831+
exitCode: 1,
832+
exports: null,
833+
error: {
834+
errorType: "TypeError",
835+
message: "变量未定义",
836+
stack: "在文件第一行",
837+
code: "ERR_UNDEFINED",
838+
},
839+
};
840+
roundtrip(frame);
841+
});
842+
843+
it("round-trips ExecutionResult with emoji in error fields", () => {
844+
const frame: BinaryFrame = {
845+
type: "ExecutionResult",
846+
sessionId: "sess-emoji",
847+
exitCode: 1,
848+
exports: null,
849+
error: {
850+
errorType: "Error",
851+
message: "Failed \u{1F4A5} boom",
852+
stack: "at \u{1F4C4} file.js:1",
853+
code: "",
854+
},
855+
};
856+
roundtrip(frame);
857+
});
858+
859+
it("round-trips ExecutionResult with all error fields containing multi-byte chars", () => {
860+
const frame: BinaryFrame = {
861+
type: "ExecutionResult",
862+
sessionId: "t",
863+
exitCode: 1,
864+
exports: Buffer.from([0x01]),
865+
error: {
866+
errorType: "Ошибка",
867+
message: "не найдено: файл.txt",
868+
stack: "в строке 日本語テスト",
869+
code: "ENOENT_テスト",
870+
},
871+
};
872+
roundtrip(frame);
873+
});
874+
});
875+
876+
// -- fnv1aHash --
877+
878+
describe("fnv1aHash", () => {
879+
it("produces consistent hash for ASCII strings", () => {
880+
expect(fnv1aHash("hello")).toBe(fnv1aHash("hello"));
881+
expect(fnv1aHash("hello")).not.toBe(fnv1aHash("world"));
882+
});
883+
884+
it("produces same hash as Rust FNV-1a over UTF-8 bytes for ASCII", () => {
885+
// FNV-1a 32-bit of "hello" over UTF-8 bytes [0x68, 0x65, 0x6c, 0x6c, 0x6f]:
886+
// hash = 0x811c9dc5
887+
// hash ^= 0x68; hash *= 0x01000193 → ...
888+
// Expected: 0x4f9f2cab (computed from reference implementation)
889+
const bytes = Buffer.from("hello", "utf8");
890+
let expected = 0x811c9dc5;
891+
for (let i = 0; i < bytes.length; i++) {
892+
expected ^= bytes[i];
893+
expected = Math.imul(expected, 0x01000193);
894+
}
895+
expected = expected >>> 0;
896+
expect(fnv1aHash("hello")).toBe(expected);
897+
});
898+
899+
it("hashes over UTF-8 bytes for non-ASCII strings", () => {
900+
// "é" is 2 bytes in UTF-8 (0xc3 0xa9) but 1 code unit in UTF-16
901+
// If we hashed over UTF-16, we'd get a different result than UTF-8
902+
const bytes = Buffer.from("café", "utf8");
903+
let expected = 0x811c9dc5;
904+
for (let i = 0; i < bytes.length; i++) {
905+
expected ^= bytes[i];
906+
expected = Math.imul(expected, 0x01000193);
907+
}
908+
expected = expected >>> 0;
909+
expect(fnv1aHash("café")).toBe(expected);
910+
// Verify it's 5 bytes, not 4 code units
911+
expect(bytes.length).toBe(5);
912+
});
913+
914+
it("produces different hash for non-ASCII vs naive charCodeAt approach", () => {
915+
// Verify the fix matters: naive charCodeAt gives different result for non-ASCII
916+
const str = "日本語";
917+
const bytes = Buffer.from(str, "utf8");
918+
919+
// UTF-8 bytes hash (correct)
920+
let utf8Hash = 0x811c9dc5;
921+
for (let i = 0; i < bytes.length; i++) {
922+
utf8Hash ^= bytes[i];
923+
utf8Hash = Math.imul(utf8Hash, 0x01000193);
924+
}
925+
utf8Hash = utf8Hash >>> 0;
926+
927+
// UTF-16 code units hash (old buggy behavior)
928+
let utf16Hash = 0x811c9dc5;
929+
for (let i = 0; i < str.length; i++) {
930+
utf16Hash ^= str.charCodeAt(i);
931+
utf16Hash = Math.imul(utf16Hash, 0x01000193);
932+
}
933+
utf16Hash = utf16Hash >>> 0;
934+
935+
// They should differ for non-ASCII
936+
expect(utf8Hash).not.toBe(utf16Hash);
937+
// Our function should match the UTF-8 version
938+
expect(fnv1aHash(str)).toBe(utf8Hash);
939+
});
940+
});

0 commit comments

Comments
 (0)