fix(sandbox): bound directory-symlink traversal in SSH key discovery

Author: euxaristia

internal/sandbox/ssh_discovery_limits_test.go

@@ -165,3 +165,86 @@ func TestLinuxAbsentSSHKeyRefusesCommandBeforeCreation(t *testing.T) {
 		})
 	}
 }
+
+func TestSSHDiscoverySymlinkDirectoryBounds(t *testing.T) {
+	t.Run("symlink directory limit exceeded", func(t *testing.T) {
+		sshDir := filepath.Join(t.TempDir(), ".ssh")
+		mustWriteFile(t, filepath.Join(sshDir, "known_hosts"), "example.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAI...")
+		external := t.TempDir()
+		curr := external
+		for i := 0; i <= sshSymlinkMaxDirs; i++ {
+			curr = filepath.Join(curr, fmt.Sprintf("dir-%03d", i))
+			if err := os.Mkdir(curr, 0700); err != nil {
+				t.Fatal(err)
+			}
+		}
+		if err := os.Symlink(external, filepath.Join(sshDir, "link")); err != nil {
+			t.Fatal(err)
+		}
+		scanner := &sshDiscovery{}
+		_ = scanner.walkPrivateKeyFiles(sshDir)
+		if len(scanner.errors) == 0 {
+			t.Fatal("expected discovery error for symlink directory limit exceeded, got none")
+		}
+		var matched bool
+		for _, e := range scanner.errors {
+			if strings.Contains(e, "symlink directory limit exceeded") {
+				matched = true
+				break
+			}
+		}
+		if !matched {
+			t.Fatalf("expected symlink directory limit exceeded, got %v", scanner.errors)
+		}
+	})
+
+	t.Run("symlink entry limit exceeded", func(t *testing.T) {
+		sshDir := filepath.Join(t.TempDir(), ".ssh")
+		if err := os.MkdirAll(sshDir, 0700); err != nil {
+			t.Fatal(err)
+		}
+		external := t.TempDir()
+		for i := 0; i <= sshSymlinkMaxEntries; i++ {
+			mustWriteFile(t, filepath.Join(external, fmt.Sprintf("file-%04d", i)), "data")
+		}
+		if err := os.Symlink(external, filepath.Join(sshDir, "link")); err != nil {
+			t.Fatal(err)
+		}
+		scanner := &sshDiscovery{}
+		_ = scanner.walkPrivateKeyFiles(sshDir)
+		if len(scanner.errors) == 0 {
+			t.Fatal("expected discovery error for symlink entry limit exceeded, got none")
+		}
+		var matched bool
+		for _, e := range scanner.errors {
+			if strings.Contains(e, "symlink entry limit exceeded") {
+				matched = true
+				break
+			}
+		}
+		if !matched {
+			t.Fatalf("expected symlink entry limit exceeded, got %v", scanner.errors)
+		}
+	})
+
+	t.Run("symlink within budget succeeds", func(t *testing.T) {
+		sshDir := filepath.Join(t.TempDir(), ".ssh")
+		if err := os.MkdirAll(sshDir, 0700); err != nil {
+			t.Fatal(err)
+		}
+		external := t.TempDir()
+		keyFile := filepath.Join(external, "custom_key")
+		mustWriteFile(t, keyFile, sshPrivateKeyFixture())
+		if err := os.Symlink(external, filepath.Join(sshDir, "link")); err != nil {
+			t.Fatal(err)
+		}
+		scanner := &sshDiscovery{}
+		keys := scanner.walkPrivateKeyFiles(sshDir)
+		if len(scanner.errors) != 0 {
+			t.Fatalf("unexpected discovery errors: %v", scanner.errors)
+		}
+		if len(keys) == 0 {
+			t.Fatal("expected discovered key in symlinked directory, got none")
+		}
+	})
+}

internal/sandbox/ssh_key_deny.go

