refactor(test): consolidate regression and override tests into #666 test files
Move regression tests and override tests from standalone files into the existing test files introduced by PR #666: - resolve-config-value.test.ts: add REGRESSION #666 describe block and setAllowedCommandPrefixes override tests - url-utils.test.ts: add REGRESSION #666 describe block and setFetchAllowedUrls override tests - Delete: regression-666.test.ts, resolve-config-value-override.test.ts, url-utils-override.test.ts Same 59 tests, fewer files, tests live next to the code they test.
This commit is contained in:
Justin Wyer
parent
d5f581fe6b
commit
95875c41c5
5 changed files with 180 additions and 256 deletions
|
|
@ -1,86 +0,0 @@
|
|||
import { describe, it, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
resolveConfigValue,
|
||||
clearConfigValueCache,
|
||||
SAFE_COMMAND_PREFIXES,
|
||||
setAllowedCommandPrefixes,
|
||||
getAllowedCommandPrefixes,
|
||||
} from "./resolve-config-value.js";
|
||||
|
||||
describe("setAllowedCommandPrefixes — user override", () => {
|
||||
beforeEach(() => {
|
||||
clearConfigValueCache();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore defaults after each test
|
||||
setAllowedCommandPrefixes(SAFE_COMMAND_PREFIXES);
|
||||
clearConfigValueCache();
|
||||
});
|
||||
|
||||
it("overrides built-in prefixes with custom list", () => {
|
||||
setAllowedCommandPrefixes(["sops", "doppler"]);
|
||||
assert.deepEqual([...getAllowedCommandPrefixes()], ["sops", "doppler"]);
|
||||
});
|
||||
|
||||
it("custom prefix is allowed through to execution", (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
setAllowedCommandPrefixes(["mycli"]);
|
||||
resolveConfigValue("!mycli get-secret");
|
||||
const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command"));
|
||||
assert.equal(blocked, false, "mycli should not be blocked when in the custom allowlist");
|
||||
});
|
||||
|
||||
it("previously-allowed prefix is blocked after override", (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
// 'pass' is in the default list
|
||||
setAllowedCommandPrefixes(["sops"]);
|
||||
const result = resolveConfigValue("!pass show secret");
|
||||
assert.equal(result, undefined);
|
||||
const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command"));
|
||||
assert.equal(blocked, true, "pass should be blocked when not in the custom allowlist");
|
||||
});
|
||||
|
||||
it("clears cache when overriding prefixes", (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
// First: block 'mycli' under defaults
|
||||
resolveConfigValue("!mycli get-secret");
|
||||
assert.ok(stderrChunks.some((line) => line.includes("Blocked")));
|
||||
|
||||
stderrChunks.length = 0;
|
||||
|
||||
// Now allow it — cache should be cleared so re-evaluation happens
|
||||
setAllowedCommandPrefixes(["mycli"]);
|
||||
resolveConfigValue("!mycli get-secret");
|
||||
const blocked = stderrChunks.some((line) => line.includes("Blocked"));
|
||||
assert.equal(blocked, false, "Should re-evaluate after allowlist change");
|
||||
});
|
||||
});
|
||||
|
|
@ -1,9 +1,11 @@
|
|||
import { describe, it, beforeEach } from "node:test";
|
||||
import { describe, it, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
resolveConfigValue,
|
||||
clearConfigValueCache,
|
||||
SAFE_COMMAND_PREFIXES,
|
||||
setAllowedCommandPrefixes,
|
||||
getAllowedCommandPrefixes,
|
||||
} from "./resolve-config-value.js";
|
||||
|
||||
beforeEach(() => {
|
||||
|
|
@ -183,3 +185,111 @@ describe("resolveConfigValue — caching", () => {
|
|||
assert.equal(stderrChunks.length, 2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("REGRESSION #666: non-default credential tool blocked with no override", () => {
|
||||
afterEach(() => {
|
||||
setAllowedCommandPrefixes(SAFE_COMMAND_PREFIXES);
|
||||
clearConfigValueCache();
|
||||
});
|
||||
|
||||
it("sops is blocked by default, then unblocked by setAllowedCommandPrefixes", (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
// Bug: sops is not in SAFE_COMMAND_PREFIXES, so it's blocked
|
||||
const result = resolveConfigValue("!sops decrypt --output-type json secrets.enc.json");
|
||||
assert.equal(result, undefined, "sops is blocked by the hardcoded allowlist");
|
||||
assert.ok(
|
||||
stderrChunks.some((line) => line.includes('Blocked disallowed command: "sops"')),
|
||||
"should log a block message for sops",
|
||||
);
|
||||
|
||||
stderrChunks.length = 0;
|
||||
clearConfigValueCache();
|
||||
|
||||
// Fix: override the allowlist to include sops
|
||||
setAllowedCommandPrefixes([...SAFE_COMMAND_PREFIXES, "sops"]);
|
||||
resolveConfigValue("!sops decrypt --output-type json secrets.enc.json");
|
||||
|
||||
const blockedAfterOverride = stderrChunks.some((line) =>
|
||||
line.includes("Blocked disallowed command"),
|
||||
);
|
||||
assert.equal(blockedAfterOverride, false, "sops must not be blocked after override");
|
||||
});
|
||||
});
|
||||
|
||||
describe("setAllowedCommandPrefixes — user override", () => {
|
||||
afterEach(() => {
|
||||
setAllowedCommandPrefixes(SAFE_COMMAND_PREFIXES);
|
||||
clearConfigValueCache();
|
||||
});
|
||||
|
||||
it("overrides built-in prefixes with custom list", () => {
|
||||
setAllowedCommandPrefixes(["sops", "doppler"]);
|
||||
assert.deepEqual([...getAllowedCommandPrefixes()], ["sops", "doppler"]);
|
||||
});
|
||||
|
||||
it("custom prefix is allowed through to execution", (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
setAllowedCommandPrefixes(["mycli"]);
|
||||
resolveConfigValue("!mycli get-secret");
|
||||
const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command"));
|
||||
assert.equal(blocked, false, "mycli should not be blocked when in the custom allowlist");
|
||||
});
|
||||
|
||||
it("previously-allowed prefix is blocked after override", (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
setAllowedCommandPrefixes(["sops"]);
|
||||
const result = resolveConfigValue("!pass show secret");
|
||||
assert.equal(result, undefined);
|
||||
const blocked = stderrChunks.some((line) => line.includes("Blocked disallowed command"));
|
||||
assert.equal(blocked, true, "pass should be blocked when not in the custom allowlist");
|
||||
});
|
||||
|
||||
it("clears cache when overriding prefixes", (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
resolveConfigValue("!mycli get-secret");
|
||||
assert.ok(stderrChunks.some((line) => line.includes("Blocked")));
|
||||
|
||||
stderrChunks.length = 0;
|
||||
|
||||
setAllowedCommandPrefixes(["mycli"]);
|
||||
resolveConfigValue("!mycli get-secret");
|
||||
const blocked = stderrChunks.some((line) => line.includes("Blocked"));
|
||||
assert.equal(blocked, false, "Should re-evaluate after allowlist change");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,113 +0,0 @@
|
|||
/**
|
||||
* Regression tests for the bug introduced by PR #666.
|
||||
*
|
||||
* Bug: PR #666 added hardcoded SAFE_COMMAND_PREFIXES and isBlockedUrl() with
|
||||
* no override mechanism. Users with non-default credential tools (sops, doppler,
|
||||
* age, etc.) or needing to fetch from internal URLs were silently broken.
|
||||
*
|
||||
* These tests import only APIs that exist on both main and this branch.
|
||||
* On main (before fix): they FAIL because the override functions don't exist.
|
||||
* On this branch (after fix): they PASS because overrides work.
|
||||
*/
|
||||
import { describe, it, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
resolveConfigValue,
|
||||
clearConfigValueCache,
|
||||
SAFE_COMMAND_PREFIXES,
|
||||
} from "../../packages/pi-coding-agent/src/core/resolve-config-value.ts";
|
||||
import {
|
||||
isBlockedUrl,
|
||||
} from "../resources/extensions/search-the-web/url-utils.ts";
|
||||
|
||||
describe("REGRESSION #666: hardcoded security lists with no override", () => {
|
||||
beforeEach(() => {
|
||||
clearConfigValueCache();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore defaults — setAllowedCommandPrefixes/setFetchAllowedUrls are
|
||||
// dynamically imported so we can restore even if they exist.
|
||||
import("../../packages/pi-coding-agent/src/core/resolve-config-value.ts").then((mod) => {
|
||||
if (typeof mod.setAllowedCommandPrefixes === "function") {
|
||||
mod.setAllowedCommandPrefixes(SAFE_COMMAND_PREFIXES);
|
||||
}
|
||||
mod.clearConfigValueCache();
|
||||
});
|
||||
import("../resources/extensions/search-the-web/url-utils.ts").then((mod) => {
|
||||
if (typeof mod.setFetchAllowedUrls === "function") {
|
||||
mod.setFetchAllowedUrls([]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("non-default credential tool (sops) can be unblocked via override", async (t) => {
|
||||
const stderrChunks: string[] = [];
|
||||
const originalWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk: string | Uint8Array, ...args: unknown[]) => {
|
||||
stderrChunks.push(chunk.toString());
|
||||
return true;
|
||||
};
|
||||
t.after(() => {
|
||||
process.stderr.write = originalWrite;
|
||||
});
|
||||
|
||||
// Confirm the bug: sops is not in the hardcoded allowlist, so it's blocked
|
||||
const blocked = resolveConfigValue("!sops decrypt --output-type json secrets.enc.json");
|
||||
assert.equal(blocked, undefined, "sops is blocked by the hardcoded allowlist");
|
||||
assert.ok(
|
||||
stderrChunks.some((line) => line.includes('Blocked disallowed command: "sops"')),
|
||||
"should log a block message for sops",
|
||||
);
|
||||
|
||||
stderrChunks.length = 0;
|
||||
clearConfigValueCache();
|
||||
|
||||
// The fix: setAllowedCommandPrefixes must exist and must unblock sops
|
||||
const mod = await import("../../packages/pi-coding-agent/src/core/resolve-config-value.ts");
|
||||
assert.equal(
|
||||
typeof mod.setAllowedCommandPrefixes,
|
||||
"function",
|
||||
"setAllowedCommandPrefixes must be exported (missing = bug #666 not fixed)",
|
||||
);
|
||||
|
||||
mod.setAllowedCommandPrefixes([...SAFE_COMMAND_PREFIXES, "sops"]);
|
||||
resolveConfigValue("!sops decrypt --output-type json secrets.enc.json");
|
||||
|
||||
const blockedAfterOverride = stderrChunks.some((line) =>
|
||||
line.includes("Blocked disallowed command"),
|
||||
);
|
||||
assert.equal(
|
||||
blockedAfterOverride,
|
||||
false,
|
||||
"sops must not be blocked after adding it to the allowlist",
|
||||
);
|
||||
});
|
||||
|
||||
it("internal company URL can be unblocked via override", async () => {
|
||||
const internalUrl = "http://192.168.1.100/internal-docs/api-reference";
|
||||
|
||||
// Confirm the bug: private IP is blocked with no way to allowlist
|
||||
assert.equal(
|
||||
isBlockedUrl(internalUrl),
|
||||
true,
|
||||
"private IP is blocked by the hardcoded SSRF blocklist",
|
||||
);
|
||||
|
||||
// The fix: setFetchAllowedUrls must exist and must unblock the host
|
||||
const mod = await import("../resources/extensions/search-the-web/url-utils.ts");
|
||||
assert.equal(
|
||||
typeof mod.setFetchAllowedUrls,
|
||||
"function",
|
||||
"setFetchAllowedUrls must be exported (missing = bug #666 not fixed)",
|
||||
);
|
||||
|
||||
mod.setFetchAllowedUrls(["192.168.1.100"]);
|
||||
|
||||
assert.equal(
|
||||
isBlockedUrl(internalUrl),
|
||||
false,
|
||||
"private IP must not be blocked after adding it to the allowlist",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,54 +0,0 @@
|
|||
import { describe, it, beforeEach, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { isBlockedUrl, setFetchAllowedUrls, getFetchAllowedUrls } from "../resources/extensions/search-the-web/url-utils.ts";
|
||||
|
||||
describe("setFetchAllowedUrls — user override", () => {
|
||||
afterEach(() => {
|
||||
// Reset to empty allowlist after each test
|
||||
setFetchAllowedUrls([]);
|
||||
});
|
||||
|
||||
it("defaults to empty allowlist", () => {
|
||||
assert.deepEqual(getFetchAllowedUrls(), []);
|
||||
});
|
||||
|
||||
it("exempts an allowed hostname from blocking", () => {
|
||||
assert.equal(isBlockedUrl("http://192.168.1.100/docs"), true, "blocked by default");
|
||||
setFetchAllowedUrls(["192.168.1.100"]);
|
||||
assert.equal(isBlockedUrl("http://192.168.1.100/docs"), false, "allowed after override");
|
||||
});
|
||||
|
||||
it("exempts localhost when explicitly allowed", () => {
|
||||
assert.equal(isBlockedUrl("http://localhost:3000/api"), true, "blocked by default");
|
||||
setFetchAllowedUrls(["localhost"]);
|
||||
assert.equal(isBlockedUrl("http://localhost:3000/api"), false, "allowed after override");
|
||||
});
|
||||
|
||||
it("exempts cloud metadata hostname when allowed", () => {
|
||||
assert.equal(isBlockedUrl("http://metadata.google.internal/computeMetadata/"), true, "blocked by default");
|
||||
setFetchAllowedUrls(["metadata.google.internal"]);
|
||||
assert.equal(isBlockedUrl("http://metadata.google.internal/computeMetadata/"), false, "allowed after override");
|
||||
});
|
||||
|
||||
it("does not affect URLs not in the allowlist", () => {
|
||||
setFetchAllowedUrls(["192.168.1.100"]);
|
||||
assert.equal(isBlockedUrl("http://192.168.1.200/secret"), true, "other private IPs still blocked");
|
||||
assert.equal(isBlockedUrl("http://localhost/admin"), true, "localhost still blocked");
|
||||
});
|
||||
|
||||
it("still allows public URLs without configuration", () => {
|
||||
setFetchAllowedUrls(["192.168.1.100"]);
|
||||
assert.equal(isBlockedUrl("https://example.com"), false);
|
||||
});
|
||||
|
||||
it("still blocks non-HTTP protocols even with allowlist", () => {
|
||||
setFetchAllowedUrls(["localhost"]);
|
||||
assert.equal(isBlockedUrl("file:///etc/passwd"), true, "file:// still blocked");
|
||||
assert.equal(isBlockedUrl("ftp://localhost/data"), true, "ftp:// still blocked");
|
||||
});
|
||||
|
||||
it("is case-insensitive for hostnames", () => {
|
||||
setFetchAllowedUrls(["MyHost.Internal"]);
|
||||
assert.equal(isBlockedUrl("http://myhost.internal/api"), false);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, it } from "node:test";
|
||||
import { describe, it, afterEach } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { isBlockedUrl } from "../resources/extensions/search-the-web/url-utils.ts";
|
||||
import { isBlockedUrl, setFetchAllowedUrls, getFetchAllowedUrls } from "../resources/extensions/search-the-web/url-utils.ts";
|
||||
|
||||
describe("isBlockedUrl — SSRF protection", () => {
|
||||
it("blocks localhost", () => {
|
||||
|
|
@ -57,3 +57,70 @@ describe("isBlockedUrl — SSRF protection", () => {
|
|||
assert.equal(isBlockedUrl("https://1.1.1.1/"), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("REGRESSION #666: private URL blocked with no override", () => {
|
||||
afterEach(() => {
|
||||
setFetchAllowedUrls([]);
|
||||
});
|
||||
|
||||
it("private IP is blocked by default, then unblocked by setFetchAllowedUrls", () => {
|
||||
const internalUrl = "http://192.168.1.100/internal-docs/api-reference";
|
||||
|
||||
// Bug: private IP is blocked with no way to allowlist
|
||||
assert.equal(isBlockedUrl(internalUrl), true, "private IP is blocked by the hardcoded blocklist");
|
||||
|
||||
// Fix: override the allowlist to include this host
|
||||
setFetchAllowedUrls(["192.168.1.100"]);
|
||||
assert.equal(isBlockedUrl(internalUrl), false, "private IP must not be blocked after override");
|
||||
});
|
||||
});
|
||||
|
||||
describe("setFetchAllowedUrls — user override", () => {
|
||||
afterEach(() => {
|
||||
setFetchAllowedUrls([]);
|
||||
});
|
||||
|
||||
it("defaults to empty allowlist", () => {
|
||||
assert.deepEqual(getFetchAllowedUrls(), []);
|
||||
});
|
||||
|
||||
it("exempts an allowed hostname from blocking", () => {
|
||||
assert.equal(isBlockedUrl("http://192.168.1.100/docs"), true, "blocked by default");
|
||||
setFetchAllowedUrls(["192.168.1.100"]);
|
||||
assert.equal(isBlockedUrl("http://192.168.1.100/docs"), false, "allowed after override");
|
||||
});
|
||||
|
||||
it("exempts localhost when explicitly allowed", () => {
|
||||
assert.equal(isBlockedUrl("http://localhost:3000/api"), true, "blocked by default");
|
||||
setFetchAllowedUrls(["localhost"]);
|
||||
assert.equal(isBlockedUrl("http://localhost:3000/api"), false, "allowed after override");
|
||||
});
|
||||
|
||||
it("exempts cloud metadata hostname when allowed", () => {
|
||||
assert.equal(isBlockedUrl("http://metadata.google.internal/computeMetadata/"), true, "blocked by default");
|
||||
setFetchAllowedUrls(["metadata.google.internal"]);
|
||||
assert.equal(isBlockedUrl("http://metadata.google.internal/computeMetadata/"), false, "allowed after override");
|
||||
});
|
||||
|
||||
it("does not affect URLs not in the allowlist", () => {
|
||||
setFetchAllowedUrls(["192.168.1.100"]);
|
||||
assert.equal(isBlockedUrl("http://192.168.1.200/secret"), true, "other private IPs still blocked");
|
||||
assert.equal(isBlockedUrl("http://localhost/admin"), true, "localhost still blocked");
|
||||
});
|
||||
|
||||
it("still allows public URLs without configuration", () => {
|
||||
setFetchAllowedUrls(["192.168.1.100"]);
|
||||
assert.equal(isBlockedUrl("https://example.com"), false);
|
||||
});
|
||||
|
||||
it("still blocks non-HTTP protocols even with allowlist", () => {
|
||||
setFetchAllowedUrls(["localhost"]);
|
||||
assert.equal(isBlockedUrl("file:///etc/passwd"), true, "file:// still blocked");
|
||||
assert.equal(isBlockedUrl("ftp://localhost/data"), true, "ftp:// still blocked");
|
||||
});
|
||||
|
||||
it("is case-insensitive for hostnames", () => {
|
||||
setFetchAllowedUrls(["MyHost.Internal"]);
|
||||
assert.equal(isBlockedUrl("http://myhost.internal/api"), false);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue