diff --git a/.changeset/onepassword-service-account-token-lifetime.md b/.changeset/onepassword-service-account-token-lifetime.md new file mode 100644 index 0000000000..f389a856ed --- /dev/null +++ b/.changeset/onepassword-service-account-token-lifetime.md @@ -0,0 +1,11 @@ +--- +"executor": patch +--- + +**The 1Password service-account token is cleared from the op-js global after each call** + +`@1password/op-js` keeps the service-account token on a module-level CLI instance (`cli.serviceAccountToken`) and reads it when it spawns `op`. The CLI backend set that global before each call and never cleared it, so one reachable reference to the token stayed live for the rest of the process. + +It is now cleared as soon as the call that needed it is done, on success, failure and interruption alike. Authentication is unaffected: every read and write of that global already happens inside the backend's semaphore, so the next operation re-sets the token before it spawns anything. + +This is hygiene rather than a boundary change. No unrelated `op` child ever received a stale token — every call routes through the same critical section that sets the correct one immediately before invoking — and the token is separately persisted in plaintext in the plugin's config blob, so an attacker's reach is unchanged. What it removes is a long-lived reachable reference that nothing needed to keep. diff --git a/packages/plugins/onepassword/src/sdk/service.test.ts b/packages/plugins/onepassword/src/sdk/service.test.ts index 3ee24dc8c6..6ae90199e0 100644 --- a/packages/plugins/onepassword/src/sdk/service.test.ts +++ b/packages/plugins/onepassword/src/sdk/service.test.ts @@ -132,4 +132,67 @@ describe("makeOnePasswordService", () => { // oxlint-enable executor/no-unknown-error-message }), ); + + // ------------------------------------------------------------------------- + // Service-account token lifetime. + // + // `op-js` parks the token on a process-global (`cli.serviceAccountToken`) and + // reads it when spawning `op`. Nothing in the library clears it, so without + // the `ensuring` in `makeCliService` a token set to serve one resolve stays + // readable for the rest of the process's life. + // + // Both halves are pinned on purpose: clearing it is only correct if it is + // still SET while the call runs. A change that cleared it too early would + // pass a "no longer parked" assertion and silently break authentication. + // ------------------------------------------------------------------------- + + it.effect("clears the service-account token from the op-js global after a CLI call", () => + Effect.gen(function* () { + opMocks.readParse.mockReturnValue("resolved-secret"); + + const service = yield* makeOnePasswordService( + { kind: "service-account", token: "ops_test_token" }, + { timeoutMs: 1_000 }, + ); + const secret = yield* service.resolveSecret("op://vault/item/field"); + + expect(secret).toBe("resolved-secret"); + // Still handed to the CLI for the call that needed it... + expect(opMocks.setServiceAccount).toHaveBeenCalledWith("ops_test_token"); + // ...and gone by the time the call is over. + expect(opMocks.setServiceAccount).toHaveBeenLastCalledWith(""); + }), + ); + + it.effect("clears the token even when the CLI call fails", () => + Effect.gen(function* () { + // The failure path is the one that matters most: an error unwinding past a + // manual "clear it afterwards" line is exactly how a token gets stranded. + opMocks.readParse.mockImplementation(() => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: simulates the untyped op-js CLI wrapper throwing + throw new Error("spawn op ENOENT"); + }); + sdkMocks.createClient.mockResolvedValue({ + secrets: { + resolve: vi.fn(async () => { + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- boundary: simulates the untyped 1Password SDK rejecting + throw new Error("sdk unavailable"); + }), + }, + vaults: { list: vi.fn(async () => []) }, + items: { list: vi.fn(async () => []) }, + }); + + yield* makeOnePasswordService( + { kind: "service-account", token: "ops_test_token" }, + { timeoutMs: 1_000 }, + ).pipe( + Effect.flatMap((service) => service.resolveSecret("op://vault/item/field")), + Effect.flip, + ); + + expect(opMocks.setServiceAccount).toHaveBeenCalledWith("ops_test_token"); + expect(opMocks.setServiceAccount).toHaveBeenLastCalledWith(""); + }), + ); }); diff --git a/packages/plugins/onepassword/src/sdk/service.ts b/packages/plugins/onepassword/src/sdk/service.ts index 19ff6b4f8e..bf725c1b6c 100644 --- a/packages/plugins/onepassword/src/sdk/service.ts +++ b/packages/plugins/onepassword/src/sdk/service.ts @@ -203,7 +203,22 @@ export const makeCliService = ( operation, message: messageWithCause(`1Password CLI ${operation} failed`, cause), }), - }), + }).pipe( + // `op-js` keeps the service-account token in a PROCESS-GLOBAL + // (`cli.serviceAccountToken`, a field on the module's single CLI + // instance) and reads it when spawning `op`. Nothing in the library + // clears it, so a token set to serve one resolve stayed readable for + // the rest of the executor's life — long after the call that needed + // it, and with no reader. The account-name branch above happens to + // blank it, but only if a differently-authenticated call comes next, + // which in a service-account-only deployment never happens. + // + // So clear it as soon as the call is done: on success, on failure and + // on interruption alike. Safe because every write and every read of + // that global happens inside this same semaphore, so the next + // operation re-sets the token before it spawns anything. + Effect.ensuring(Effect.sync(() => op.setServiceAccount(""))), + ), ) .pipe(Effect.withSpan(`onepassword.cli.${operation}`));