@@ -22,6 +22,13 @@ const sshPrivateKeyWalkPageSize = 256
 
 const sshPrivateKeySniffBytes = 128
 
+// sshSymlinkMaxDirs and sshSymlinkMaxEntries bound traversal of directory
+// trees reached through symlinks inside ~/.ssh. Real ~/.ssh directory trees
+// remain unbounded.
+const sshSymlinkMaxDirs = 64
+
+const sshSymlinkMaxEntries = 1024
+
 // sshWellKnownPrivateKeyNames are the OpenSSH default private-key basenames.
 // They are emitted even when ~/.ssh is absent so pathname-policy backends can
 // reserve them; mount-based Linux must refuse unprotected future key paths.
@@ -85,10 +92,19 @@ func (s *sshDiscovery) privateKeyDenyCandidates(home string) []string {
 var testSSHWalkChildHook func(dir string)
 
 func (s *sshDiscovery) walkPrivateKeyFiles(sshDir string) []string {
+	type pendingDir struct {
+		path       string
+		viaSymlink bool
+	}
+
 	var out []string
 	visitedDirs := make(map[string]bool)
-	pending := []string{sshDir}
-	walk := func(dir string) {
+	pending := []pendingDir{{path: sshDir, viaSymlink: false}}
+	var symlinkDirsVisited int
+	var symlinkEntriesSeen int
+
+	walk := func(item pendingDir) {
+		dir := item.path
 		realDir := dir
 		if resolved, err := filepath.EvalSymlinks(dir); err == nil {
 			realDir = resolved
@@ -98,6 +114,14 @@ func (s *sshDiscovery) walkPrivateKeyFiles(sshDir string) []string {
 		}
 		visitedDirs[realDir] = true
 
+		if item.viaSymlink {
+			symlinkDirsVisited++
+			if symlinkDirsVisited > sshSymlinkMaxDirs {
+				s.fail(dir, "symlink directory limit exceeded")
+				return
+			}
+		}
+
 		root, err := os.OpenRoot(dir)
 		if err != nil {
 			if !os.IsNotExist(err) {
@@ -123,6 +147,13 @@ func (s *sshDiscovery) walkPrivateKeyFiles(sshDir string) []string {
 				s.fail(dir, err.Error())
 				return
 			}
+			if item.viaSymlink {
+				symlinkEntriesSeen += len(entries)
+				if symlinkEntriesSeen > sshSymlinkMaxEntries {
+					s.fail(dir, "symlink entry limit exceeded")
+					return
+				}
+			}
 			if testSSHWalkChildHook != nil {
 				testSSHWalkChildHook(dir)
 			}
@@ -156,7 +187,7 @@ func (s *sshDiscovery) walkPrivateKeyFiles(sshDir string) []string {
 					}
 					targetStat, err := os.Stat(targetPath)
 					if err == nil && targetStat.IsDir() {
-						pending = append(pending, targetPath)
+						pending = append(pending, pendingDir{path: targetPath, viaSymlink: true})
 						continue
 					}
 					// Inspect leaf symlinks (bounded, specials rejected) so a
@@ -170,7 +201,7 @@ func (s *sshDiscovery) walkPrivateKeyFiles(sshDir string) []string {
 					continue
 				}
 				if info.IsDir() {
-					pending = append(pending, path)
+					pending = append(pending, pendingDir{path: path, viaSymlink: item.viaSymlink})
 					continue
 				}
 				if !mode.IsRegular() {
@@ -188,9 +219,9 @@ func (s *sshDiscovery) walkPrivateKeyFiles(sshDir string) []string {
 	// Iteration keeps open directory handles and call-stack depth constant even
 	// for deeply nested layouts. The physical-path set still breaks link cycles.
 	for len(pending) > 0 {
-		dir := pending[len(pending)-1]
+		item := pending[len(pending)-1]
 		pending = pending[:len(pending)-1]
-		walk(dir)
+		walk(item)
 	}
 	return out
 }