fix: critical symlink/../ escape via realpathSync lexical normalization divergence

Gemini 3.1 Pro found that `realpathSync` and the OS kernel disagree on
`symlink/../file.txt`:
- `realpathSync("project/link/..")` → `project/` (lexical normalization)
- `writeFile("project/link/../f")` → writes to parent of symlink TARGET (kernel)

This means `containsReal` would approve a write that the OS places OUTSIDE
the project boundary. The fix rejects any unresolved path containing `..`
segments, since their behavior through symlinks is fundamentally unpredictable
at the application level.

Also adds `.github` to `SENSITIVE_DIRS` (workflow injection vector).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: anandgupta42

packages/opencode/.github/meta/commit.txt

@@ -1,13 +1,13 @@
-fix: address code review findings — rule ordering bug, cross-platform paths, TOCTOU docs
+fix: address multi-model review consensus — movePath guard, case-insensitive matching, expanded patterns
 
-- Fix critical bug: bash deny defaults had `"*": "ask"` LAST which overrode deny rules
-  due to last-match-wins semantics. Moved `"*": "ask"` to first position so deny rules
-  take precedence.
-- Fix all doc examples with same ordering bug (security-faq.md, permissions.md)
-- Fix `isSensitiveWrite` to use regex split `/[/\\]/` for cross-platform path handling
-- Allow per-path "Always" approval for sensitive file writes (reduces approval fatigue)
-- Document TOCTOU limitation in `containsReal` JSDoc
-- Add doc clarification about last-match-wins rule ordering with examples
-- Add tests: bash deny defaults evaluation, user override merge, Windows backslash paths
+Fixes from consensus across GPT 5.2, Kimi K2.5, MiniMax M2.5, and GLM-5 reviews:
+
+- Add `assertSensitiveWrite(ctx, movePath)` for move destinations in `apply_patch`
+  (CRITICAL: 3 models flagged that moves to `.ssh/`, `.env` bypassed sensitive check)
+- Add case-insensitive matching on macOS/Windows for sensitive dirs and files
+  (`.GIT/config`, `.SSH/id_rsa` now correctly detected on case-insensitive FS)
+- Expand `SENSITIVE_FILES` with `.htpasswd`, `.pgpass`
+- Add `SENSITIVE_EXTENSIONS` for private keys: `.pem`, `.key`, `.p12`, `.pfx`
+- Add tests: case-insensitive matching, certificate extensions, credential files
 
 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

packages/opencode/src/file/protected.ts

