From 4a997a0402a4821414a80d1e0903c4ad2cba192f Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 13:48:19 -0500 Subject: [PATCH 01/19] Use SDK with service account --- package-lock.json | 16 +++++++++ package.json | 1 + src/index.ts | 11 ++++-- src/utils.test.ts | 90 +++++++++++++++++++++++++++++++++++++++++++++++ src/utils.ts | 80 +++++++++++++++++++++++++++++++++++++++-- 5 files changed, 193 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 01950d9..7d61a9d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "license": "MIT", "dependencies": { "@1password/op-js": "^0.1.11", + "@1password/sdk": "^0.4.0", "@actions/core": "^1.10.1", "@actions/exec": "^1.1.1", "@actions/tool-cache": "^2.0.2", @@ -72,6 +73,21 @@ "prettier": "^2.0.0 || ^3.0.0" } }, + "node_modules/@1password/sdk": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/@1password/sdk/-/sdk-0.4.0.tgz", + "integrity": "sha512-RIypujc9R/UeUaobjyClTYokqRFpcaIkHq+EO/X9XoHId98Vg+SbjwGV+yygRC4MyHwYNo1KP1iEbZcqJ4ZTdw==", + "license": "MIT", + "dependencies": { + "@1password/sdk-core": "0.4.0" + } + }, + "node_modules/@1password/sdk-core": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/@1password/sdk-core/-/sdk-core-0.4.0.tgz", + "integrity": "sha512-vjeI1o4wiONY+t1naA4dtUp6HktdLH1D2S+tN1Lh4l41S9XIUHxrljov9B5u6G+VHr7f2MUoxmzXA9zT3aokQQ==", + "license": "MIT" + }, "node_modules/@actions/core": { "version": "1.11.1", "resolved": "https://registry.npmjs.org/@actions/core/-/core-1.11.1.tgz", diff --git a/package.json b/package.json index 6c75c15..b265942 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "homepage": "https://github.com/1Password/load-secrets-action#readme", "dependencies": { "@1password/op-js": "^0.1.11", + "@1password/sdk": "^0.4.0", "@actions/core": "^1.10.1", "@actions/exec": "^1.1.1", "@actions/tool-cache": "^2.0.2", diff --git a/src/index.ts b/src/index.ts index fcc2552..7cb7b76 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,7 +3,7 @@ import * as core from "@actions/core"; import { validateCli } from "@1password/op-js"; import { installCliOnGithubActionRunner } from "./op-cli-installer"; import { loadSecrets, unsetPrevious, validateAuth } from "./utils"; -import { envFilePath } from "./constants"; +import { envFilePath, envConnectHost, envConnectToken } from "./constants"; const loadSecretsAction = async () => { try { @@ -26,8 +26,13 @@ const loadSecretsAction = async () => { dotenv.config({ path: file }); } - // Download and install the CLI - await installCLI(); + + const isConnect = + process.env[envConnectHost] && process.env[envConnectToken]; + // If Connect is used, download and install the CLI + if (isConnect) { + await installCLI(); + } // Load secrets await loadSecrets(shouldExportEnv); diff --git a/src/utils.test.ts b/src/utils.test.ts index 0dd0ffe..99d3079 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,6 +1,7 @@ import * as core from "@actions/core"; import * as exec from "@actions/exec"; import { read, setClientInfo } from "@1password/op-js"; +import { createClient } from "@1password/sdk"; import { extractSecret, loadSecrets, @@ -22,6 +23,9 @@ jest.mock("@actions/exec", () => ({ })), })); jest.mock("@1password/op-js"); +jest.mock("@1password/sdk", () => ({ + createClient: jest.fn(), +})); beforeEach(() => { jest.clearAllMocks(); @@ -181,6 +185,92 @@ describe("loadSecrets", () => { }); }); +describe("loadSecrets when using Service Account", () => { + const mockResolve = jest.fn(); + + beforeEach(() => { + process.env[envConnectHost] = ""; + process.env[envConnectToken] = ""; + process.env[envServiceAccountToken] = "ops_token"; + + Object.keys(process.env).forEach((key) => { + if ( + typeof process.env[key] === "string" && + process.env[key]?.startsWith("op://") + ) { + delete process.env[key]; + } + }); + process.env.MY_SECRET = "op://vault/item/field"; + + (createClient as jest.Mock).mockResolvedValue({ + secrets: { resolve: mockResolve }, + }); + + mockResolve.mockResolvedValue("resolved-secret-value"); + }); + + + it("does not call op env ls when using Service Account", async () => { + await loadSecrets(false); + expect(exec.getExecOutput).not.toHaveBeenCalled(); + }); + + it("sets step output with resolved value when export-env is false", async () => { + await loadSecrets(false); + expect(core.setOutput).toHaveBeenCalledTimes(1); + expect(core.setOutput).toHaveBeenCalledWith("MY_SECRET", "resolved-secret-value"); + }); + + it("masks secret with setSecret when export-env is false", async () => { + await loadSecrets(false); + expect(core.setSecret).toHaveBeenCalledTimes(1); + expect(core.setSecret).toHaveBeenCalledWith("resolved-secret-value"); + }); + + it("does not call exportVariable when export-env is false", async () => { + await loadSecrets(false); + expect(core.exportVariable).not.toHaveBeenCalled(); + }); + + it("exports env and sets OP_MANAGED_VARIABLES when export-env is true", async () => { + await loadSecrets(true); + expect(core.exportVariable).toHaveBeenCalledWith( + "MY_SECRET", + "resolved-secret-value", + ); + expect(core.exportVariable).toHaveBeenCalledWith( + envManagedVariables, + "MY_SECRET", + ); + }); + + it("does not set step output when export-env is true", async () => { + await loadSecrets(true); + expect(core.setOutput).not.toHaveBeenCalledWith("MY_SECRET", expect.anything()); + }); + + it("masks secret with setSecret when export-env is true", async () => { + await loadSecrets(true); + expect(core.setSecret).toHaveBeenCalledTimes(1); + expect(core.setSecret).toHaveBeenCalledWith("resolved-secret-value"); + }); + + it("returns early when no env vars have op:// refs", async () => { + Object.keys(process.env).forEach((key) => { + if ( + typeof process.env[key] === "string" && + process.env[key]?.startsWith("op://") + ) { + delete process.env[key]; + } + }); + await loadSecrets(true); + expect(exec.getExecOutput).not.toHaveBeenCalled(); + expect(core.exportVariable).not.toHaveBeenCalled(); + }); +}); + describe("unsetPrevious", () => { const testManagedEnv = "TEST_SECRET"; const testSecretValue = "MyS3cr#T"; diff --git a/src/utils.ts b/src/utils.ts index 571620d..7962a35 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,6 +1,7 @@ import * as core from "@actions/core"; import * as exec from "@actions/exec"; import { read, setClientInfo, semverToInt } from "@1password/op-js"; +import { createClient } from "@1password/sdk"; import { version } from "../package.json"; import { authErr, @@ -29,6 +30,30 @@ export const validateAuth = (): void => { core.info(`Authenticated with ${authType}.`); }; +export const getEnvVarNamesWithSecretRefs = (): string[] => + Object.keys(process.env).filter( + (key) => + typeof process.env[key] === "string" && + process.env[key]?.startsWith("op://"), + ); + +const setResolvedSecret = ( + envName: string, + secretValue: string, + shouldExportEnv: boolean, +): void => { + core.info(`Populating variable: ${envName}`); + + if (shouldExportEnv) { + core.exportVariable(envName, secretValue); + } else { + core.setOutput(envName, secretValue); + } + if (secretValue) { + core.setSecret(secretValue); + } +}; + export const extractSecret = ( envName: string, shouldExportEnv: boolean, @@ -57,8 +82,10 @@ export const extractSecret = ( } }; -export const loadSecrets = async (shouldExportEnv: boolean): Promise => { - // Pass User-Agent Information to the 1Password CLI +// Connect loads secrets via the 1Password CLI +const loadSecretsViaConnect = async ( + shouldExportEnv: boolean, +): Promise => { setClientInfo({ name: "1Password GitHub Action", id: "GHA", @@ -83,6 +110,55 @@ export const loadSecrets = async (shouldExportEnv: boolean): Promise => { } }; +// Service Account loads secrets via the 1Password SDK +const loadSecretsViaServiceAccount = async ( + shouldExportEnv: boolean, +): Promise => { + const envs = getEnvVarNamesWithSecretRefs(); + if (envs.length === 0) { + return; + } + + const token = process.env[envServiceAccountToken]; + if (!token) { + throw new Error(authErr); + } + + const client = await createClient({ + auth: token, + integrationName: "1Password GitHub Action", + integrationVersion: version, + }); + + for (const envName of envs) { + const ref = process.env[envName]; + if (!ref) { + continue; + } + + // Resolve the secret value using the 1Password SDK + // and make it available either as step outputs or as environment variables + const secretValue = await client.secrets.resolve(ref); + setResolvedSecret(envName, secretValue, shouldExportEnv); + } + + if (shouldExportEnv) { + core.exportVariable(envManagedVariables, envs.join()); + } +}; + +export const loadSecrets = async (shouldExportEnv: boolean): Promise => { + const isConnect = + process.env[envConnectHost] && process.env[envConnectToken]; + + if (isConnect) { + await loadSecretsViaConnect(shouldExportEnv); + return; + } + + await loadSecretsViaServiceAccount(shouldExportEnv); +}; + export const unsetPrevious = (): void => { if (process.env[envManagedVariables]) { core.info("Unsetting previous values ..."); From 95478552e85b0b14498da22da2ec34e8957a507f Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 13:57:08 -0500 Subject: [PATCH 02/19] Fix linting issues --- src/index.ts | 1 - src/utils.test.ts | 11 ++++++++--- src/utils.ts | 3 +-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/index.ts b/src/index.ts index 7cb7b76..2cf0995 100644 --- a/src/index.ts +++ b/src/index.ts @@ -26,7 +26,6 @@ const loadSecretsAction = async () => { dotenv.config({ path: file }); } - const isConnect = process.env[envConnectHost] && process.env[envConnectToken]; // If Connect is used, download and install the CLI diff --git a/src/utils.test.ts b/src/utils.test.ts index 99d3079..a6a1f24 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -210,7 +210,6 @@ describe("loadSecrets when using Service Account", () => { mockResolve.mockResolvedValue("resolved-secret-value"); }); - it("does not call op env ls when using Service Account", async () => { await loadSecrets(false); expect(exec.getExecOutput).not.toHaveBeenCalled(); @@ -219,7 +218,10 @@ describe("loadSecrets when using Service Account", () => { it("sets step output with resolved value when export-env is false", async () => { await loadSecrets(false); expect(core.setOutput).toHaveBeenCalledTimes(1); - expect(core.setOutput).toHaveBeenCalledWith("MY_SECRET", "resolved-secret-value"); + expect(core.setOutput).toHaveBeenCalledWith( + "MY_SECRET", + "resolved-secret-value", + ); }); it("masks secret with setSecret when export-env is false", async () => { @@ -247,7 +249,10 @@ describe("loadSecrets when using Service Account", () => { it("does not set step output when export-env is true", async () => { await loadSecrets(true); - expect(core.setOutput).not.toHaveBeenCalledWith("MY_SECRET", expect.anything()); + expect(core.setOutput).not.toHaveBeenCalledWith( + "MY_SECRET", + expect.anything(), + ); }); it("masks secret with setSecret when export-env is true", async () => { diff --git a/src/utils.ts b/src/utils.ts index 7962a35..682fd37 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -148,8 +148,7 @@ const loadSecretsViaServiceAccount = async ( }; export const loadSecrets = async (shouldExportEnv: boolean): Promise => { - const isConnect = - process.env[envConnectHost] && process.env[envConnectToken]; + const isConnect = process.env[envConnectHost] && process.env[envConnectToken]; if (isConnect) { await loadSecretsViaConnect(shouldExportEnv); From d2fdd9df66ec115f638a1c52e3eee5a2f5a5b355 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 14:12:14 -0500 Subject: [PATCH 03/19] Update unit test --- src/utils.test.ts | 62 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index a6a1f24..7b066e5 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -147,7 +147,7 @@ describe("extractSecret", () => { }); }); -describe("loadSecrets", () => { +describe("loadSecrets when using Connect", () => { it("sets the client info and gets the executed output", async () => { await loadSecrets(true); @@ -274,6 +274,66 @@ describe("loadSecrets when using Service Account", () => { expect(exec.getExecOutput).not.toHaveBeenCalled(); expect(core.exportVariable).not.toHaveBeenCalled(); }); + + describe("multiple refs", () => { + const ref1 = "op://vault/item/field"; + const ref2 = "op://vault/other/item"; + const ref3 = "op://vault/file/secret"; + + beforeEach(() => { + process.env.MY_SECRET = ref1; + process.env.ANOTHER_SECRET = ref2; + process.env.FILE_SECRET = ref3; + + mockResolve + .mockResolvedValueOnce("value1") + .mockResolvedValueOnce("value2") + .mockResolvedValueOnce("value3"); + }); + + it("resolves each ref and sets step output for each when export-env is false", async () => { + await loadSecrets(false); + + expect(mockResolve).toHaveBeenCalledTimes(3); + expect(mockResolve).toHaveBeenCalledWith(ref1); + expect(mockResolve).toHaveBeenCalledWith(ref2); + expect(mockResolve).toHaveBeenCalledWith(ref3); + + expect(core.setOutput).toHaveBeenCalledTimes(3); + expect(core.setOutput).toHaveBeenCalledWith("MY_SECRET", "value1"); + expect(core.setOutput).toHaveBeenCalledWith("ANOTHER_SECRET", "value2"); + expect(core.setOutput).toHaveBeenCalledWith("FILE_SECRET", "value3"); + + expect(core.setSecret).toHaveBeenCalledTimes(3); + }); + + it("resolves each ref and exports each and sets OP_MANAGED_VARIABLES when export-env is true", async () => { + await loadSecrets(true); + + expect(mockResolve).toHaveBeenCalledTimes(3); + + expect(core.exportVariable).toHaveBeenCalledWith("MY_SECRET", "value1"); + expect(core.exportVariable).toHaveBeenCalledWith( + "ANOTHER_SECRET", + "value2", + ); + expect(core.exportVariable).toHaveBeenCalledWith("FILE_SECRET", "value3"); + + const exportVariableCalls = (core.exportVariable as jest.Mock).mock + .calls as [string, string][]; + const managedVarsCall = exportVariableCalls.find( + ([name]) => name === envManagedVariables, + ); + expect(managedVarsCall).toBeDefined(); + const managedList = (managedVarsCall as [string, string])[1].split(","); + expect(managedList).toContain("MY_SECRET"); + expect(managedList).toContain("ANOTHER_SECRET"); + expect(managedList).toContain("FILE_SECRET"); + expect(managedList).toHaveLength(3); + + expect(core.setSecret).toHaveBeenCalledTimes(3); + }); + }); }); describe("unsetPrevious", () => { From a2ce22dd394f4a28ae38221c55f5304761a12425 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 16:35:10 -0500 Subject: [PATCH 04/19] Add error handling --- src/utils.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/utils.ts b/src/utils.ts index 682fd37..7d54916 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -124,11 +124,18 @@ const loadSecretsViaServiceAccount = async ( throw new Error(authErr); } - const client = await createClient({ + // Authenticate with the 1Password SDK + let client; + try { + client = await createClient({ auth: token, integrationName: "1Password GitHub Action", integrationVersion: version, }); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw new Error(`Service account authentication failed: ${message}`); + } for (const envName of envs) { const ref = process.env[envName]; From 24235f3b6baf98e8eeab461097345ea7089f3492 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 16:37:41 -0500 Subject: [PATCH 05/19] Fix formatting --- src/utils.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 7d54916..383e103 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -128,10 +128,10 @@ const loadSecretsViaServiceAccount = async ( let client; try { client = await createClient({ - auth: token, - integrationName: "1Password GitHub Action", - integrationVersion: version, - }); + auth: token, + integrationName: "1Password GitHub Action", + integrationVersion: version, + }); } catch (err) { const message = err instanceof Error ? err.message : String(err); throw new Error(`Service account authentication failed: ${message}`); From 6911316fe31cc37f0c21d2f690e8ee8ad6928a23 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 17:24:55 -0500 Subject: [PATCH 06/19] Add secret ref validation --- src/utils.test.ts | 54 +++++++++++++++++++++++++++++++++++++++++++++-- src/utils.ts | 29 ++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index 7b066e5..d981470 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,7 +1,7 @@ import * as core from "@actions/core"; import * as exec from "@actions/exec"; import { read, setClientInfo } from "@1password/op-js"; -import { createClient } from "@1password/sdk"; +import { createClient, Secrets } from "@1password/sdk"; import { extractSecret, loadSecrets, @@ -25,6 +25,9 @@ jest.mock("@actions/exec", () => ({ jest.mock("@1password/op-js"); jest.mock("@1password/sdk", () => ({ createClient: jest.fn(), + Secrets: { + validateSecretReference: jest.fn(), + }, })); beforeEach(() => { @@ -170,11 +173,24 @@ describe("loadSecrets when using Connect", () => { expect(core.exportVariable).not.toHaveBeenCalled(); }); + it("fails with clear message when a secret reference is invalid", async () => { + (Secrets.validateSecretReference as jest.Mock).mockImplementationOnce( + () => { + throw new Error("invalid reference format"); + }, + ); + process.env.MOCK_SECRET = "op://bad/invalid-ref"; + + await expect(loadSecrets(true)).rejects.toThrow( + "Invalid secret reference(s): MOCK_SECRET", + ); + }); + describe("core.exportVariable", () => { it("is called when shouldExportEnv is true", async () => { await loadSecrets(true); - expect(core.exportVariable).toHaveBeenCalledTimes(1); + expect(core.exportVariable).toHaveBeenCalledTimes(2); }); it("is not called when shouldExportEnv is false", async () => { @@ -334,6 +350,40 @@ describe("loadSecrets when using Service Account", () => { expect(core.setSecret).toHaveBeenCalledTimes(3); }); }); + + describe("secret reference validation", () => { + it("fails with clear message when a secret reference is invalid", async () => { + process.env.MY_SECRET = "op://invalid/ref/form"; + (Secrets.validateSecretReference as jest.Mock).mockImplementationOnce( + () => { + throw new Error("invalid reference format"); + }, + ); + + await expect(loadSecrets(true)).rejects.toThrow( + "Invalid secret reference(s): MY_SECRET", + ); + expect(mockResolve).not.toHaveBeenCalled(); + }); + + it("validates all refs before resolving any secrets", async () => { + process.env.MY_SECRET = "op://vault/item/field"; + process.env.OTHER = "op://vault/other/item"; + (Secrets.validateSecretReference as jest.Mock).mockImplementation( + (ref: string) => { + if (ref === "op://vault/other/item") { + throw new Error("invalid"); + } + }, + ); + mockResolve.mockResolvedValue("value1"); + + await expect(loadSecrets(false)).rejects.toThrow( + "Invalid secret reference(s): OTHER", + ); + expect(mockResolve).not.toHaveBeenCalled(); + }); + }); }); describe("unsetPrevious", () => { diff --git a/src/utils.ts b/src/utils.ts index 383e103..9653956 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,7 +1,7 @@ import * as core from "@actions/core"; import * as exec from "@actions/exec"; import { read, setClientInfo, semverToInt } from "@1password/op-js"; -import { createClient } from "@1password/sdk"; +import { createClient, Secrets } from "@1password/sdk"; import { version } from "../package.json"; import { authErr, @@ -37,6 +37,29 @@ export const getEnvVarNamesWithSecretRefs = (): string[] => process.env[key]?.startsWith("op://"), ); +const validateSecretRefs = (envNames: string[]): void => { + const invalid: string[] = []; + + for (const envName of envNames) { + const ref = process.env[envName]; + if (!ref) { + continue; + } + + try { + Secrets.validateSecretReference(ref); + } catch { + invalid.push(envName); + } + } + + // Throw an error if any secret references are invalid + if (invalid.length > 0) { + const names = invalid.join(", "); + throw new Error(`Invalid secret reference(s): ${names}`); + } +}; + const setResolvedSecret = ( envName: string, secretValue: string, @@ -102,6 +125,8 @@ const loadSecretsViaConnect = async ( } const envs = res.stdout.replace(/\n+$/g, "").split(/\r?\n/); + validateSecretRefs(envs); + for (const envName of envs) { extractSecret(envName, shouldExportEnv); } @@ -119,6 +144,8 @@ const loadSecretsViaServiceAccount = async ( return; } + validateSecretRefs(envs); + const token = process.env[envServiceAccountToken]; if (!token) { throw new Error(authErr); From e7fe4397d919c139550ed539df72c7c9c45b2a6a Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 17:38:00 -0500 Subject: [PATCH 07/19] Add e2e test --- .github/workflows/e2e-tests.yml | 15 +++++++++++++++ tests/assert-invalid-ref-failed.sh | 7 +++++++ 2 files changed, 22 insertions(+) create mode 100644 tests/assert-invalid-ref-failed.sh diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 3105fb6..cd77b93 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -105,6 +105,21 @@ jobs: shell: bash run: ./tests/assert-env-unset.sh + - name: Load secrets (invalid ref - expect failure) + id: load_invalid + continue-on-error: true + uses: ./ + env: + BAD_REF: "op://x" + OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} + with: + export-env: true + + - name: Assert invalid ref failed + run: ./tests/assert-invalid-ref-failed.sh + env: + STEP_OUTCOME: ${{ steps.load_invalid.outcome }} + test-connect: name: Connect (ubuntu-latest, ${{ matrix.version }}, export-env=${{ matrix.export-env }}) runs-on: ubuntu-latest diff --git a/tests/assert-invalid-ref-failed.sh b/tests/assert-invalid-ref-failed.sh new file mode 100644 index 0000000..3d07973 --- /dev/null +++ b/tests/assert-invalid-ref-failed.sh @@ -0,0 +1,7 @@ +#!/bin/bash +set -e +if [ "$STEP_OUTCOME" != "failure" ]; then + echo "Expected action to fail on invalid ref, got: $STEP_OUTCOME" + exit 1 +fi +echo "Action correctly failed on invalid ref" From 7998453500147ece20e760e0ffebf16dfa8adade Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 17:41:39 -0500 Subject: [PATCH 08/19] Update script --- .github/workflows/e2e-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index cd77b93..958b75a 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -116,6 +116,7 @@ jobs: export-env: true - name: Assert invalid ref failed + shell: bash run: ./tests/assert-invalid-ref-failed.sh env: STEP_OUTCOME: ${{ steps.load_invalid.outcome }} From 604a86ce4e79b0e501658d43dbda0ce70227f53a Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 17:44:26 -0500 Subject: [PATCH 09/19] Make assert-invalid-ref-failed.sh executable --- tests/assert-invalid-ref-failed.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tests/assert-invalid-ref-failed.sh diff --git a/tests/assert-invalid-ref-failed.sh b/tests/assert-invalid-ref-failed.sh old mode 100644 new mode 100755 From 2a828228a826030c69cd594d989e158999eafbc5 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 17:48:21 -0500 Subject: [PATCH 10/19] Try woth test failure --- .github/workflows/e2e-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 958b75a..ef47b56 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -110,7 +110,7 @@ jobs: continue-on-error: true uses: ./ env: - BAD_REF: "op://x" + BAD_REF: "op://${{ secrets.VAULT }}/test-secret/password" OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} with: export-env: true From d456b725135925f84d1826ca34e658e0c1315150 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 17:50:34 -0500 Subject: [PATCH 11/19] Add connect e2e test --- .github/workflows/e2e-tests.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index ef47b56..fa18f8b 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -110,7 +110,7 @@ jobs: continue-on-error: true uses: ./ env: - BAD_REF: "op://${{ secrets.VAULT }}/test-secret/password" + BAD_REF: "op://x" OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} with: export-env: true @@ -205,3 +205,18 @@ jobs: - name: Assert removed secrets [exported env] if: ${{ matrix.export-env }} run: ./tests/assert-env-unset.sh + + - name: Load secrets (invalid ref - expect failure) + id: load_invalid + continue-on-error: true + uses: ./ + env: + BAD_REF: "op://x" + with: + export-env: true + + - name: Assert invalid ref failed + shell: bash + run: ./tests/assert-invalid-ref-failed.sh + env: + STEP_OUTCOME: ${{ steps.load_invalid.outcome }} From af49dd18deafd85fcc3773c65ad880bb17977ea4 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 18:00:42 -0500 Subject: [PATCH 12/19] Remove connect handling --- .github/workflows/e2e-tests.yml | 15 --------------- src/utils.ts | 2 -- 2 files changed, 17 deletions(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index fa18f8b..958b75a 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -205,18 +205,3 @@ jobs: - name: Assert removed secrets [exported env] if: ${{ matrix.export-env }} run: ./tests/assert-env-unset.sh - - - name: Load secrets (invalid ref - expect failure) - id: load_invalid - continue-on-error: true - uses: ./ - env: - BAD_REF: "op://x" - with: - export-env: true - - - name: Assert invalid ref failed - shell: bash - run: ./tests/assert-invalid-ref-failed.sh - env: - STEP_OUTCOME: ${{ steps.load_invalid.outcome }} diff --git a/src/utils.ts b/src/utils.ts index 9653956..97922bb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -125,8 +125,6 @@ const loadSecretsViaConnect = async ( } const envs = res.stdout.replace(/\n+$/g, "").split(/\r?\n/); - validateSecretRefs(envs); - for (const envName of envs) { extractSecret(envName, shouldExportEnv); } From ab44f9f69c6e396597459da2a4c53a1871d9183c Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Wed, 18 Feb 2026 18:01:22 -0500 Subject: [PATCH 13/19] Remove unit test --- src/utils.test.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index d981470..ca954a2 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -173,24 +173,11 @@ describe("loadSecrets when using Connect", () => { expect(core.exportVariable).not.toHaveBeenCalled(); }); - it("fails with clear message when a secret reference is invalid", async () => { - (Secrets.validateSecretReference as jest.Mock).mockImplementationOnce( - () => { - throw new Error("invalid reference format"); - }, - ); - process.env.MOCK_SECRET = "op://bad/invalid-ref"; - - await expect(loadSecrets(true)).rejects.toThrow( - "Invalid secret reference(s): MOCK_SECRET", - ); - }); - describe("core.exportVariable", () => { it("is called when shouldExportEnv is true", async () => { await loadSecrets(true); - expect(core.exportVariable).toHaveBeenCalledTimes(2); + expect(core.exportVariable).toHaveBeenCalledTimes(1); }); it("is not called when shouldExportEnv is false", async () => { From 015b03300e4a3a52bef22d67bac1d0a88e336ac9 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Thu, 19 Feb 2026 14:17:40 -0500 Subject: [PATCH 14/19] Code cleanup --- src/utils.test.ts | 15 +++++++++++++++ src/utils.ts | 16 +++------------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index 7b066e5..e44b4af 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -148,6 +148,12 @@ describe("extractSecret", () => { }); describe("loadSecrets when using Connect", () => { + beforeEach(() => { + process.env[envConnectHost] = "https://localhost:8000"; + process.env[envConnectToken] = "token"; + process.env[envServiceAccountToken] = ""; + }); + it("sets the client info and gets the executed output", async () => { await loadSecrets(true); @@ -275,6 +281,15 @@ describe("loadSecrets when using Service Account", () => { expect(core.exportVariable).not.toHaveBeenCalled(); }); + it("wraps createClient errors with a descriptive message", async () => { + (createClient as jest.Mock).mockRejectedValue( + new Error("invalid token format"), + ); + await expect(loadSecrets(false)).rejects.toThrow( + "Service account authentication failed: invalid token format", + ); + }); + describe("multiple refs", () => { const ref1 = "op://vault/item/field"; const ref2 = "op://vault/other/item"; diff --git a/src/utils.ts b/src/utils.ts index 383e103..77505e1 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -30,7 +30,7 @@ export const validateAuth = (): void => { core.info(`Authenticated with ${authType}.`); }; -export const getEnvVarNamesWithSecretRefs = (): string[] => +const getEnvVarNamesWithSecretRefs = (): string[] => Object.keys(process.env).filter( (key) => typeof process.env[key] === "string" && @@ -58,8 +58,6 @@ export const extractSecret = ( envName: string, shouldExportEnv: boolean, ): void => { - core.info(`Populating variable: ${envName}`); - const ref = process.env[envName]; if (!ref) { return; @@ -70,16 +68,8 @@ export const extractSecret = ( return; } - if (shouldExportEnv) { - core.exportVariable(envName, secretValue); - } else { - core.setOutput(envName, secretValue); - } - // Skip setSecret for empty strings to avoid the warning: - // "Can't add secret mask for empty string in ##[add-mask] command." - if (secretValue) { - core.setSecret(secretValue); - } + setResolvedSecret(envName, secretValue, shouldExportEnv); + }; // Connect loads secrets via the 1Password CLI From 1e8273d4beecd40215786aeda3f8026d9cc7131e Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Thu, 19 Feb 2026 14:24:51 -0500 Subject: [PATCH 15/19] Fix formatting --- src/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils.ts b/src/utils.ts index 77505e1..6b61d48 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -69,7 +69,6 @@ export const extractSecret = ( } setResolvedSecret(envName, secretValue, shouldExportEnv); - }; // Connect loads secrets via the 1Password CLI From 04984a6c91714492720f156812c0eb8416059cda Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Fri, 20 Feb 2026 08:31:14 -0500 Subject: [PATCH 16/19] Add eslint disable --- src/utils.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils.test.ts b/src/utils.test.ts index d21fa24..f2d9429 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -23,6 +23,7 @@ jest.mock("@actions/exec", () => ({ })), })); jest.mock("@1password/op-js"); +// eslint-disable-next-line @typescript-eslint/naming-convention jest.mock("@1password/sdk", () => ({ createClient: jest.fn(), Secrets: { From 9d7acefac986f92b643437d44d0a263b330b84cb Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Fri, 20 Feb 2026 08:34:52 -0500 Subject: [PATCH 17/19] Move comment --- src/utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index f2d9429..575c30f 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -23,9 +23,9 @@ jest.mock("@actions/exec", () => ({ })), })); jest.mock("@1password/op-js"); -// eslint-disable-next-line @typescript-eslint/naming-convention jest.mock("@1password/sdk", () => ({ createClient: jest.fn(), + // eslint-disable-next-line @typescript-eslint/naming-convention Secrets: { validateSecretReference: jest.fn(), }, From dc90451a94226792d67567dd0243df96debe84e1 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Sun, 22 Feb 2026 12:31:37 -0500 Subject: [PATCH 18/19] Apply code suggestions --- src/utils.test.ts | 3 +-- src/utils.ts | 11 ++++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index 575c30f..a74874f 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -356,7 +356,7 @@ describe("loadSecrets when using Service Account", () => { describe("secret reference validation", () => { it("fails with clear message when a secret reference is invalid", async () => { - process.env.MY_SECRET = "op://invalid/ref/form"; + process.env.MY_SECRET = "op://x"; (Secrets.validateSecretReference as jest.Mock).mockImplementationOnce( () => { throw new Error("invalid reference format"); @@ -379,7 +379,6 @@ describe("loadSecrets when using Service Account", () => { } }, ); - mockResolve.mockResolvedValue("value1"); await expect(loadSecrets(false)).rejects.toThrow( "Invalid secret reference(s): OTHER", diff --git a/src/utils.ts b/src/utils.ts index e66cc18..6a3e0bb 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -38,7 +38,7 @@ const getEnvVarNamesWithSecretRefs = (): string[] => ); const validateSecretRefs = (envNames: string[]): void => { - const invalid: string[] = []; + const invalid: { name: string; message: string }[] = []; for (const envName of envNames) { const ref = process.env[envName]; @@ -48,15 +48,16 @@ const validateSecretRefs = (envNames: string[]): void => { try { Secrets.validateSecretReference(ref); - } catch { - invalid.push(envName); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + invalid.push({ name: envName, message }); } } // Throw an error if any secret references are invalid if (invalid.length > 0) { - const names = invalid.join(", "); - throw new Error(`Invalid secret reference(s): ${names}`); + const details = invalid.map(({ name, message }) => `${name}: ${message}`).join("; "); + throw new Error(`Invalid secret reference(s): ${details}`); } }; From 398c918d600e677d45e96fb6e8c819c157c7295c Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Mon, 23 Feb 2026 08:13:30 -0500 Subject: [PATCH 19/19] Fix format --- src/utils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/utils.ts b/src/utils.ts index 6a3e0bb..9753e18 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -56,7 +56,9 @@ const validateSecretRefs = (envNames: string[]): void => { // Throw an error if any secret references are invalid if (invalid.length > 0) { - const details = invalid.map(({ name, message }) => `${name}: ${message}`).join("; "); + const details = invalid + .map(({ name, message }) => `${name}: ${message}`) + .join("; "); throw new Error(`Invalid secret reference(s): ${details}`); } };