Skip to content

Commit e46031a

Browse files
authored
fix(scripts): don't reject miner package source files merely named with "secret" (#8266)
check-miner-package.ts's FORBIDDEN_PATH filter matched the keyword "secret" (or "private...key") anywhere in a path, including inside legitimately-named .js/.ts source files -- caught while shipping PR #8263, whose tenant-secret-resolution.ts had to be renamed to tenant-credential-resolution.ts to work around it. Splits the filter: exact-name dotfiles (.env/.npmrc/.dev.vars) stay forbidden regardless of extension, and the keyword heuristic now only applies to non-source files. Source files are exempt because a real leaked secret VALUE is already caught unconditionally by the separate FORBIDDEN_CONTENT scan regardless of filename -- the keyword check was only ever a coarse, filename-based backstop for stray credential-shaped files like .pem/.json.
1 parent 264f26a commit e46031a

2 files changed

Lines changed: 57 additions & 2 deletions

File tree

scripts/check-miner-package.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,21 @@ const REQUIRED = [
2929
"Dockerfile",
3030
"schema/miner-goal-spec.schema.json",
3131
];
32-
const FORBIDDEN_PATH = /(^|\/)(\.dev\.vars|\.env|\.npmrc|.*\.pem|.*private.*key.*|.*secret.*)$/i;
32+
// Exact-name dotfiles that are inherently credential-shaped regardless of extension.
33+
const FORBIDDEN_DOTFILE = /(^|\/)(\.dev\.vars|\.env|\.npmrc)$/i;
34+
// A filename merely SUGGESTING secret-shaped content -- a coarse heuristic that's meaningful for data/config
35+
// files (.json, .txt, .yml, .pem, etc.), where the filename is a fair proxy for what's likely inside, but not
36+
// for .js/.ts source (source files are exempted below): a descriptively-named source file that IMPLEMENTS
37+
// secret/credential-handling logic is normal and expected in this codebase, and a real leaked secret VALUE in
38+
// source is already caught unconditionally by FORBIDDEN_CONTENT below, regardless of the file's name.
39+
const FORBIDDEN_KEYWORD = /(^|\/)(.*\.pem|.*private.*key.*|.*secret.*)$/i;
40+
// .ts$ also matches .d.ts (which always ends in the literal characters ".ts").
41+
const SOURCE_FILE = /\.(js|ts)$/i;
42+
43+
function isForbiddenPath(file: string): boolean {
44+
return FORBIDDEN_DOTFILE.test(file) || (!SOURCE_FILE.test(file) && FORBIDDEN_KEYWORD.test(file));
45+
}
46+
3347
// Stale public-package wording the published README must never ship with (#7013). The sibling
3448
// check-mcp-package.ts has always guarded its README against this; the miner-package check did not, so a
3549
// pre-release "private beta"/"preview URL" phrasing could ship in the public `@loopover/miner` README unnoticed.
@@ -41,7 +55,7 @@ type ReadContentFn = (file: string) => string;
4155
export function validateMinerPackFileList(files: readonly PackedFile[], readContent: ReadContentFn): string[] {
4256
const paths = files.map((file) => (typeof file === "string" ? file : file.path)).sort();
4357
for (const file of paths) {
44-
if (FORBIDDEN_PATH.test(file)) throw new Error(`Forbidden file in miner package: ${file}`);
58+
if (isForbiddenPath(file)) throw new Error(`Forbidden file in miner package: ${file}`);
4559
if (!ALLOWED.some((pattern) => pattern.test(file))) throw new Error(`Unexpected file in miner package: ${file}`);
4660
const content = readContent(file);
4761
if (FORBIDDEN_CONTENT.test(content)) throw new Error(`Secret-like content found in miner package file: ${file}`);

test/unit/check-miner-package.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,47 @@ describe("check-miner-package script", () => {
3434
expect(result.out).toContain("Forbidden file in miner package: .env");
3535
});
3636

37+
describe("the secret-filename heuristic only applies to non-source files", () => {
38+
it("accepts a source file whose name merely mentions 'secret' or 'private key' (no leaked value inside)", () => {
39+
const result = runChecker({
40+
CHECK_MINER_PACK_TEST_FILES: JSON.stringify([
41+
"package.json",
42+
"bin/loopover-miner.js",
43+
"lib/secret-helper.js",
44+
"lib/secret-helper.d.ts",
45+
"lib/private-key-rotation.js",
46+
"DEPLOYMENT.md",
47+
"Dockerfile",
48+
"docs/coding-agent-driver.md",
49+
"schema/miner-goal-spec.schema.json",
50+
]),
51+
CHECK_MINER_PACK_TEST_CONTENT: "console.log('this file discusses secrets but holds no secret value');",
52+
});
53+
expect(result.status).toBe(0);
54+
expect(result.out).toMatch(/^Miner package dry-run ok:/);
55+
expect(result.out).toContain("lib/secret-helper.js");
56+
expect(result.out).toContain("lib/private-key-rotation.js");
57+
});
58+
59+
it("still rejects a non-code file whose name suggests it IS a secret (.pem, secrets.json)", () => {
60+
const pem = runChecker({ CHECK_MINER_PACK_TEST_FILES: JSON.stringify(["my-private-key.pem"]) });
61+
expect(pem.status).toBe(1);
62+
expect(pem.out).toContain("Forbidden file in miner package: my-private-key.pem");
63+
64+
const json = runChecker({ CHECK_MINER_PACK_TEST_FILES: JSON.stringify(["config/secrets.json"]) });
65+
expect(json.status).toBe(1);
66+
expect(json.out).toContain("Forbidden file in miner package: config/secrets.json");
67+
});
68+
69+
it("still rejects the exact-name dotfiles (.env/.npmrc/.dev.vars) regardless of this change", () => {
70+
for (const dotfile of [".env", ".npmrc", ".dev.vars"]) {
71+
const result = runChecker({ CHECK_MINER_PACK_TEST_FILES: JSON.stringify([dotfile]) });
72+
expect(result.status).toBe(1);
73+
expect(result.out).toContain(`Forbidden file in miner package: ${dotfile}`);
74+
}
75+
});
76+
});
77+
3778
it("rejects a README carrying stale public-package wording (#7013)", () => {
3879
const result = runChecker({
3980
CHECK_MINER_PACK_TEST_FILES: JSON.stringify([

0 commit comments

Comments
 (0)