From e7ac183d5230e373b82086383e28973dab33ebfe Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:48:34 +0200 Subject: [PATCH 1/3] Clear the 1Password service-account token once the call that needed it is done op-js keeps the service-account token on a module-level CLI instance and reads it when spawning `op`. The CLI backend set that global before each call and nothing cleared it, so a token handed over for one secret resolution stayed readable for the rest of the process's life, with nothing left to read it. The account-name branch blanks it, but only if a differently-authenticated call comes next -- which in a service-account-only deployment never happens. Clear it on success, failure and interruption alike. 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. --- ...password-service-account-token-lifetime.md | 9 +++ .../onepassword/src/sdk/service.test.ts | 63 +++++++++++++++++++ .../plugins/onepassword/src/sdk/service.ts | 17 ++++- 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 .changeset/onepassword-service-account-token-lifetime.md diff --git a/.changeset/onepassword-service-account-token-lifetime.md b/.changeset/onepassword-service-account-token-lifetime.md new file mode 100644 index 0000000000..0937158e16 --- /dev/null +++ b/.changeset/onepassword-service-account-token-lifetime.md @@ -0,0 +1,9 @@ +--- +"executor": patch +--- + +**The 1Password service-account token no longer stays parked in a process global** + +`@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 nothing ever cleared it, so a token handed over to serve one secret resolution stayed readable for the rest of the process's life — long after the call that needed it, and with nothing left to read it. The account-name branch happens to blank the global, but only if a differently-authenticated call comes next, which in a service-account-only deployment never happens. + +The token is now cleared as soon as the call that needed it is done, on success, failure and interruption alike. 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 and authentication is unaffected. 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}`)); From c2329e4a908cda22790a4fb1914875b22e887d58 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:56:43 +0200 Subject: [PATCH 2/3] Describe the token-clearing change as hygiene rather than a boundary change --- .changeset/onepassword-service-account-token-lifetime.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.changeset/onepassword-service-account-token-lifetime.md b/.changeset/onepassword-service-account-token-lifetime.md index 0937158e16..f389a856ed 100644 --- a/.changeset/onepassword-service-account-token-lifetime.md +++ b/.changeset/onepassword-service-account-token-lifetime.md @@ -2,8 +2,10 @@ "executor": patch --- -**The 1Password service-account token no longer stays parked in a process global** +**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 nothing ever cleared it, so a token handed over to serve one secret resolution stayed readable for the rest of the process's life — long after the call that needed it, and with nothing left to read it. The account-name branch happens to blank the global, but only if a differently-authenticated call comes next, which in a service-account-only deployment never happens. +`@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. -The token is now cleared as soon as the call that needed it is done, on success, failure and interruption alike. 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 and authentication is unaffected. +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. From 434d1f1be2b52ea587356d906632b6626239d3ef Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:13:03 +0200 Subject: [PATCH 3/3] test(onepassword): capture the token at spawn time so an early clear cannot pass The token-lifetime pair claimed to pin "still set while the call runs", but both assertions read the mock's call ledger after the fact. Hoisting the clear before the spawn leaves the sequence [token, "", ""], which keeps toHaveBeenCalledWith(token) and toHaveBeenLastCalledWith("") green while real service-account auth breaks: op-js builds the op child's env from the global at the moment it spawns, inside the call. Record the global's value from inside the spawned call and assert on that, so a clear that lands too early now fails the suite instead of slipping past it. Asserting in the test body (rather than expecting inside the mock) keeps the failure from being swallowed by Effect.try and rerouted through the SDK fallback. --- .../onepassword/src/sdk/service.test.ts | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/plugins/onepassword/src/sdk/service.test.ts b/packages/plugins/onepassword/src/sdk/service.test.ts index 6ae90199e0..13d4c5f97a 100644 --- a/packages/plugins/onepassword/src/sdk/service.test.ts +++ b/packages/plugins/onepassword/src/sdk/service.test.ts @@ -144,11 +144,21 @@ describe("makeOnePasswordService", () => { // 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. + // + // The "still set" half has to be observed from INSIDE the spawn: `op-js` + // builds the child's env from the global at spawn time, and the mock's call + // ledger cannot see the ordering between the token set and the spawn — a + // clear hoisted before `fn()` leaves [token, "", ""], which keeps both + // after-the-fact assertions green. // ------------------------------------------------------------------------- it.effect("clears the service-account token from the op-js global after a CLI call", () => Effect.gen(function* () { - opMocks.readParse.mockReturnValue("resolved-secret"); + let tokenAtSpawn: unknown; + opMocks.readParse.mockImplementation(() => { + tokenAtSpawn = opMocks.setServiceAccount.mock.lastCall?.[0]; + return "resolved-secret"; + }); const service = yield* makeOnePasswordService( { kind: "service-account", token: "ops_test_token" }, @@ -157,8 +167,8 @@ describe("makeOnePasswordService", () => { 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"); + // Still set while the spawn ran — clearing any earlier would break auth... + expect(tokenAtSpawn).toBe("ops_test_token"); // ...and gone by the time the call is over. expect(opMocks.setServiceAccount).toHaveBeenLastCalledWith(""); }), @@ -168,7 +178,9 @@ describe("makeOnePasswordService", () => { 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. + let tokenAtSpawn: unknown; opMocks.readParse.mockImplementation(() => { + tokenAtSpawn = opMocks.setServiceAccount.mock.lastCall?.[0]; // 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"); }); @@ -191,7 +203,7 @@ describe("makeOnePasswordService", () => { Effect.flip, ); - expect(opMocks.setServiceAccount).toHaveBeenCalledWith("ops_test_token"); + expect(tokenAtSpawn).toBe("ops_test_token"); expect(opMocks.setServiceAccount).toHaveBeenLastCalledWith(""); }), );