From 3d15bf513e79a9c0678f7c32054caf0a69b791af Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 14 Jul 2026 01:12:04 -0700 Subject: [PATCH] fix(onepassword): preserve pending tool authorization (#107275) * fix(onepassword): preserve pending tool authorization * refactor(onepassword): consume pending authorization in helper * chore: defer release note generation --- extensions/onepassword/src/broker.test.ts | 74 +++++++++++++++++++ extensions/onepassword/src/broker.ts | 12 +-- .../onepassword/src/pending-authorization.ts | 40 ++++++++++ 3 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 extensions/onepassword/src/pending-authorization.ts diff --git a/extensions/onepassword/src/broker.test.ts b/extensions/onepassword/src/broker.test.ts index d28362497366..3cac472134ff 100644 --- a/extensions/onepassword/src/broker.test.ts +++ b/extensions/onepassword/src/broker.test.ts @@ -265,6 +265,80 @@ describe("OnePasswordBroker validation and policy", () => { ]); }); + it("authorizes when hook and execute contexts disagree on session fields", async () => { + const { broker, audit } = setup(); + await broker.beforeToolCall( + { + toolName: "onepassword", + params: { action: "get", slug: "automatic", reason: "asymmetric contexts" }, + toolCallId: "call_x|fc_y", + }, + { + toolName: "onepassword", + toolCallId: "call_x|fc_y", + agentId: "main", + sessionKey: "agent:main:main", + sessionId: "hook-run-uuid", + }, + ); + + await expect( + broker.get( + "call_x|fc_y", + { action: "get", slug: "automatic", reason: "asymmetric contexts" }, + { agentId: "main", sessionKey: "agent:main:main" }, + ), + ).resolves.toMatchObject({ slug: "automatic" }); + expect((await audit.entries()).map((entry) => entry.value.outcome)).toEqual(["auto"]); + }); + + it("rejects an ambiguous fallback across sessions", async () => { + const { broker } = setup(); + for (const sessionKey of ["session-a", "session-b"]) { + await broker.beforeToolCall( + { + toolName: "onepassword", + params: { action: "get", slug: "automatic", reason: "same request" }, + toolCallId: "call-1", + }, + { toolName: "onepassword", toolCallId: "call-1", agentId: "agent-a", sessionKey }, + ); + } + + await expect( + broker.get( + "call-1", + { action: "get", slug: "automatic", reason: "same request" }, + { agentId: "agent-a", sessionKey: "session-c" }, + ), + ).rejects.toMatchObject({ code: "POLICY_NOT_EVALUATED" }); + }); + + it("rejects a unique fallback from another agent", async () => { + const { broker } = setup(); + await broker.beforeToolCall( + { + toolName: "onepassword", + params: { action: "get", slug: "automatic", reason: "same request" }, + toolCallId: "call-1", + }, + { + toolName: "onepassword", + toolCallId: "call-1", + agentId: "agent-a", + sessionKey: "session-a", + }, + ); + + await expect( + broker.get( + "call-1", + { action: "get", slug: "automatic", reason: "same request" }, + { agentId: "agent-b", sessionKey: "session-b" }, + ), + ).rejects.toMatchObject({ code: "POLICY_NOT_EVALUATED" }); + }); + it("persists allow-always grants and expires them", async () => { const { broker, audit, grants, getItem, advance } = setup(); const first = await before(broker, "grant-1", { diff --git a/extensions/onepassword/src/broker.ts b/extensions/onepassword/src/broker.ts index b10583e234df..53e3bf8a4372 100644 --- a/extensions/onepassword/src/broker.ts +++ b/extensions/onepassword/src/broker.ts @@ -13,6 +13,7 @@ import { } from "./config.js"; import { OnePasswordError, type OnePasswordErrorCode } from "./errors.js"; import type { OpClient, ResolvedSecret } from "./op-client.js"; +import { takePendingAuthorization } from "./pending-authorization.js"; type AuditOutcome = | "auto" @@ -233,12 +234,8 @@ export class OnePasswordBroker { private pendingKey( context: Pick, ): string { - return JSON.stringify([ - context.agentId, - context.sessionKey, - context.sessionId, - context.toolCallId, - ]); + const { agentId, sessionKey, sessionId, toolCallId } = context; + return JSON.stringify([agentId, sessionKey, sessionId, toolCallId]); } private async audit( @@ -458,8 +455,7 @@ export class OnePasswordBroker { reason: input.reason, }; const key = this.pendingKey(fallbackContext); - const authorization = this.pending.get(key); - this.pending.delete(key); + const authorization = takePendingAuthorization(this.pending, key, fallbackContext); if ( !authorization || authorization.slug !== input.slug || diff --git a/extensions/onepassword/src/pending-authorization.ts b/extensions/onepassword/src/pending-authorization.ts new file mode 100644 index 000000000000..b1ccd540f195 --- /dev/null +++ b/extensions/onepassword/src/pending-authorization.ts @@ -0,0 +1,40 @@ +type PendingRequest = { + agentId: string; + toolCallId: string; + slug: string; + reason: string; +}; + +export function takePendingAuthorization( + pending: Map, + exactKey: string, + request: PendingRequest, +): T | undefined { + const exact = pending.get(exactKey); + if (exact) { + pending.delete(exactKey); + return exact; + } + + let match: [string, T] | undefined; + for (const entry of pending) { + const candidate = entry[1]; + if ( + candidate.agentId !== request.agentId || + candidate.toolCallId !== request.toolCallId || + candidate.slug !== request.slug || + candidate.reason !== request.reason + ) { + continue; + } + if (match) { + return undefined; + } + match = entry; + } + if (!match) { + return undefined; + } + pending.delete(match[0]); + return match[1]; +}