Code clean up and add tests

This commit is contained in:
Jill Regan
2026-02-20 15:59:25 -05:00
parent 41f600a118
commit 3c643fe809
5 changed files with 454 additions and 101 deletions

View File

@@ -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);
}

View File

@@ -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:\/\/<vault>\/<item>\/<field>/,
);
expect(() => parseOpRef("op://a/b/c/d/e")).toThrow(
/use op:\/\/<vault>\/<item>\/<field>/,
);
});
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");
});
});

View File

@@ -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 = <T>(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<string, string>,
): Promise<string> => {
// 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<string, string>();
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) {