fix(sandbox): keep relative in-repo symlinks on download

docs/guides/dev/cli-internals.md

@@ -271,7 +271,7 @@ Per-repo mode does not use the layer stack — it runs the same phases inline in
 │             ▼                                                   │
 │  ┌──────────────────┐                                           │
 │  │ Extract output    │ SafeDownload() with sanitization:        │
-│  │                   │ - Remove symlinks (sandbox escape)       │
+│  │                   │ - Remove dangerous symlinks (sandbox escape) │
 │  │                   │ - Remove .git/hooks/ (hook injection)    │
 │  └──────┬───────────┘                                           │
 │         ▼                                                       │
@@ -316,15 +316,15 @@ SandboxClaudeConfig = "/tmp/claude-config"
 | `ExecStreamReader()` | `openshell sandbox exec ...` | Streaming stdout reader |
 | `Upload()` | `openshell sandbox upload ...` | Copy files into sandbox |
 | `Download()` | `openshell sandbox download ...` | Copy files out of sandbox |
-| `SafeDownload()` | Download + sanitize | Remove symlinks, .git/hooks |
+| `SafeDownload()` | Download + sanitize | Remove dangerous symlinks (absolute or repo-escaping), .git/hooks |
 | `CollectLogs()` | Download logs dir | Extract sandbox logs |
 | `ExtractTranscripts()` | Download transcripts | Extract conversation transcripts |
 | `Delete()` | `openshell sandbox delete` | Destroy container |
 
 ### Security: sanitizeDownload()
 
 After downloading files from the sandbox, `sanitizeDownload()` removes:
