From 7d4e24a43ff243f56efa13564531f01f7fb52300 Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Mon, 2 Mar 2026 18:41:50 -0500 Subject: [PATCH] Revert "Merge pull request #137 from 1Password/feature/migrate-to-sdk" This reverts commit 38b333095dd181f4fbb51e943024c6f2e28717e1, reversing changes made to 652d567877240d6cd42fe4e77cce6c9548b15bad. --- .github/workflows/e2e-tests.yml | 20 +-- package-lock.json | 16 --- package.json | 1 - src/index.ts | 10 +- src/utils.test.ts | 209 +---------------------------- src/utils.ts | 127 ++---------------- tests/assert-invalid-ref-failed.sh | 7 - 7 files changed, 21 insertions(+), 369 deletions(-) delete mode 100755 tests/assert-invalid-ref-failed.sh diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index a274491..3105fb6 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -26,12 +26,13 @@ on: jobs: test-service-account: - name: Service Account (${{ matrix.os }}, export-env=${{ matrix.export-env }}) + name: Service Account (${{ matrix.os }}, ${{ matrix.version }}, 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 @@ -68,6 +69,7 @@ jobs: id: load_secrets uses: ./ with: + version: ${{ matrix.version }} export-env: ${{ matrix.export-env }} env: SECRET: op://${{ secrets.VAULT }}/test-secret/password @@ -103,22 +105,6 @@ 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 42ae9bd..2f5dc91 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,6 @@ "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", @@ -73,21 +72,6 @@ "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 e5ecb3c..5751ca5 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,6 @@ "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 2cf0995..fcc2552 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, envConnectHost, envConnectToken } from "./constants"; +import { envFilePath } from "./constants"; const loadSecretsAction = async () => { try { @@ -26,12 +26,8 @@ const loadSecretsAction = async () => { dotenv.config({ path: file }); } - const isConnect = - process.env[envConnectHost] && process.env[envConnectToken]; - // If Connect is used, download and install the CLI - if (isConnect) { - await installCLI(); - } + // Download and install the CLI + await installCLI(); // Load secrets await loadSecrets(shouldExportEnv); diff --git a/src/utils.test.ts b/src/utils.test.ts index 22a9c51..f66a31f 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,7 +1,6 @@ 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, @@ -17,13 +16,6 @@ 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(); @@ -145,13 +137,7 @@ describe("extractSecret", () => { }); }); -describe("loadSecrets when using Connect", () => { - beforeEach(() => { - process.env[envConnectHost] = "https://localhost:8000"; - process.env[envConnectToken] = "token"; - process.env[envServiceAccountToken] = ""; - }); - +describe("loadSecrets", () => { it("sets the client info and gets the executed output", async () => { await loadSecrets(true); @@ -189,199 +175,6 @@ describe("loadSecrets when using Connect", () => { }); }); -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 9753e18..571620d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,7 +1,6 @@ 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, @@ -30,60 +29,12 @@ export const validateAuth = (): void => { core.info(`Authenticated with ${authType}.`); }; -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 => { + core.info(`Populating variable: ${envName}`); + const ref = process.env[envName]; if (!ref) { return; @@ -94,13 +45,20 @@ export const extractSecret = ( return; } - setResolvedSecret(envName, secretValue, shouldExportEnv); + 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); + } }; -// Connect loads secrets via the 1Password CLI -const loadSecretsViaConnect = async ( - shouldExportEnv: boolean, -): Promise => { +export const loadSecrets = async (shouldExportEnv: boolean): Promise => { + // Pass User-Agent Information to the 1Password CLI setClientInfo({ name: "1Password GitHub Action", id: "GHA", @@ -125,63 +83,6 @@ const loadSecretsViaConnect = async ( } }; -// 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 deleted file mode 100755 index 3d07973..0000000 --- a/tests/assert-invalid-ref-failed.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/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"