mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(agents): skip bootstrap files with undefined path (#22698)
* fix(agents): skip bootstrap files with undefined path buildBootstrapContextFiles() called file.path.replace() without checking that path was defined. If a hook pushed a bootstrap file using 'filePath' instead of 'path', the function threw TypeError and crashed every agent session — not just the misconfigured hook. Fix: add a null-guard before the path.replace() call. Files with undefined path are skipped with a warning so one bad hook can't take down all agents. Also adds a test covering the undefined-path case. Fixes #22693 * fix: harden bootstrap path validation and report guards (#22698) (thanks @arosstale) --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -138,6 +138,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Agents/Bootstrap: skip malformed bootstrap files with missing/invalid paths instead of crashing agent sessions; hooks using `filePath` (or non-string `path`) are skipped with a warning. (#22693, #22698) Thanks @arosstale.
|
||||
- Security/Agents: cap embedded Pi runner outer retry loop with a higher profile-aware dynamic limit (32-160 attempts) and return an explicit `retry_limit` error payload when retries never converge, preventing unbounded internal retry cycles (`GHSA-76m6-pj3w-v7mf`).
|
||||
- Telegram: detect duplicate bot-token ownership across Telegram accounts at startup/status time, mark secondary accounts as not configured with an explicit fix message, and block duplicate account startup before polling to avoid endless `getUpdates` conflict loops.
|
||||
- Agents/Tool images: include source filenames in `agents/tool-images` resize logs so compression events can be traced back to specific files.
|
||||
|
||||
@@ -24,6 +24,33 @@ function registerExtraBootstrapFileHook() {
|
||||
});
|
||||
}
|
||||
|
||||
function registerMalformedBootstrapFileHook() {
|
||||
registerInternalHook("agent:bootstrap", (event) => {
|
||||
const context = event.context as AgentBootstrapHookContext;
|
||||
context.bootstrapFiles = [
|
||||
...context.bootstrapFiles,
|
||||
{
|
||||
name: "EXTRA.md",
|
||||
filePath: path.join(context.workspaceDir, "BROKEN.md"),
|
||||
content: "broken",
|
||||
missing: false,
|
||||
} as unknown as WorkspaceBootstrapFile,
|
||||
{
|
||||
name: "EXTRA.md",
|
||||
path: 123,
|
||||
content: "broken",
|
||||
missing: false,
|
||||
} as unknown as WorkspaceBootstrapFile,
|
||||
{
|
||||
name: "EXTRA.md",
|
||||
path: " ",
|
||||
content: "broken",
|
||||
missing: false,
|
||||
} as unknown as WorkspaceBootstrapFile,
|
||||
];
|
||||
});
|
||||
}
|
||||
|
||||
describe("resolveBootstrapFilesForRun", () => {
|
||||
beforeEach(() => clearInternalHooks());
|
||||
afterEach(() => clearInternalHooks());
|
||||
@@ -36,6 +63,23 @@ describe("resolveBootstrapFilesForRun", () => {
|
||||
|
||||
expect(files.some((file) => file.path === path.join(workspaceDir, "EXTRA.md"))).toBe(true);
|
||||
});
|
||||
|
||||
it("drops malformed hook files with missing/invalid paths", async () => {
|
||||
registerMalformedBootstrapFileHook();
|
||||
|
||||
const workspaceDir = await makeTempWorkspace("openclaw-bootstrap-");
|
||||
const warnings: string[] = [];
|
||||
const files = await resolveBootstrapFilesForRun({
|
||||
workspaceDir,
|
||||
warn: (message) => warnings.push(message),
|
||||
});
|
||||
|
||||
expect(
|
||||
files.every((file) => typeof file.path === "string" && file.path.trim().length > 0),
|
||||
).toBe(true);
|
||||
expect(warnings).toHaveLength(3);
|
||||
expect(warnings[0]).toContain('missing or invalid "path" field');
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveBootstrapContextForRun", () => {
|
||||
|
||||
@@ -22,12 +22,31 @@ export function makeBootstrapWarn(params: {
|
||||
return (message: string) => params.warn?.(`${message} (sessionKey=${params.sessionLabel})`);
|
||||
}
|
||||
|
||||
function sanitizeBootstrapFiles(
|
||||
files: WorkspaceBootstrapFile[],
|
||||
warn?: (message: string) => void,
|
||||
): WorkspaceBootstrapFile[] {
|
||||
const sanitized: WorkspaceBootstrapFile[] = [];
|
||||
for (const file of files) {
|
||||
const pathValue = typeof file.path === "string" ? file.path.trim() : "";
|
||||
if (!pathValue) {
|
||||
warn?.(
|
||||
`skipping bootstrap file "${file.name}" — missing or invalid "path" field (hook may have used "filePath" instead)`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
sanitized.push({ ...file, path: pathValue });
|
||||
}
|
||||
return sanitized;
|
||||
}
|
||||
|
||||
export async function resolveBootstrapFilesForRun(params: {
|
||||
workspaceDir: string;
|
||||
config?: OpenClawConfig;
|
||||
sessionKey?: string;
|
||||
sessionId?: string;
|
||||
agentId?: string;
|
||||
warn?: (message: string) => void;
|
||||
}): Promise<WorkspaceBootstrapFile[]> {
|
||||
const sessionKey = params.sessionKey ?? params.sessionId;
|
||||
const bootstrapFiles = filterBootstrapFilesForSession(
|
||||
@@ -35,7 +54,7 @@ export async function resolveBootstrapFilesForRun(params: {
|
||||
sessionKey,
|
||||
);
|
||||
|
||||
return applyBootstrapHookOverrides({
|
||||
const updated = await applyBootstrapHookOverrides({
|
||||
files: bootstrapFiles,
|
||||
workspaceDir: params.workspaceDir,
|
||||
config: params.config,
|
||||
@@ -43,6 +62,7 @@ export async function resolveBootstrapFilesForRun(params: {
|
||||
sessionId: params.sessionId,
|
||||
agentId: params.agentId,
|
||||
});
|
||||
return sanitizeBootstrapFiles(updated, params.warn);
|
||||
}
|
||||
|
||||
export async function resolveBootstrapContextForRun(params: {
|
||||
|
||||
@@ -116,6 +116,40 @@ describe("buildBootstrapContextFiles", () => {
|
||||
expect(result[0]?.content.length).toBeLessThanOrEqual(20);
|
||||
expect(result[0]?.content.startsWith("[MISSING]")).toBe(true);
|
||||
});
|
||||
|
||||
it("skips files with missing or invalid paths and emits warnings", () => {
|
||||
const malformedMissingPath = {
|
||||
name: "SKILL-SECURITY.md",
|
||||
missing: false,
|
||||
content: "secret",
|
||||
} as unknown as WorkspaceBootstrapFile;
|
||||
const malformedNonStringPath = {
|
||||
name: "SKILL-SECURITY.md",
|
||||
path: 123,
|
||||
missing: false,
|
||||
content: "secret",
|
||||
} as unknown as WorkspaceBootstrapFile;
|
||||
const malformedWhitespacePath = {
|
||||
name: "SKILL-SECURITY.md",
|
||||
path: " ",
|
||||
missing: false,
|
||||
content: "secret",
|
||||
} as unknown as WorkspaceBootstrapFile;
|
||||
const good = makeFile({ content: "hello" });
|
||||
const warnings: string[] = [];
|
||||
const result = buildBootstrapContextFiles(
|
||||
[malformedMissingPath, malformedNonStringPath, malformedWhitespacePath, good],
|
||||
{
|
||||
warn: (msg) => warnings.push(msg),
|
||||
},
|
||||
);
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0]?.path).toBe("/tmp/AGENTS.md");
|
||||
expect(warnings).toHaveLength(3);
|
||||
expect(warnings.every((warning) => warning.includes('missing or invalid "path" field'))).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
type BootstrapLimitResolverCase = {
|
||||
|
||||
@@ -199,15 +199,22 @@ export function buildBootstrapContextFiles(
|
||||
if (remainingTotalChars <= 0) {
|
||||
break;
|
||||
}
|
||||
const pathValue = typeof file.path === "string" ? file.path.trim() : "";
|
||||
if (!pathValue) {
|
||||
opts?.warn?.(
|
||||
`skipping bootstrap file "${file.name}" — missing or invalid "path" field (hook may have used "filePath" instead)`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
if (file.missing) {
|
||||
const missingText = `[MISSING] Expected at: ${file.path}`;
|
||||
const missingText = `[MISSING] Expected at: ${pathValue}`;
|
||||
const cappedMissingText = clampToBudget(missingText, remainingTotalChars);
|
||||
if (!cappedMissingText) {
|
||||
break;
|
||||
}
|
||||
remainingTotalChars = Math.max(0, remainingTotalChars - cappedMissingText.length);
|
||||
result.push({
|
||||
path: file.path,
|
||||
path: pathValue,
|
||||
content: cappedMissingText,
|
||||
});
|
||||
continue;
|
||||
@@ -231,7 +238,7 @@ export function buildBootstrapContextFiles(
|
||||
}
|
||||
remainingTotalChars = Math.max(0, remainingTotalChars - contentWithinBudget.length);
|
||||
result.push({
|
||||
path: file.path,
|
||||
path: pathValue,
|
||||
content: contentWithinBudget,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -93,4 +93,23 @@ describe("buildSystemPromptReport", () => {
|
||||
expect(report.injectedWorkspaceFiles[0]?.injectedChars).toBe(0);
|
||||
expect(report.injectedWorkspaceFiles[0]?.truncated).toBe(true);
|
||||
});
|
||||
|
||||
it("ignores malformed injected file paths and still matches valid entries", () => {
|
||||
const file = makeBootstrapFile({ path: "/tmp/workspace/policies/AGENTS.md" });
|
||||
const report = buildSystemPromptReport({
|
||||
source: "run",
|
||||
generatedAt: 0,
|
||||
bootstrapMaxChars: 20_000,
|
||||
systemPrompt: "system",
|
||||
bootstrapFiles: [file],
|
||||
injectedFiles: [
|
||||
{ path: 123 as unknown as string, content: "bad" },
|
||||
{ path: "/tmp/workspace/policies/AGENTS.md", content: "trimmed" },
|
||||
],
|
||||
skillsPrompt: "",
|
||||
tools: [],
|
||||
});
|
||||
|
||||
expect(report.injectedWorkspaceFiles[0]?.injectedChars).toBe("trimmed".length);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -40,26 +40,34 @@ function buildInjectedWorkspaceFiles(params: {
|
||||
bootstrapFiles: WorkspaceBootstrapFile[];
|
||||
injectedFiles: EmbeddedContextFile[];
|
||||
}): SessionSystemPromptReport["injectedWorkspaceFiles"] {
|
||||
const injectedByPath = new Map(params.injectedFiles.map((f) => [f.path, f.content]));
|
||||
const injectedByPath = new Map<string, string>();
|
||||
const injectedByBaseName = new Map<string, string>();
|
||||
for (const file of params.injectedFiles) {
|
||||
const normalizedPath = file.path.replace(/\\/g, "/");
|
||||
const pathValue = typeof file.path === "string" ? file.path.trim() : "";
|
||||
if (!pathValue) {
|
||||
continue;
|
||||
}
|
||||
if (!injectedByPath.has(pathValue)) {
|
||||
injectedByPath.set(pathValue, file.content);
|
||||
}
|
||||
const normalizedPath = pathValue.replace(/\\/g, "/");
|
||||
const baseName = path.posix.basename(normalizedPath);
|
||||
if (!injectedByBaseName.has(baseName)) {
|
||||
injectedByBaseName.set(baseName, file.content);
|
||||
}
|
||||
}
|
||||
return params.bootstrapFiles.map((file) => {
|
||||
const pathValue = typeof file.path === "string" ? file.path.trim() : "";
|
||||
const rawChars = file.missing ? 0 : (file.content ?? "").trimEnd().length;
|
||||
const injected =
|
||||
injectedByPath.get(file.path) ??
|
||||
(pathValue ? injectedByPath.get(pathValue) : undefined) ??
|
||||
injectedByPath.get(file.name) ??
|
||||
injectedByBaseName.get(file.name);
|
||||
const injectedChars = injected ? injected.length : 0;
|
||||
const truncated = !file.missing && injectedChars < rawChars;
|
||||
return {
|
||||
name: file.name,
|
||||
path: file.path,
|
||||
path: pathValue || file.name,
|
||||
missing: file.missing,
|
||||
rawChars,
|
||||
injectedChars,
|
||||
|
||||
Reference in New Issue
Block a user