diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 3105fb6..a274491 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.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 diff --git a/package-lock.json b/package-lock.json index b96d951..05b6c69 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": "^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", diff --git a/package.json b/package.json index 02013ed..f799ff6 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": "^3.0.0", "@actions/exec": "^3.0.0", "@actions/tool-cache": "^4.0.0", diff --git a/src/index.ts b/src/index.ts index fcc2552..2cf0995 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,12 @@ 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 f66a31f..22a9c51 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, 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"; diff --git a/src/utils.ts b/src/utils.ts index 571620d..9753e18 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, 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) { - 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); }; -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 +125,63 @@ 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; + } + + 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 => { + 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 ..."); 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"