test(config): enforce allow+alsoAllow mutual exclusion
This commit is contained in:
53
src/config/config.tools-alsoAllow.test.ts
Normal file
53
src/config/config.tools-alsoAllow.test.ts
Normal file
@@ -0,0 +1,53 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
|
||||||
|
import { validateConfigObject } from "./validation.js";
|
||||||
|
|
||||||
|
// NOTE: These tests ensure allow + alsoAllow cannot be set in the same scope.
|
||||||
|
|
||||||
|
describe("config: tools.alsoAllow", () => {
|
||||||
|
it("rejects tools.allow + tools.alsoAllow together", () => {
|
||||||
|
const res = validateConfigObject({
|
||||||
|
tools: {
|
||||||
|
allow: ["group:fs"],
|
||||||
|
alsoAllow: ["lobster"],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.ok).toBe(false);
|
||||||
|
if (!res.ok) {
|
||||||
|
expect(res.issues.some((i) => i.path === "tools")).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects agents.list[].tools.allow + alsoAllow together", () => {
|
||||||
|
const res = validateConfigObject({
|
||||||
|
agents: {
|
||||||
|
list: [
|
||||||
|
{
|
||||||
|
id: "main",
|
||||||
|
tools: {
|
||||||
|
allow: ["group:fs"],
|
||||||
|
alsoAllow: ["lobster"],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.ok).toBe(false);
|
||||||
|
if (!res.ok) {
|
||||||
|
expect(res.issues.some((i) => i.path.includes("agents.list"))).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows profile + alsoAllow", () => {
|
||||||
|
const res = validateConfigObject({
|
||||||
|
tools: {
|
||||||
|
profile: "coding",
|
||||||
|
alsoAllow: ["lobster"],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.ok).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -9,83 +9,9 @@ import {
|
|||||||
resolveDefaultAgentIdFromRaw,
|
resolveDefaultAgentIdFromRaw,
|
||||||
} from "./legacy.shared.js";
|
} from "./legacy.shared.js";
|
||||||
|
|
||||||
function mergeAlsoAllowIntoAllow(node: unknown): boolean {
|
// NOTE: tools.alsoAllow was introduced after legacy migrations; no legacy migration needed.
|
||||||
if (!isRecord(node)) return false;
|
|
||||||
const allow = node.allow;
|
|
||||||
const alsoAllow = node.alsoAllow;
|
|
||||||
if (!Array.isArray(allow) || allow.length === 0) return false;
|
|
||||||
if (!Array.isArray(alsoAllow) || alsoAllow.length === 0) return false;
|
|
||||||
const merged = Array.from(new Set([...(allow as unknown[]), ...(alsoAllow as unknown[])]));
|
|
||||||
node.allow = merged;
|
|
||||||
delete node.alsoAllow;
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|
||||||
function migrateAlsoAllowInToolConfig(raw: Record<string, unknown>, changes: string[]) {
|
// tools.alsoAllow legacy migration intentionally omitted (field not shipped in prod).
|
||||||
let mutated = false;
|
|
||||||
|
|
||||||
// Global tools
|
|
||||||
const tools = getRecord(raw.tools);
|
|
||||||
if (mergeAlsoAllowIntoAllow(tools)) {
|
|
||||||
mutated = true;
|
|
||||||
changes.push("Merged tools.alsoAllow into tools.allow (and removed tools.alsoAllow).");
|
|
||||||
}
|
|
||||||
|
|
||||||
// tools.byProvider.*
|
|
||||||
const byProvider = getRecord(tools?.byProvider);
|
|
||||||
if (byProvider) {
|
|
||||||
for (const [key, value] of Object.entries(byProvider)) {
|
|
||||||
if (mergeAlsoAllowIntoAllow(value)) {
|
|
||||||
mutated = true;
|
|
||||||
changes.push(`Merged tools.byProvider.${key}.alsoAllow into allow (and removed alsoAllow).`);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// agents.list[].tools
|
|
||||||
const agentsList = getAgentsList(raw);
|
|
||||||
for (const agent of agentsList) {
|
|
||||||
const agentTools = getRecord(agent.tools);
|
|
||||||
if (mergeAlsoAllowIntoAllow(agentTools)) {
|
|
||||||
mutated = true;
|
|
||||||
const id = typeof agent.id === "string" ? agent.id : "<unknown>";
|
|
||||||
changes.push(`Merged agents.list[${id}].tools.alsoAllow into allow (and removed alsoAllow).`);
|
|
||||||
}
|
|
||||||
|
|
||||||
const agentByProvider = getRecord(agentTools?.byProvider);
|
|
||||||
if (agentByProvider) {
|
|
||||||
for (const [key, value] of Object.entries(agentByProvider)) {
|
|
||||||
if (mergeAlsoAllowIntoAllow(value)) {
|
|
||||||
mutated = true;
|
|
||||||
const id = typeof agent.id === "string" ? agent.id : "<unknown>";
|
|
||||||
changes.push(
|
|
||||||
`Merged agents.list[${id}].tools.byProvider.${key}.alsoAllow into allow (and removed alsoAllow).`,
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Provider group tool policies: channels.<provider>.groups.*.tools and similar nested tool policy objects.
|
|
||||||
const channels = getRecord(raw.channels);
|
|
||||||
if (channels) {
|
|
||||||
for (const [provider, providerCfg] of Object.entries(channels)) {
|
|
||||||
const groups = getRecord(getRecord(providerCfg)?.groups);
|
|
||||||
if (!groups) continue;
|
|
||||||
for (const [groupKey, groupCfg] of Object.entries(groups)) {
|
|
||||||
const toolsCfg = getRecord(getRecord(groupCfg)?.tools);
|
|
||||||
if (mergeAlsoAllowIntoAllow(toolsCfg)) {
|
|
||||||
mutated = true;
|
|
||||||
changes.push(
|
|
||||||
`Merged channels.${provider}.groups.${groupKey}.tools.alsoAllow into allow (and removed alsoAllow).`,
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return mutated;
|
|
||||||
}
|
|
||||||
|
|
||||||
export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [
|
export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [
|
||||||
{
|
{
|
||||||
@@ -102,13 +28,7 @@ export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [
|
|||||||
changes.push('Updated auth.profiles["anthropic:claude-cli"].mode → "oauth".');
|
changes.push('Updated auth.profiles["anthropic:claude-cli"].mode → "oauth".');
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
{
|
// tools.alsoAllow migration removed (field not shipped in prod; enforce via schema instead).
|
||||||
id: "tools.alsoAllow-merge",
|
|
||||||
describe: "Merge tools.alsoAllow into allow when allow is present",
|
|
||||||
apply: (raw, changes) => {
|
|
||||||
migrateAlsoAllowInToolConfig(raw, changes);
|
|
||||||
},
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
id: "tools.bash->tools.exec",
|
id: "tools.bash->tools.exec",
|
||||||
describe: "Move tools.bash to tools.exec",
|
describe: "Move tools.bash to tools.exec",
|
||||||
|
|||||||
Reference in New Issue
Block a user