Skip to content

Commit bf7d156

Browse files
pashpashpashpashpashpash
authored
Bound native hook permission fingerprints (#71758)
* fix: bound native hook permission fingerprints * fix: address native hook fingerprint review * test: isolate native jiti runtime assertions
1 parent 4a72e1b commit bf7d156

4 files changed

Lines changed: 218 additions & 9 deletions

File tree

‎src/agents/harness/native-hook-relay.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,130 @@ describe("native hook relay registry", () => {
937937
expect(responses.at(-1)).toEqual({ stdout: "", stderr: "", exitCode: 0 });
938938
});
939939

940+
it("deduplicates pending PermissionRequest approvals before consuming approval budget", async () => {
941+
const relay = registerNativeHookRelay({
942+
provider: "codex",
943+
sessionId: "session-1",
944+
runId: "run-1",
945+
});
946+
const resolvers: Array<(decision: "allow") => void> = [];
947+
const approvalRequester = vi.fn(
948+
() =>
949+
new Promise<"allow">((resolve) => {
950+
resolvers.push(resolve);
951+
}),
952+
);
953+
__testing.setNativeHookRelayPermissionApprovalRequesterForTests(approvalRequester);
954+
955+
const duplicatePayload = {
956+
hook_event_name: "PermissionRequest",
957+
tool_name: "Bash",
958+
tool_use_id: "native-call-1",
959+
tool_input: { command: "git push" },
960+
};
961+
const duplicateRequests = Array.from({ length: 12 }, () =>
962+
invokeNativeHookRelay({
963+
provider: "codex",
964+
relayId: relay.relayId,
965+
event: "permission_request",
966+
rawPayload: duplicatePayload,
967+
}),
968+
);
969+
await Promise.resolve();
970+
expect(approvalRequester).toHaveBeenCalledTimes(1);
971+
972+
const newRequest = invokeNativeHookRelay({
973+
provider: "codex",
974+
relayId: relay.relayId,
975+
event: "permission_request",
976+
rawPayload: {
977+
...duplicatePayload,
978+
tool_use_id: "native-call-2",
979+
tool_input: { command: "curl https://example.com" },
980+
},
981+
});
982+
await Promise.resolve();
983+
expect(approvalRequester).toHaveBeenCalledTimes(2);
984+
985+
for (const resolve of resolvers) {
986+
resolve("allow");
987+
}
988+
await expect(Promise.all([...duplicateRequests, newRequest])).resolves.toHaveLength(13);
989+
});
990+
991+
it("uses canonical PermissionRequest content fingerprints for ordinary objects", () => {
992+
const first = __testing.permissionRequestContentFingerprintForTests({
993+
provider: "codex",
994+
sessionId: "session-1",
995+
runId: "run-1",
996+
toolName: "exec",
997+
toolInput: { a: 1, b: { x: 2, y: 3 } },
998+
});
999+
const second = __testing.permissionRequestContentFingerprintForTests({
1000+
provider: "codex",
1001+
sessionId: "session-1",
1002+
runId: "run-1",
1003+
toolName: "exec",
1004+
toolInput: { b: { y: 3, x: 2 }, a: 1 },
1005+
});
1006+
1007+
expect(second).toBe(first);
1008+
});
1009+
1010+
it("keeps broad PermissionRequest content fingerprints sensitive to tail changes", () => {
1011+
const firstToolInput = Object.fromEntries(
1012+
Array.from({ length: 205 }, (_, index) => [`key-${index}`, `value-${index}`]),
1013+
);
1014+
const secondToolInput = {
1015+
...firstToolInput,
1016+
"key-204": "changed",
1017+
};
1018+
1019+
expect(
1020+
__testing.permissionRequestContentFingerprintForTests({
1021+
provider: "codex",
1022+
sessionId: "session-1",
1023+
runId: "run-1",
1024+
toolName: "exec",
1025+
toolInput: firstToolInput,
1026+
}),
1027+
).not.toBe(
1028+
__testing.permissionRequestContentFingerprintForTests({
1029+
provider: "codex",
1030+
sessionId: "session-1",
1031+
runId: "run-1",
1032+
toolName: "exec",
1033+
toolInput: secondToolInput,
1034+
}),
1035+
);
1036+
});
1037+
1038+
it("fingerprints broad PermissionRequest inputs without Object.keys enumeration", () => {
1039+
const toolInput = Object.fromEntries(
1040+
Array.from({ length: 300 }, (_, index) => [`key-${index}`, `value-${index}`]),
1041+
);
1042+
const objectKeys = vi.spyOn(Object, "keys").mockImplementation(() => {
1043+
throw new Error("Object.keys should not be used for permission fingerprints");
1044+
});
1045+
1046+
try {
1047+
expect(__testing.permissionRequestToolInputKeyFingerprintForTests(toolInput)).toContain(
1048+
"key-",
1049+
);
1050+
expect(
1051+
__testing.permissionRequestContentFingerprintForTests({
1052+
provider: "codex",
1053+
sessionId: "session-1",
1054+
runId: "run-1",
1055+
toolName: "exec",
1056+
toolInput,
1057+
}),
1058+
).toMatch(/^[a-f0-9]{64}$/);
1059+
} finally {
1060+
objectKeys.mockRestore();
1061+
}
1062+
});
1063+
9401064
it("sanitizes PermissionRequest approval previews and reports omitted keys", () => {
9411065
expect(
9421066
__testing.formatPermissionApprovalDescriptionForTests({

‎src/agents/harness/native-hook-relay.ts‎

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ const MAX_NATIVE_HOOK_RELAY_HISTORY_ARRAY_ITEMS = 50;
122122
const MAX_NATIVE_HOOK_RELAY_HISTORY_OBJECT_KEYS = 50;
123123
const MAX_PERMISSION_FALLBACK_KEYS = 200;
124124
const MAX_PERMISSION_FALLBACK_KEY_CHARS = 240;
125+
const MAX_PERMISSION_FINGERPRINT_SORT_KEYS = 200;
125126
const MAX_APPROVAL_TITLE_LENGTH = 80;
126127
const MAX_APPROVAL_DESCRIPTION_LENGTH = 700;
127128
const MAX_PERMISSION_APPROVALS_PER_WINDOW = 12;
@@ -442,7 +443,7 @@ async function runNativeHookRelayPermissionRequest(params: {
442443
const pendingApproval = pendingPermissionApprovals.get(approvalKey);
443444
try {
444445
const decision = await (pendingApproval ??
445-
requestNativeHookRelayPermissionApprovalWithBudget({
446+
startNativeHookRelayPermissionApprovalWithBudget({
446447
registration: params.registration,
447448
approvalKey,
448449
request,
@@ -463,7 +464,7 @@ async function runNativeHookRelayPermissionRequest(params: {
463464
return params.adapter.renderNoopResponse(params.invocation.event);
464465
}
465466

466-
async function requestNativeHookRelayPermissionApprovalWithBudget(params: {
467+
async function startNativeHookRelayPermissionApprovalWithBudget(params: {
467468
registration: NativeHookRelayRegistration;
468469
approvalKey: string;
469470
request: NativeHookRelayPermissionApprovalRequest;
@@ -505,18 +506,18 @@ function permissionRequestFallbackKey(request: NativeHookRelayPermissionApproval
505506

506507
function permissionRequestToolInputKeyFingerprint(toolInput: Record<string, unknown>): string {
507508
let fingerprint = "";
508-
let processed = 0;
509-
for (const key of Object.keys(toolInput).toSorted()) {
510-
if (processed >= MAX_PERMISSION_FALLBACK_KEYS) {
511-
break;
512-
}
509+
const { keys, truncated } = readBoundedOwnKeys(toolInput, MAX_PERMISSION_FALLBACK_KEYS);
510+
for (const key of keys) {
513511
const separator = fingerprint ? "," : "";
514512
const remaining = MAX_PERMISSION_FALLBACK_KEY_CHARS - fingerprint.length - separator.length;
515513
if (remaining <= 0) {
516514
break;
517515
}
518516
fingerprint += `${separator}${key.slice(0, remaining)}`;
519-
processed += 1;
517+
}
518+
if (truncated && fingerprint.length < MAX_PERMISSION_FALLBACK_KEY_CHARS) {
519+
const marker = `${fingerprint ? "," : ""}...`;
520+
fingerprint += marker.slice(0, MAX_PERMISSION_FALLBACK_KEY_CHARS - fingerprint.length);
520521
}
521522
return fingerprint || "none";
522523
}
@@ -559,15 +560,51 @@ function updateJsonHash(hash: ReturnType<typeof createHash>, value: JsonValue):
559560
return;
560561
}
561562
hash.update("{");
562-
for (const key of Object.keys(value).toSorted()) {
563+
const { keys, truncated } = readBoundedOwnKeys(value, MAX_PERMISSION_FINGERPRINT_SORT_KEYS);
564+
for (const key of keys) {
563565
hash.update(JSON.stringify(key));
564566
hash.update(":");
565567
updateJsonHash(hash, value[key]);
566568
hash.update(",");
567569
}
570+
if (truncated) {
571+
// Keep ordinary objects order-independent without sorting a broad native
572+
// hook payload. The tail remains content-sensitive in traversal order.
573+
const sortedKeySet = new Set(keys);
574+
hash.update("#object-tail:");
575+
for (const key in value) {
576+
if (!Object.prototype.hasOwnProperty.call(value, key) || sortedKeySet.has(key)) {
577+
continue;
578+
}
579+
hash.update(JSON.stringify(key));
580+
hash.update(":");
581+
updateJsonHash(hash, value[key]);
582+
hash.update(",");
583+
}
584+
}
568585
hash.update("}");
569586
}
570587

588+
function readBoundedOwnKeys(
589+
value: Record<string, unknown>,
590+
maxKeys: number,
591+
): { keys: string[]; truncated: boolean } {
592+
const keys: string[] = [];
593+
let truncated = false;
594+
for (const key in value) {
595+
if (!Object.prototype.hasOwnProperty.call(value, key)) {
596+
continue;
597+
}
598+
if (keys.length >= maxKeys) {
599+
truncated = true;
600+
break;
601+
}
602+
keys.push(key);
603+
}
604+
keys.sort();
605+
return { keys, truncated };
606+
}
607+
571608
function consumeNativeHookRelayPermissionBudget(relayId: string, now = Date.now()): boolean {
572609
const windowStart = now - PERMISSION_APPROVAL_WINDOW_MS;
573610
const timestamps = (permissionApprovalWindows.get(relayId) ?? []).filter(
@@ -1032,6 +1069,14 @@ export const __testing = {
10321069
): string {
10331070
return formatPermissionApprovalDescription(request);
10341071
},
1072+
permissionRequestContentFingerprintForTests(
1073+
request: NativeHookRelayPermissionApprovalRequest,
1074+
): string {
1075+
return permissionRequestContentFingerprint(request);
1076+
},
1077+
permissionRequestToolInputKeyFingerprintForTests(toolInput: Record<string, unknown>): string {
1078+
return permissionRequestToolInputKeyFingerprint(toolInput);
1079+
},
10351080
setNativeHookRelayPermissionApprovalRequesterForTests(
10361081
requester: NativeHookRelayPermissionApprovalRequester,
10371082
): void {

‎src/plugin-sdk/facade-loader.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,24 @@ const originalBundledPluginsDir = process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
1919
const FACADE_LOADER_GLOBAL = "__openclawTestLoadBundledPluginPublicSurfaceModuleSync";
2020
type FacadeLoaderJitiFactory = NonNullable<Parameters<typeof setFacadeLoaderJitiFactoryForTest>[0]>;
2121

22+
function forceNodeRuntimeVersionsForTest(): () => void {
23+
const originalVersions = process.versions;
24+
const nodeVersions = { ...originalVersions } as NodeJS.ProcessVersions & {
25+
bun?: string | undefined;
26+
};
27+
delete nodeVersions.bun;
28+
Object.defineProperty(process, "versions", {
29+
configurable: true,
30+
value: nodeVersions,
31+
});
32+
return () => {
33+
Object.defineProperty(process, "versions", {
34+
configurable: true,
35+
value: originalVersions,
36+
});
37+
};
38+
}
39+
2240
function createBundledPluginDir(prefix: string, marker: string): string {
2341
return createBundledPluginPublicSurfaceFixture({ createTempDirSync, marker, prefix });
2442
}
@@ -127,6 +145,7 @@ describe("plugin-sdk facade loader", () => {
127145
})) as unknown as ReturnType<FacadeLoaderJitiFactory>;
128146
}) as FacadeLoaderJitiFactory);
129147
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
148+
const restoreVersions = forceNodeRuntimeVersionsForTest();
130149

131150
try {
132151
expect(
@@ -143,6 +162,7 @@ describe("plugin-sdk facade loader", () => {
143162
}),
144163
);
145164
} finally {
165+
restoreVersions();
146166
platformSpy.mockRestore();
147167
}
148168
});

‎src/plugins/setup-registry.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,24 @@ let resolvePluginSetupProvider: typeof import("./setup-registry.js").resolvePlug
1717
let resolvePluginSetupCliBackend: typeof import("./setup-registry.js").resolvePluginSetupCliBackend;
1818
let runPluginSetupConfigMigrations: typeof import("./setup-registry.js").runPluginSetupConfigMigrations;
1919

20+
function forceNodeRuntimeVersionsForTest(): () => void {
21+
const originalVersions = process.versions;
22+
const nodeVersions = { ...originalVersions } as NodeJS.ProcessVersions & {
23+
bun?: string | undefined;
24+
};
25+
delete nodeVersions.bun;
26+
Object.defineProperty(process, "versions", {
27+
configurable: true,
28+
value: nodeVersions,
29+
});
30+
return () => {
31+
Object.defineProperty(process, "versions", {
32+
configurable: true,
33+
value: originalVersions,
34+
});
35+
};
36+
}
37+
2038
function makeTempDir(): string {
2139
return makeTrackedTempDir("openclaw-setup-registry", tempDirs);
2240
}
@@ -166,13 +184,15 @@ describe("setup-registry getJiti", () => {
166184
diagnostics: [],
167185
});
168186
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
187+
const restoreVersions = forceNodeRuntimeVersionsForTest();
169188

170189
try {
171190
resolvePluginSetupRegistry({
172191
workspaceDir: pluginRoot,
173192
env: {},
174193
});
175194
} finally {
195+
restoreVersions();
176196
platformSpy.mockRestore();
177197
}
178198

0 commit comments

Comments
 (0)