Skip to content

Commit 54eb03f

Browse files
steipetesteipete
authored
fix(migrate): harden importer path resolution and memory copy safety (#109314)
1 parent ce91959 commit 54eb03f

12 files changed

Lines changed: 425 additions & 52 deletions

File tree

‎extensions/codex/src/migration/memory-plan.ts‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,25 @@ import {
77
canonicalPathFromExistingAncestor,
88
isPathInside,
99
} from "openclaw/plugin-sdk/security-runtime";
10-
import { exists } from "./helpers.js";
1110
import type { CodexMemorySource } from "./source-files.js";
1211

12+
const MIGRATION_REASON_TARGET_NOT_REGULAR = "target is not a regular file";
13+
14+
async function lstatIfExists(filePath: string) {
15+
try {
16+
return await fs.lstat(filePath);
17+
} catch (error) {
18+
const code =
19+
error && typeof error === "object" && "code" in error
20+
? String((error as { code?: unknown }).code)
21+
: undefined;
22+
if (code === "ENOENT" || code === "ENOTDIR") {
23+
return undefined;
24+
}
25+
throw error;
26+
}
27+
}
28+
1329
async function assertSafeMemoryDestination(params: {
1430
source: string;
1531
workspaceDir: string;
@@ -45,21 +61,30 @@ export async function buildCodexMemoryItems(params: {
4561
"codex",
4662
path.basename(memory.path),
4763
);
48-
await assertSafeMemoryDestination({
49-
source: memory.path,
50-
workspaceDir: params.workspaceDir,
51-
target,
52-
});
53-
const targetExists = await exists(target);
64+
const targetStat = await lstatIfExists(target);
65+
const targetExists = targetStat !== undefined;
66+
const targetNotRegular = targetExists && !targetStat.isFile();
67+
if (!targetNotRegular) {
68+
await assertSafeMemoryDestination({
69+
source: memory.path,
70+
workspaceDir: params.workspaceDir,
71+
target,
72+
});
73+
}
74+
const targetConflict = targetNotRegular || (targetExists && !params.overwrite);
5475
items.push(
5576
createMigrationItem({
5677
id: memory.id,
5778
kind: "memory",
5879
action: "copy",
5980
source: memory.path,
6081
target,
61-
status: targetExists && !params.overwrite ? "conflict" : "planned",
62-
reason: targetExists && !params.overwrite ? MIGRATION_REASON_TARGET_EXISTS : undefined,
82+
status: targetConflict ? "conflict" : "planned",
83+
reason: targetNotRegular
84+
? MIGRATION_REASON_TARGET_NOT_REGULAR
85+
: targetConflict
86+
? MIGRATION_REASON_TARGET_EXISTS
87+
: undefined,
6388
message: "Copy consolidated Codex memory into the OpenClaw memory index.",
6489
details: {
6590
sourceType: "codex-memory",

‎extensions/codex/src/migration/provider.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { CODEX_PLUGINS_MARKETPLACE_NAME } from "../app-server/config.js";
1414
import { buildCodexPluginAppCacheKey } from "../app-server/plugin-app-cache-key.js";
1515
import type { CodexGetAccountResponse, v2 } from "../app-server/protocol.js";
1616
import { buildCodexMigrationProvider } from "./provider.js";
17+
import { discoverCodexSource } from "./source.js";
1718

1819
const appServerRequest = vi.hoisted(() => vi.fn());
1920

@@ -200,6 +201,20 @@ describe("buildCodexMigrationProvider", () => {
200201
appServerRequest.mockRejectedValue(new Error("codex app-server unavailable"));
201202
});
202203

204+
it("preserves whitespace in nonempty CODEX_HOME values", async () => {
205+
const root = await makeTempRoot();
206+
const codexHome = path.join(root, " spaced ");
207+
await writeFile(path.join(codexHome, "memories", "MEMORY.md"), "# Memory\n");
208+
vi.stubEnv("CODEX_HOME", codexHome);
209+
210+
const source = await discoverCodexSource({ memoryOnly: true });
211+
212+
expect(source.codexHome).toBe(codexHome);
213+
expect(source.memoryFiles.map((entry) => entry.path)).toEqual([
214+
path.join(codexHome, "memories", "MEMORY.md"),
215+
]);
216+
});
217+
203218
it("plans and imports only consolidated Codex memory into the selected agent", async () => {
204219
const fixture = await createCodexFixture();
205220
const targetWorkspace = path.join(fixture.root, "workspace-research");
@@ -333,6 +348,33 @@ describe("buildCodexMigrationProvider", () => {
333348
},
334349
);
335350

351+
it.runIf(process.platform !== "win32")(
352+
"marks a dangling Codex memory destination symlink as a conflict",
353+
async () => {
354+
const fixture = await createCodexFixture();
355+
const target = path.join(fixture.workspaceDir, "memory", "imports", "codex", "MEMORY.md");
356+
await writeFile(path.join(fixture.codexHome, "memories", "MEMORY.md"), "# Memory\n");
357+
await fs.mkdir(path.dirname(target), { recursive: true });
358+
await fs.symlink(path.join(fixture.root, "missing-memory.md"), target);
359+
const provider = buildCodexMigrationProvider();
360+
361+
const plan = await provider.plan(
362+
makeContext({
363+
source: fixture.codexHome,
364+
stateDir: fixture.stateDir,
365+
workspaceDir: fixture.workspaceDir,
366+
itemKinds: ["memory"],
367+
overwrite: true,
368+
}),
369+
);
370+
371+
expect(findItem(plan.items, "memory:codex:MEMORY.md")).toMatchObject({
372+
status: "conflict",
373+
reason: "target is not a regular file",
374+
});
375+
},
376+
);
377+
336378
it("plans Codex skills while keeping plugins and native config explicit", async () => {
337379
const fixture = await createCodexFixture();
338380
const provider = buildCodexMigrationProvider();

‎extensions/codex/src/migration/source.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ type PluginReadResult =
7474
};
7575

7676
function defaultCodexHome(): string {
77-
return resolveHomePath(process.env.CODEX_HOME?.trim() || "~/.codex");
77+
const configuredHome = process.env.CODEX_HOME;
78+
// Codex preserves nonempty CODEX_HOME verbatim; --from remains trimmed below as CLI convenience.
79+
return resolveHomePath(
80+
configuredHome !== undefined && configuredHome.length > 0 ? configuredHome : "~/.codex",
81+
);
7882
}
7983

8084
function personalAgentsSkillsDir(): string {

‎extensions/migrate-claude/memory.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,30 @@ import {
88
canonicalPathFromExistingAncestor,
99
isPathInside,
1010
} from "openclaw/plugin-sdk/security-runtime";
11-
import { exists } from "./helpers.js";
1211
import {
1312
CLAUDE_AUTO_MEMORY_MAX_FILES,
1413
CLAUDE_AUTO_MEMORY_MAX_SCAN_ENTRIES,
1514
type ClaudeSource,
1615
} from "./source.js";
1716
import type { PlannedTargets } from "./targets.js";
1817

18+
const MIGRATION_REASON_TARGET_NOT_REGULAR = "target is not a regular file";
19+
20+
async function lstatIfExists(filePath: string) {
21+
try {
22+
return await fs.lstat(filePath);
23+
} catch (error) {
24+
const code =
25+
error && typeof error === "object" && "code" in error
26+
? String((error as { code?: unknown }).code)
27+
: undefined;
28+
if (code === "ENOENT" || code === "ENOTDIR") {
29+
return undefined;
30+
}
31+
throw error;
32+
}
33+
}
34+
1935
async function addMemoryItem(params: {
2036
items: MigrationItem[];
2137
id: string;
@@ -28,8 +44,12 @@ async function addMemoryItem(params: {
2844
if (!params.source) {
2945
return;
3046
}
31-
const targetExists = await exists(params.target);
47+
const targetStat = await lstatIfExists(params.target);
48+
const targetExists = targetStat !== undefined;
49+
const targetNotRegular = targetExists && !targetStat.isFile();
3250
const action = params.copyWhenMissing && !targetExists ? "copy" : "append";
51+
const targetConflict =
52+
targetNotRegular || (action === "copy" && targetExists && !params.overwrite);
3353
params.items.push(
3454
createMigrationItem({
3555
id: params.id,
@@ -39,9 +59,10 @@ async function addMemoryItem(params: {
3959
action,
4060
source: params.source,
4161
target: params.target,
42-
status: action === "copy" && targetExists && !params.overwrite ? "conflict" : "planned",
43-
reason:
44-
action === "copy" && targetExists && !params.overwrite
62+
status: targetConflict ? "conflict" : "planned",
63+
reason: targetNotRegular
64+
? MIGRATION_REASON_TARGET_NOT_REGULAR
65+
: targetConflict
4566
? MIGRATION_REASON_TARGET_EXISTS
4667
: undefined,
4768
details: { sourceLabel: params.sourceLabel },
@@ -174,17 +195,26 @@ async function buildAutoMemoryItems(params: {
174195
for (const relativePath of files) {
175196
const source = path.join(collection.path, relativePath);
176197
const target = path.join(targetRoot, relativePath);
177-
await assertSafeMemoryDestination(destinationBoundary, target);
178-
const targetExists = await exists(target);
198+
const targetStat = await lstatIfExists(target);
199+
const targetExists = targetStat !== undefined;
200+
const targetNotRegular = targetExists && !targetStat.isFile();
201+
if (!targetNotRegular) {
202+
await assertSafeMemoryDestination(destinationBoundary, target);
203+
}
204+
const targetConflict = targetNotRegular || (targetExists && !params.overwrite);
179205
items.push(
180206
createMigrationItem({
181207
id: `memory:claude-auto:${collection.id}:${relativePath.replaceAll(path.sep, "/")}`,
182208
kind: "memory",
183209
action: "copy",
184210
source,
185211
target,
186-
status: targetExists && !params.overwrite ? "conflict" : "planned",
187-
reason: targetExists && !params.overwrite ? MIGRATION_REASON_TARGET_EXISTS : undefined,
212+
status: targetConflict ? "conflict" : "planned",
213+
reason: targetNotRegular
214+
? MIGRATION_REASON_TARGET_NOT_REGULAR
215+
: targetConflict
216+
? MIGRATION_REASON_TARGET_EXISTS
217+
: undefined,
188218
message: "Copy Claude Code auto-memory Markdown into the OpenClaw memory index.",
189219
details: {
190220
sourceType: "claude-auto-memory",

‎extensions/migrate-claude/provider.test.ts‎

Lines changed: 117 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import fs from "node:fs/promises";
33
import os from "node:os";
44
import path from "node:path";
55
import { redactMigrationPlan } from "openclaw/plugin-sdk/migration";
6-
import { afterEach, describe, expect, it } from "vitest";
6+
import { afterEach, describe, expect, it, vi } from "vitest";
77
import { resolveHomePath } from "./helpers.js";
88
import { buildMemoryItems } from "./memory.js";
99
import { buildClaudeMigrationProvider } from "./provider.js";
10-
import { CLAUDE_AUTO_MEMORY_MAX_FILES, type ClaudeSource } from "./source.js";
10+
import { CLAUDE_AUTO_MEMORY_MAX_FILES, type ClaudeSource, discoverClaudeSource } from "./source.js";
1111
import {
1212
cleanupTempRoots,
1313
makeConfigRuntime,
@@ -29,6 +29,7 @@ function planItemById(
2929

3030
describe("Claude migration provider", () => {
3131
afterEach(async () => {
32+
vi.unstubAllEnvs();
3233
await cleanupTempRoots();
3334
});
3435

@@ -140,6 +141,32 @@ describe("Claude migration provider", () => {
140141
expect(plan.items[0]?.source).toBe(path.join(customMemory, "MEMORY.md"));
141142
});
142143

144+
it("honors CLAUDE_CONFIG_DIR for a relocated Claude home", async () => {
145+
const root = await makeTempRoot();
146+
const relocatedHome = path.join(root, "relocated-claude");
147+
const memoryDir = path.join(relocatedHome, "projects", "-tmp-project", "memory");
148+
await writeFile(path.join(memoryDir, "MEMORY.md"), "# Relocated memory\n");
149+
vi.stubEnv("CLAUDE_CONFIG_DIR", relocatedHome);
150+
151+
const source = await discoverClaudeSource();
152+
153+
expect(source.root).toBe(relocatedHome);
154+
expect(source.homeDir).toBe(relocatedHome);
155+
expect(source.autoMemorySources.map((entry) => entry.path)).toEqual([memoryDir]);
156+
});
157+
158+
it("treats an explicit repo root with a top-level projects/ dir as a project, not a home", async () => {
159+
const root = await makeTempRoot();
160+
const projectRoot = path.join(root, "my-monorepo");
161+
await writeFile(path.join(projectRoot, "projects", "svc-a", "readme.md"), "# svc\n");
162+
await writeFile(path.join(projectRoot, "settings.json"), "{}\n");
163+
164+
const source = await discoverClaudeSource(projectRoot);
165+
166+
expect(source.projectDir).toBe(projectRoot);
167+
expect(source.homeDir).toBeUndefined();
168+
});
169+
143170
it.runIf(process.platform !== "win32")(
144171
"reports an unreadable configured Claude Code auto-memory directory",
145172
async () => {
@@ -171,6 +198,38 @@ describe("Claude migration provider", () => {
171198
},
172199
);
173200

201+
it.runIf(process.platform !== "win32" && process.getuid?.() !== 0)(
202+
"reports an inaccessible configured Claude Code auto-memory directory",
203+
async () => {
204+
const root = await makeTempRoot();
205+
const source = path.join(root, ".claude");
206+
const lockedParent = path.join(root, "locked-parent");
207+
const customMemory = path.join(lockedParent, "custom-memory");
208+
await writeFile(
209+
path.join(source, "settings.json"),
210+
JSON.stringify({ autoMemoryDirectory: customMemory }),
211+
);
212+
await writeFile(path.join(customMemory, "MEMORY.md"), "# Custom memory\n");
213+
await fs.chmod(lockedParent, 0o000);
214+
const provider = buildClaudeMigrationProvider();
215+
216+
try {
217+
await expect(
218+
provider.plan(
219+
makeContext({
220+
source,
221+
stateDir: path.join(root, "state"),
222+
workspaceDir: path.join(root, "workspace"),
223+
itemKinds: ["memory"],
224+
}),
225+
),
226+
).rejects.toThrow(customMemory);
227+
} finally {
228+
await fs.chmod(lockedParent, 0o700);
229+
}
230+
},
231+
);
232+
174233
it.runIf(process.platform !== "win32")(
175234
"reports an unreadable standard Claude Code projects directory",
176235
async () => {
@@ -219,6 +278,27 @@ describe("Claude migration provider", () => {
219278
).rejects.toThrow("autoMemoryDirectory must be absolute or start with ~/");
220279
});
221280

281+
it('rejects bare "~" as a Claude Code auto-memory directory', async () => {
282+
const root = await makeTempRoot();
283+
const source = path.join(root, ".claude");
284+
await writeFile(
285+
path.join(source, "settings.json"),
286+
JSON.stringify({ autoMemoryDirectory: "~" }),
287+
);
288+
const provider = buildClaudeMigrationProvider();
289+
290+
await expect(
291+
provider.plan(
292+
makeContext({
293+
source,
294+
stateDir: path.join(root, "state"),
295+
workspaceDir: path.join(root, "workspace"),
296+
itemKinds: ["memory"],
297+
}),
298+
),
299+
).rejects.toThrow("autoMemoryDirectory must be absolute or start with ~/");
300+
});
301+
222302
it("rejects Claude Code auto-memory that contains the import destination", async () => {
223303
const root = await makeTempRoot();
224304
const source = path.join(root, ".claude");
@@ -268,6 +348,41 @@ describe("Claude migration provider", () => {
268348
},
269349
);
270350

351+
it.runIf(process.platform !== "win32")(
352+
"marks a dangling Claude Code memory destination symlink as a conflict",
353+
async () => {
354+
const root = await makeTempRoot();
355+
const source = path.join(root, ".claude");
356+
const workspaceDir = path.join(root, "workspace");
357+
await writeFile(
358+
path.join(source, "projects", "-tmp-linked", "memory", "MEMORY.md"),
359+
"# Source memory\n",
360+
);
361+
const provider = buildClaudeMigrationProvider();
362+
const context = makeContext({
363+
source,
364+
stateDir: path.join(root, "state"),
365+
workspaceDir,
366+
itemKinds: ["memory"],
367+
overwrite: true,
368+
});
369+
const initial = await provider.plan(context);
370+
const target = initial.items[0]?.target;
371+
if (!target) {
372+
throw new Error("expected planned Claude memory target");
373+
}
374+
await fs.mkdir(path.dirname(target), { recursive: true });
375+
await fs.symlink(path.join(root, "missing-memory.md"), target);
376+
377+
const plan = await provider.plan(context);
378+
379+
expect(plan.items[0]).toMatchObject({
380+
status: "conflict",
381+
reason: "target is not a regular file",
382+
});
383+
},
384+
);
385+
271386
it("fails planning when a discovered Claude Code memory directory cannot be read", async () => {
272387
const root = await makeTempRoot();
273388
const missingMemory = path.join(root, "missing-memory");

0 commit comments

Comments
 (0)