fix(config): reject shared agentDir
This commit is contained in:
109
src/config/agent-dirs.ts
Normal file
109
src/config/agent-dirs.ts
Normal file
@@ -0,0 +1,109 @@
|
|||||||
|
import os from "node:os";
|
||||||
|
import path from "node:path";
|
||||||
|
import { DEFAULT_AGENT_ID, normalizeAgentId } from "../routing/session-key.js";
|
||||||
|
import { resolveUserPath } from "../utils.js";
|
||||||
|
import { resolveStateDir } from "./paths.js";
|
||||||
|
import type { ClawdbotConfig } from "./types.js";
|
||||||
|
|
||||||
|
export type DuplicateAgentDir = {
|
||||||
|
agentDir: string;
|
||||||
|
agentIds: string[];
|
||||||
|
};
|
||||||
|
|
||||||
|
export class DuplicateAgentDirError extends Error {
|
||||||
|
readonly duplicates: DuplicateAgentDir[];
|
||||||
|
|
||||||
|
constructor(duplicates: DuplicateAgentDir[]) {
|
||||||
|
super(formatDuplicateAgentDirError(duplicates));
|
||||||
|
this.name = "DuplicateAgentDirError";
|
||||||
|
this.duplicates = duplicates;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function canonicalizeAgentDir(agentDir: string): string {
|
||||||
|
const resolved = path.resolve(agentDir);
|
||||||
|
if (process.platform === "darwin" || process.platform === "win32") {
|
||||||
|
return resolved.toLowerCase();
|
||||||
|
}
|
||||||
|
return resolved;
|
||||||
|
}
|
||||||
|
|
||||||
|
function collectReferencedAgentIds(cfg: ClawdbotConfig): string[] {
|
||||||
|
const ids = new Set<string>();
|
||||||
|
|
||||||
|
const defaultAgentId =
|
||||||
|
cfg.routing?.defaultAgentId?.trim() || DEFAULT_AGENT_ID;
|
||||||
|
ids.add(normalizeAgentId(defaultAgentId));
|
||||||
|
|
||||||
|
const agents = cfg.routing?.agents;
|
||||||
|
if (agents && typeof agents === "object") {
|
||||||
|
for (const id of Object.keys(agents)) {
|
||||||
|
ids.add(normalizeAgentId(id));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const bindings = cfg.routing?.bindings;
|
||||||
|
if (Array.isArray(bindings)) {
|
||||||
|
for (const binding of bindings) {
|
||||||
|
const id = binding?.agentId;
|
||||||
|
if (typeof id === "string" && id.trim()) {
|
||||||
|
ids.add(normalizeAgentId(id));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return [...ids];
|
||||||
|
}
|
||||||
|
|
||||||
|
function resolveEffectiveAgentDir(
|
||||||
|
cfg: ClawdbotConfig,
|
||||||
|
agentId: string,
|
||||||
|
deps?: { env?: NodeJS.ProcessEnv; homedir?: () => string },
|
||||||
|
): string {
|
||||||
|
const id = normalizeAgentId(agentId);
|
||||||
|
const configured = cfg.routing?.agents?.[id]?.agentDir?.trim();
|
||||||
|
if (configured) return resolveUserPath(configured);
|
||||||
|
const root = resolveStateDir(
|
||||||
|
deps?.env ?? process.env,
|
||||||
|
deps?.homedir ?? os.homedir,
|
||||||
|
);
|
||||||
|
return path.join(root, "agents", id, "agent");
|
||||||
|
}
|
||||||
|
|
||||||
|
export function findDuplicateAgentDirs(
|
||||||
|
cfg: ClawdbotConfig,
|
||||||
|
deps?: { env?: NodeJS.ProcessEnv; homedir?: () => string },
|
||||||
|
): DuplicateAgentDir[] {
|
||||||
|
const byDir = new Map<string, { agentDir: string; agentIds: string[] }>();
|
||||||
|
|
||||||
|
for (const agentId of collectReferencedAgentIds(cfg)) {
|
||||||
|
const agentDir = resolveEffectiveAgentDir(cfg, agentId, deps);
|
||||||
|
const key = canonicalizeAgentDir(agentDir);
|
||||||
|
const entry = byDir.get(key);
|
||||||
|
if (entry) {
|
||||||
|
entry.agentIds.push(agentId);
|
||||||
|
} else {
|
||||||
|
byDir.set(key, { agentDir, agentIds: [agentId] });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return [...byDir.values()].filter((v) => v.agentIds.length > 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function formatDuplicateAgentDirError(
|
||||||
|
dups: DuplicateAgentDir[],
|
||||||
|
): string {
|
||||||
|
const lines: string[] = [
|
||||||
|
"Duplicate agentDir detected (multi-agent config).",
|
||||||
|
"Each agent must have a unique agentDir; sharing it causes auth/session state collisions and token invalidation.",
|
||||||
|
"",
|
||||||
|
"Conflicts:",
|
||||||
|
...dups.map(
|
||||||
|
(d) => `- ${d.agentDir}: ${d.agentIds.map((id) => `"${id}"`).join(", ")}`,
|
||||||
|
),
|
||||||
|
"",
|
||||||
|
"Fix: remove the shared routing.agents.*.agentDir override (or give each agent its own directory).",
|
||||||
|
"If you want to share credentials, copy auth-profiles.json instead of sharing the entire agentDir.",
|
||||||
|
];
|
||||||
|
return lines.join("\n");
|
||||||
|
}
|
||||||
@@ -981,3 +981,55 @@ describe("legacy config detection", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("multi-agent agentDir validation", () => {
|
||||||
|
it("rejects shared routing.agents.*.agentDir", async () => {
|
||||||
|
vi.resetModules();
|
||||||
|
const { validateConfigObject } = await import("./config.js");
|
||||||
|
const shared = path.join(os.tmpdir(), "clawdbot-shared-agentdir");
|
||||||
|
const res = validateConfigObject({
|
||||||
|
routing: {
|
||||||
|
agents: {
|
||||||
|
a: { agentDir: shared },
|
||||||
|
b: { agentDir: shared },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
expect(res.ok).toBe(false);
|
||||||
|
if (!res.ok) {
|
||||||
|
expect(res.issues.some((i) => i.path === "routing.agents")).toBe(true);
|
||||||
|
expect(res.issues[0]?.message).toContain("Duplicate agentDir");
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws on shared agentDir during loadConfig()", async () => {
|
||||||
|
await withTempHome(async (home) => {
|
||||||
|
const configDir = path.join(home, ".clawdbot");
|
||||||
|
await fs.mkdir(configDir, { recursive: true });
|
||||||
|
await fs.writeFile(
|
||||||
|
path.join(configDir, "clawdbot.json"),
|
||||||
|
JSON.stringify(
|
||||||
|
{
|
||||||
|
routing: {
|
||||||
|
agents: {
|
||||||
|
a: { agentDir: "~/.clawdbot/agents/shared/agent" },
|
||||||
|
b: { agentDir: "~/.clawdbot/agents/shared/agent" },
|
||||||
|
},
|
||||||
|
bindings: [{ agentId: "a", match: { provider: "telegram" } }],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
null,
|
||||||
|
2,
|
||||||
|
),
|
||||||
|
"utf-8",
|
||||||
|
);
|
||||||
|
|
||||||
|
vi.resetModules();
|
||||||
|
const spy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||||
|
const { loadConfig } = await import("./config.js");
|
||||||
|
expect(() => loadConfig()).toThrow(/duplicate agentDir/i);
|
||||||
|
expect(spy.mock.calls.flat().join(" ")).toMatch(/Duplicate agentDir/i);
|
||||||
|
spy.mockRestore();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -8,6 +8,10 @@ import {
|
|||||||
resolveShellEnvFallbackTimeoutMs,
|
resolveShellEnvFallbackTimeoutMs,
|
||||||
shouldEnableShellEnvFallback,
|
shouldEnableShellEnvFallback,
|
||||||
} from "../infra/shell-env.js";
|
} from "../infra/shell-env.js";
|
||||||
|
import {
|
||||||
|
DuplicateAgentDirError,
|
||||||
|
findDuplicateAgentDirs,
|
||||||
|
} from "./agent-dirs.js";
|
||||||
import {
|
import {
|
||||||
applyIdentityDefaults,
|
applyIdentityDefaults,
|
||||||
applyLoggingDefaults,
|
applyLoggingDefaults,
|
||||||
@@ -140,6 +144,14 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
|||||||
),
|
),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
const duplicates = findDuplicateAgentDirs(cfg, {
|
||||||
|
env: deps.env,
|
||||||
|
homedir: deps.homedir,
|
||||||
|
});
|
||||||
|
if (duplicates.length > 0) {
|
||||||
|
throw new DuplicateAgentDirError(duplicates);
|
||||||
|
}
|
||||||
|
|
||||||
const enabled =
|
const enabled =
|
||||||
shouldEnableShellEnvFallback(deps.env) ||
|
shouldEnableShellEnvFallback(deps.env) ||
|
||||||
cfg.env?.shellEnv?.enabled === true;
|
cfg.env?.shellEnv?.enabled === true;
|
||||||
@@ -157,6 +169,10 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
|||||||
|
|
||||||
return cfg;
|
return cfg;
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
if (err instanceof DuplicateAgentDirError) {
|
||||||
|
deps.logger.error(err.message);
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
deps.logger.error(`Failed to read config at ${configPath}`, err);
|
deps.logger.error(`Failed to read config at ${configPath}`, err);
|
||||||
return {};
|
return {};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,3 +1,7 @@
|
|||||||
|
import {
|
||||||
|
findDuplicateAgentDirs,
|
||||||
|
formatDuplicateAgentDirError,
|
||||||
|
} from "./agent-dirs.js";
|
||||||
import {
|
import {
|
||||||
applyIdentityDefaults,
|
applyIdentityDefaults,
|
||||||
applyModelDefaults,
|
applyModelDefaults,
|
||||||
@@ -32,6 +36,18 @@ export function validateConfigObject(
|
|||||||
})),
|
})),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
const duplicates = findDuplicateAgentDirs(validated.data as ClawdbotConfig);
|
||||||
|
if (duplicates.length > 0) {
|
||||||
|
return {
|
||||||
|
ok: false,
|
||||||
|
issues: [
|
||||||
|
{
|
||||||
|
path: "routing.agents",
|
||||||
|
message: formatDuplicateAgentDirError(duplicates),
|
||||||
|
},
|
||||||
|
],
|
||||||
|
};
|
||||||
|
}
|
||||||
return {
|
return {
|
||||||
ok: true,
|
ok: true,
|
||||||
config: applyModelDefaults(
|
config: applyModelDefaults(
|
||||||
|
|||||||
Reference in New Issue
Block a user