fix(workspace): check the deny list through links; write files atomically

CHANGELOG.md

@@ -16,6 +16,8 @@ opt-out.
 - `lousho eval` without vitest installed now says it runs `*.eval.ts` files through vitest, the directory it searched from, and the install command; [Evals](docs/evals.md) states the requirement up front.
 
 ### Fixed
+- The file tools' `deny` list (`.git` by default) is now also checked where a path really leads: a symlink or Windows junction inside the workspace pointing at `.git` or into it (`gitj -> .git`) let `edit_file gitj/config` rewrite `.git/config` (for example `core.hooksPath`, which runs code on the next git command) and `write_file gitj/PWNED` create files in `.git`. Reads, writes, edits, `list_dir`, and `glob`/`grep` given such a link as `path` are now refused. Providers with links can implement the new optional `FsProvider.realPath(path)`; `NodeWorkspace` does. See [Workspace tools](docs/workspace-tools.md#security-model).
+- `NodeWorkspace.writeFile` (and so `write_file` / `edit_file`) writes a temp file in the same directory and renames it into place, so a process killed mid-write no longer leaves a truncated file. A replaced file keeps its permission bits; a read-only file is still refused.
 - MCP tool schemas with OpenAPI's `nullable: true` (any type) now accept `null`; the model's valid `null` was rejected with `ToolArgumentsValidationError`.
 - A failed MCP tool call (server exited, timeout, protocol error) now rejects with `LOUSHO_MCP_CALL_FAILED`, naming `<server>__<tool>` with the MCP error as `cause`, instead of a bare `MCP error -32000: Connection closed`. [MCP](docs/mcp.md) now says stdio `env` is merged onto the MCP SDK's default allow-list, not `process.env`.
 - Flows (consumer harness): a `{{variable}}` holding an object (a `toolCall` result, an MCP result) no longer reaches a prompt as `[object Object]`: an object whose only field is `text` (an MCP tool result) is that text, a raw MCP `{ content }` result its text parts, and any other object or array its JSON. Placeholders take dotted paths (`{{res.user.name}}`, `{{list.0}}`, own properties only), also as whole tool-argument values. An `evaluator` step stores its result in `outputVariable`. See [Template values](docs/flows.md#template-values).

api/index.api.md

@@ -2578,6 +2578,7 @@ export interface FsProvider {
     readdir(path: string): Promise<WorkspaceDirEntry[]>;
     readFile(path: string): Promise<string>;
     readFileBytes?(path: string): Promise<Uint8Array>;
+    realPath?(path: string): Promise<string>;
     rm(path: string, options?: {
         recursive?: boolean;
     }): Promise<void>;
@@ -4098,14 +4099,14 @@ export class NodeWorkspace implements Workspace {
     readFile(path: string): Promise<string>;
     // (undocumented)
     readFileBytes(path: string): Promise<Uint8Array>;
+    realPath(path: string): Promise<string>;
     rm(path: string, options?: {
         recursive?: boolean;
     }): Promise<void>;
     readonly root: string;
     readonly shell: string;
     // (undocumented)
     stat(path: string): Promise<WorkspaceStat | undefined>;
-    // (undocumented)
     writeFile(path: string, content: string): Promise<void>;
 }
 

api/tools.api.md

@@ -857,6 +857,7 @@ export interface FsProvider {
     readdir(path: string): Promise<WorkspaceDirEntry[]>;
     readFile(path: string): Promise<string>;
     readFileBytes?(path: string): Promise<Uint8Array>;
+    realPath?(path: string): Promise<string>;
     rm(path: string, options?: {
         recursive?: boolean;
     }): Promise<void>;
@@ -1531,14 +1532,14 @@ export class NodeWorkspace implements Workspace {
     readFile(path: string): Promise<string>;
     // (undocumented)
     readFileBytes(path: string): Promise<Uint8Array>;
+    realPath(path: string): Promise<string>;
     rm(path: string, options?: {
         recursive?: boolean;
     }): Promise<void>;
     readonly root: string;
     readonly shell: string;
     // (undocumented)
     stat(path: string): Promise<WorkspaceStat | undefined>;
-    // (undocumented)
     writeFile(path: string, content: string): Promise<void>;
 }
 

docs/workspace-tools.md

@@ -241,6 +241,12 @@ path. This means:
 - `glob` and `grep` never follow links while walking the tree.
 - `rm` on a link removes the link itself and leaves its target alone.
 
+**Writes are atomic.** `NodeWorkspace` writes a file (for `write_file` and
+`edit_file` too) to a temp file in the same directory, flushes it and renames
+it over the target, so a process killed mid-write leaves the old file or the
+new one, never half of it. A replaced file keeps its permission bits, and a
+read-only file is still refused.
+
 **The file tools keep out of `.git`.** A checkout often has credentials in
 `.git/config`: `actions/checkout` keeps the job's token there as
 `http.https://github.com/.extraheader = AUTHORIZATION: basic ...`, and a
@@ -264,9 +270,19 @@ const workspace = new NodeWorkspace({ root: '.' });
 const tools = createFsTools(workspace, { readOnly: true, deny: ['.git', '.env'] });
 ```
 
-The check is by name, so a symlink elsewhere in the workspace that points
-into `.git` is not caught (`glob` and `grep` never follow links, but
-`read_file` on the link does). **The `shell` tool is not covered:** a command
+Links into `.git` are refused too. With `NodeWorkspace` (or any provider that
+implements `realPath`), the check also runs on where a path really leads, so a
+symlink or Windows junction elsewhere in the workspace that points at `.git`
+or into it (`gitj -> .git`, `hooks -> .git/hooks`, `cfg -> .git/config`)
+cannot be used to read, write or edit what is behind it: `read_file
+gitj/config`, `edit_file gitj/config` and `write_file gitj/PWNED` are tool
+errors, and so are `list_dir`, `glob` and `grep` given the link as their
+`path` (`grep` with `path: 'gitj'` or `path: 'gitj/config'`). `glob` and
+`grep` never follow links while walking the tree, so a search from the root
+does not reach `.git` through a link either. A provider without `realPath`
+(`MemoryWorkspace` has no links) gets the name check only.
+
+**The `shell` tool is not covered:** a command
 is free text, so it cannot be path-filtered, and `cat .git/config` or
 `git config --get-regexp extraheader` still works. Git commands that coding
 agents need (`git status`, `git diff`) go through the shell tool, so keep its

llms-full.txt

@@ -20017,6 +20017,12 @@ path. This means:
 - `glob` and `grep` never follow links while walking the tree.
 - `rm` on a link removes the link itself and leaves its target alone.
 
+**Writes are atomic.** `NodeWorkspace` writes a file (for `write_file` and
+`edit_file` too) to a temp file in the same directory, flushes it and renames
+it over the target, so a process killed mid-write leaves the old file or the
+new one, never half of it. A replaced file keeps its permission bits, and a
+read-only file is still refused.
+
 **The file tools keep out of `.git`.** A checkout often has credentials in
 `.git/config`: `actions/checkout` keeps the job's token there as
 `http.https://github.com/.extraheader = AUTHORIZATION: basic ...`, and a
@@ -20040,9 +20046,19 @@ const workspace = new NodeWorkspace({ root: '.' });
 const tools = createFsTools(workspace, { readOnly: true, deny: ['.git', '.env'] });
 ```
 
-The check is by name, so a symlink elsewhere in the workspace that points
-into `.git` is not caught (`glob` and `grep` never follow links, but
-`read_file` on the link does). **The `shell` tool is not covered:** a command
+Links into `.git` are refused too. With `NodeWorkspace` (or any provider that
+implements `realPath`), the check also runs on where a path really leads, so a
+symlink or Windows junction elsewhere in the workspace that points at `.git`
+or into it (`gitj -> .git`, `hooks -> .git/hooks`, `cfg -> .git/config`)
+cannot be used to read, write or edit what is behind it: `read_file
+gitj/config`, `edit_file gitj/config` and `write_file gitj/PWNED` are tool
+errors, and so are `list_dir`, `glob` and `grep` given the link as their
+`path` (`grep` with `path: 'gitj'` or `path: 'gitj/config'`). `glob` and
+`grep` never follow links while walking the tree, so a search from the root
+does not reach `.git` through a link either. A provider without `realPath`
+(`MemoryWorkspace` has no links) gets the name check only.
+
+**The `shell` tool is not covered:** a command
 is free text, so it cannot be path-filtered, and `cat .git/config` or
 `git config --get-regexp extraheader` still works. Git commands that coding
 agents need (`git status`, `git diff`) go through the shell tool, so keep its

src/storage/atomicFile.ts

@@ -73,10 +73,11 @@ async function syncDirectory(dir: string): Promise<void> {
   }
 }
 
-/** Writes and flushes `content` to a new file `temp`. */
-async function writeSynced(temp: string, content: string): Promise<void> {
+/** Writes and flushes `content` to a new file `temp`, with permission bits `mode` when given. */
+async function writeSynced(temp: string, content: string, mode: number | undefined): Promise<void> {
   const handle = await open(temp, 'wx');
   try {
+    if (mode !== undefined) await handle.chmod(mode);
     await handle.writeFile(content, 'utf8');
     await handle.sync();
   } finally {
@@ -86,17 +87,18 @@ async function writeSynced(temp: string, content: string): Promise<void> {
 
 /**
  * Writes `content` to `file` atomically and durably (see the module comment);
- * creates the directory when it is missing.
+ * creates the directory when it is missing. `mode` sets the new file's
+ * permission bits (to keep those of the file it replaces).
  */
-export async function writeFileAtomic(file: string, content: string): Promise<void> {
+export async function writeFileAtomic(file: string, content: string, { mode }: { mode?: number } = {}): Promise<void> {
   const temp = `${file}.${process.pid}.${randomUUID()}.tmp`;
   try {
     try {
-      await writeSynced(temp, content);
+      await writeSynced(temp, content, mode);
     } catch (error) {
       if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error;
       await mkdir(dirname(file), { recursive: true });
-      await writeSynced(temp, content);
+      await writeSynced(temp, content, mode);
     }
     await renameWithRetry(temp, file);
   } catch (error) {

src/tools/workspace/NodeWorkspace.test.ts

@@ -233,6 +233,152 @@ describe('NodeWorkspace file system', () => {
       expect(fs.existsSync(path.join(rootDir, 'src', 'a.ts'))).toBe(true);
     });
   });
+
+  // A link inside the workspace pointing at .git let edit_file set core.hooksPath in .git/config.
+  describe('deny list through links', () => {
+    const CONFIG = '[core]\n\tbare = false\n';
+    let gitRoot: string;
+    let gitWs: NodeWorkspace;
+    let gitTools: Record<string, DefinedTool>;
+
+    beforeAll(() => {
+      gitRoot = path.join(tmp, 'git-root');
+      fs.mkdirSync(path.join(gitRoot, '.git', 'hooks'), { recursive: true });
+      fs.writeFileSync(path.join(gitRoot, '.git', 'config'), CONFIG);
+      fs.writeFileSync(path.join(gitRoot, '.git', 'hooks', 'pre-commit.sample'), 'TOKEN\n');
+      fs.writeFileSync(path.join(gitRoot, 'readme.md'), 'hello\n');
+      gitWs = new NodeWorkspace({ root: gitRoot });
+      gitTools = Object.fromEntries(createFsTools(gitWs).map((t) => [t.name, t]));
+    });
+
+    const config = () => fs.readFileSync(path.join(gitRoot, '.git', 'config'), 'utf8');
+
+    /** Every way the file tools could reach `.git` through the directory link `name` (to `.git`) or `sub` (to `.git/hooks`). */
+    async function expectRefused(name: string, sub: string): Promise<void> {
+      const denied = /is denied: it resolves through a link/;
+      await expect(exec(gitTools.read_file, { path: `${name}/config` })).rejects.toThrow(denied);
+      await expect(exec(gitTools.edit_file, { path: `${name}/config`, old_string: 'bare = false', new_string: 'hooksPath = /tmp/x' })).rejects.toThrow(denied);
+      await expect(exec(gitTools.write_file, { path: `${name}/PWNED`, content: 'x' })).rejects.toThrow(denied);
+      await expect(exec(gitTools.write_file, { path: `${name}/new/dir/PWNED`, content: 'x' })).rejects.toThrow(denied);
+      await expect(exec(gitTools.write_file, { path: `${sub}/pre-commit`, content: 'x' })).rejects.toThrow(denied);
+      await expect(exec(gitTools.list_dir, { path: name })).rejects.toThrow(denied);
+      await expect(exec(gitTools.glob, { pattern: '**/*', path: name })).rejects.toThrow(denied);
+      await expect(exec(gitTools.grep, { pattern: 'TOKEN', path: name })).rejects.toThrow(denied);
+      await expect(exec(gitTools.grep, { pattern: 'bare', path: `${name}/config` })).rejects.toThrow(denied);
+      await expect(exec(gitTools.grep, { pattern: 'TOKEN', path: sub })).rejects.toThrow(denied);
+      // A walk from the root never follows the link.
+      expect(await exec(gitTools.grep, { pattern: 'TOKEN' })).toBe('No matches for "TOKEN".');
+      expect(config()).toBe(CONFIG);
+      expect(fs.existsSync(path.join(gitRoot, '.git', 'PWNED'))).toBe(false);
+      expect(fs.existsSync(path.join(gitRoot, '.git', 'new'))).toBe(false);
+      expect(fs.existsSync(path.join(gitRoot, '.git', 'hooks', 'pre-commit'))).toBe(false);
+    }
+
+    it('refuses reads, writes, edits, listings and searches through a junction to .git (or a directory symlink elsewhere)', async () => {
+      const type = IS_WINDOWS ? 'junction' : 'dir';
+      fs.symlinkSync(path.join(gitRoot, '.git'), path.join(gitRoot, 'gitj'), type);
+      fs.symlinkSync(path.join(gitRoot, '.git', 'hooks'), path.join(gitRoot, 'hooksj'), type);
+      try {
+        await expectRefused('gitj', 'hooksj');
+      } finally {
+        fs.rmSync(path.join(gitRoot, 'gitj'));
+        fs.rmSync(path.join(gitRoot, 'hooksj'));
+      }
+    });
+
+    it('refuses the same through a directory symlink', async (ctx) => {
+      const refused = trySymlink(path.join(gitRoot, '.git'), path.join(gitRoot, 'gits'), 'dir');
+      if (refused) {
+        console.warn(`Skipping .git directory-symlink test: the OS refused to create a symlink (${refused}).`);
+        ctx.skip();
+        return;
+      }
+      fs.symlinkSync(path.join(gitRoot, '.git', 'hooks'), path.join(gitRoot, 'hookss'), 'dir');
+      try {
+        await expectRefused('gits', 'hookss');
+      } finally {
+        fs.rmSync(path.join(gitRoot, 'gits'));
+        fs.rmSync(path.join(gitRoot, 'hookss'));
+      }
+    });
+
+    it('refuses a file symlink to .git/config', async (ctx) => {
+      const refused = trySymlink(path.join(gitRoot, '.git', 'config'), path.join(gitRoot, 'cfg'), 'file');
+      if (refused) {
+        console.warn(`Skipping .git file-symlink test: the OS refused to create a symlink (${refused}).`);
+        ctx.skip();
+        return;
+      }
+      try {
+        await expect(exec(gitTools.read_file, { path: 'cfg' })).rejects.toThrow(/is denied/);
+        await expect(exec(gitTools.edit_file, { path: 'cfg', old_string: 'bare', new_string: 'x' })).rejects.toThrow(/is denied/);
+        await expect(exec(gitTools.write_file, { path: 'cfg', content: 'x' })).rejects.toThrow(/is denied/);
+        expect(config()).toBe(CONFIG);
+      } finally {
+        fs.rmSync(path.join(gitRoot, 'cfg'));
+      }
+    });
+
+    it.runIf(IS_WINDOWS)('refuses a junction whose target is spelled .GIT', async () => {
+      fs.symlinkSync(path.join(gitRoot, '.GIT'), path.join(gitRoot, 'upper'), 'junction');
+      try {
+        await expect(exec(gitTools.write_file, { path: 'upper/PWNED', content: 'x' })).rejects.toThrow(/is denied/);
+        await expect(exec(gitTools.read_file, { path: 'upper/config' })).rejects.toThrow(/is denied/);
+      } finally {
+        fs.rmSync(path.join(gitRoot, 'upper'));
+      }
+    });
+
+    it('keeps links to allowed places working, and deny: [] opens .git through a link too', async () => {
+      const type = IS_WINDOWS ? 'junction' : 'dir';
+      fs.symlinkSync(path.join(gitRoot, '.git'), path.join(gitRoot, 'gitj'), type);
+      try {
+        expect(await gitWs.realPath('gitj/config')).toBe('.git/config');
+        expect(await gitWs.realPath('gitj/not/yet')).toBe('.git/not/yet');
+        expect(await gitWs.realPath('.')).toBe('.');
+        const open = Object.fromEntries(createFsTools(gitWs, { deny: [] }).map((t) => [t.name, t]));
+        expect(await exec(open.read_file, { path: 'gitj/config' })).toMatch(/bare = false/);
+        expect(await exec(gitTools.read_file, { path: 'readme.md' })).toBe('     1\thello');
+      } finally {
+        fs.rmSync(path.join(gitRoot, 'gitj'));
+      }
+    });
+  });
+
+  describe('writeFile replaces the file atomically', () => {
+    it('renames a new file into place: a hard link to the old file keeps the old content, and no temp file is left', async () => {
+      await ws.writeFile('atomic/a.txt', 'old');
+      fs.linkSync(path.join(rootDir, 'atomic', 'a.txt'), path.join(rootDir, 'atomic', 'old-link.txt'));
+      await ws.writeFile('atomic/a.txt', 'new');
+      expect(fs.readFileSync(path.join(rootDir, 'atomic', 'a.txt'), 'utf8')).toBe('new');
+      expect(fs.readFileSync(path.join(rootDir, 'atomic', 'old-link.txt'), 'utf8')).toBe('old');
+      expect(fs.readdirSync(path.join(rootDir, 'atomic')).sort()).toEqual(['a.txt', 'old-link.txt']);
+    });
+
+    it.skipIf(IS_WINDOWS)('keeps the permission bits of the file it replaces', async () => {
+      const file = path.join(rootDir, 'atomic', 'run.sh');
+      await ws.writeFile('atomic/run.sh', 'echo 1');
+      fs.chmodSync(file, 0o750);
+      await ws.writeFile('atomic/run.sh', 'echo 2');
+      expect(fs.statSync(file).mode & 0o777).toBe(0o750);
+    });
+
+    it('still refuses a read-only file and a directory', async () => {
+      const file = path.join(rootDir, 'atomic', 'locked.txt');
+      await ws.writeFile('atomic/locked.txt', 'keep');
+      fs.chmodSync(file, 0o444);
+      try {
+        // root may write any file, so there is nothing to refuse.
+        if (process.getuid?.() !== 0) {
+          await expect(ws.writeFile('atomic/locked.txt', 'x')).rejects.toThrow(/Permission denied|not permitted/);
+          expect(fs.readFileSync(file, 'utf8')).toBe('keep');
+        }
+      } finally {
+        fs.chmodSync(file, 0o644);
+      }
+      await expect(ws.writeFile('atomic', 'x')).rejects.toThrow('atomic is a directory, not a file.');
+    });
+  });
 });
 
 describe('NodeWorkspace shell', () => {