diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 3105fb6..958b75a 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -105,6 +105,22 @@ 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 + shell: bash + 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/src/utils.test.ts b/src/utils.test.ts index e44b4af..a74874f 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,10 @@ jest.mock("@actions/exec", () => ({ jest.mock("@1password/op-js"); jest.mock("@1password/sdk", () => ({ createClient: jest.fn(), + // eslint-disable-next-line @typescript-eslint/naming-convention + Secrets: { + validateSecretReference: jest.fn(), + }, })); beforeEach(() => { @@ -349,6 +353,39 @@ 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://x"; + (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"); + } + }, + ); + + 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 6b61d48..9753e18 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,32 @@ const getEnvVarNamesWithSecretRefs = (): string[] => process.env[key]?.startsWith("op://"), ); +const validateSecretRefs = (envNames: string[]): void => { + const invalid: { name: string; message: string }[] = []; + + for (const envName of envNames) { + const ref = process.env[envName]; + if (!ref) { + continue; + } + + try { + Secrets.validateSecretReference(ref); + } 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 details = invalid + .map(({ name, message }) => `${name}: ${message}`) + .join("; "); + throw new Error(`Invalid secret reference(s): ${details}`); + } +}; + const setResolvedSecret = ( envName: string, secretValue: string, @@ -108,6 +134,8 @@ const loadSecretsViaServiceAccount = async ( return; } + validateSecretRefs(envs); + const token = process.env[envServiceAccountToken]; if (!token) { throw new Error(authErr); diff --git a/tests/assert-invalid-ref-failed.sh b/tests/assert-invalid-ref-failed.sh new file mode 100755 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"