Skip to content

fix(sandbox): canonicalize paths when detecting mount overlap#83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chriswritescode-dev merged 1 commit into from
Aug 2, 2026
Merged

fix(sandbox): canonicalize paths when detecting mount overlap #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/sandbox/manager.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,7 @@ import type { Logger, SandboxResources, SandboxMountConfig } from '../types'
import { resolve, join, isAbsolute, posix as posixPath } from 'path'
import { mkdirSync, existsSync, writeFileSync, chmodSync, rmSync } from 'fs'
import { defaultGitService, type GitService } from '../utils/git-service'
import { isSameOrDescendantPath, type SandboxMount } from './path'
import { canonicalizePath, isSameOrDescendantPath, type SandboxMount } from './path'

export interface SandboxManagerConfig {
image: string
Expand DownExpand Up@@ -45,8 +45,8 @@ function normalizeContainerPath(path: string): string {
}

function containerPathsOverlap(a: string, b: string): boolean {
const left = normalizeContainerPath(a)
const right = normalizeContainerPath(b)
const left = normalizeContainerPath(canonicalizePath(a))
const right = normalizeContainerPath(canonicalizePath(b))
Comment on lines +48 to +49

@coderabbitaicoderabbitaiBotAug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use canonical paths for all workspace-boundary checks.

resolveToolOutputMount at Line [249] and resolveTempMount at Line [263] still use raw startsWith checks. If a configured path is a symlink under workspaceDir but resolves outside it, these guards skip the mount before the canonical overlap check runs. Reuse canonicalizePath and isSameOrDescendantPath in both guards. Keep the original path in emitted mounts.

Proposed fix
- if (resolved === workspaceDir || resolved.startsWith(workspaceDir + '/')) return undefined+ if (isSameOrDescendantPath(canonicalizePath(resolved), canonicalizePath(workspaceDir))) {+ return undefined+ }

Apply the same change in both workspace-boundary guards.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/sandbox/manager.ts` around lines 48 - 49, Update the workspace-boundary
guards in resolveToolOutputMount and resolveTempMount to canonicalize the
configured paths and use isSameOrDescendantPath instead of raw startsWith
checks, ensuring symlinked paths resolving outside workspaceDir are rejected
before mounting. Preserve the original configured paths in emitted mounts.

return isSameOrDescendantPath(left, right) || isSameOrDescendantPath(right, left)
}

Expand DownExpand Up@@ -279,11 +279,11 @@ export function createSandboxManager(
const resolvedGitDir = resolve(projectDir, gitDirResult.stdout.trim())
const resolvedCommonDir = resolve(projectDir, commonDirResult.stdout.trim())

if (!resolvedGitDir.startsWith(projectDir + '/')) {
if (!isSameOrDescendantPath(canonicalizePath(resolvedGitDir), canonicalizePath(projectDir))) {
paths.add(resolvedGitDir)
}

if (!resolvedCommonDir.startsWith(projectDir + '/')) {
if (!isSameOrDescendantPath(canonicalizePath(resolvedCommonDir), canonicalizePath(projectDir))) {
paths.add(resolvedCommonDir)
}

Expand Down
10 changes: 10 additions & 0 deletions src/sandbox/path.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,19 @@
import { realpathSync } from 'fs'

export interface SandboxMount {
hostDir: string
containerDir: string
readOnly?: boolean
}

export function canonicalizePath(path: string): string {
try {
return realpathSync(path)
} catch {
return path
}
}

export function isSameOrDescendantPath(path: string, prefix: string): boolean {
if (path === prefix) return true
return path.startsWith(prefix + '/')
Expand Down