feat: add path validation options to restore action

This commit is contained in:
Jason Ginchereau 2026-05-18 12:28:44 -10:00
parent 27d5ce7f10
commit dabc4c2ca1
25 changed files with 201047 additions and 164350 deletions

View file

@ -0,0 +1,119 @@
/**
* Local CommonJS stub for the `@actions/cache` toolkit package.
*
* The published toolkit is an ESM-only package, which Jest's CJS resolver
* cannot load directly. The action's runtime is bundled by `@vercel/ncc`
* (which handles ESM deps), but Jest tests run uncompiled and therefore
* need a CJS-compatible surface to import.
*
* This file re-implements just the public surface that the action's source
* code imports, with no-op implementations. Tests use `jest.spyOn` or
* `jest.mock("@actions/cache")` to override the implementations as needed.
*
* Wired up via `moduleNameMapper` in `jest.config.js`.
*
* Types are pulled from the real `@actions/cache` package via type-only
* imports, so a TypeScript build (via `tsc --noEmit` or ts-jest) verifies
* that the stub's runtime surface still satisfies the real package's
* signatures a signature drift (renamed parameter, added property,
* changed return type) will surface here as a compile error rather than
* as a silent test-only behavior change. `import type` is fully erased at
* compile time, so the Jest `moduleNameMapper` redirect for this file is
* not affected at runtime (no self-referential require loop).
*/
import type * as Cache from "@actions/cache";
// Re-export the toolkit's types so consumers of this stub and consumers of
// the real package see identical types — there is no second source of truth.
export type {
CacheIntegrityErrorCode,
DownloadOptions,
PathValidationMode,
PathValidationViolation,
UploadOptions
} from "@actions/cache";
// Each `typeof Cache.X` annotation forces the local implementation to be
// assignable to the real package's exported signature.
export const ValidationError: typeof Cache.ValidationError = class extends Error {
constructor(message: string) {
super(message);
this.name = "ValidationError";
}
};
export const ReserveCacheError: typeof Cache.ReserveCacheError = class extends Error {
constructor(message: string) {
super(message);
this.name = "ReserveCacheError";
}
};
export const FinalizeCacheError: typeof Cache.FinalizeCacheError = class extends Error {
constructor(message: string) {
super(message);
this.name = "FinalizeCacheError";
}
};
export const CacheIntegrityError: typeof Cache.CacheIntegrityError = class extends Error {
readonly code: Cache.CacheIntegrityErrorCode;
readonly violations?: Cache.PathValidationViolation[];
constructor(
code: Cache.CacheIntegrityErrorCode,
message: string,
violations?: Cache.PathValidationViolation[]
) {
super(message);
this.name = "CacheIntegrityError";
this.code = code;
this.violations = violations;
}
};
export const isFeatureAvailable: typeof Cache.isFeatureAvailable = () => true;
function checkKey(key: string): void {
if (key.length > 512) {
throw new ValidationError(
`Key Validation Error: ${key} cannot be larger than 512 characters.`
);
}
const regex = /^[^,]*$/;
if (!regex.test(key)) {
throw new ValidationError(
`Key Validation Error: ${key} cannot contain commas.`
);
}
}
export const restoreCache: typeof Cache.restoreCache = async (
_paths,
primaryKey,
restoreKeys,
_options,
_enableCrossOsArchive
) => {
const keys = [primaryKey, ...(restoreKeys ?? [])];
if (keys.length > 10) {
throw new ValidationError(
`Key Validation Error: Keys are limited to a maximum of 10.`
);
}
for (const key of keys) {
checkKey(key);
}
return undefined;
};
export const saveCache: typeof Cache.saveCache = async (
_paths,
key,
_options,
_enableCrossOsArchive
) => {
checkKey(key);
return -1;
};

View file

