Merge pull request #137 from 1Password/feature/migrate-to-sdk

feat: migrate service account ot use 1Password sdk

.github/workflows/e2e-tests.yml

@@ -26,13 +26,12 @@ on:
 
 jobs:
   test-service-account:
-    name: Service Account (${{ matrix.os }}, ${{ matrix.version }}, export-env=${{ matrix.export-env }})
+    name: Service Account (${{ matrix.os }}, export-env=${{ matrix.export-env }})
     runs-on: ${{ matrix.os }}
     strategy:
       fail-fast: true
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        version: [latest, 2.30.0]
         export-env: [true, false]
     steps:
       - name: Checkout
@@ -69,7 +68,6 @@ jobs:
         id: load_secrets
         uses: ./
         with:
-          version: ${{ matrix.version }}
           export-env: ${{ matrix.export-env }}
         env:
           SECRET: op://${{ secrets.VAULT }}/test-secret/password
@@ -105,6 +103,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

package-lock.json

@@ -10,6 +10,7 @@
 			"license": "MIT",
 			"dependencies": {
 				"@1password/op-js": "^0.1.11",
+				"@1password/sdk": "^0.4.0",
 				"@actions/core": "^3.0.0",
 				"@actions/exec": "^3.0.0",
 				"@actions/tool-cache": "^4.0.0",
@@ -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": "3.0.0",
 			"resolved": "https://registry.npmjs.org/@actions/core/-/core-3.0.0.tgz",

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": "^3.0.0",
 		"@actions/exec": "^3.0.0",
 		"@actions/tool-cache": "^4.0.0",

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,12 @@ const loadSecretsAction = async () => {
 			dotenv.config({ path: file });
 		}
 
-		// Download and install the CLI
+		const isConnect =
-		await installCLI();
+			process.env[envConnectHost] && process.env[envConnectToken];
+		// If Connect is used, download and install the CLI
+		if (isConnect) {
+			await installCLI();
+		}
 
 		// Load secrets
 		await loadSecrets(shouldExportEnv);

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, Secrets } from "@1password/sdk";
 import {
 	extractSecret,
 	loadSecrets,
@@ -16,6 +17,13 @@ import {
 } from "./constants";
 
 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(() => {
 	jest.clearAllMocks();
@@ -137,7 +145,13 @@ describe("extractSecret", () => {
 	});
 });
 
-describe("loadSecrets", () => {
+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);
 
@@ -175,6 +189,199 @@ 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();
+	});
+
+	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";
+		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("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", () => {
 	const testManagedEnv = "TEST_SECRET";
 	const testSecretValue = "MyS3cr#T";

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, Secrets } from "@1password/sdk";
 import { version } from "../package.json";
 import {
 	authErr,
@@ -29,12 +30,60 @@ export const validateAuth = (): void => {
 	core.info(`Authenticated with ${authType}.`);
 };
 
-export const extractSecret = (
+const getEnvVarNamesWithSecretRefs = (): string[] =>
+	Object.keys(process.env).filter(
+		(key) =>
+			typeof process.env[key] === "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,
 	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,
+): void => {
 	const ref = process.env[envName];
 	if (!ref) {
 		return;
@@ -45,20 +94,13 @@ export const extractSecret = (
 		return;
 	}
 
-	if (shouldExportEnv) {
+	setResolvedSecret(envName, secretValue, 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);
-	}
 };
 
-export const loadSecrets = async (shouldExportEnv: boolean): Promise<void> => {
+// Connect loads secrets via the 1Password CLI
-	// Pass User-Agent Information to the 1Password CLI
+const loadSecretsViaConnect = async (
+	shouldExportEnv: boolean,
+): Promise<void> => {
 	setClientInfo({
 		name: "1Password GitHub Action",
 		id: "GHA",
@@ -83,6 +125,63 @@ export const loadSecrets = async (shouldExportEnv: boolean): Promise<void> => {
 	}
 };
 
+// Service Account loads secrets via the 1Password SDK
+const loadSecretsViaServiceAccount = async (
+	shouldExportEnv: boolean,
+): Promise<void> => {
+	const envs = getEnvVarNamesWithSecretRefs();
+	if (envs.length === 0) {
+		return;
+	}
+
+	validateSecretRefs(envs);
+
+	const token = process.env[envServiceAccountToken];
+	if (!token) {
+		throw new Error(authErr);
+	}
+
+	// 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];
+		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<void> => {
+	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 ...");

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"