Colin McDonnell
d5f881e9fc
* action: trim sensitive env values before GitHub Actions log masking GitHub Actions' log masking is line-based: a secret value containing a newline only registers the first line as a mask, leaving the remainder exposed verbatim in logs. A trailing newline copied from a terminal into a GitHub Actions secret (e.g. ANTHROPIC_API_KEY) was enough to leak "a large part of the key" in run logs (pullfrog/pullfrog#41). normalizeEnv now trims leading/trailing whitespace from any value whose key matches the sensitive name pattern, masks the cleaned value, and warns when whitespace was stripped so the user notices the source. sanitizeSecret is reused for dbSecrets injection in main.ts. The three secret-store PUT/POST routes also trim values defensively, matching the existing name.trim() pattern. Real multi-line secrets are not used in practice — even GITHUB_PRIVATE_KEY PEMs are stored single-line with escaped \n and unescaped at the point of use — so a straight trim() is safe. * action: address review — use core.setSecret for masking, don't zero whitespace-only Pullfrog's review of #698 caught two real issues in the original fix: 1. `console.log(\`::add-mask::\${trimmed}\`)` doesn't escape \r/\n. If a value survives trim with an embedded newline (PEMs, kubeconfigs, JSON), the runner only registers the first line as a mask and the rest leaks. `core.setSecret(trimmed)` routes through @actions/core which percent-encodes \r/\n so the runner V2 parser decodes back to the full value and registers every non-empty line as a separate mask. Removes the load-bearing "no embedded newlines" invariant from the fix. 2. Whitespace-only sensitive values silently became "". Downstream truthy checks would flip from "set" to "missing" with no log. Now sanitizeSecret returns null in that case and callers skip the process.env write, surfacing a clear missing-key error instead. Tests rewritten to assert process.env state directly — no stdout spies. Masking correctness is delegated to @actions/core (trusted dependency).
92 lines
3.5 KiB
TypeScript
92 lines
3.5 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
|
import { normalizeEnv, sanitizeSecret } from "./normalizeEnv.ts";
|
|
|
|
/**
|
|
* These tests pin the load-bearing invariants of secret sanitisation:
|
|
* - sensitive values are trimmed before downstream code reads them
|
|
* - whitespace-only values are NOT silently zeroed (leave env unchanged)
|
|
* - case normalisation still happens
|
|
*
|
|
* Masking (`core.setSecret`) is delegated to `@actions/core` and trusted to
|
|
* work as documented — we don't spy on stdout to re-test the toolkit.
|
|
*/
|
|
|
|
describe("normalizeEnv: process.env state contract", () => {
|
|
let originalEnv: NodeJS.ProcessEnv;
|
|
|
|
beforeEach(() => {
|
|
// normalizeEnv() iterates the entire process.env, so the test must
|
|
// control it. snapshot + full wipe + restore is the cleanest isolation.
|
|
originalEnv = { ...process.env };
|
|
for (const k of Object.keys(process.env)) delete process.env[k];
|
|
});
|
|
|
|
afterEach(() => {
|
|
for (const k of Object.keys(process.env)) delete process.env[k];
|
|
Object.assign(process.env, originalEnv);
|
|
});
|
|
|
|
it("trims trailing newline from sensitive env vars", () => {
|
|
process.env.ANTHROPIC_API_KEY = "sk-ant-secret-value\n";
|
|
normalizeEnv();
|
|
expect(process.env.ANTHROPIC_API_KEY).toBe("sk-ant-secret-value");
|
|
});
|
|
|
|
it("trims surrounding whitespace including \\r\\n and spaces", () => {
|
|
process.env.OPENAI_API_KEY = " sk-openai-value\r\n ";
|
|
normalizeEnv();
|
|
expect(process.env.OPENAI_API_KEY).toBe("sk-openai-value");
|
|
});
|
|
|
|
it("leaves clean sensitive values untouched", () => {
|
|
process.env.ANTHROPIC_API_KEY = "sk-ant-clean";
|
|
normalizeEnv();
|
|
expect(process.env.ANTHROPIC_API_KEY).toBe("sk-ant-clean");
|
|
});
|
|
|
|
it("ignores non-sensitive env vars", () => {
|
|
process.env.NODE_ENV = "production\n";
|
|
normalizeEnv();
|
|
expect(process.env.NODE_ENV).toBe("production\n");
|
|
});
|
|
|
|
it("canonicalises case and trims the value", () => {
|
|
process.env.anthropic_api_key = "sk-ant-lowercase\n";
|
|
normalizeEnv();
|
|
expect(process.env.ANTHROPIC_API_KEY).toBe("sk-ant-lowercase");
|
|
expect(process.env.anthropic_api_key).toBeUndefined();
|
|
});
|
|
|
|
it("preserves whitespace-only values rather than silently zeroing them", () => {
|
|
// contract: don't mutate when value is whitespace-only. caller sees the
|
|
// misconfigured value verbatim and either fails clearly downstream or
|
|
// logs a missing-key error.
|
|
process.env.ANTHROPIC_API_KEY = " \n ";
|
|
normalizeEnv();
|
|
expect(process.env.ANTHROPIC_API_KEY).toBe(" \n ");
|
|
});
|
|
|
|
it("preserves embedded newlines (toolkit masks each line)", () => {
|
|
// multi-line PEMs aren't used in practice, but if one slipped in via a
|
|
// DB secret we don't want to silently mutate it. trim() only touches
|
|
// the ends; @actions/core handles per-line masking via the runner.
|
|
process.env.ANTHROPIC_API_KEY = "line1\nline2";
|
|
normalizeEnv();
|
|
expect(process.env.ANTHROPIC_API_KEY).toBe("line1\nline2");
|
|
});
|
|
});
|
|
|
|
describe("sanitizeSecret return value", () => {
|
|
it("returns the trimmed value for a sensitive secret with trailing newline", () => {
|
|
expect(sanitizeSecret("ANTHROPIC_API_KEY", "sk-ant-secret\n")).toBe("sk-ant-secret");
|
|
});
|
|
|
|
it("returns the value unchanged when no trimming is needed", () => {
|
|
expect(sanitizeSecret("ANTHROPIC_API_KEY", "sk-ant-clean")).toBe("sk-ant-clean");
|
|
});
|
|
|
|
it("returns null for whitespace-only input so caller can skip injection", () => {
|
|
expect(sanitizeSecret("ANTHROPIC_API_KEY", " \n")).toBeNull();
|
|
});
|
|
});
|