Harden tokenPath() against path prefix and symlink attacks

- Fix incomplete path prefix check: Replace strings.HasPrefix with
  hasPathPrefix() that properly handles directory separators, preventing
  attacks where tokensDir-evil would pass the check for tokensDir

- Add hasPathPrefix() helper for safe directory prefix checking without
  symlink resolution, keeping isPathWithinDir() for resolved symlink checks

- Document TOCTOU limitation in symlink check with explanation of the
  low practical risk due to required attacker capabilities

- Improve test to verify exact hash-based fallback path format

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Author: hughdbrown

internal/oauth/oauth.go

@@ -308,13 +308,19 @@ func (m *Manager) tokenPath(email string) string {
 	cleanPath := filepath.Clean(path)
 	cleanTokensDir := filepath.Clean(m.tokensDir)
 
-	// Verify the path is still within tokensDir
-	if !strings.HasPrefix(cleanPath, cleanTokensDir) {
+	// Verify the path is still within tokensDir (using proper directory check
+	// to avoid prefix attacks like tokensDir-evil matching tokensDir)
+	if !hasPathPrefix(cleanPath, cleanTokensDir) {
 		// If path escapes tokensDir, use a hash-based fallback
 		return filepath.Join(m.tokensDir, fmt.Sprintf("%x.json", sha256.Sum256([]byte(email))))
 	}
 
-	// Check if path is a symlink that could escape tokensDir
+	// Check if path is a symlink that could escape tokensDir.
+	// Note: There is an inherent TOCTOU (time-of-check to time-of-use) race between
+	// this check and when the token is actually written. An attacker could create a
+	// symlink after this check passes but before the write occurs. However, exploiting
+	// this would require the attacker to have write access to the tokens directory and
+	// precise timing, making it difficult to exploit in practice.
 	if info, err := os.Lstat(cleanPath); err == nil && info.Mode()&os.ModeSymlink != 0 {
 		// Path exists and is a symlink - resolve it and verify it stays within tokensDir
 		resolved, err := filepath.EvalSymlinks(cleanPath)
@@ -327,7 +333,18 @@ func (m *Manager) tokenPath(email string) string {
 	return cleanPath
 }
 
+// hasPathPrefix checks if path starts with dir using proper separator handling.
+// This prevents prefix attacks like tokensDir-evil matching tokensDir.
+// Does not resolve symlinks - use isPathWithinDir when symlink resolution is needed.
+func hasPathPrefix(path, dir string) bool {
+	cleanPath := filepath.Clean(path)
+	cleanDir := filepath.Clean(dir)
+	return strings.HasPrefix(cleanPath, cleanDir+string(filepath.Separator)) ||
+		cleanPath == cleanDir
+}
+
 // isPathWithinDir checks if path is within dir, resolving symlinks in dir.
+// Use this when checking resolved symlink targets.
 func isPathWithinDir(path, dir string) bool {
 	// Resolve symlinks in dir to get the real base directory
 	resolvedDir, err := filepath.EvalSymlinks(dir)

internal/oauth/oauth_test.go

@@ -1,7 +1,9 @@
 package oauth
 
 import (
+	"crypto/sha256"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -290,10 +292,10 @@ func TestTokenPath_SymlinkEscape(t *testing.T) {
 		t.Errorf("tokenPath returned symlink path %q, should use hash-based fallback to prevent escape", gotPath)
 	}
 
-	// Verify the returned path, when resolved, stays within tokensDir
-	// (the hash-based fallback should be used)
-	if !strings.HasPrefix(gotPath, tokensDir) {
-		t.Errorf("tokenPath %q is not within tokensDir %q", gotPath, tokensDir)
+	// Verify the returned path is exactly the expected hash-based fallback
+	expectedPath := filepath.Join(tokensDir, fmt.Sprintf("%x.json", sha256.Sum256([]byte("evil"))))
+	if gotPath != expectedPath {
+		t.Errorf("tokenPath = %q, want hash-based fallback %q", gotPath, expectedPath)
 	}
 }