@ -265,3 +265,61 @@ test("isGhes returns true when the GITHUB_SERVER_URL environment variable is set
process.env["GITHUB_SERVER_URL"] = "https://src.onpremise.fabrikam.com";
expect(actionUtils.isGhes()).toBeTruthy();
});
describe("getPathValidationInput", () => {
const inputEnv = "INPUT_STRICT-PATHS";
beforeEach(() => {
delete process.env[inputEnv];
// Re-mock getInput so the each-test environment reads INPUT_STRICT-PATHS
jest.spyOn(core, "getInput").mockImplementation((name, options) => {
return jest.requireActual("@actions/core").getInput(name, options);
});
});
afterEach(() => {
delete process.env[inputEnv];
});
test("returns 'warn' when input is unset", () => {
expect(actionUtils.getPathValidationInput()).toBe("warn");
});
test.each([
["off", "off"],
["warn", "warn"],
["error", "error"],
["OFF", "off"],
["Warn", "warn"],
["ERROR", "error"]
])("normalizes %s to %s", (input, expected) => {
process.env[inputEnv] = input;
expect(actionUtils.getPathValidationInput()).toBe(expected);
});
test("falls back to 'warn' for unrecognized values and logs a warning", () => {
process.env[inputEnv] = "strict";
// getPathValidationInput() calls the same module's logWarning() via a
// local function reference (TypeScript compiles intra-module calls as
// direct references, not exports.X lookups), so a jest.spyOn on
// actionUtils.logWarning cannot intercept it. Spy on core.info — the
// only externally observable side effect — and suppress the real
// implementation so the warning does not pollute the Jest log.
const infoSpy = jest
.spyOn(core, "info")
.mockImplementation(() => undefined);
try {
expect(actionUtils.getPathValidationInput()).toBe("warn");
expect(infoSpy).toHaveBeenCalledWith(
expect.stringContaining("Unrecognized value for strict-paths")
);
} finally {
infoSpy.mockRestore();
}
});
test("treats empty string as default 'warn'", () => {
process.env[inputEnv] = "";
expect(actionUtils.getPathValidationInput()).toBe("warn");
});
});

View file

@ -0,0 +1,45 @@
// @ts-check
/**
* save-poisoned-cache.mjs
*
* Helper script used by the path-validation E2E workflow to upload a cache
* archive that contains entries outside the declared `path` inputs. This
* simulates a poisoned cache that would have been produced by a build job
* that had write access to the workspace's parent directory (the canonical
* cache-poisoning scenario being defended against).
*
* Usage:
* node save-poisoned-cache.mjs <cache-key> <declared-path> [extra-path ...]
*
* The script invokes `@actions/cache.saveCache()` with the declared path(s)
* AND extra paths that escape the workspace. The toolkit's saveCache packs
* everything into the archive, so the resulting cache entry will contain
* "escape" entries that resolve outside the declared `path` when the action's
* `restore` step later extracts it (because the restore step only declares the
* legitimate `path`).
*
* Important: this script is NOT shipped to users. It is purely a test fixture
* generator used by the E2E workflow to validate that the action's client-side
* validation correctly rejects (or warns about) such caches.
*/
import * as cache from '@actions/cache';
const [, , key, ...paths] = process.argv;
if (!key || paths.length === 0) {
console.error(
'Usage: node save-poisoned-cache.mjs <cache-key> <path> [extra-path ...]'
);
process.exit(2);
}
console.log(`Saving poisoned cache with key="${key}" paths=${JSON.stringify(paths)}`);
try {
const cacheId = await cache.saveCache(paths, key);
console.log(`Saved poisoned cache (cacheId=${cacheId})`);
} catch (err) {
console.error(`Failed to save poisoned cache: ${err?.message ?? err}`);
process.exit(1);
}

View file