-- **Symlinks** — Prevents sandbox escape via symlink-to-host-path attacks
+- **Dangerous symlinks** (absolute targets or targets that escape the repo) — Prevents sandbox escape via symlink-to-host-path attacks; relative in-repo symlinks are kept
 - **.git/hooks/** — Prevents hook injection that would execute on the host
 
 ---

internal/cli/run.go

@@ -554,8 +554,8 @@ func runAgent(agentName, fullsendDir, outputBase, targetRepo, fullsendBinary str
 			}
 		}
 
-		// 9d. Extract target repo back to host. SafeDownload removes symlinks
-		// and .git/hooks/ after download to prevent sandbox escape.
+		// 9d. Extract target repo back to host. SafeDownload removes dangerous
+		// symlinks (absolute or repo-escaping) and .git/hooks/ to prevent sandbox escape.
 		if clearErr := os.RemoveAll(repoSrc); clearErr != nil {
 			return fmt.Errorf("clearing local repo %s before extraction: %w", repoSrc, clearErr)
 		}

internal/sandbox/sandbox.go

@@ -25,12 +25,42 @@ const (
 )
 
 func sanitizeDownload(localDir string) error {
-	return filepath.WalkDir(localDir, func(path string, d fs.DirEntry, err error) error {
+	absLocal, err := filepath.Abs(localDir)
+	if err != nil {
+		return err
+	}
+	absLocal, err = filepath.EvalSymlinks(absLocal)
+	if err != nil {
+		return err
+	}
+
+	return filepath.WalkDir(absLocal, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
 		if d.Type()&fs.ModeSymlink != 0 {
-			return os.Remove(path)
+			target, readErr := os.Readlink(path)
+			if readErr != nil {
+				return os.Remove(path)
+			}
+			// Absolute targets always point outside the repo root.
+			if filepath.IsAbs(target) {
+				return os.Remove(path)
+			}
+			// Use EvalSymlinks, not filepath.Clean: Clean is textual and misses
+			// chains where an in-repo dir-symlink is used as a component
+			// (e.g. "sub/link/../../etc/passwd" cleans to inside the repo but
+			// follows the link to outside). Fall back to remove on error
+			// (dangling or looping).
+			rawPath := filepath.Dir(path) + string(filepath.Separator) + target
+			resolved, evalErr := filepath.EvalSymlinks(rawPath)
+			if evalErr != nil {
+				return os.Remove(path)
+			}
+			if !strings.HasPrefix(resolved+string(filepath.Separator), absLocal+string(filepath.Separator)) {
+				return os.Remove(path)
+			}
+			return nil
 		}
 
 		if d.IsDir() && d.Name() == "hooks" && filepath.Base(filepath.Dir(path)) == ".git" {
@@ -343,7 +373,7 @@ func DownloadFile(sandboxName, remotePath, localPath string) error {
 }
 
 // SafeDownload copies a directory from a sandbox to the local machine and then
-// sanitizes the result by removing symlinks and .git/hooks/.
+// sanitizes the result by removing dangerous symlinks (absolute or repo-escaping) and .git/hooks/.
 func SafeDownload(sandboxName, remoteDir, localDir string) error {
 	if err := Download(sandboxName, remoteDir, localDir); err != nil {
 		return err

internal/sandbox/sandbox_test.go

@@ -149,25 +149,92 @@ func TestOsRootContainment(t *testing.T) {
 	assert.Error(t, err)
 }
 
-func TestSanitizeDownload_RemovesSymlinks(t *testing.T) {
+func TestSanitizeDownload_RemovesAbsoluteSymlinks(t *testing.T) {
 	dir := t.TempDir()
 
-	// Create a regular file.
 	require.NoError(t, os.WriteFile(filepath.Join(dir, "real.txt"), []byte("ok"), 0o644))
-
-	// Create a symlink (dangling is fine — we just need it to exist).
 	require.NoError(t, os.Symlink("/nonexistent/target", filepath.Join(dir, "danger")))
 
 	err := sanitizeDownload(dir)
 	require.NoError(t, err)
 
-	// Regular file should survive.
 	_, err = os.Stat(filepath.Join(dir, "real.txt"))
 	assert.NoError(t, err)
 
-	// Symlink should be removed.
 	_, err = os.Lstat(filepath.Join(dir, "danger"))
-	assert.True(t, os.IsNotExist(err), "symlink should have been removed")
+	assert.True(t, os.IsNotExist(err), "absolute symlink should have been removed")
+}
+
+func TestSanitizeDownload_KeepsRelativeSymlinksInsideRepo(t *testing.T) {
+	dir := t.TempDir()
+
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "sub"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "target.txt"), []byte("ok"), 0o644))
+	// Relative symlink: sub/link -> ../target.txt (stays inside dir)
+	require.NoError(t, os.Symlink("../target.txt", filepath.Join(dir, "sub", "link")))
+
+	err := sanitizeDownload(dir)
+	require.NoError(t, err)
+
+	_, err = os.Lstat(filepath.Join(dir, "sub", "link"))
+	assert.NoError(t, err, "relative in-repo symlink should be preserved")
+}
+
+func TestSanitizeDownload_RemovesSymlinkChainEscape(t *testing.T) {
+	dir := t.TempDir()
+
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "real"), 0o755))
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "sub"), 0o755))
+	// dirlink -> ../real: relative, inside repo — sanitizeDownload keeps it.
+	require.NoError(t, os.Symlink("../real", filepath.Join(dir, "sub", "dirlink")))
+	// escape -> "sub/dirlink/../../etc/passwd":
+	//   filepath.Clean sees: dir/sub/dirlink/../../etc/passwd → dir/etc/passwd (inside, passes textual check)
+	//   EvalSymlinks follows: sub/dirlink → dir/real → ../../etc/passwd → outside dir (escapes)
+	require.NoError(t, os.Symlink("sub/dirlink/../../etc/passwd", filepath.Join(dir, "escape")))
+
+	err := sanitizeDownload(dir)
+	require.NoError(t, err)
+
+	_, err = os.Lstat(filepath.Join(dir, "sub", "dirlink"))
+	assert.NoError(t, err, "in-repo dirlink should be preserved")
+
+	_, err = os.Lstat(filepath.Join(dir, "escape"))
+	assert.True(t, os.IsNotExist(err), "symlink-chain escape should be removed")
+}
+
+func TestSanitizeDownload_RemovesRelativeSymlinksEscapingRepo(t *testing.T) {
+	dir := t.TempDir()
+
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "sub"), 0o755))
+	// Relative symlink that traverses above dir root.
+	require.NoError(t, os.Symlink("../../etc/passwd", filepath.Join(dir, "sub", "escape")))
+
+	err := sanitizeDownload(dir)
+	require.NoError(t, err)
+
+	_, err = os.Lstat(filepath.Join(dir, "sub", "escape"))
+	assert.True(t, os.IsNotExist(err), "escaping relative symlink should have been removed")
+}
+
+func TestSanitizeDownload_RemovesDirSymlinkIndirection(t *testing.T) {
+	repo := t.TempDir()
+
+	// Place a secret file outside the repo root.
+	secret := filepath.Join(filepath.Dir(repo), "secret")
+	require.NoError(t, os.WriteFile(secret, []byte("leaked"), 0o644))
+	t.Cleanup(func() { os.Remove(secret) })
+
+	// d/x is a directory symlink to "." — relative, inside repo, so kept.
+	// e targets "d/x/../../secret" which textually cleans to repo/secret (inside),
+	// but on the filesystem d/x resolves to d/, so ../../secret escapes.
+	require.NoError(t, os.MkdirAll(filepath.Join(repo, "d"), 0o755))
+	require.NoError(t, os.Symlink(".", filepath.Join(repo, "d", "x")))
+	require.NoError(t, os.Symlink("d/x/../../secret", filepath.Join(repo, "e")))
+
+	require.NoError(t, sanitizeDownload(repo))
+
+	_, err := os.Lstat(filepath.Join(repo, "e"))
+	assert.True(t, os.IsNotExist(err), "dir-symlink indirection escape should have been removed")
 }
 
 func TestSanitizeDownload_RemovesGitHooks(t *testing.T) {