fix: close medium and low severity findings from the 2026-10-20 security audit

internal/filepathext/bounds.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync"
 )
 
 // IsInside reports whether absPath is located within the directory tree rooted
@@ -23,7 +24,67 @@ func IsInside(absPath, absWorkspace string) bool {
 	// doesn't exist on disk, fall back to using it as-is.
 	resolvedPath := resolveSymlinks(absPath)
 	resolvedWorkspace := resolveSymlinks(absWorkspace)
-	rel, err := filepath.Rel(strings.ToLower(resolvedWorkspace), strings.ToLower(resolvedPath))
+	if insideRel(resolvedWorkspace, resolvedPath) {
+		return true
+	}
+	// Case-folded fallback for case-insensitive filesystems: the workspace
+	// and target may legitimately differ in case (e.g. C:\Users\test\Project
+	// vs C:\Users\test\project) and still denote the same directory. It is
+	// applied only where the hosting filesystem ignores case — always on
+	// Windows, and probed at runtime elsewhere (the default macOS APFS layout
+	// is case-insensitive). On case-sensitive filesystems, case-differing
+	// siblings are *different* directories and must not false-pass here.
+	if caseInsensitiveFS(absWorkspace) {
+		return insideRel(strings.ToLower(resolvedWorkspace), strings.ToLower(resolvedPath))
+	}
+	return false
+}
+
+// caseProbeCache memoizes the probe result per workspace root; the property
+// belongs to the filesystem, not the path, so one probe per root is enough.
+var caseProbeCache sync.Map // string -> bool
+
+// caseInsensitiveFS reports whether the filesystem hosting dir ignores case.
+// Windows is treated as case-insensitive; everywhere else the workspace root
+// itself is probed: if dir and its fully upper-cased spelling resolve to the
+// same directory, the filesystem does not preserve case. Unknown (missing
+// dir, network path) answers conservatively as case-sensitive, which keeps
+// the bounds check strict.
+func caseInsensitiveFS(dir string) bool {
+	if runtime.GOOS == "windows" {
+		return true
+	}
+	if v, ok := caseProbeCache.Load(dir); ok {
+		return v.(bool)
+	}
+	insensitive := false
+	if !isNetworkPath(dir) {
+		// codeql[go/path-injection] -- probe-only: the workspace root is
+		// stat'ed in both case spellings solely to decide whether the
+		// filesystem ignores case. No file content is read or written,
+		// network paths are short-circuited, and any probe failure fails
+		// closed to the strict comparison.
+		fi1, err1 := os.Stat(dir)
+		if err1 == nil {
+			upper := strings.ToUpper(dir)
+			if upper != dir {
+				// codeql[go/path-injection] -- upper-cased spelling of the
+				// same probe target; see the comment above.
+				fi2, err2 := os.Stat(upper)
+				if err2 == nil && os.SameFile(fi1, fi2) {
+					insensitive = true
+				}
+			}
+		}
+	}
+	caseProbeCache.Store(dir, insensitive)
+	return insensitive
+}
+
+// insideRel reports whether target is at or below base, using raw (case-
+// preserving) string comparison of the two already-resolved absolute paths.
+func insideRel(base, target string) bool {
+	rel, err := filepath.Rel(base, target)
 	if err != nil {
 		return false
 	}
@@ -47,14 +108,14 @@ func resolveSymlinks(absPath string) string {
 	}
 
 	if resolved, err := filepath.EvalSymlinks(absPath); err == nil {
-		return resolved
+		return followReparseTargets(resolved)
 	}
 
 	root := filepath.Clean(absPath)
 	var missing []string
 	for {
 		if resolved, err := filepath.EvalSymlinks(root); err == nil {
-			result := resolved
+			result := followReparseTargets(resolved)
 			for i := len(missing) - 1; i >= 0; i-- {
 				result = filepath.Join(result, missing[i])
 			}
@@ -70,6 +131,29 @@ func resolveSymlinks(absPath string) string {
 	}
 }
 
+// followReparseTargets resolves reparse points that EvalSymlinks reports as
+// resolved but leaves unresolved on Windows — notably NTFS junctions and
+// mount points, which EvalSymlinks hands back unchanged. os.Readlink still
+// reports the link target for those, so follow it until the path stops
+// changing. The hop cap bounds cycles through self-referential links.
+func followReparseTargets(path string) string {
+	for range 16 {
+		// codeql[go/path-injection] -- Readlink is part of the containment
+		// check itself: it only reads the link target for comparison
+		// against the workspace root. No file is opened and nothing is
+		// returned to the caller.
+		target, err := os.Readlink(path)
+		if err != nil || target == "" {
+			return path
+		}
+		if !filepath.IsAbs(target) {
+			target = filepath.Join(filepath.Dir(path), target)
+		}
+		path = filepath.Clean(target)
+	}
+	return path
+}
+
 // isNetworkPath reports whether a Windows path names a UNC resource. Checking
 // such paths on disk can make the SMB redirecter start NetBIOS/DNS resolution
 // and TCP connection attempts, so bounds checks compare them lexically instead

internal/filepathext/bounds_test.go

@@ -61,16 +61,20 @@ func TestIsInside(t *testing.T) {
 			expectInside: false,
 		},
 		{
+			// Case-folding only applies on case-insensitive filesystems.
+			// These roots do not exist on disk, so the probe conservatively
+			// treats them as case-sensitive (strict) everywhere except
+			// Windows, where the filesystem is case-insensitive by contract.
 			name:         "case-different paths on case-insensitive FS",
 			path:         "/Home/User/Project/src/main.go",
 			workspace:    "/home/user/project",
-			expectInside: true,
+			expectInside: runtime.GOOS == "windows",
 		},
 		{
 			name:         "case-different workspace root",
 			path:         "/home/user/PROJECT/src/main.go",
 			workspace:    "/home/user/project",
-			expectInside: true,
+			expectInside: runtime.GOOS == "windows",
 		},
 	}
 
@@ -102,6 +106,37 @@ func TestIsInside_RealPaths(t *testing.T) {
 	require.False(t, sibling, "parent of workspace should be outside")
 }
 
+func TestIsInside_CaseInsensitiveFilesystems(t *testing.T) {
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+
+	// Re-spell the workspace root in a different case. On a case-insensitive
+	// filesystem (Windows, default macOS APFS) this denotes the same
+	// directory and must pass the bounds check; on a case-sensitive one it is
+	// a different path and must not.
+	flipped := flipCase(tmpDir)
+	require.NotEqual(t, tmpDir, flipped)
+
+	inside := IsInside(filepath.Join(flipped, "sub", "main.go"), tmpDir)
+	require.Equal(t, caseInsensitiveFS(tmpDir), inside,
+		"case folding must match the hosting filesystem's case behavior")
+}
+
+// flipCase flips the case of the first ASCII letter in p.
+func flipCase(p string) string {
+	for i := 0; i < len(p); i++ {
+		c := p[i]
+		if c >= 'A' && c <= 'Z' {
+			return p[:i] + string(c+32) + p[i+1:]
+		}
+		if c >= 'a' && c <= 'z' {
+			return p[:i] + string(c-32) + p[i+1:]
+		}
+	}
+	return p
+}
+
 func TestIsInside_UncPaths(t *testing.T) {
 	if runtime.GOOS != "windows" {
 		t.Skip("UNC paths are Windows-only")

pkg/agent/agent.go

@@ -192,8 +192,12 @@ type sessionAgent struct {
 	redactOutgoingSecrets bool
 	redactOutgoingPII     bool
 	wireSecretsForced     bool
-	notify                pubsub.Publisher[notify.Notification]
-	runComplete           pubsub.Publisher[notify.RunComplete]
+	// redactionPolicy is the per-agent frozen snapshot of the secret-protection
+	// switches, shared with this agent's tools so the wire mask and the read
+	// path cannot drift apart and a later agent build cannot alter it.
+	redactionPolicy *tools.Policy
+	notify          pubsub.Publisher[notify.Notification]
+	runComplete     pubsub.Publisher[notify.RunComplete]
 
 	// workingDir anchors the cwd hooks and the memory vault are opened against.
 	workingDir string
@@ -273,17 +277,21 @@ type sessionAgent struct {
 }
 
 type SessionAgentOptions struct {
-	LargeModel             Model
-	SmallModel             Model
-	SystemPromptPrefix     string
-	SystemPrompt           string
-	IsSubAgent             bool
-	DisableAutoSummarize   bool
-	SummarizeThreshold     float64
-	IsYolo                 bool
-	RedactOutgoingSecrets  bool
-	RedactOutgoingPII      bool
-	WireSecretsForced      bool
+	LargeModel            Model
+	SmallModel            Model
+	SystemPromptPrefix    string
+	SystemPrompt          string
+	IsSubAgent            bool
+	DisableAutoSummarize  bool
+	SummarizeThreshold    float64
+	IsYolo                bool
+	RedactOutgoingSecrets bool
+	RedactOutgoingPII     bool
+	WireSecretsForced     bool
+	// RedactionPolicy is the per-agent frozen snapshot of the secret-protection
+	// switches, shared with this agent's tools. Nil falls back to the built-in
+	// secure defaults.
+	RedactionPolicy        *tools.Policy
 	Sessions               session.Service
 	Messages               message.Service
 	GoalService            goal.Service
@@ -333,6 +341,7 @@ func NewSessionAgent(
 		redactOutgoingSecrets:  opts.RedactOutgoingSecrets,
 		redactOutgoingPII:      opts.RedactOutgoingPII,
 		wireSecretsForced:      opts.WireSecretsForced,
+		redactionPolicy:        opts.RedactionPolicy,
 		notify:                 opts.Notify,
 		runComplete:            opts.RunComplete,
 		reflectionEnabled:      csync.NewValue(opts.ReflectionEnabled),

pkg/agent/agentic_fetch_tool.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"net/http"
 	"os"
-	"time"
 
 	"charm.land/fantasy"
 
@@ -16,6 +15,7 @@ import (
 	"github.com/hackafterdark/phosphor/pkg/egress"
 	"github.com/hackafterdark/phosphor/pkg/permission"
 	"github.com/hackafterdark/phosphor/pkg/security/externalcontent"
+	"github.com/hackafterdark/phosphor/pkg/security/urlguard"
 )
 
 //go:embed templates/agentic_fetch.md
@@ -54,10 +54,7 @@ var agenticFetchPromptTmpl []byte
 
 func (c *coordinator) agenticFetchTool(_ context.Context, client *http.Client) (fantasy.AgentTool, error) {
 	if client == nil {
-		client = &http.Client{
-			Timeout:   30 * time.Second,
-			Transport: egress.NewHTTPTransport(),
-		}
+		client = tools.NewSecuredHTTPClient(c.cfg.Config(), c.tuiAllowPrompt)
 	} else {
 		client = egress.WrapClient(client)
 	}
@@ -107,7 +104,13 @@ func (c *coordinator) agenticFetchTool(_ context.Context, client *http.Client) (
 			var fullPrompt string
 
 			if params.URL != "" {
-				// URL mode: fetch the URL content first.
+				// URL mode: fetch the URL content first. The side-channel
+				// exfiltration defense and the trust hierarchy are enforced
+				// here because this path dials directly, not through the
+				// web_fetch sub-tool.
+				if err := urlguard.Check(params.URL); err != nil {
+					return fantasy.NewTextErrorResponse(err.Error()), nil
+				}
 				content, err := tools.FetchURLAndConvert(ctx, client, params.URL)
 				if err != nil {
 					return fantasy.NewTextErrorResponse(fmt.Sprintf("Failed to fetch URL: %s", err)), nil
@@ -172,15 +175,15 @@ func (c *coordinator) agenticFetchTool(_ context.Context, client *http.Client) (
 				return fantasy.ToolResponse{}, errors.New("small model provider not configured")
 			}
 
-			webFetchTool := tools.NewWebFetchTool(tmpDir, client, c.cfg.Config(), c.tuiAllowPrompt)
+			webFetchTool := tools.NewWebFetchTool(tmpDir, c.cfg.Config(), c.tuiAllowPrompt)
 			webSearchTool := tools.NewWebSearchTool(client)
 			fetchTools := []fantasy.AgentTool{
 				webFetchTool,
 				webSearchTool,
 				tools.NewGlobTool(tmpDir),
-				tools.NewGrepTool(tmpDir, c.cfg.Config().Tools.Grep),
+				tools.NewGrepTool(tmpDir, c.cfg.Config().Tools.Grep, c.redactionPolicy),
 				tools.NewSourcegraphTool(client),
-				tools.NewViewTool(c.lspManager, c.permissions, c.filetracker, nil, tmpDir),
+				tools.NewViewTool(c.lspManager, c.permissions, c.filetracker, nil, tmpDir, c.redactionPolicy),
 			}
 
 			// Sub-agent tools run without hook interception. The top-level
@@ -201,6 +204,7 @@ func (c *coordinator) agenticFetchTool(_ context.Context, client *http.Client) (
 				Sessions:             c.sessions,
 				Messages:             c.messages,
 				Tools:                fetchTools,
+				RedactionPolicy:      c.redactionPolicy,
 			})
 
 			return c.runSubAgent(ctx, subAgentParams{

pkg/agent/common_test.go

@@ -167,18 +167,18 @@ func coderAgent(r *vcr.Recorder, env fakeEnv, large, small fantasy.LanguageModel
 	}
 
 	allTools := []fantasy.AgentTool{
-		tools.NewBashTool(env.permissions, env.workingDir, cfg.Config().Tools.Bash, cfg.Config().Options.Attribution, modelName),
+		tools.NewBashTool(env.permissions, env.workingDir, cfg.Config().Tools.Bash, cfg.Config().Options.Attribution, modelName, nil),
 		tools.NewDownloadTool(env.permissions, env.workingDir, r.GetDefaultClient()),
-		tools.NewEditTool(nil, env.permissions, env.history, *env.filetracker, env.workingDir),
-		tools.NewMultiEditTool(nil, env.permissions, env.history, *env.filetracker, env.workingDir),
+		tools.NewEditTool(nil, env.permissions, env.history, *env.filetracker, env.workingDir, nil),
+		tools.NewMultiEditTool(nil, env.permissions, env.history, *env.filetracker, env.workingDir, nil),
 		tools.NewFetchTool(env.permissions, env.workingDir, r.GetDefaultClient()),
 		tools.NewGlobTool(env.workingDir),
-		tools.NewGrepTool(env.workingDir, cfg.Config().Tools.Grep),
+		tools.NewGrepTool(env.workingDir, cfg.Config().Tools.Grep, nil),
 		tools.NewStructuralSearchTool(env.workingDir, nil),
 		tools.NewLsTool(env.permissions, env.workingDir, cfg.Config().Tools.Ls),
 		tools.NewSourcegraphTool(r.GetDefaultClient()),
-		tools.NewViewTool(nil, env.permissions, *env.filetracker, nil, env.workingDir),
-		tools.NewWriteTool(nil, env.permissions, env.history, *env.filetracker, env.workingDir),
+		tools.NewViewTool(nil, env.permissions, *env.filetracker, nil, env.workingDir, nil),
+		tools.NewWriteTool(nil, env.permissions, env.history, *env.filetracker, env.workingDir, nil),
 	}
 
 	return testSessionAgent(env, large, small, systemPrompt, allTools...), nil