@ -34,6 +34,11 @@ beforeAll(() => {
return actualUtils.getInputAsBool(name, options);
}
);
jest.spyOn(actionUtils, "getPathValidationInput").mockImplementation(() => {
const actualUtils = jest.requireActual("../src/utils/actionUtils");
return actualUtils.getPathValidationInput();
});
});
beforeEach(() => {
@ -79,7 +84,8 @@ test("restore with no cache found", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -122,7 +128,8 @@ test("restore with restore keys and no cache found", async () => {
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -164,7 +171,8 @@ test("restore with cache found for key", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -209,7 +217,8 @@ test("restore with cache found for restore key", async () => {
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -254,7 +263,8 @@ test("Fail restore when fail on cache miss is enabled and primary + restore keys
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -297,7 +307,8 @@ test("restore when fail on cache miss is enabled and primary key doesn't match r
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -343,7 +354,8 @@ test("restore with fail on cache miss disabled and no cache found", async () =>
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);

View file

@ -35,6 +35,16 @@ beforeAll(() => {
return actualUtils.getInputAsBool(name, options);
}
);
jest.spyOn(actionUtils, "getPathValidationInput").mockImplementation(() => {
const actualUtils = jest.requireActual("../src/utils/actionUtils");
return actualUtils.getPathValidationInput();
});
jest.spyOn(actionUtils, "logWarning").mockImplementation(message => {
const actualUtils = jest.requireActual("../src/utils/actionUtils");
return actualUtils.logWarning(message);
});
});
beforeEach(() => {
@ -127,7 +137,8 @@ test("restore on GHES with AC available ", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -181,7 +192,8 @@ test("restore with too many keys should fail", async () => {
key,
restoreKeys,
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -207,7 +219,8 @@ test("restore with large key should fail", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -233,7 +246,8 @@ test("restore with invalid key should fail", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -268,7 +282,8 @@ test("restore with no cache found", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -309,7 +324,8 @@ test("restore with restore keys and no cache found", async () => {
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -349,7 +365,8 @@ test("restore with cache found for key", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -391,7 +408,8 @@ test("restore with cache found for restore key", async () => {
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -432,7 +450,8 @@ test("restore with lookup-only set", async () => {
key,
[],
{
lookupOnly: true
lookupOnly: true,
pathValidation: "warn"
},
false
);
@ -465,3 +484,246 @@ test("restore failure with earlyExit should call process exit", async () => {
);
expect(processExitMock).toHaveBeenCalledWith(1);
});
// ---------------------------------------------------------------------------
// Path validation tests
//
// These tests verify that the action correctly forwards the `strict-paths`
// input to the @actions/cache toolkit and handles `CacheIntegrityError`
// rejections according to the `fail-on-cache-invalid` input.
// ---------------------------------------------------------------------------
test("restore defaults strict-paths to 'warn' and forwards it to restoreCache", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({ path, key });
const restoreCacheMock = jest
.spyOn(cache, "restoreCache")
.mockResolvedValueOnce(key);
await restoreImpl(new StateProvider());
expect(restoreCacheMock).toHaveBeenCalledTimes(1);
expect(restoreCacheMock).toHaveBeenCalledWith(
[path],
key,
[],
{ lookupOnly: false, pathValidation: "warn" },
false
);
});
test.each(["off", "warn", "error"])(
"restore forwards strict-paths value '%s' to restoreCache",
async value => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({ path, key, strictPaths: value });
const restoreCacheMock = jest
.spyOn(cache, "restoreCache")
.mockResolvedValueOnce(key);
await restoreImpl(new StateProvider());
expect(restoreCacheMock).toHaveBeenCalledTimes(1);
expect(restoreCacheMock).toHaveBeenCalledWith(
[path],
key,
[],
{ lookupOnly: false, pathValidation: value },
false
);
}
);
test("restore falls back to 'warn' when strict-paths input is unrecognized", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({ path, key, strictPaths: "STRICT" });
const restoreCacheMock = jest
.spyOn(cache, "restoreCache")
.mockResolvedValueOnce(key);
// getPathValidationInput()'s call to logWarning() is intra-module so a
// spy on actionUtils.logWarning would not intercept it. Spy on core.info
// (the underlying transport for logWarning) and suppress the real
// implementation so the warning does not print into the Jest log.
const infoMock = jest
.spyOn(core, "info")
.mockImplementation(() => undefined);
await restoreImpl(new StateProvider());
expect(restoreCacheMock).toHaveBeenCalledWith(
[path],
key,
[],
{ lookupOnly: false, pathValidation: "warn" },
false
);
expect(infoMock).toHaveBeenCalledWith(
expect.stringContaining("Unrecognized value for strict-paths")
);
});
test("restore treats CacheIntegrityError as a cache miss by default", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({ path, key, strictPaths: "error" });
const integrityError = new Error("entries escape declared paths");
integrityError.name = "CacheIntegrityError";
(integrityError as Error & { code?: string }).code = "PATH_VIOLATION";
const restoreCacheMock = jest
.spyOn(cache, "restoreCache")
.mockRejectedValueOnce(integrityError);
const setOutputMock = jest.spyOn(core, "setOutput");
const failedMock = jest.spyOn(core, "setFailed");
// Suppress the real logWarning so the discarded-cache warning does not
// pollute test output. beforeEach's jest.restoreAllMocks() handles
// cross-test cleanup.
const logWarningMock = jest
.spyOn(actionUtils, "logWarning")
.mockImplementation(() => undefined);
await restoreImpl(new StateProvider());
expect(restoreCacheMock).toHaveBeenCalledTimes(1);
// Intentionally NOT set: a discarded cache must look identical to a
// regular cache miss to downstream `if:` checks (see issue #1466).
const cacheHitCalls = setOutputMock.mock.calls.filter(
c => c[0] === "cache-hit"
);
expect(cacheHitCalls).toHaveLength(0);
expect(failedMock).not.toHaveBeenCalled();
expect(logWarningMock).toHaveBeenCalledWith(
expect.stringContaining("PATH_VIOLATION")
);
});
test("restore fails when CacheIntegrityError is raised and fail-on-cache-invalid is true", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({
path,
key,
strictPaths: "error",
failOnCacheInvalid: true
});
const integrityError = new Error("entry escapes workspace");
integrityError.name = "CacheIntegrityError";
(integrityError as Error & { code?: string }).code = "PATH_VIOLATION";
jest.spyOn(cache, "restoreCache").mockRejectedValueOnce(integrityError);
const failedMock = jest.spyOn(core, "setFailed");
await restoreImpl(new StateProvider());
expect(failedMock).toHaveBeenCalledTimes(1);
expect(failedMock.mock.calls[0][0]).toContain("integrity validation");
expect(failedMock.mock.calls[0][0]).toContain("PATH_VIOLATION");
});
test("restore propagates non-integrity errors normally", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({ path, key });
const networkError = new Error("Network timeout");
jest.spyOn(cache, "restoreCache").mockRejectedValueOnce(networkError);
const failedMock = jest.spyOn(core, "setFailed");
const logWarningMock = jest
.spyOn(actionUtils, "logWarning")
.mockImplementation(() => undefined);
await restoreImpl(new StateProvider());
expect(failedMock).toHaveBeenCalledWith("Network timeout");
expect(logWarningMock).not.toHaveBeenCalledWith(
expect.stringContaining("integrity")
);
});
test("restore parse-error integrity failure also treated as miss by default", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({ path, key, strictPaths: "error" });
const parseError = new Error("malformed gzip header");
parseError.name = "CacheIntegrityError";
(parseError as Error & { code?: string }).code = "PARSE_ERROR";
jest.spyOn(cache, "restoreCache").mockRejectedValueOnce(parseError);
const setOutputMock = jest.spyOn(core, "setOutput");
const failedMock = jest.spyOn(core, "setFailed");
const logWarningMock = jest
.spyOn(actionUtils, "logWarning")
.mockImplementation(() => undefined);
await restoreImpl(new StateProvider());
const cacheHitCalls = setOutputMock.mock.calls.filter(
c => c[0] === "cache-hit"
);
expect(cacheHitCalls).toHaveLength(0);
expect(failedMock).not.toHaveBeenCalled();
expect(logWarningMock).toHaveBeenCalledWith(
expect.stringContaining("PARSE_ERROR")
);
});
test("restore tolerates CacheIntegrityError without explicit code", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({ path, key, strictPaths: "error" });
const integrityError = new Error("bad archive");
integrityError.name = "CacheIntegrityError";
// intentionally no .code property
jest.spyOn(cache, "restoreCache").mockRejectedValueOnce(integrityError);
const setOutputMock = jest.spyOn(core, "setOutput");
const logWarningMock = jest
.spyOn(actionUtils, "logWarning")
.mockImplementation(() => undefined);
await restoreImpl(new StateProvider());
const cacheHitCalls = setOutputMock.mock.calls.filter(
c => c[0] === "cache-hit"
);
expect(cacheHitCalls).toHaveLength(0);
expect(logWarningMock).toHaveBeenCalledWith(
expect.stringContaining("unknown")
);
});
test("restore does not set cache-hit output when integrity error is rethrown", async () => {
const path = "node_modules";
const key = "node-test";
testUtils.setInputs({
path,
key,
strictPaths: "error",
failOnCacheInvalid: true
});
const integrityError = new Error("rejected");
integrityError.name = "CacheIntegrityError";
(integrityError as Error & { code?: string }).code = "PATH_VIOLATION";
jest.spyOn(cache, "restoreCache").mockRejectedValueOnce(integrityError);
const setOutputMock = jest.spyOn(core, "setOutput");
await restoreImpl(new StateProvider());
// setOutput should NOT have been called with cache-hit at all in this path
const cacheHitCalls = setOutputMock.mock.calls.filter(
c => c[0] === "cache-hit"
);
expect(cacheHitCalls).toHaveLength(0);
});

View file

@ -35,6 +35,11 @@ beforeAll(() => {
.getInputAsBool(name, options);
}
);
jest.spyOn(actionUtils, "getPathValidationInput").mockImplementation(() => {
const actualUtils = jest.requireActual("../src/utils/actionUtils");
return actualUtils.getPathValidationInput();
});
});
beforeEach(() => {
@ -80,7 +85,8 @@ test("restore with no cache found", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -122,7 +128,8 @@ test("restore with restore keys and no cache found", async () => {
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -161,7 +168,8 @@ test("restore with cache found for key", async () => {
key,
[],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);
@ -204,7 +212,8 @@ test("restore with cache found for restore key", async () => {
key,
[restoreKey],
{
lookupOnly: false
lookupOnly: false,
pathValidation: "warn"
},
false
);