Conversation
…ctory Introduces GetClaudeConfigDir() in internal/claudeconfig as a central helper that returns CLAUDE_CONFIG_DIR if set, falling back to ~/.claude. Updates all four call sites (plugins, process_manager container mount, credentialsFileExists, and mcp validatePlanPath) to use it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Session TranscriptThis function will have a corresponding test file covering both the env-var path and the fallback path. Step 2: Update
|
There was a problem hiding this comment.
Pull request overview
Adds support for CLAUDE_CONFIG_DIR so the codebase consistently resolves Claude Code’s configuration directory (instead of assuming ~/.claude) across container execution, credentials lookup, MCP plan-path validation, and plugins.
Changes:
- Introduces
internal/claudeconfig.GetClaudeConfigDir()to resolve the Claude config directory usingCLAUDE_CONFIG_DIRwith a fallback to~/.claude. - Updates Claude container volume mounting and host credentials discovery to use the resolved config directory.
- Updates MCP plan path validation to validate against the resolved Claude config directory and adds targeted tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/claudeconfig/config_dir.go | Adds shared helper to resolve Claude config directory from CLAUDE_CONFIG_DIR or ~/.claude. |
| internal/claudeconfig/config_dir_test.go | Unit tests for GetClaudeConfigDir() behavior with/without env var. |
| internal/claude/process_manager.go | Uses resolved Claude config dir for docker volume mount and .credentials.json discovery. |
| internal/claude/process_manager_test.go | Adds tests ensuring credentials detection and container mount respect CLAUDE_CONFIG_DIR. |
| internal/mcp/server.go | Updates plan-path validation to constrain to <claudeDir>/plans. |
| internal/mcp/server_test.go | Adds test coverage for plan-path validation when CLAUDE_CONFIG_DIR is set. |
| internal/plugins/plugins.go | Switches plugins package to use shared Claude config dir helper. |
Comments suppressed due to low confidence (1)
internal/mcp/server.go:923
- validatePlanPath() only checks string prefixes on a cleaned absolute path and does not account for symlinks. A symlink placed inside the allowed plans directory that points outside it would pass validation but cause readPlanFromPath() to read an arbitrary file. Consider validating using resolved (EvalSymlinks) paths and/or rejecting symlinks when opening the file so the allowed-dir constraint can't be bypassed.
// Clean and resolve the path to eliminate ../ traversal
absPath, err := filepath.Abs(planPath)
if err != nil {
return fmt.Errorf("cannot resolve path: %w", err)
}
cleanPath := filepath.Clean(absPath)
// Ensure the resolved path is within the allowed directory.
// We append os.PathSeparator to prevent prefix matches like
// ~/.claude/plans-evil/ matching ~/.claude/plans
if !strings.HasPrefix(cleanPath, allowedDir+string(os.PathSeparator)) && cleanPath != allowedDir {
return fmt.Errorf("path must be within %s", allowedDir)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| func GetClaudeConfigDir() (string, error) { | ||
| if dir := os.Getenv("CLAUDE_CONFIG_DIR"); dir != "" { | ||
| return dir, nil | ||
| } | ||
| home, err := os.UserHomeDir() | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return filepath.Join(home, ".claude"), nil |
There was a problem hiding this comment.
GetClaudeConfigDir() returns CLAUDE_CONFIG_DIR as-is. If the env var contains leading/trailing whitespace or a relative path, callers can misbehave (e.g., validatePlanPath compares against an absolute path, and docker -v bind mounts require an absolute host path). Consider trimming whitespace and normalizing to an absolute, cleaned path before returning (or clearly documenting that only absolute paths are supported).
| name: "path in default ~/.claude/plans is rejected when CLAUDE_CONFIG_DIR is set", | ||
| path: filepath.Join(os.Getenv("HOME"), ".claude", "plans", "plan.md"), | ||
| wantErr: true, | ||
| }, |
There was a problem hiding this comment.
This test case uses os.Getenv("HOME") to construct the default ~/.claude path. On platforms/environments where HOME isn't set (or doesn't reflect os.UserHomeDir()), the test may still pass but won't actually validate the intended behavior. Prefer using os.UserHomeDir() and filepath.Join(home, ".claude", ...) for the default-path case.
| // getClaudeDir returns the Claude config directory path | ||
| func getClaudeDir() string { | ||
| home, err := os.UserHomeDir() | ||
| dir, err := claudeconfig.GetClaudeConfigDir() | ||
| if err != nil { | ||
| return "" | ||
| } | ||
| return filepath.Join(home, ".claude") | ||
| return dir | ||
| } |
There was a problem hiding this comment.
getClaudeDir() now hides the error from GetClaudeConfigDir() and forces callers to handle failure via an empty string, which leads to misleading downstream errors like "could not find home directory" even though the home dir may be fine. Consider returning (string, error) here and propagating a more accurate error message (claude config dir), especially now that the source may be CLAUDE_CONFIG_DIR.
| @@ -1013,7 +1014,7 @@ func buildContainerRunArgs(config ProcessConfig, claudeArgs []string) (container | |||
| "run", "-i", "--rm", | |||
| "--name", containerName, | |||
| "-v", config.WorkingDir + ":/workspace", | |||
| "-v", homeDir + "/.claude:/home/claude/.claude-host:ro", | |||
| "-v", claudeDir + ":/home/claude/.claude-host:ro", | |||
| "-w", "/workspace", | |||
There was a problem hiding this comment.
buildContainerRunArgs() now mounts the host Claude config dir directly. If CLAUDE_CONFIG_DIR is set to a relative path (or includes whitespace), docker bind mounts may fail at runtime (bind mounts generally require absolute host paths) and the failure won't be caught here. Consider normalizing/validating the resolved directory (e.g., ensure absolute) before constructing the -v argument and returning a clear error if it's invalid.
Summary
Adds support for the
CLAUDE_CONFIG_DIRenvironment variable so that Plural correctly locates Claude Code's configuration directory when it's customized, instead of always assuming~/.claude.Changes
internal/claudeconfig/config_dir.gowithGetClaudeConfigDir()helper that checksCLAUDE_CONFIG_DIRenv var before falling back to~/.claudeprocess_manager.goto use the resolved config dircredentialsFileExists()to look for.credentials.jsonin the resolved config dirvalidatePlanPath()in MCP server to validate against the resolved config dirgetClaudeDir()in plugins package to use the shared helperTest plan
go test ./internal/claudeconfig/...— unit tests forGetClaudeConfigDir()with and without env vargo test ./internal/claude/...— tests for container args and credentials file detection with custom config dirgo test ./internal/mcp/...— tests for plan path validation with custom config dirgo test ./...— full test suite passesFixes #324