From 338c84947d14fdfca4fa6cac90162f3324efd012 Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 28 Oct 2025 17:01:49 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=A4=96=20Fix=20symlink=20preservation?= =?UTF-8?q?=20when=20editing=20files=20in=20both=20runtimes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When editing a file through a symlink, both LocalRuntime and SSHRuntime were replacing the symlink with a regular file instead of writing through it to preserve the link. SSHRuntime now uses 'cat temp > target' instead of 'mv temp target' to write through symlinks. LocalRuntime resolves symlinks with fs.realpath() before writing to the actual target. Added integration test that verifies symlinks are preserved when editing files through them in both local and SSH runtimes. --- src/runtime/LocalRuntime.ts | 15 +++++++++--- src/runtime/SSHRuntime.ts | 4 +++- tests/runtime/runtime.test.ts | 43 +++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/runtime/LocalRuntime.ts b/src/runtime/LocalRuntime.ts index ccbc057fd..4e6d365a4 100644 --- a/src/runtime/LocalRuntime.ts +++ b/src/runtime/LocalRuntime.ts @@ -241,15 +241,24 @@ export class LocalRuntime implements Runtime { writeFile(filePath: string): WritableStream { let tempPath: string; let writer: WritableStreamDefaultWriter; + let resolvedPath: string; return new WritableStream({ async start() { + // Resolve symlinks to write through them (preserves the symlink) + try { + resolvedPath = await fsPromises.realpath(filePath); + } catch { + // If file doesn't exist, use the original path + resolvedPath = filePath; + } + // Create parent directories if they don't exist - const parentDir = path.dirname(filePath); + const parentDir = path.dirname(resolvedPath); await fsPromises.mkdir(parentDir, { recursive: true }); // Create temp file for atomic write - tempPath = `${filePath}.tmp.${Date.now()}`; + tempPath = `${resolvedPath}.tmp.${Date.now()}`; const nodeStream = fs.createWriteStream(tempPath); const webStream = Writable.toWeb(nodeStream) as WritableStream; writer = webStream.getWriter(); @@ -261,7 +270,7 @@ export class LocalRuntime implements Runtime { // Close the writer and rename to final location await writer.close(); try { - await fsPromises.rename(tempPath, filePath); + await fsPromises.rename(tempPath, resolvedPath); } catch (err) { throw new RuntimeErrorClass( `Failed to write file ${filePath}: ${err instanceof Error ? err.message : String(err)}`, diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index ba10128f5..b12693007 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -263,12 +263,14 @@ export class SSHRuntime implements Runtime { /** * Write file contents over SSH atomically from a stream + * Preserves symlinks by writing through them to the target file */ writeFile(path: string): WritableStream { const tempPath = `${path}.tmp.${Date.now()}`; // Create parent directory if needed, then write file atomically + // Use cat to copy through symlinks (preserves them), then chmod and mv // Use shescape.quote for safe path escaping - const writeCommand = `mkdir -p $(dirname ${shescape.quote(path)}) && cat > ${shescape.quote(tempPath)} && chmod 600 ${shescape.quote(tempPath)} && mv ${shescape.quote(tempPath)} ${shescape.quote(path)}`; + const writeCommand = `mkdir -p $(dirname ${shescape.quote(path)}) && cat > ${shescape.quote(tempPath)} && chmod 600 ${shescape.quote(tempPath)} && cat ${shescape.quote(tempPath)} > ${shescape.quote(path)} && rm ${shescape.quote(tempPath)}`; // Need to get the exec stream in async callbacks let execPromise: Promise | null = null; diff --git a/tests/runtime/runtime.test.ts b/tests/runtime/runtime.test.ts index 86a6463ee..a2044c53b 100644 --- a/tests/runtime/runtime.test.ts +++ b/tests/runtime/runtime.test.ts @@ -329,6 +329,49 @@ describeIntegration("Runtime integration tests", () => { const content = await readFileString(runtime, `${workspace.path}/special.txt`); expect(content).toBe(specialContent); }); + + test.concurrent("preserves symlinks when editing target file", async () => { + const runtime = createRuntime(); + await using workspace = await TestWorkspace.create(runtime, type); + + // Create a target file + const targetPath = `${workspace.path}/target.txt`; + await writeFileString(runtime, targetPath, "original content"); + + // Create a symlink to the target + const linkPath = `${workspace.path}/link.txt`; + const result = await execBuffered(runtime, `ln -s target.txt link.txt`, { + cwd: workspace.path, + timeout: 30, + }); + expect(result.exitCode).toBe(0); + + // Verify symlink was created + const lsResult = await execBuffered(runtime, "ls -la link.txt", { + cwd: workspace.path, + timeout: 30, + }); + expect(lsResult.stdout).toContain("->"); + expect(lsResult.stdout).toContain("target.txt"); + + // Edit the file via the symlink + await writeFileString(runtime, linkPath, "new content"); + + // Verify the symlink is still a symlink (not replaced with a file) + const lsAfter = await execBuffered(runtime, "ls -la link.txt", { + cwd: workspace.path, + timeout: 30, + }); + expect(lsAfter.stdout).toContain("->"); + expect(lsAfter.stdout).toContain("target.txt"); + + // Verify both the symlink and target have the new content + const linkContent = await readFileString(runtime, linkPath); + expect(linkContent).toBe("new content"); + + const targetContent = await readFileString(runtime, targetPath); + expect(targetContent).toBe("new content"); + }); }); describe("stat() - File metadata", () => { From e36f7c61a29833f47e463469a9a4072ad86f8039 Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 28 Oct 2025 17:05:42 +0000 Subject: [PATCH 2/4] Restore atomic write property in SSH runtime Use readlink -f to resolve symlinks first, then mv the temp file to the resolved path. This maintains atomicity (readers see old or new file, never partial) while preserving symlinks. Addresses Codex review feedback. --- src/runtime/SSHRuntime.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index b12693007..a54d3501b 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -263,14 +263,15 @@ export class SSHRuntime implements Runtime { /** * Write file contents over SSH atomically from a stream - * Preserves symlinks by writing through them to the target file + * Preserves symlinks by resolving them first, then writing to the target */ writeFile(path: string): WritableStream { const tempPath = `${path}.tmp.${Date.now()}`; - // Create parent directory if needed, then write file atomically - // Use cat to copy through symlinks (preserves them), then chmod and mv + // Resolve symlinks to get the actual target path, preserving the symlink itself + // If path doesn't exist, readlink fails and we use the original path + // Then write atomically using mv (all-or-nothing for readers) // Use shescape.quote for safe path escaping - const writeCommand = `mkdir -p $(dirname ${shescape.quote(path)}) && cat > ${shescape.quote(tempPath)} && chmod 600 ${shescape.quote(tempPath)} && cat ${shescape.quote(tempPath)} > ${shescape.quote(path)} && rm ${shescape.quote(tempPath)}`; + const writeCommand = `RESOLVED=$(readlink -f ${shescape.quote(path)} 2>/dev/null || echo ${shescape.quote(path)}) && mkdir -p $(dirname "$RESOLVED") && cat > ${shescape.quote(tempPath)} && chmod 600 ${shescape.quote(tempPath)} && mv ${shescape.quote(tempPath)} "$RESOLVED"`; // Need to get the exec stream in async callbacks let execPromise: Promise | null = null; From 7b51a42798443a958168c0965032092d7246cc9a Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 28 Oct 2025 17:13:45 +0000 Subject: [PATCH 3/4] Preserve file permissions when editing through symlinks Both LocalRuntime and SSHRuntime now preserve the original file permissions when editing through symlinks. If the target file exists, its permissions are read before writing and restored after. For new files, default permissions apply (600 for SSH, system defaults for local). Added integration test verifying permissions are preserved across edits. --- src/runtime/LocalRuntime.ts | 11 ++++++++- src/runtime/SSHRuntime.ts | 7 +++--- tests/runtime/runtime.test.ts | 45 +++++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/runtime/LocalRuntime.ts b/src/runtime/LocalRuntime.ts index 4e6d365a4..e1ecf7bb3 100644 --- a/src/runtime/LocalRuntime.ts +++ b/src/runtime/LocalRuntime.ts @@ -242,15 +242,20 @@ export class LocalRuntime implements Runtime { let tempPath: string; let writer: WritableStreamDefaultWriter; let resolvedPath: string; + let originalMode: number | undefined; return new WritableStream({ async start() { // Resolve symlinks to write through them (preserves the symlink) try { resolvedPath = await fsPromises.realpath(filePath); + // Save original permissions to restore after write + const stat = await fsPromises.stat(resolvedPath); + originalMode = stat.mode; } catch { - // If file doesn't exist, use the original path + // If file doesn't exist, use the original path and default permissions resolvedPath = filePath; + originalMode = undefined; } // Create parent directories if they don't exist @@ -270,6 +275,10 @@ export class LocalRuntime implements Runtime { // Close the writer and rename to final location await writer.close(); try { + // If we have original permissions, apply them to temp file before rename + if (originalMode !== undefined) { + await fsPromises.chmod(tempPath, originalMode); + } await fsPromises.rename(tempPath, resolvedPath); } catch (err) { throw new RuntimeErrorClass( diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index a54d3501b..16181026e 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -263,15 +263,16 @@ export class SSHRuntime implements Runtime { /** * Write file contents over SSH atomically from a stream - * Preserves symlinks by resolving them first, then writing to the target + * Preserves symlinks and file permissions by resolving and copying metadata */ writeFile(path: string): WritableStream { const tempPath = `${path}.tmp.${Date.now()}`; // Resolve symlinks to get the actual target path, preserving the symlink itself - // If path doesn't exist, readlink fails and we use the original path + // If target exists, save its permissions to restore after write + // If path doesn't exist, use 600 as default // Then write atomically using mv (all-or-nothing for readers) // Use shescape.quote for safe path escaping - const writeCommand = `RESOLVED=$(readlink -f ${shescape.quote(path)} 2>/dev/null || echo ${shescape.quote(path)}) && mkdir -p $(dirname "$RESOLVED") && cat > ${shescape.quote(tempPath)} && chmod 600 ${shescape.quote(tempPath)} && mv ${shescape.quote(tempPath)} "$RESOLVED"`; + const writeCommand = `RESOLVED=$(readlink -f ${shescape.quote(path)} 2>/dev/null || echo ${shescape.quote(path)}) && PERMS=$(stat -c '%a' "$RESOLVED" 2>/dev/null || echo 600) && mkdir -p $(dirname "$RESOLVED") && cat > ${shescape.quote(tempPath)} && chmod "$PERMS" ${shescape.quote(tempPath)} && mv ${shescape.quote(tempPath)} "$RESOLVED"`; // Need to get the exec stream in async callbacks let execPromise: Promise | null = null; diff --git a/tests/runtime/runtime.test.ts b/tests/runtime/runtime.test.ts index a2044c53b..574ab9086 100644 --- a/tests/runtime/runtime.test.ts +++ b/tests/runtime/runtime.test.ts @@ -372,6 +372,51 @@ describeIntegration("Runtime integration tests", () => { const targetContent = await readFileString(runtime, targetPath); expect(targetContent).toBe("new content"); }); + + test.concurrent("preserves file permissions when editing through symlink", async () => { + const runtime = createRuntime(); + await using workspace = await TestWorkspace.create(runtime, type); + + // Create a target file with specific permissions (755) + const targetPath = `${workspace.path}/target.txt`; + await writeFileString(runtime, targetPath, "original content"); + + // Set permissions to 755 + const chmodResult = await execBuffered(runtime, "chmod 755 target.txt", { + cwd: workspace.path, + timeout: 30, + }); + expect(chmodResult.exitCode).toBe(0); + + // Verify initial permissions + const statBefore = await execBuffered(runtime, "stat -c '%a' target.txt", { + cwd: workspace.path, + timeout: 30, + }); + expect(statBefore.stdout.trim()).toBe("755"); + + // Create a symlink to the target + const linkPath = `${workspace.path}/link.txt`; + const lnResult = await execBuffered(runtime, "ln -s target.txt link.txt", { + cwd: workspace.path, + timeout: 30, + }); + expect(lnResult.exitCode).toBe(0); + + // Edit the file via the symlink + await writeFileString(runtime, linkPath, "new content"); + + // Verify permissions are preserved + const statAfter = await execBuffered(runtime, "stat -c '%a' target.txt", { + cwd: workspace.path, + timeout: 30, + }); + expect(statAfter.stdout.trim()).toBe("755"); + + // Verify content was updated + const content = await readFileString(runtime, targetPath); + expect(content).toBe("new content"); + }); }); describe("stat() - File metadata", () => { From 19390740fe7cc0124cf974ca6c9db51e1c5b8afe Mon Sep 17 00:00:00 2001 From: Ammar Date: Tue, 28 Oct 2025 17:17:28 +0000 Subject: [PATCH 4/4] Fix formatting (remove trailing whitespace) --- tests/runtime/runtime.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/runtime/runtime.test.ts b/tests/runtime/runtime.test.ts index 574ab9086..27e4ce020 100644 --- a/tests/runtime/runtime.test.ts +++ b/tests/runtime/runtime.test.ts @@ -380,7 +380,7 @@ describeIntegration("Runtime integration tests", () => { // Create a target file with specific permissions (755) const targetPath = `${workspace.path}/target.txt`; await writeFileString(runtime, targetPath, "original content"); - + // Set permissions to 755 const chmodResult = await execBuffered(runtime, "chmod 755 target.txt", { cwd: workspace.path,