@@ -45,6 +45,7 @@ const WIN32_HOME = ["AppData", "Downloads", "Desktop", "Documents", "Pictures",
  */
 const SENSITIVE_DIRS = [
   ".git",
+  ".github",
   ".ssh",
   ".gnupg",
   ".aws",

packages/opencode/src/util/filesystem.ts

@@ -181,11 +181,27 @@ export namespace Filesystem {
       // Child doesn't exist — walk up to find nearest existing ancestor
     }
 
+    // SECURITY: If the raw child path contains '..' segments, reject it.
+    // realpathSync normalizes '..' lexically (before symlink resolution),
+    // but the OS kernel resolves symlinks THEN applies '..'. For example:
+    //   realpathSync("project/symlink/..") → project/  (lexical)
+    //   writeFile("project/symlink/../f")  → writes outside project (kernel)
+    // Since we can't trust realpathSync's resolution of paths with '..',
+    // any path containing '..' that couldn't be fully resolved above is denied.
+    const segments = child.split(/[/\\]/)
+    if (segments.includes("..")) return false
+
     // Walk up the directory tree to find the nearest existing ancestor,
     // then append the remaining segments. This handles write operations
     // where the target directory hasn't been created yet.
-    const resolved = pathResolve(child)
-    let current = resolved
+    //
+    // CRITICAL: realpathSync normalizes '..' lexically (before symlink resolution),
+    // but the OS kernel resolves symlinks THEN applies '..'. For example:
+    //   realpathSync("project/symlink/..") → project/  (lexical)
+    //   writeFile("project/symlink/../f")  → writes outside project (kernel)
+    // Therefore, if any trailing segment is '..', we MUST deny the access since
+    // we cannot predict where the OS will actually write.
+    let current = child
     const trailing: string[] = []
     while (true) {
       try {

packages/opencode/test/file/security-e2e.test.ts

@@ -131,6 +131,32 @@ describe("E2E: symlink escape attacks", () => {
     })
   })
 
+  test("symlink/../file.txt escape for non-existent write target is blocked", async () => {
+    // CRITICAL: This tests the Gemini-found vulnerability where path.resolve()
+    // strips '..' lexically before symlinks are resolved. The agent writes to
+    // /project/symlink/../secret.txt. Without the fix, pathResolve normalizes
+    // this to /project/secret.txt (looks safe). With the fix, realpathSync on
+    // /project/symlink/.. correctly follows the symlink first.
+    await using outside = await tmpdir({
+      init: async (dir) => {
+        await Bun.write(path.join(dir, "existing.txt"), "data")
+      },
+    })
+
+    await using project = await tmpdir({
+      init: async (dir) => {
+        // symlink inside project pointing outside
+        await fs.symlink(outside.path, path.join(dir, "link"))
+      },
+    })
+
+    // /project/link/../new-file.txt — OS resolves link to outside, then ..
+    // goes to outside's parent. This must be DENIED.
+    // NOTE: path.join normalizes away '..', so we construct the path manually
+    const escapePath = project.path + "/link/../new-file.txt"
+    expect(Filesystem.containsReal(project.path, escapePath)).toBe(false)
+  })
+
   test("relative symlink that resolves outside is blocked", async () => {
     await using outside = await tmpdir({
       init: async (dir) => {

test_bypass.cjs

@@ -0,0 +1,44 @@
+const fs = require('fs')
+const path = require('path')
+
+// Fake containsReal implementation matching the one in the codebase
+function containsReal(parent, child) {
+    let realParent;
+    try {
+      realParent = fs.realpathSync(parent)
+    } catch {
+      return false;
+    }
+
+    try {
+      const realChild = fs.realpathSync(child)
+      const rel = path.relative(realParent, realChild)
+      return !path.isAbsolute(rel) && !rel.startsWith("..")
+    } catch {
+      // Child doesn't exist — walk up to find nearest existing ancestor
+    }
+
+    const resolved = path.resolve(child)
+    let current = resolved
+    const trailing = []
+    while (true) {
+      try {
+        const realAncestor = fs.realpathSync(current)
+        const realChild = trailing.length > 0 ? path.join(realAncestor, ...trailing) : realAncestor
+        const rel = path.relative(realParent, realChild)
+        return !path.isAbsolute(rel) && !rel.startsWith("..")
+      } catch {
+        const parent_ = path.dirname(current)
+        if (parent_ === current) {
+          return false;
+        }
+        trailing.unshift(path.basename(current))
+        current = parent_
+      }
+    }
+}
+
+const parent = '/tmp/project'
+const child = '/tmp/project/symlink/../new_secret.txt'
+
+console.log("containsReal allows bypass write?:", containsReal(parent, child))

test_bypass_fix.cjs

@@ -0,0 +1,30 @@
+const fs = require('fs')
+const path = require('path')
+
+function containsReal(parent, child) {
+    let realParent = fs.realpathSync(parent)
+
+    let current = path.isAbsolute(child) ? child : path.resolve(child) // wait, path.resolve normalizes. 
+    // If it's relative, we can do path.join(process.cwd(), child) instead of path.resolve? 
+    // Let's test with absolute child to keep it simple.
+    current = child;
+
+    const trailing = []
+    while (true) {
+      try {
+        const realAncestor = fs.realpathSync(current)
+        const realChild = trailing.length > 0 ? path.join(realAncestor, ...trailing) : realAncestor
+        const rel = path.relative(realParent, realChild)
+        return !path.isAbsolute(rel) && !rel.startsWith("..")
+      } catch {
+        const parent_ = path.dirname(current)
+        if (parent_ === current) {
+          return false;
+        }
+        trailing.unshift(path.basename(current))
+        current = parent_
+      }
+    }
+}
+
+console.log("Fixed allows bypass write?:", containsReal('/tmp/project', '/tmp/project/symlink/../new_secret.txt'))

test_bypass_fix2.cjs

@@ -0,0 +1,28 @@
+const fs = require('fs')
+const path = require('path')
+
+function containsReal(parent, child) {
+    let realParent = fs.realpathSync(parent)
+
+    let current = child;
+    const trailing = []
+    while (true) {
+      try {
+        const realAncestor = fs.realpathSync(current)
+        console.log("Resolved", current, "->", realAncestor)
+        const realChild = trailing.length > 0 ? path.join(realAncestor, ...trailing) : realAncestor
+        console.log("realChild:", realChild)
+        const rel = path.relative(realParent, realChild)
+        return !path.isAbsolute(rel) && !rel.startsWith("..")
+      } catch (e) {
+        const parent_ = path.dirname(current)
+        if (parent_ === current) {
+          return false;
+        }
+        trailing.unshift(path.basename(current))
+        current = parent_
+      }
+    }
+}
+
+console.log("Fixed allows bypass write?:", containsReal('/tmp/project', '/tmp/project/symlink/../new_secret.txt'))

test_bypass_fix3.cjs

@@ -0,0 +1,35 @@
+const fs = require('fs')
+const path = require('path')
+
+fs.rmSync('/tmp/project2', {recursive: true, force: true})
+fs.rmSync('/tmp/outside2', {recursive: true, force: true})
+
+fs.mkdirSync('/tmp/project2', {recursive: true})
+fs.mkdirSync('/tmp/outside2/sub', {recursive: true})
+fs.symlinkSync('/tmp/outside2/sub', '/tmp/project2/symlink')
+
+function containsReal(parent, child) {
+    let realParent = fs.realpathSync(parent)
+
+    let current = child;
+    const trailing = []
+    while (true) {
+      try {
+        const realAncestor = fs.realpathSync(current)
+        console.log("Resolved", current, "->", realAncestor)
+        const realChild = trailing.length > 0 ? path.join(realAncestor, ...trailing) : realAncestor
+        console.log("realChild:", realChild)
+        const rel = path.relative(realParent, realChild)
+        return !path.isAbsolute(rel) && !rel.startsWith("..")
+      } catch (e) {
+        const parent_ = path.dirname(current)
+        if (parent_ === current) {
+          return false;
+        }
+        trailing.unshift(path.basename(current))
+        current = parent_
+      }
+    }
+}
+
+console.log("Fixed allows bypass write?:", containsReal('/tmp/project2', '/tmp/project2/symlink/../new_secret.txt'))

test_bypass_fix4.cjs

@@ -0,0 +1,26 @@
+const fs = require('fs')
+const path = require('path')
+
+function containsRealNative(parent, child) {
+    let realParent = fs.realpathSync.native(parent)
+    const resolved = path.resolve(child)
+    let current = resolved
+    const trailing = []
+    while (true) {
+      try {
+        const realAncestor = fs.realpathSync.native(current)
+        const realChild = trailing.length > 0 ? path.join(realAncestor, ...trailing) : realAncestor
+        const rel = path.relative(realParent, realChild)
+        return !path.isAbsolute(rel) && !rel.startsWith("..")
+      } catch (e) {
+        const parent_ = path.dirname(current)
+        if (parent_ === current) {
+          return false;
+        }
+        trailing.unshift(path.basename(current))
+        current = parent_
+      }
+    }
+}
+
+console.log("With .native but using path.resolve. bypass write?:", containsRealNative('/tmp/project2', '/tmp/project2/symlink/../new_secret2.txt'))

test_bypass_fix5.cjs

@@ -0,0 +1,25 @@
+const fs = require('fs')
+const path = require('path')
+
+function containsRealNativeWithDirname(parent, child) {
+    let realParent = fs.realpathSync.native(parent)
+    let current = child // NO path.resolve(child)
+    const trailing = []
+    while (true) {
+      try {
+        const realAncestor = fs.realpathSync.native(current)
+        const realChild = trailing.length > 0 ? path.join(realAncestor, ...trailing) : realAncestor
+        const rel = path.relative(realParent, realChild)
+        return !path.isAbsolute(rel) && !rel.startsWith("..")
+      } catch (e) {
+        const parent_ = path.dirname(current)
+        if (parent_ === current) {
+          return false;
+        }
+        trailing.unshift(path.basename(current))
+        current = parent_
+      }
+    }
+}
+
+console.log("With dirname and .native bypass write?:", containsRealNativeWithDirname('/tmp/project2', '/tmp/project2/symlink/../new_secret5.txt'))