From 21385b0c31192a81c7f785081563e10ed821835d Mon Sep 17 00:00:00 2001 From: Jill Regan Date: Fri, 20 Feb 2026 10:43:17 -0500 Subject: [PATCH] Add e2e test --- .github/workflows/e2e-tests.yml | 75 +++++++++++++++- src/index.ts | 7 +- src/utils.test.ts | 154 ++++++++++++++++++++++++++++++++ src/utils.ts | 64 ++++++++----- 4 files changed, 277 insertions(+), 23 deletions(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 958b75a..05a3743 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -21,7 +21,10 @@ on: OP_SERVICE_ACCOUNT_TOKEN: required: true VAULT: - description: "1Password vault name or UUID" + description: "1Password vault name" + required: true + VAULT_ID: + description: "1Password vault UUID" required: true jobs: @@ -89,6 +92,30 @@ jobs: FILE_MULTILINE_SECRET: ${{ steps.load_secrets.outputs.FILE_MULTILINE_SECRET }} run: ./tests/assert-env-set.sh + - name: Load secrets by vault ID + id: load_secrets_by_vault_id + uses: ./ + with: + version: ${{ matrix.version }} + export-env: ${{ matrix.export-env }} + env: + SECRET: op://${{ secrets.VAULT_ID }}/test-secret/password + SECRET_IN_SECTION: op://${{ secrets.VAULT_ID }}/test-secret/test-section/password + MULTILINE_SECRET: op://${{ secrets.VAULT_ID }}/multiline-secret/notesPlain + OP_ENV_FILE: ./tests/.env.tpl + + - name: Assert test secret values [vault by ID] + if: ${{ !matrix.export-env }} + shell: bash + env: + SECRET: ${{ steps.load_secrets_by_vault_id.outputs.SECRET }} + SECRET_IN_SECTION: ${{ steps.load_secrets_by_vault_id.outputs.SECRET_IN_SECTION }} + MULTILINE_SECRET: ${{ steps.load_secrets_by_vault_id.outputs.MULTILINE_SECRET }} + FILE_SECRET: ${{ steps.load_secrets_by_vault_id.outputs.FILE_SECRET }} + FILE_SECRET_IN_SECTION: ${{ steps.load_secrets_by_vault_id.outputs.FILE_SECRET_IN_SECTION }} + FILE_MULTILINE_SECRET: ${{ steps.load_secrets_by_vault_id.outputs.FILE_MULTILINE_SECRET }} + run: ./tests/assert-env-set.sh + - name: Assert test secret values [exported env] if: ${{ matrix.export-env }} shell: bash @@ -111,7 +138,6 @@ jobs: uses: ./ env: BAD_REF: "op://x" - OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }} with: export-env: true @@ -156,6 +182,12 @@ jobs: echo "FILE_SECRET_IN_SECTION=op://${{ secrets.VAULT }}/test-secret/test-section/password" >> tests/.env.tpl echo "FILE_MULTILINE_SECRET=op://${{ secrets.VAULT }}/multiline-secret/notesPlain" >> tests/.env.tpl + - name: Generate .vaultId_env.tpl + run: | + echo "FILE_SECRET=op://${{ secrets.VAULT_ID }}/test-secret/password" > tests/.vaultId_env.tpl + echo "FILE_SECRET_IN_SECTION=op://${{ secrets.VAULT_ID }}/test-secret/test-section/password" >> tests/.vaultId_env.tpl + echo "FILE_MULTILINE_SECRET=op://${{ secrets.VAULT_ID }}/multiline-secret/notesPlain" >> tests/.vaultId_env.tpl + - name: Launch 1Password Connect instance env: OP_CONNECT_CREDENTIALS: ${{ secrets.OP_CONNECT_CREDENTIALS }} @@ -192,6 +224,30 @@ jobs: FILE_MULTILINE_SECRET: ${{ steps.load_secrets.outputs.FILE_MULTILINE_SECRET }} run: ./tests/assert-env-set.sh + - name: Load secrets by vault ID + id: load_secrets_by_vault_id + uses: ./ + with: + version: ${{ matrix.version }} + export-env: ${{ matrix.export-env }} + env: + SECRET: op://${{ secrets.VAULT_ID }}/test-secret/password + SECRET_IN_SECTION: op://${{ secrets.VAULT_ID }}/test-secret/test-section/password + MULTILINE_SECRET: op://${{ secrets.VAULT_ID }}/multiline-secret/notesPlain + OP_ENV_FILE: ./tests/.vaultId_env.tpl + + - name: Assert test secret values [vault by ID] + if: ${{ !matrix.export-env }} + shell: bash + env: + SECRET: ${{ steps.load_secrets_by_vault_id.outputs.SECRET }} + SECRET_IN_SECTION: ${{ steps.load_secrets_by_vault_id.outputs.SECRET_IN_SECTION }} + MULTILINE_SECRET: ${{ steps.load_secrets_by_vault_id.outputs.MULTILINE_SECRET }} + FILE_SECRET: ${{ steps.load_secrets_by_vault_id.outputs.FILE_SECRET }} + FILE_SECRET_IN_SECTION: ${{ steps.load_secrets_by_vault_id.outputs.FILE_SECRET_IN_SECTION }} + FILE_MULTILINE_SECRET: ${{ steps.load_secrets_by_vault_id.outputs.FILE_MULTILINE_SECRET }} + run: ./tests/assert-env-set.sh + - name: Assert test secret values [exported env] if: ${{ matrix.export-env }} run: ./tests/assert-env-set.sh @@ -205,3 +261,18 @@ jobs: - name: Assert removed secrets [exported env] if: ${{ matrix.export-env }} 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" + 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 }} diff --git a/src/index.ts b/src/index.ts index 4913b3e..280c857 100644 --- a/src/index.ts +++ b/src/index.ts @@ -33,7 +33,12 @@ const loadSecretsAction = async () => { if (message === "Unknown Error" && error.cause instanceof Error) { message = error.cause.message; } - } else if (error && typeof error === "object" && "message" in error && typeof (error as { message: unknown }).message === "string") { + } else if ( + error && + typeof error === "object" && + "message" in error && + typeof (error as { message: unknown }).message === "string" + ) { message = (error as { message: string }).message; } else if (error !== null && error !== undefined) { message = typeof error === "string" ? error : JSON.stringify(error); diff --git a/src/utils.test.ts b/src/utils.test.ts index d6b7ced..00b3877 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -170,6 +170,7 @@ describe("loadSecrets when using Connect", () => { process.env.MY_SECRET = "op://vault/item/field"; (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: jest.fn().mockResolvedValue({ id: "vault-id-123" }), getItem: jest.fn().mockResolvedValue({ fields: [ { label: "field", value: "resolved-via-connect", section: undefined }, @@ -178,6 +179,7 @@ describe("loadSecrets when using Connect", () => { }), }); }); + it("resolves ref via Connect SDK and exports secret", async () => { await loadSecrets(true); @@ -198,6 +200,158 @@ describe("loadSecrets when using Connect", () => { expect(core.exportVariable).not.toHaveBeenCalled(); }); + it("sets step output when shouldExportEnv is false", async () => { + await loadSecrets(false); + + expect(core.setOutput).toHaveBeenCalledWith("MY_SECRET", "resolved-via-connect"); + expect(core.exportVariable).not.toHaveBeenCalled(); + }); + + it("masks resolved secret with setSecret", async () => { + await loadSecrets(true); + + expect(core.setSecret).toHaveBeenCalledWith("resolved-via-connect"); + }); + + it("calls getVault with vault segment from ref", async () => { + process.env.MY_SECRET = "op://my-vault-name/my-item/field"; + const mockGetVault = jest.fn().mockResolvedValue({ id: "vault-uuid" }); + const mockGetItem = jest.fn().mockResolvedValue({ + fields: [ + { label: "field", value: "secret-value", section: undefined }, + ], + sections: [], + }); + (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: mockGetVault, + getItem: mockGetItem, + }); + + await loadSecrets(false); + + expect(mockGetVault).toHaveBeenCalledWith("my-vault-name"); + }); + + it("throws when getVault returns vault without id", async () => { + const mockGetVault = jest.fn().mockResolvedValue({}); + (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: mockGetVault, + getItem: jest.fn(), + }); + + await expect(loadSecrets(true)).rejects.toThrow( + /Could not find valid vault "vault" for ref "op:\/\/vault\/item\/field"/, + ); + expect(mockGetVault).toHaveBeenCalledWith("vault"); + }); + + it("resolves vault by name and uses returned id for getItem", async () => { + process.env.MY_SECRET = "op://My Vault/My Item/field"; + const mockGetVault = jest.fn().mockResolvedValue({ id: "uuid-for-my-vault" }); + const mockGetItem = jest.fn().mockResolvedValue({ + fields: [ + { label: "field", value: "secret-from-named-vault", section: undefined }, + ], + sections: [], + }); + (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: mockGetVault, + getItem: mockGetItem, + }); + + await loadSecrets(true); + + expect(mockGetVault).toHaveBeenCalledWith("My Vault"); + expect(mockGetItem).toHaveBeenCalledWith("uuid-for-my-vault", "My Item"); + expect(core.exportVariable).toHaveBeenCalledWith( + "MY_SECRET", + "secret-from-named-vault", + ); + }); + + it("calls getItem with vault id from getVault, not ref vault segment", async () => { + const mockGetVault = jest.fn().mockResolvedValue({ id: "resolved-vault-id" }); + const mockGetItem = jest.fn().mockResolvedValue({ + fields: [ + { label: "field", value: "resolved-via-connect", section: undefined }, + ], + sections: [], + }); + (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: mockGetVault, + getItem: mockGetItem, + }); + + await loadSecrets(true); + + expect(mockGetVault).toHaveBeenCalledWith("vault"); + expect(mockGetItem).toHaveBeenCalledWith("resolved-vault-id", "item"); + }); + + it("rejects when getItem fails", async () => { + const mockGetVault = jest.fn().mockResolvedValue({ id: "vault-id-123" }); + const mockGetItem = jest.fn().mockRejectedValue(new Error("Item not found")); + (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: mockGetVault, + getItem: mockGetItem, + }); + + await expect(loadSecrets(true)).rejects.toThrow("Item not found"); + }); + + it("resolves refs in different vaults using each vault id", async () => { + delete process.env.MY_SECRET; + process.env.SECRET_A = "op://vault-a/item1/field1"; + process.env.SECRET_B = "op://vault-b/item2/field2"; + const mockGetVault = jest.fn().mockImplementation(async (vaultName: string) => + Promise.resolve({ + id: vaultName === "vault-a" ? "id-a" : "id-b", + }), + ); + const mockGetItem = jest + .fn() + .mockResolvedValueOnce({ + fields: [ + { label: "field1", value: "value-a", section: undefined }, + ], + sections: [], + }) + .mockResolvedValueOnce({ + fields: [ + { label: "field2", value: "value-b", section: undefined }, + ], + sections: [], + }); + (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: mockGetVault, + getItem: mockGetItem, + }); + + await loadSecrets(true); + + expect(mockGetVault).toHaveBeenCalledWith("vault-a"); + expect(mockGetVault).toHaveBeenCalledWith("vault-b"); + expect(mockGetItem).toHaveBeenNthCalledWith(1, "id-a", "item1"); + expect(mockGetItem).toHaveBeenNthCalledWith(2, "id-b", "item2"); + expect(core.exportVariable).toHaveBeenCalledWith("SECRET_A", "value-a"); + expect(core.exportVariable).toHaveBeenCalledWith("SECRET_B", "value-b"); + }); + + it("throws on invalid ref before calling Connect", async () => { + delete process.env.MY_SECRET; + process.env.BAD_REF = "op://x"; + const mockGetVault = jest.fn(); + const mockGetItem = jest.fn(); + (OnePasswordConnect as jest.Mock).mockReturnValue({ + getVault: mockGetVault, + getItem: mockGetItem, + }); + + await expect(loadSecrets(true)).rejects.toThrow(/invalid|reference/i); + expect(mockGetVault).not.toHaveBeenCalled(); + expect(mockGetItem).not.toHaveBeenCalled(); + }); + describe("core.exportVariable", () => { it("is called when shouldExportEnv is true", async () => { await loadSecrets(true); diff --git a/src/utils.ts b/src/utils.ts index 583ac67..77c38a6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -58,7 +58,9 @@ const parseOpRef = (ref: string): ParsedOpRef => { if (segments.length === 4) { section = segments[2]; if (!section) { - throw new Error(`Invalid op reference: section is required when using 4 path segments`); + throw new Error( + `Invalid op reference: section is required when using 4 path segments`, + ); } } @@ -68,7 +70,7 @@ const parseOpRef = (ref: string): ParsedOpRef => { field, section, }; -} +}; // #endregion // #region Connect item resolution @@ -77,8 +79,14 @@ const getSecretFromConnectItem = async ( item: FullItem, parsed: ParsedOpRef, ): Promise => { - const sectionIds = parsed.section ? findSectionIdsByQuery(item.sections, parsed.section) : []; - const { fieldValue, fileId } = findMatchingFieldAndFile(item, parsed.field, sectionIds); + const sectionIds = parsed.section + ? findSectionIdsByQuery(item.sections, parsed.section) + : []; + const { fieldValue, fileId } = findMatchingFieldAndFile( + item, + parsed.field, + sectionIds, + ); if (fieldValue !== undefined) { return fieldValue; @@ -103,7 +111,7 @@ const getSecretFromConnectItem = async ( throw new Error( `could not find field or file ${parsed.field} on item ${parsed.item} in vault ${parsed.vault}`, ); -} +}; const findSectionIdsByQuery = ( sections: FullItem["sections"], @@ -117,9 +125,9 @@ const findSectionIdsByQuery = ( } const ids = sections - .filter((s) => s.id === sectionQuery || s.label === sectionQuery) - .map((s) => s.id!) - .filter(Boolean); + .filter((s) => s.id === sectionQuery || s.label === sectionQuery) + .map((s) => s.id!) + .filter(Boolean); // If no sections were found with the given query throw an error if (ids.length === 0) { @@ -148,9 +156,13 @@ const findMatchingFieldAndFile = ( if (sectionFilter) { // Filter fields by section const matchingFields = fields.filter((f) => { - const fieldIdOrLabelMatchesQuery = f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; + const fieldIdOrLabelMatchesQuery = + f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; const sectionId = f.section?.id; - const fieldSectionIsInRefSections = sectionId !== null && sectionId !== undefined && sectionIds.includes(sectionId); + const fieldSectionIsInRefSections = + sectionId !== null && + sectionId !== undefined && + sectionIds.includes(sectionId); return fieldIdOrLabelMatchesQuery && fieldSectionIsInRefSections; }); @@ -161,9 +173,13 @@ const findMatchingFieldAndFile = ( matchedField = matchingFields[0]; const matchingFiles = files.filter((f) => { - const fileIdOrNameMatchesQuery = f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; + const fileIdOrNameMatchesQuery = + f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; const sectionId = f.section?.id; - const fileSectionIsInRefSections = sectionId !== null && sectionId !== undefined && sectionIds.includes(sectionId); + const fileSectionIsInRefSections = + sectionId !== null && + sectionId !== undefined && + sectionIds.includes(sectionId); return fileIdOrNameMatchesQuery && fileSectionIsInRefSections; }); @@ -174,8 +190,10 @@ const findMatchingFieldAndFile = ( matchedFile = matchingFiles[0]; } else { const matchingFields = fields.filter((f) => { - const fieldIdOrLabelMatchesQuery = f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; - const fieldHasNoSection = (f.section?.id === null || f.section?.id === undefined); + const fieldIdOrLabelMatchesQuery = + f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; + const fieldHasNoSection = + f.section?.id === null || f.section?.id === undefined; return fieldIdOrLabelMatchesQuery && fieldHasNoSection; }); @@ -188,7 +206,8 @@ const findMatchingFieldAndFile = ( // If no field was found with no section, find a field in any section if (!matchedField) { const matchingFieldsInAnySection = fields.filter((f) => { - const fieldIdOrLabelMatchesQuery = f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; + const fieldIdOrLabelMatchesQuery = + f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; return fieldIdOrLabelMatchesQuery; }); @@ -199,8 +218,10 @@ const findMatchingFieldAndFile = ( } const matchingFiles = files.filter((f) => { - const fileIdOrNameMatchesQuery = f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; - const fileHasNoSection = (f.section?.id === null || f.section?.id === undefined); + const fileIdOrNameMatchesQuery = + f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; + const fileHasNoSection = + f.section?.id === null || f.section?.id === undefined; return fileIdOrNameMatchesQuery && fileHasNoSection; }); @@ -213,7 +234,8 @@ const findMatchingFieldAndFile = ( // If no file was found with no section, find a file in any section if (!matchedFile) { const matchingFilesInAnySection = files.filter((f) => { - const fileIdOrNameMatchesQuery = f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; + const fileIdOrNameMatchesQuery = + f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; return fileIdOrNameMatchesQuery; }); @@ -245,7 +267,7 @@ const findMatchingFieldAndFile = ( } return {}; -} +}; // #endregion // #region Shared helpers and auth @@ -385,7 +407,9 @@ const loadSecretsViaConnect = async ( const parsed = parseOpRef(ref); const vault = await client.getVault(parsed.vault); if (!vault.id) { - throw new Error(`Vault "${parsed.vault}" has no id`); + throw new Error( + `Could not find valid vault "${parsed.vault}" for ref "${ref}"`, + ); } const item = await client.getItem(vault.id, parsed.item);