Merge pull request #135 from 1Password/jill/validate-secret-reference

Add secret ref validation

.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

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", () => {

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);

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"