mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:00:43 +00:00
Bound native hook permission fingerprints (#71758)
* fix: bound native hook permission fingerprints * fix: address native hook fingerprint review * test: isolate native jiti runtime assertions
This commit is contained in:
@@ -937,6 +937,130 @@ describe("native hook relay registry", () => {
|
||||
expect(responses.at(-1)).toEqual({ stdout: "", stderr: "", exitCode: 0 });
|
||||
});
|
||||
|
||||
it("deduplicates pending PermissionRequest approvals before consuming approval budget", async () => {
|
||||
const relay = registerNativeHookRelay({
|
||||
provider: "codex",
|
||||
sessionId: "session-1",
|
||||
runId: "run-1",
|
||||
});
|
||||
const resolvers: Array<(decision: "allow") => void> = [];
|
||||
const approvalRequester = vi.fn(
|
||||
() =>
|
||||
new Promise<"allow">((resolve) => {
|
||||
resolvers.push(resolve);
|
||||
}),
|
||||
);
|
||||
__testing.setNativeHookRelayPermissionApprovalRequesterForTests(approvalRequester);
|
||||
|
||||
const duplicatePayload = {
|
||||
hook_event_name: "PermissionRequest",
|
||||
tool_name: "Bash",
|
||||
tool_use_id: "native-call-1",
|
||||
tool_input: { command: "git push" },
|
||||
};
|
||||
const duplicateRequests = Array.from({ length: 12 }, () =>
|
||||
invokeNativeHookRelay({
|
||||
provider: "codex",
|
||||
relayId: relay.relayId,
|
||||
event: "permission_request",
|
||||
rawPayload: duplicatePayload,
|
||||
}),
|
||||
);
|
||||
await Promise.resolve();
|
||||
expect(approvalRequester).toHaveBeenCalledTimes(1);
|
||||
|
||||
const newRequest = invokeNativeHookRelay({
|
||||
provider: "codex",
|
||||
relayId: relay.relayId,
|
||||
event: "permission_request",
|
||||
rawPayload: {
|
||||
...duplicatePayload,
|
||||
tool_use_id: "native-call-2",
|
||||
tool_input: { command: "curl https://example.com" },
|
||||
},
|
||||
});
|
||||
await Promise.resolve();
|
||||
expect(approvalRequester).toHaveBeenCalledTimes(2);
|
||||
|
||||
for (const resolve of resolvers) {
|
||||
resolve("allow");
|
||||
}
|
||||
await expect(Promise.all([...duplicateRequests, newRequest])).resolves.toHaveLength(13);
|
||||
});
|
||||
|
||||
it("uses canonical PermissionRequest content fingerprints for ordinary objects", () => {
|
||||
const first = __testing.permissionRequestContentFingerprintForTests({
|
||||
provider: "codex",
|
||||
sessionId: "session-1",
|
||||
runId: "run-1",
|
||||
toolName: "exec",
|
||||
toolInput: { a: 1, b: { x: 2, y: 3 } },
|
||||
});
|
||||
const second = __testing.permissionRequestContentFingerprintForTests({
|
||||
provider: "codex",
|
||||
sessionId: "session-1",
|
||||
runId: "run-1",
|
||||
toolName: "exec",
|
||||
toolInput: { b: { y: 3, x: 2 }, a: 1 },
|
||||
});
|
||||
|
||||
expect(second).toBe(first);
|
||||
});
|
||||
|
||||
it("keeps broad PermissionRequest content fingerprints sensitive to tail changes", () => {
|
||||
const firstToolInput = Object.fromEntries(
|
||||
Array.from({ length: 205 }, (_, index) => [`key-${index}`, `value-${index}`]),
|
||||
);
|
||||
const secondToolInput = {
|
||||
...firstToolInput,
|
||||
"key-204": "changed",
|
||||
};
|
||||
|
||||
expect(
|
||||
__testing.permissionRequestContentFingerprintForTests({
|
||||
provider: "codex",
|
||||
sessionId: "session-1",
|
||||
runId: "run-1",
|
||||
toolName: "exec",
|
||||
toolInput: firstToolInput,
|
||||
}),
|
||||
).not.toBe(
|
||||
__testing.permissionRequestContentFingerprintForTests({
|
||||
provider: "codex",
|
||||
sessionId: "session-1",
|
||||
runId: "run-1",
|
||||
toolName: "exec",
|
||||
toolInput: secondToolInput,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("fingerprints broad PermissionRequest inputs without Object.keys enumeration", () => {
|
||||
const toolInput = Object.fromEntries(
|
||||
Array.from({ length: 300 }, (_, index) => [`key-${index}`, `value-${index}`]),
|
||||
);
|
||||
const objectKeys = vi.spyOn(Object, "keys").mockImplementation(() => {
|
||||
throw new Error("Object.keys should not be used for permission fingerprints");
|
||||
});
|
||||
|
||||
try {
|
||||
expect(__testing.permissionRequestToolInputKeyFingerprintForTests(toolInput)).toContain(
|
||||
"key-",
|
||||
);
|
||||
expect(
|
||||
__testing.permissionRequestContentFingerprintForTests({
|
||||
provider: "codex",
|
||||
sessionId: "session-1",
|
||||
runId: "run-1",
|
||||
toolName: "exec",
|
||||
toolInput,
|
||||
}),
|
||||
).toMatch(/^[a-f0-9]{64}$/);
|
||||
} finally {
|
||||
objectKeys.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("sanitizes PermissionRequest approval previews and reports omitted keys", () => {
|
||||
expect(
|
||||
__testing.formatPermissionApprovalDescriptionForTests({
|
||||
|
||||
@@ -122,6 +122,7 @@ const MAX_NATIVE_HOOK_RELAY_HISTORY_ARRAY_ITEMS = 50;
|
||||
const MAX_NATIVE_HOOK_RELAY_HISTORY_OBJECT_KEYS = 50;
|
||||
const MAX_PERMISSION_FALLBACK_KEYS = 200;
|
||||
const MAX_PERMISSION_FALLBACK_KEY_CHARS = 240;
|
||||
const MAX_PERMISSION_FINGERPRINT_SORT_KEYS = 200;
|
||||
const MAX_APPROVAL_TITLE_LENGTH = 80;
|
||||
const MAX_APPROVAL_DESCRIPTION_LENGTH = 700;
|
||||
const MAX_PERMISSION_APPROVALS_PER_WINDOW = 12;
|
||||
@@ -442,7 +443,7 @@ async function runNativeHookRelayPermissionRequest(params: {
|
||||
const pendingApproval = pendingPermissionApprovals.get(approvalKey);
|
||||
try {
|
||||
const decision = await (pendingApproval ??
|
||||
requestNativeHookRelayPermissionApprovalWithBudget({
|
||||
startNativeHookRelayPermissionApprovalWithBudget({
|
||||
registration: params.registration,
|
||||
approvalKey,
|
||||
request,
|
||||
@@ -463,7 +464,7 @@ async function runNativeHookRelayPermissionRequest(params: {
|
||||
return params.adapter.renderNoopResponse(params.invocation.event);
|
||||
}
|
||||
|
||||
async function requestNativeHookRelayPermissionApprovalWithBudget(params: {
|
||||
async function startNativeHookRelayPermissionApprovalWithBudget(params: {
|
||||
registration: NativeHookRelayRegistration;
|
||||
approvalKey: string;
|
||||
request: NativeHookRelayPermissionApprovalRequest;
|
||||
@@ -505,18 +506,18 @@ function permissionRequestFallbackKey(request: NativeHookRelayPermissionApproval
|
||||
|
||||
function permissionRequestToolInputKeyFingerprint(toolInput: Record<string, unknown>): string {
|
||||
let fingerprint = "";
|
||||
let processed = 0;
|
||||
for (const key of Object.keys(toolInput).toSorted()) {
|
||||
if (processed >= MAX_PERMISSION_FALLBACK_KEYS) {
|
||||
break;
|
||||
}
|
||||
const { keys, truncated } = readBoundedOwnKeys(toolInput, MAX_PERMISSION_FALLBACK_KEYS);
|
||||
for (const key of keys) {
|
||||
const separator = fingerprint ? "," : "";
|
||||
const remaining = MAX_PERMISSION_FALLBACK_KEY_CHARS - fingerprint.length - separator.length;
|
||||
if (remaining <= 0) {
|
||||
break;
|
||||
}
|
||||
fingerprint += `${separator}${key.slice(0, remaining)}`;
|
||||
processed += 1;
|
||||
}
|
||||
if (truncated && fingerprint.length < MAX_PERMISSION_FALLBACK_KEY_CHARS) {
|
||||
const marker = `${fingerprint ? "," : ""}...`;
|
||||
fingerprint += marker.slice(0, MAX_PERMISSION_FALLBACK_KEY_CHARS - fingerprint.length);
|
||||
}
|
||||
return fingerprint || "none";
|
||||
}
|
||||
@@ -559,15 +560,51 @@ function updateJsonHash(hash: ReturnType<typeof createHash>, value: JsonValue):
|
||||
return;
|
||||
}
|
||||
hash.update("{");
|
||||
for (const key of Object.keys(value).toSorted()) {
|
||||
const { keys, truncated } = readBoundedOwnKeys(value, MAX_PERMISSION_FINGERPRINT_SORT_KEYS);
|
||||
for (const key of keys) {
|
||||
hash.update(JSON.stringify(key));
|
||||
hash.update(":");
|
||||
updateJsonHash(hash, value[key]);
|
||||
hash.update(",");
|
||||
}
|
||||
if (truncated) {
|
||||
// Keep ordinary objects order-independent without sorting a broad native
|
||||
// hook payload. The tail remains content-sensitive in traversal order.
|
||||
const sortedKeySet = new Set(keys);
|
||||
hash.update("#object-tail:");
|
||||
for (const key in value) {
|
||||
if (!Object.prototype.hasOwnProperty.call(value, key) || sortedKeySet.has(key)) {
|
||||
continue;
|
||||
}
|
||||
hash.update(JSON.stringify(key));
|
||||
hash.update(":");
|
||||
updateJsonHash(hash, value[key]);
|
||||
hash.update(",");
|
||||
}
|
||||
}
|
||||
hash.update("}");
|
||||
}
|
||||
|
||||
function readBoundedOwnKeys(
|
||||
value: Record<string, unknown>,
|
||||
maxKeys: number,
|
||||
): { keys: string[]; truncated: boolean } {
|
||||
const keys: string[] = [];
|
||||
let truncated = false;
|
||||
for (const key in value) {
|
||||
if (!Object.prototype.hasOwnProperty.call(value, key)) {
|
||||
continue;
|
||||
}
|
||||
if (keys.length >= maxKeys) {
|
||||
truncated = true;
|
||||
break;
|
||||
}
|
||||
keys.push(key);
|
||||
}
|
||||
keys.sort();
|
||||
return { keys, truncated };
|
||||
}
|
||||
|
||||
function consumeNativeHookRelayPermissionBudget(relayId: string, now = Date.now()): boolean {
|
||||
const windowStart = now - PERMISSION_APPROVAL_WINDOW_MS;
|
||||
const timestamps = (permissionApprovalWindows.get(relayId) ?? []).filter(
|
||||
@@ -1032,6 +1069,14 @@ export const __testing = {
|
||||
): string {
|
||||
return formatPermissionApprovalDescription(request);
|
||||
},
|
||||
permissionRequestContentFingerprintForTests(
|
||||
request: NativeHookRelayPermissionApprovalRequest,
|
||||
): string {
|
||||
return permissionRequestContentFingerprint(request);
|
||||
},
|
||||
permissionRequestToolInputKeyFingerprintForTests(toolInput: Record<string, unknown>): string {
|
||||
return permissionRequestToolInputKeyFingerprint(toolInput);
|
||||
},
|
||||
setNativeHookRelayPermissionApprovalRequesterForTests(
|
||||
requester: NativeHookRelayPermissionApprovalRequester,
|
||||
): void {
|
||||
|
||||
@@ -19,6 +19,24 @@ const originalBundledPluginsDir = process.env.OPENCLAW_BUNDLED_PLUGINS_DIR;
|
||||
const FACADE_LOADER_GLOBAL = "__openclawTestLoadBundledPluginPublicSurfaceModuleSync";
|
||||
type FacadeLoaderJitiFactory = NonNullable<Parameters<typeof setFacadeLoaderJitiFactoryForTest>[0]>;
|
||||
|
||||
function forceNodeRuntimeVersionsForTest(): () => void {
|
||||
const originalVersions = process.versions;
|
||||
const nodeVersions = { ...originalVersions } as NodeJS.ProcessVersions & {
|
||||
bun?: string | undefined;
|
||||
};
|
||||
delete nodeVersions.bun;
|
||||
Object.defineProperty(process, "versions", {
|
||||
configurable: true,
|
||||
value: nodeVersions,
|
||||
});
|
||||
return () => {
|
||||
Object.defineProperty(process, "versions", {
|
||||
configurable: true,
|
||||
value: originalVersions,
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
function createBundledPluginDir(prefix: string, marker: string): string {
|
||||
return createBundledPluginPublicSurfaceFixture({ createTempDirSync, marker, prefix });
|
||||
}
|
||||
@@ -127,6 +145,7 @@ describe("plugin-sdk facade loader", () => {
|
||||
})) as unknown as ReturnType<FacadeLoaderJitiFactory>;
|
||||
}) as FacadeLoaderJitiFactory);
|
||||
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
const restoreVersions = forceNodeRuntimeVersionsForTest();
|
||||
|
||||
try {
|
||||
expect(
|
||||
@@ -143,6 +162,7 @@ describe("plugin-sdk facade loader", () => {
|
||||
}),
|
||||
);
|
||||
} finally {
|
||||
restoreVersions();
|
||||
platformSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -17,6 +17,24 @@ let resolvePluginSetupProvider: typeof import("./setup-registry.js").resolvePlug
|
||||
let resolvePluginSetupCliBackend: typeof import("./setup-registry.js").resolvePluginSetupCliBackend;
|
||||
let runPluginSetupConfigMigrations: typeof import("./setup-registry.js").runPluginSetupConfigMigrations;
|
||||
|
||||
function forceNodeRuntimeVersionsForTest(): () => void {
|
||||
const originalVersions = process.versions;
|
||||
const nodeVersions = { ...originalVersions } as NodeJS.ProcessVersions & {
|
||||
bun?: string | undefined;
|
||||
};
|
||||
delete nodeVersions.bun;
|
||||
Object.defineProperty(process, "versions", {
|
||||
configurable: true,
|
||||
value: nodeVersions,
|
||||
});
|
||||
return () => {
|
||||
Object.defineProperty(process, "versions", {
|
||||
configurable: true,
|
||||
value: originalVersions,
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
function makeTempDir(): string {
|
||||
return makeTrackedTempDir("openclaw-setup-registry", tempDirs);
|
||||
}
|
||||
@@ -166,6 +184,7 @@ describe("setup-registry getJiti", () => {
|
||||
diagnostics: [],
|
||||
});
|
||||
const platformSpy = vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
const restoreVersions = forceNodeRuntimeVersionsForTest();
|
||||
|
||||
try {
|
||||
resolvePluginSetupRegistry({
|
||||
@@ -173,6 +192,7 @@ describe("setup-registry getJiti", () => {
|
||||
env: {},
|
||||
});
|
||||
} finally {
|
||||
restoreVersions();
|
||||
platformSpy.mockRestore();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user