diff --git a/dist/index.js b/dist/index.js index 679fe63..edb37b2 100644 --- a/dist/index.js +++ b/dist/index.js @@ -33684,7 +33684,7 @@ const loadSecretsAction = async () => { message = error.message; } else { - message = String(error); + String(error); } core.setFailed(message); } diff --git a/docs/local-testing.md b/docs/local-testing.md index 21f76df..9e52ef9 100644 --- a/docs/local-testing.md +++ b/docs/local-testing.md @@ -17,7 +17,8 @@ This document explains how to run e2e tests locally using `act`. | Secret | Description | | -------------------------- | --------------------- | | `OP_SERVICE_ACCOUNT_TOKEN` | Service Account token | -| `VAULT` | Vault name or UUID | +| `VAULT` | Vault name | +| `VAULT_ID` | Vault UUID | ## Building Before Testing diff --git a/src/index.ts b/src/index.ts index 280c857..712cadc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,21 +27,14 @@ const loadSecretsAction = async () => { // Load secrets await loadSecrets(shouldExportEnv); } catch (error) { + // It's possible for the Error constructor to be modified to be anything + // in JavaScript, so the following code accounts for this possibility. + // https://kentcdodds.com/blog/get-a-catch-block-error-message-with-typescript let message = "Unknown Error"; if (error instanceof Error) { message = error.message; - 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" - ) { - message = (error as { message: string }).message; - } else if (error !== null && error !== undefined) { - message = typeof error === "string" ? error : JSON.stringify(error); + } else { + message = String(error); } core.setFailed(message); } diff --git a/src/utils.test.ts b/src/utils.test.ts index 3de8a08..85966de 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -2,12 +2,16 @@ import * as core from "@actions/core"; import * as exec from "@actions/exec"; import { read } from "@1password/op-js"; import { createClient, Secrets } from "@1password/sdk"; -import { OnePasswordConnect } from "@1password/connect"; +import { OnePasswordConnect, FullItem } from "@1password/connect"; import { extractSecret, loadSecrets, unsetPrevious, validateAuth, + findMatchingFieldAndFile, + findSectionIdsByQuery, + parseOpRef, + getEnvVarNamesWithSecretRefs, } from "./utils"; import { authErr, @@ -594,3 +598,360 @@ describe("unsetPrevious", () => { expect(core.exportVariable).toHaveBeenCalledWith("TEST_SECRET", ""); }); }); + +describe("findMatchingFieldAndFile", () => { + interface TestField { + id?: string; + label?: string; + value?: string | null; + section?: { id: string } | null | undefined; + } + interface TestFile { + id?: string; + name?: string; + section?: { id: string } | null | undefined; + } + + const item = (opts: { fields?: TestField[]; files?: TestFile[] }): FullItem => + ({ + fields: opts.fields ?? [], + files: opts.files ?? [], + sections: [], + }) as unknown as FullItem; + + const find = ( + opts: { fields?: TestField[]; files?: TestFile[] }, + sectionIds: string[] = [], + ) => findMatchingFieldAndFile(item(opts), "password", sectionIds); + + describe("when section filter is used (sectionIds.length > 0)", () => { + it.each<{ + name: string; + itemOpts: { fields?: TestField[]; files?: TestFile[] }; + expected: { fieldValue?: string; fileId?: string }; + }>([ + { + name: "returns field value when one field matches query and is in ref sections", + itemOpts: { + fields: [ + { + id: "f1", + label: "password", + value: "secret123", + section: { id: "section-1" }, + }, + ], + }, + expected: { fieldValue: "secret123" }, + }, + { + name: "returns file id when one file matches query and is in ref sections", + itemOpts: { + files: [ + { + id: "file-uuid", + name: "password", + section: { id: "section-1" }, + }, + ], + }, + expected: { fileId: "file-uuid" }, + }, + { + name: "returns empty object when no field or file matches", + itemOpts: { + fields: [ + { label: "other", value: "x", section: { id: "section-1" } }, + ], + files: [], + }, + expected: {}, + }, + { + name: "returns field value when field matches by id", + itemOpts: { + fields: [ + { + id: "password", + label: "Password Label", + value: "secret-by-id", + section: { id: "section-1" }, + }, + ], + }, + expected: { fieldValue: "secret-by-id" }, + }, + ])("$name", ({ itemOpts, expected }) => { + expect(find(itemOpts, ["section-1"])).toEqual(expected); + }); + + it.each<{ + name: string; + itemOpts: { fields?: TestField[]; files?: TestFile[] }; + error: RegExp; + }>([ + { + name: "throws when multiple fields match", + itemOpts: { + fields: [ + { label: "password", value: "a", section: { id: "section-1" } }, + { label: "password", value: "b", section: { id: "section-1" } }, + ], + }, + error: /Multiple matches/, + }, + { + name: "throws when multiple files match", + itemOpts: { + files: [ + { id: "id1", name: "password", section: { id: "section-1" } }, + { id: "id2", name: "password", section: { id: "section-1" } }, + ], + }, + error: /Multiple matches/, + }, + { + name: "throws when both a field and a file match", + itemOpts: { + fields: [ + { label: "password", value: "v", section: { id: "section-1" } }, + ], + files: [ + { id: "fid", name: "password", section: { id: "section-1" } }, + ], + }, + error: /Both a field and a file match/, + }, + { + name: "throws when field has no value", + itemOpts: { + fields: [ + { label: "password", value: null, section: { id: "section-1" } }, + ], + }, + error: /has no value/, + }, + ])("$name", ({ itemOpts, error }) => { + expect(() => find(itemOpts, ["section-1"])).toThrow(error); + }); + }); + + describe("when no section filter (sectionIds.length === 0)", () => { + const sectionIds: string[] = []; + + it.each<{ + name: string; + itemOpts: { fields?: TestField[]; files?: TestFile[] }; + expected: { fieldValue?: string; fileId?: string }; + }>([ + { + name: "returns field value when one field has no section and matches query", + itemOpts: { + fields: [{ label: "password", value: "secret", section: undefined }], + }, + expected: { fieldValue: "secret" }, + }, + { + name: "returns file id when one file has no section and matches query", + itemOpts: { + files: [{ id: "file-id", name: "password", section: undefined }], + }, + expected: { fileId: "file-id" }, + }, + { + name: "returns field value from fallback (any section) when no field with no section matches", + itemOpts: { + fields: [ + { label: "other", value: "x", section: undefined }, + { + label: "password", + value: "from-any-section", + section: { id: "sec" }, + }, + ], + }, + expected: { fieldValue: "from-any-section" }, + }, + { + name: "returns file id from fallback (any section) when no file with no section matches", + itemOpts: { + files: [ + { id: "other", name: "x", section: undefined }, + { id: "file-any", name: "password", section: { id: "sec" } }, + ], + }, + expected: { fileId: "file-any" }, + }, + { + name: "returns empty object when no match", + itemOpts: { + fields: [{ label: "other", value: "x", section: undefined }], + files: [], + }, + expected: {}, + }, + ])("$name", ({ itemOpts, expected }) => { + expect(find(itemOpts, sectionIds)).toEqual(expected); + }); + + it.each<{ + name: string; + itemOpts: { fields?: TestField[]; files?: TestFile[] }; + error: RegExp; + }>([ + { + name: "throws when multiple fields with no section match", + itemOpts: { + fields: [ + { label: "password", value: "a", section: undefined }, + { label: "password", value: "b", section: undefined }, + ], + }, + error: /Multiple matches/, + }, + { + name: "throws when multiple files with no section match", + itemOpts: { + files: [ + { id: "1", name: "password", section: undefined }, + { id: "2", name: "password", section: undefined }, + ], + }, + error: /Multiple matches/, + }, + { + name: "throws when both field and file match", + itemOpts: { + fields: [{ label: "password", value: "value", section: undefined }], + files: [{ id: "fid", name: "password", section: undefined }], + }, + error: /Both a field and a file match/, + }, + ])("$name", ({ itemOpts, error }) => { + expect(() => find(itemOpts, sectionIds)).toThrow(error); + }); + }); +}); + +describe("findSectionIdsByQuery", () => { + it("throws when sections is empty", () => { + expect(() => findSectionIdsByQuery([], "section-1")).toThrow( + /section section-1 could not be found/, + ); + }); + + it("throws when sections is null/undefined", () => { + expect(() => + findSectionIdsByQuery(undefined as unknown as FullItem["sections"], "x"), + ).toThrow(/could not be found/); + }); + + it("returns section id when section matches by id", () => { + const sections = [{ id: "sec-1", label: "Section 1" }]; + expect( + findSectionIdsByQuery(sections as FullItem["sections"], "sec-1"), + ).toEqual(["sec-1"]); + }); + + it("returns section id when section matches by label", () => { + const sections = [{ id: "sec-1", label: "My Section" }]; + expect( + findSectionIdsByQuery(sections as FullItem["sections"], "My Section"), + ).toEqual(["sec-1"]); + }); + + it("throws when section query matches no section", () => { + const sections = [{ id: "sec-1", label: "Other" }]; + expect(() => + findSectionIdsByQuery(sections as FullItem["sections"], "nonexistent"), + ).toThrow(/could not be found/); + }); + + it("returns multiple ids when multiple sections match", () => { + const sections = [ + { id: "sec-1", label: "A" }, + { id: "sec-2", label: "A" }, + ]; + expect( + findSectionIdsByQuery(sections as FullItem["sections"], "A"), + ).toEqual(["sec-1", "sec-2"]); + }); +}); + +describe("parseOpRef", () => { + it("parses 3-segment ref (vault/item/field)", () => { + expect(parseOpRef("op://vault/item/field")).toEqual({ + vault: "vault", + item: "item", + field: "field", + section: undefined, + }); + }); + + it("parses 4-segment ref (vault/item/section/field)", () => { + expect(parseOpRef("op://vault/item/MySection/password")).toEqual({ + vault: "vault", + item: "item", + section: "MySection", + field: "password", + }); + }); + + it("decodes URI-encoded segments", () => { + expect(parseOpRef("op://my%20vault/my%20item/field")).toEqual({ + vault: "my vault", + item: "my item", + field: "field", + section: undefined, + }); + }); + + it("throws when ref does not start with op://", () => { + expect(() => parseOpRef("invalid-ref")).toThrow( + /Invalid op reference: invalid-ref/, + ); + }); + + it("throws when segment count is invalid", () => { + expect(() => parseOpRef("op://vault/item")).toThrow( + /use op:\/\/\/\//, + ); + expect(() => parseOpRef("op://a/b/c/d/e")).toThrow( + /use op:\/\/\/\//, + ); + }); + + it("throws when vault or item or field is empty", () => { + expect(() => parseOpRef("op:///item/field")).toThrow(/vault is required/); + expect(() => parseOpRef("op://vault//field")).toThrow(/item is required/); + expect(() => parseOpRef("op://vault/item/")).toThrow(/field is required/); + }); + + it("throws when 4-segment ref has empty section", () => { + expect(() => parseOpRef("op://vault/item//field")).toThrow( + /section is required when using 4 path segments/, + ); + }); + + it("throws when last segment is empty (trailing slash)", () => { + expect(() => parseOpRef("op://vault/item/field/")).toThrow( + /field is required/, + ); + }); +}); + +describe("getEnvVarNamesWithSecretRefs", () => { + it("returns only env var names whose value is a string starting with op://", () => { + process.env.OP_REF = "op://vault/item/field"; + process.env.NOT_OP_REF = "https://example.com"; + process.env.EMPTY_REF = ""; + process.env.OP_REF_OTHER = "op://other/vault/item/secret"; + + const result = getEnvVarNamesWithSecretRefs(); + + expect(result).toContain("OP_REF"); + expect(result).toContain("OP_REF_OTHER"); + expect(result).not.toContain("NOT_OP_REF"); + expect(result).not.toContain("EMPTY_REF"); + }); +}); diff --git a/src/utils.ts b/src/utils.ts index 77c38a6..f2511cc 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -19,7 +19,7 @@ interface ParsedOpRef { field: string; } -const parseOpRef = (ref: string): ParsedOpRef => { +export const parseOpRef = (ref: string): ParsedOpRef => { // Safety check: refs are validated by validateSecretRefs before this runs // this guards against parseOpRef being called directly with invalid input if (!ref.startsWith("op://")) { @@ -113,7 +113,7 @@ const getSecretFromConnectItem = async ( ); }; -const findSectionIdsByQuery = ( +export const findSectionIdsByQuery = ( sections: FullItem["sections"], sectionQuery: string | undefined, ): string[] => { @@ -126,8 +126,7 @@ const findSectionIdsByQuery = ( const ids = sections .filter((s) => s.id === sectionQuery || s.label === sectionQuery) - .map((s) => s.id!) - .filter(Boolean); + .flatMap((s) => (s.id ? [s.id] : [])); // If no sections were found with the given query throw an error if (ids.length === 0) { @@ -139,116 +138,76 @@ const findSectionIdsByQuery = ( return ids; }; -const findMatchingFieldAndFile = ( +export const findMatchingFieldAndFile = ( item: FullItem, fieldOrFileQuery: string, sectionIds: string[], ): { fieldValue?: string; fileId?: string } => { - const errMultiple = `multiple fields ${fieldOrFileQuery} that match the provided reference have been found`; - + // Get the fields/files from the item and check if the ref has a section filter const fields = item.fields ?? []; const files = item.files ?? []; const sectionFilter = sectionIds.length > 0; + const fieldMatchesQuery = (f: (typeof fields)[0]) => + f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; + const fileMatchesQuery = (f: (typeof files)[0]) => + f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; + let matchedField: (typeof fields)[0] | undefined; let matchedFile: (typeof files)[0] | undefined; if (sectionFilter) { - // Filter fields by section + // If the ref has a section filter only accept matches inside the referenced sections const matchingFields = fields.filter((f) => { - const fieldIdOrLabelMatchesQuery = - f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; const sectionId = f.section?.id; - const fieldSectionIsInRefSections = + const inRefSections = sectionId !== null && sectionId !== undefined && sectionIds.includes(sectionId); - return fieldIdOrLabelMatchesQuery && fieldSectionIsInRefSections; + return fieldMatchesQuery(f) && inRefSections; }); - - // If multiple fields match the query throw an error otherwise set first matching field - if (matchingFields.length > 1) { - throw new Error(errMultiple); - } - matchedField = matchingFields[0]; + matchedField = findSingleMatch(matchingFields); const matchingFiles = files.filter((f) => { - const fileIdOrNameMatchesQuery = - f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; const sectionId = f.section?.id; - const fileSectionIsInRefSections = + const inRefSections = sectionId !== null && sectionId !== undefined && sectionIds.includes(sectionId); - return fileIdOrNameMatchesQuery && fileSectionIsInRefSections; + return fileMatchesQuery(f) && inRefSections; }); - - // If multiple files match the query throw an error otherwise set first matching file - if (matchingFiles.length > 1) { - throw new Error(errMultiple); - } - matchedFile = matchingFiles[0]; + matchedFile = findSingleMatch(matchingFiles); } else { + // If the ref has no section filter search for matches with no section const matchingFields = fields.filter((f) => { - const fieldIdOrLabelMatchesQuery = - f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; - const fieldHasNoSection = + const hasNoSection = f.section?.id === null || f.section?.id === undefined; - return fieldIdOrLabelMatchesQuery && fieldHasNoSection; + return fieldMatchesQuery(f) && hasNoSection; }); + matchedField = findSingleMatch(matchingFields); - // If multiple fields match the query throw an error otherwise set first matching field - if (matchingFields.length > 1) { - throw new Error(errMultiple); - } - matchedField = matchingFields[0]; - - // If no field was found with no section, find a field in any section + // If no matches were found with no section, search for matches in any section if (!matchedField) { - const matchingFieldsInAnySection = fields.filter((f) => { - const fieldIdOrLabelMatchesQuery = - f.id === fieldOrFileQuery || f.label === fieldOrFileQuery; - return fieldIdOrLabelMatchesQuery; - }); - - if (matchingFieldsInAnySection.length > 1) { - throw new Error(errMultiple); - } - matchedField = matchingFieldsInAnySection[0]; + const matchingFieldsInAnySection = fields.filter(fieldMatchesQuery); + matchedField = findSingleMatch(matchingFieldsInAnySection); } const matchingFiles = files.filter((f) => { - const fileIdOrNameMatchesQuery = - f.id === fieldOrFileQuery || f.name === fieldOrFileQuery; - const fileHasNoSection = + const hasNoSection = f.section?.id === null || f.section?.id === undefined; - return fileIdOrNameMatchesQuery && fileHasNoSection; + return fileMatchesQuery(f) && hasNoSection; }); + matchedFile = findSingleMatch(matchingFiles); - // If multiple files match the query throw an error otherwise set first matching file - if (matchingFiles.length > 1) { - throw new Error(errMultiple); - } - matchedFile = matchingFiles[0]; - - // 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; - return fileIdOrNameMatchesQuery; - }); - - if (matchingFilesInAnySection.length > 1) { - throw new Error(errMultiple); - } - matchedFile = matchingFilesInAnySection[0]; + const matchingFilesInAnySection = files.filter(fileMatchesQuery); + matchedFile = findSingleMatch(matchingFilesInAnySection); } } if (matchedField && matchedFile) { throw new Error( - `you cannot query fields/files that are identically named.`, + `Both a field and a file match "${fieldOrFileQuery}". Rename one or use the ID in your op:// reference.`, ); } @@ -262,16 +221,24 @@ const findMatchingFieldAndFile = ( } if (matchedFile?.id) { - const fileId = matchedFile.id; - return { fileId }; + return { fileId: matchedFile.id }; } return {}; }; + +const findSingleMatch = (matches: T[]): T | undefined => { + if (matches.length > 1) { + throw new Error( + "Multiple matches found. Rename one or use an ID in your op:// reference.", + ); + } + return matches[0]; +}; // #endregion // #region Shared helpers and auth -const getEnvVarNamesWithSecretRefs = (): string[] => +export const getEnvVarNamesWithSecretRefs = (): string[] => Object.keys(process.env).filter( (key) => typeof process.env[key] === "string" && @@ -364,6 +331,29 @@ export const unsetPrevious = (): void => { } } }; + +const fetchVaultId = async ( + client: OPConnect, + vaultQuery: string, + ref: string, + cache: Map, +): Promise => { + // Check if the vault ID is already cached + const cached = cache.get(vaultQuery); + if (cached !== undefined) { + return cached; + } + + const vault = await client.getVault(vaultQuery); + if (!vault.id) { + throw new Error( + `Could not find valid vault "${vaultQuery}" for ref "${ref}"`, + ); + } + + cache.set(vaultQuery, vault.id); + return vault.id; +}; // #endregion // #region Load secrets @@ -397,25 +387,33 @@ const loadSecretsViaConnect = async ( throw new Error(`Connect authentication failed: ${message}`); } + const vaultIdByQuery = new Map(); + for (const envName of envs) { const ref = process.env[envName]; if (!ref) { continue; } - // Parse the op ref and get the item from the Connect SDK - const parsed = parseOpRef(ref); - const vault = await client.getVault(parsed.vault); - if (!vault.id) { - throw new Error( - `Could not find valid vault "${parsed.vault}" for ref "${ref}"`, - ); - } - const item = await client.getItem(vault.id, parsed.item); + try { + // Parse the op ref and get the item from the Connect SDK + const parsed = parseOpRef(ref); - // Get the secret value from the item as Connect returns a full item object - const secretValue = await getSecretFromConnectItem(client, item, parsed); - setResolvedSecret(envName, secretValue, shouldExportEnv); + const vaultId = await fetchVaultId( + client, + parsed.vault, + ref, + vaultIdByQuery, + ); + const item = await client.getItem(vaultId, parsed.item); + + // Get the secret value from the item as Connect returns a full item object + const secretValue = await getSecretFromConnectItem(client, item, parsed); + setResolvedSecret(envName, secretValue, shouldExportEnv); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + throw new Error(`Failed to load ref "${ref}": ${msg}`); + } } if (shouldExportEnv) {