From 9f67dc7688c8d76023f533a7e8f34765bfe3e721 Mon Sep 17 00:00:00 2001 From: Alessio Gogna <5177307+alecsg77@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:37:03 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20fix:=20resolve=20Dev=20Container?= =?UTF-8?q?=20file-tool=20paths=20remotely?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route tilde file-tool paths through the active runtime so Plan Mode uses the Dev Container user's home rather than the Mux host home. Validate that the remote Mux home is writable at runtime startup and cover the non-root user flow across reads and edits. --- _Generated with `mux` • Model: `openai:gpt-5.6-terra` • Thinking: `high` • Cost: `$0.29`_ --- src/node/runtime/DevcontainerRuntime.test.ts | 56 +++++++++++++++++++ src/node/runtime/DevcontainerRuntime.ts | 27 +++++++++ src/node/services/tools/fileCommon.test.ts | 50 +++++++++++++++-- src/node/services/tools/fileCommon.ts | 16 +++++- .../services/tools/file_edit_insert.test.ts | 44 +++++++++++++++ src/node/services/tools/file_edit_insert.ts | 2 +- .../tools/file_edit_operation.test.ts | 48 ++++++++++++++++ .../services/tools/file_edit_operation.ts | 2 +- src/node/services/tools/file_read.test.ts | 44 +++++++++++++++ src/node/services/tools/file_read.ts | 2 +- .../attachments/readAttachmentFromPath.ts | 2 +- 11 files changed, 280 insertions(+), 13 deletions(-) diff --git a/src/node/runtime/DevcontainerRuntime.test.ts b/src/node/runtime/DevcontainerRuntime.test.ts index 826c53b9c90..e8d47b73942 100644 --- a/src/node/runtime/DevcontainerRuntime.test.ts +++ b/src/node/runtime/DevcontainerRuntime.test.ts @@ -67,6 +67,62 @@ describe("DevcontainerRuntime.stat", () => { }); }); +interface DevcontainerRuntimeWithHomeValidation { + remoteHomeDir?: string; + remoteUser?: string; + verifyRemoteMuxHomeWritable(abortSignal?: AbortSignal): Promise; +} + +class HomeValidationDevcontainerRuntime extends DevcontainerRuntime { + commands: string[] = []; + exitCode = 0; + + override exec(command: string, _options: ExecOptions): Promise { + this.commands.push(command); + return Promise.resolve({ + stdout: createTextStream(""), + stderr: createTextStream(""), + stdin: createSinkStream(), + exitCode: Promise.resolve(this.exitCode), + duration: Promise.resolve(0), + }); + } +} + +describe("DevcontainerRuntime remote Mux home validation", () => { + it("checks the configured remote user's Mux home without creating it", async () => { + const runtime = new HomeValidationDevcontainerRuntime({ + srcBaseDir: "/tmp/mux", + configPath: ".devcontainer/devcontainer.json", + }); + const internals = runtime as unknown as DevcontainerRuntimeWithHomeValidation; + internals.remoteUser = "node"; + internals.remoteHomeDir = "/home/node"; + + await internals.verifyRemoteMuxHomeWritable(); + + expect(runtime.commands).toEqual([ + 'if [ -e "$HOME/.mux" ]; then test -w "$HOME/.mux"; else test -w "$HOME"; fi', + ]); + }); + + it("reports an actionable error when its Mux home is not writable", async () => { + const runtime = new HomeValidationDevcontainerRuntime({ + srcBaseDir: "/tmp/mux", + configPath: ".devcontainer/devcontainer.json", + }); + runtime.exitCode = 1; + const internals = runtime as unknown as DevcontainerRuntimeWithHomeValidation; + internals.remoteUser = "node"; + internals.remoteHomeDir = "/home/node"; + + // eslint-disable-next-line @typescript-eslint/await-thenable -- bun:test expect().rejects requires await + await expect(internals.verifyRemoteMuxHomeWritable()).rejects.toThrow( + "Devcontainer Mux home is not writable; verify the remote user's HOME permissions." + ); + }); +}); + describe("DevcontainerRuntime.resolvePath", () => { it("resolves ~ to cached remoteHomeDir", async () => { const runtime = createRuntime({ remoteHomeDir: "/home/coder" }); diff --git a/src/node/runtime/DevcontainerRuntime.ts b/src/node/runtime/DevcontainerRuntime.ts index ae7a3ad9ffb..a6065401f75 100644 --- a/src/node/runtime/DevcontainerRuntime.ts +++ b/src/node/runtime/DevcontainerRuntime.ts @@ -267,6 +267,31 @@ export class DevcontainerRuntime extends LocalBaseRuntime { } } + /** + * Verify that Plan Mode can create its runtime-local Mux home without assuming + * the host user's ownership. This keeps a misconfigured remoteUser actionable + * instead of failing later from an unrelated file tool invocation. + */ + private async verifyRemoteMuxHomeWritable(abortSignal?: AbortSignal): Promise { + const stream = await this.exec( + 'if [ -e "$HOME/.mux" ]; then test -w "$HOME/.mux"; else test -w "$HOME"; fi', + { + cwd: this.getContainerBasePath(), + timeout: 10, + abortSignal, + } + ); + await stream.stdin.close(); + const exitCode = await stream.exitCode; + + if (exitCode !== 0) { + throw new RuntimeError( + "Devcontainer Mux home is not writable; verify the remote user's HOME permissions.", + "file_io" + ); + } + } + private async fetchRemoteHome(abortSignal?: AbortSignal): Promise { if (!this.currentWorkspacePath) return; if (abortSignal?.aborted) return; @@ -549,6 +574,7 @@ export class DevcontainerRuntime extends LocalBaseRuntime { this.remoteUser = result.remoteUser; this.currentWorkspacePath = workspacePath; await this.fetchRemoteHome(abortSignal); + await this.verifyRemoteMuxHomeWritable(abortSignal); await this.setupCredentials(env, abortSignal); @@ -851,6 +877,7 @@ export class DevcontainerRuntime extends LocalBaseRuntime { this.remoteWorkspaceFolder = result.remoteWorkspaceFolder; this.remoteUser = result.remoteUser; await this.fetchRemoteHome(options?.signal); + await this.verifyRemoteMuxHomeWritable(options?.signal); await this.setupCredentials(this.lastCredentialEnv, options?.signal); diff --git a/src/node/services/tools/fileCommon.test.ts b/src/node/services/tools/fileCommon.test.ts index 78f514e2a07..3ee0d2d717a 100644 --- a/src/node/services/tools/fileCommon.test.ts +++ b/src/node/services/tools/fileCommon.test.ts @@ -10,6 +10,7 @@ import { MAX_FILE_SIZE, } from "./fileCommon"; import type { createRuntime as CreateRuntimeFn } from "@/node/runtime/runtimeFactory"; +import { DevcontainerRuntime } from "@/node/runtime/DevcontainerRuntime"; /* eslint-disable @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment */ const { @@ -205,28 +206,65 @@ describe("fileCommon", () => { const cwd = "/workspace/project"; const runtime = createRuntime({ type: "local", srcBaseDir: cwd }); - it("keeps resolving configured plan files outside cwd", () => { + it("keeps resolving configured plan files outside cwd", async () => { const planFilePath = "/home/user/.mux/plans/plan.md"; - const result = resolvePathWithinCwd(planFilePath, cwd, runtime); + const result = await resolvePathWithinCwd(planFilePath, cwd, runtime); expect(result.correctedPath).toBe(planFilePath); expect(result.resolvedPath).toBe(planFilePath); }); - it("keeps resolving unrelated absolute paths outside cwd", () => { + it("keeps resolving unrelated absolute paths outside cwd", async () => { const otherPath = "/home/user/.mux/plans/other.md"; - const result = resolvePathWithinCwd(otherPath, cwd, runtime); + const result = await resolvePathWithinCwd(otherPath, cwd, runtime); expect(result.correctedPath).toBe(otherPath); expect(result.resolvedPath).toBe(otherPath); }); - it("resolves relative paths that traverse outside cwd", () => { - const result = resolvePathWithinCwd("../plans/ancestor.md", cwd, runtime); + it("resolves relative paths that traverse outside cwd", async () => { + const result = await resolvePathWithinCwd("../plans/ancestor.md", cwd, runtime); expect(result.correctedPath).toBe("../plans/ancestor.md"); expect(result.resolvedPath).toBe("/workspace/plans/ancestor.md"); }); + + it("resolves tilde paths using the Dev Container remote home", async () => { + const runtime = new DevcontainerRuntime({ + srcBaseDir: "/tmp/mux", + configPath: ".devcontainer/devcontainer.json", + }); + const runtimeState = runtime as unknown as { remoteHomeDir?: string }; + runtimeState.remoteHomeDir = "/home/node"; + + const result = await resolvePathWithinCwd( + "~/.mux/plans/test-project/devcontainer-plan.md", + cwd, + runtime + ); + + expect(result.resolvedPath).toBe("/home/node/.mux/plans/test-project/devcontainer-plan.md"); + expect(result.resolvedPath).not.toBe( + "/home/host-user/.mux/plans/test-project/devcontainer-plan.md" + ); + }); + + it("uses the Dev Container remote-user fallback before its home is cached", async () => { + const runtime = new DevcontainerRuntime({ + srcBaseDir: "/tmp/mux", + configPath: ".devcontainer/devcontainer.json", + }); + const runtimeState = runtime as unknown as { remoteUser?: string }; + runtimeState.remoteUser = "node"; + + const result = await resolvePathWithinCwd( + "~/.mux/plans/test-project/devcontainer-plan.md", + cwd, + runtime + ); + + expect(result.resolvedPath).toBe("/home/node/.mux/plans/test-project/devcontainer-plan.md"); + }); }); describe("validateNoRedundantPrefix", () => { diff --git a/src/node/services/tools/fileCommon.ts b/src/node/services/tools/fileCommon.ts index 9d87ceee7b1..9c2dc5e7757 100644 --- a/src/node/services/tools/fileCommon.ts +++ b/src/node/services/tools/fileCommon.ts @@ -278,16 +278,26 @@ export function validatePathInCwd( * the exact path the user asked us to touch instead of imposing a stricter * workspace-only rule. */ -export function resolvePathWithinCwd( +export async function resolvePathWithinCwd( filePath: string, cwd: string, runtime: Runtime -): { correctedPath: string; resolvedPath: string; warning?: string } { +): Promise<{ correctedPath: string; resolvedPath: string; warning?: string }> { const redundantPrefixResult = validateNoRedundantPrefix(filePath, cwd, runtime); const correctedPath = redundantPrefixResult?.correctedPath ?? filePath; + const trimmedPath = correctedPath.trim(); + + // A Dev Container's home belongs to its remote user, not the Mux host process. + // Resolve tilde paths through the runtime so a tool never leaks host HOME into a + // command that will execute inside the container. + const resolvedPath = + trimmedPath === "~" || trimmedPath.startsWith("~/") + ? await runtime.resolvePath(correctedPath) + : runtime.normalizePath(correctedPath, cwd); + return { correctedPath, - resolvedPath: runtime.normalizePath(correctedPath, cwd), + resolvedPath, warning: redundantPrefixResult?.warning, }; } diff --git a/src/node/services/tools/file_edit_insert.test.ts b/src/node/services/tools/file_edit_insert.test.ts index d29451c25ea..dbe84b102d3 100644 --- a/src/node/services/tools/file_edit_insert.test.ts +++ b/src/node/services/tools/file_edit_insert.test.ts @@ -6,6 +6,7 @@ import { createFileEditInsertTool } from "./file_edit_insert"; import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/common/types/tools"; import type { ToolExecutionOptions } from "ai"; import { createRuntime } from "@/node/runtime/runtimeFactory"; +import { DevcontainerRuntime } from "@/node/runtime/DevcontainerRuntime"; import { getTestDeps } from "./testHelpers"; const mockToolCallOptions: ToolExecutionOptions = { @@ -172,6 +173,49 @@ describe("file_edit_insert tool", () => { }); }); +class RecordingDevcontainerRuntime extends DevcontainerRuntime { + writtenPaths: string[] = []; + + override stat(): Promise { + return Promise.reject(new Error("file does not exist")); + } + + override writeFile(filePath: string): WritableStream { + this.writtenPaths.push(filePath); + return new WritableStream(); + } +} + +it("creates plan files in the Dev Container user's home", async () => { + const planPath = "~/.mux/plans/test-project/devcontainer-plan.md"; + const runtime = new RecordingDevcontainerRuntime({ + srcBaseDir: "/tmp/mux", + configPath: ".devcontainer/devcontainer.json", + }); + const runtimeState = runtime as unknown as { remoteUser?: string }; + runtimeState.remoteUser = "node"; + + const tool = createFileEditInsertTool({ + ...getTestDeps(), + cwd: "/workspace/project", + runtime, + runtimeTempDir: "/tmp", + planFileOnly: true, + planFilePath: planPath, + }); + + const result = (await tool.execute!( + { path: planPath, content: "# Plan\n" }, + mockToolCallOptions + )) as FileEditInsertToolResult; + + expect(result.success).toBe(true); + expect(runtime.writtenPaths).toEqual(["/home/node/.mux/plans/test-project/devcontainer-plan.md"]); + expect(runtime.writtenPaths).not.toContain( + "/home/host-user/.mux/plans/test-project/devcontainer-plan.md" + ); +}); + describe("file_edit_insert outside-cwd access", () => { it("allows traversal outside cwd", async () => { const rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "file-edit-insert-outside-")); diff --git a/src/node/services/tools/file_edit_insert.ts b/src/node/services/tools/file_edit_insert.ts index 12a7f6a6f6b..bf1a1fdae3d 100644 --- a/src/node/services/tools/file_edit_insert.ts +++ b/src/node/services/tools/file_edit_insert.ts @@ -57,7 +57,7 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) correctedPath, warning: pathWarning, resolvedPath, - } = resolvePathWithinCwd(path, config.cwd, config.runtime); + } = await resolvePathWithinCwd(path, config.cwd, config.runtime); path = correctedPath; // Validate plan mode access restrictions diff --git a/src/node/services/tools/file_edit_operation.test.ts b/src/node/services/tools/file_edit_operation.test.ts index 519c1ac7e07..d69f4916e62 100644 --- a/src/node/services/tools/file_edit_operation.test.ts +++ b/src/node/services/tools/file_edit_operation.test.ts @@ -71,6 +71,54 @@ describe("executeFileEditOperation", () => { expect(normalizeCallForFilePath.basePath).toBe(testCwd); } }); + + test("uses the runtime home for Dev Container tilde edits", async () => { + const planPath = "~/.mux/plans/test-project/devcontainer-plan.md"; + const resolvedPlanPath = "/home/node/.mux/plans/test-project/devcontainer-plan.md"; + const writtenPaths: string[] = []; + const runtime = { + normalizePath: jest.fn<(targetPath: string, basePath: string) => string>( + () => "/home/host-user/.mux/plans/test-project/devcontainer-plan.md" + ), + resolvePath: jest.fn<(targetPath: string) => Promise>(() => + Promise.resolve(resolvedPlanPath) + ), + stat: jest + .fn<() => Promise<{ size: number; modifiedTime: Date; isDirectory: boolean }>>() + .mockResolvedValue({ size: 5, modifiedTime: new Date(), isDirectory: false }), + readFile: jest.fn<() => ReadableStream>( + () => + new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("old\n")); + controller.close(); + }, + }) + ), + writeFile: jest.fn<(filePath: string) => WritableStream>((filePath: string) => { + writtenPaths.push(filePath); + return new WritableStream(); + }), + } as unknown as Runtime; + + const result = await executeFileEditOperation({ + config: { + cwd: "/workspace/project", + runtime, + runtimeTempDir: "/tmp", + ...getTestDeps(), + }, + filePath: planPath, + operation: () => ({ success: true, newContent: "updated\n", metadata: {} }), + }); + + expect(result.success).toBe(true); + // eslint-disable-next-line @typescript-eslint/unbound-method -- asserting calls on a Jest mock + expect(runtime.resolvePath).toHaveBeenCalledWith(planPath); + // eslint-disable-next-line @typescript-eslint/unbound-method -- asserting calls on a Jest mock + expect(runtime.normalizePath).not.toHaveBeenCalled(); + expect(writtenPaths).toEqual([resolvedPlanPath]); + }); }); describe("executeFileEditOperation outside-cwd access", () => { diff --git a/src/node/services/tools/file_edit_operation.ts b/src/node/services/tools/file_edit_operation.ts index a99c76e33ee..936abe4f90f 100644 --- a/src/node/services/tools/file_edit_operation.ts +++ b/src/node/services/tools/file_edit_operation.ts @@ -117,7 +117,7 @@ export async function executeFileEditOperation({ correctedPath: validatedPath, warning: pathWarning, resolvedPath, - } = resolvePathWithinCwd(filePath, config.cwd, config.runtime); + } = await resolvePathWithinCwd(filePath, config.cwd, config.runtime); filePath = validatedPath; // Validate plan mode access restrictions diff --git a/src/node/services/tools/file_read.test.ts b/src/node/services/tools/file_read.test.ts index bf63ad0f52c..5471233ab7d 100644 --- a/src/node/services/tools/file_read.test.ts +++ b/src/node/services/tools/file_read.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, afterEach } from "bun:test"; import { LocalRuntime } from "@/node/runtime/LocalRuntime"; +import { DevcontainerRuntime } from "@/node/runtime/DevcontainerRuntime"; import * as fs from "fs/promises"; import * as path from "path"; import * as os from "os"; @@ -32,6 +33,24 @@ function createTestFileReadTool(options?: { cwd?: string }) { }; } +class RecordingDevcontainerRuntime extends DevcontainerRuntime { + readPaths: string[] = []; + + override stat(): Promise<{ size: number; modifiedTime: Date; isDirectory: boolean }> { + return Promise.resolve({ size: 5, modifiedTime: new Date(), isDirectory: false }); + } + + override readFile(filePath: string): ReadableStream { + this.readPaths.push(filePath); + return new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("plan\n")); + controller.close(); + }, + }); + } +} + describe("file_read tool", () => { let testDir: string; let testFilePath: string; @@ -47,6 +66,31 @@ describe("file_read tool", () => { await fs.rm(testDir, { recursive: true, force: true }); }); + it("reads tilde paths from the Dev Container user's home", async () => { + const planPath = "~/.mux/plans/test-project/devcontainer-plan.md"; + const runtime = new RecordingDevcontainerRuntime({ + srcBaseDir: "/tmp/mux", + configPath: ".devcontainer/devcontainer.json", + }); + const runtimeState = runtime as unknown as { remoteUser?: string }; + runtimeState.remoteUser = "node"; + + const tool = createFileReadTool({ + ...getTestDeps(), + cwd: "/workspace/project", + runtime, + runtimeTempDir: "/tmp", + }); + + const result = (await tool.execute!( + { path: planPath }, + mockToolCallOptions + )) as FileReadToolResult; + + expect(result.success).toBe(true); + expect(runtime.readPaths).toEqual(["/home/node/.mux/plans/test-project/devcontainer-plan.md"]); + }); + it("should read entire file with line numbers", async () => { // Setup const content = "line one\nline two\nline three"; diff --git a/src/node/services/tools/file_read.ts b/src/node/services/tools/file_read.ts index f9bda35efb7..7967603bced 100644 --- a/src/node/services/tools/file_read.ts +++ b/src/node/services/tools/file_read.ts @@ -28,7 +28,7 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { correctedPath: validatedPath, warning: pathWarning, resolvedPath, - } = resolvePathWithinCwd(filePath, config.cwd, config.runtime); + } = await resolvePathWithinCwd(filePath, config.cwd, config.runtime); filePath = validatedPath; // Check if file exists using runtime diff --git a/src/node/utils/attachments/readAttachmentFromPath.ts b/src/node/utils/attachments/readAttachmentFromPath.ts index 1b239a7ad3a..b61f3d721a5 100644 --- a/src/node/utils/attachments/readAttachmentFromPath.ts +++ b/src/node/utils/attachments/readAttachmentFromPath.ts @@ -220,7 +220,7 @@ export async function readAttachFileFromPath( "attach_file requires a path" ); - const { resolvedPath } = resolvePathWithinCwd(args.path, args.cwd, args.runtime); + const { resolvedPath } = await resolvePathWithinCwd(args.path, args.cwd, args.runtime); const fileStat = await statRegularFile(args, resolvedPath); const filename = getFallbackFilename(resolvedPath, args.filename); const mediaType = getSupportedAttachmentMediaType({