mirror of
https://github.com/cexll/myclaude.git
synced 2026-02-05 02:30:26 +08:00
fix: allow claude backend to read env from setting.json while preventing recursion (#92)
* fix: allow claude backend to read env from setting.json while preventing recursion Fixes #89 Problem: - --setting-sources "" prevents claude from reading ~/.claude/setting.json env - Removing it causes infinite recursion via skills/commands/agents loading Solution: - Keep --setting-sources "" to block all config sources - Add loadMinimalEnvSettings() to extract only env from setting.json - Pass env explicitly via --settings parameter - Update tests to validate dynamic --settings parameter Benefits: - Claude backend can access ANTHROPIC_API_KEY and other env vars - Skills/commands/agents remain blocked, preventing recursion - Graceful degradation if setting.json doesn't exist Generated with SWE-Agent.ai Co-Authored-By: SWE-Agent.ai <noreply@swe-agent.ai> * security: pass env via process environment instead of command line Critical security fix for issue #89: - Prevents ANTHROPIC_API_KEY leakage in process command line (ps) - Prevents sensitive values from being logged in wrapper logs Changes: 1. executor.go: - Add SetEnv() method to commandRunner interface - realCmd merges env with os.Environ() and sets to cmd.Env - All test mocks implement SetEnv() 2. backend.go: - Change loadMinimalEnvSettings() to return map[string]string - Use os.UserHomeDir() instead of os.Getenv("HOME") - Add 1MB file size limit check - Only accept string values in env (reject non-strings) - Remove --settings parameter (no longer in command line) 3. Tests: - Add loadMinimalEnvSettings() unit tests - Remove --settings validation (no longer in args) - All test mocks implement SetEnv() Security improvements: - No sensitive values in argv (safe from ps/logs) - Type-safe env parsing (string-only) - File size limit prevents memory issues - Graceful degradation if setting.json missing Tests: All pass (30.912s) Generated with SWE-Agent.ai Co-Authored-By: SWE-Agent.ai <noreply@swe-agent.ai> --------- Co-authored-by: SWE-Agent.ai <noreply@swe-agent.ai>
This commit is contained in:
@@ -1,5 +1,11 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"os"
|
||||
"path/filepath"
|
||||
)
|
||||
|
||||
// Backend defines the contract for invoking different AI CLI backends.
|
||||
// Each backend is responsible for supplying the executable command and
|
||||
// building the argument list based on the wrapper config.
|
||||
@@ -29,6 +35,51 @@ func (ClaudeBackend) BuildArgs(cfg *Config, targetArg string) []string {
|
||||
return buildClaudeArgs(cfg, targetArg)
|
||||
}
|
||||
|
||||
const maxClaudeSettingsBytes = 1 << 20 // 1MB
|
||||
|
||||
// loadMinimalEnvSettings 从 ~/.claude/setting.json 只提取 env 配置。
|
||||
// 只接受字符串类型的值;文件缺失/解析失败/超限都返回空。
|
||||
func loadMinimalEnvSettings() map[string]string {
|
||||
home, err := os.UserHomeDir()
|
||||
if err != nil || home == "" {
|
||||
return nil
|
||||
}
|
||||
|
||||
settingPath := filepath.Join(home, ".claude", "setting.json")
|
||||
info, err := os.Stat(settingPath)
|
||||
if err != nil || info.Size() > maxClaudeSettingsBytes {
|
||||
return nil
|
||||
}
|
||||
|
||||
data, err := os.ReadFile(settingPath)
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
var cfg struct {
|
||||
Env map[string]any `json:"env"`
|
||||
}
|
||||
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||
return nil
|
||||
}
|
||||
if len(cfg.Env) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
env := make(map[string]string, len(cfg.Env))
|
||||
for k, v := range cfg.Env {
|
||||
s, ok := v.(string)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
env[k] = s
|
||||
}
|
||||
if len(env) == 0 {
|
||||
return nil
|
||||
}
|
||||
return env
|
||||
}
|
||||
|
||||
func buildClaudeArgs(cfg *Config, targetArg string) []string {
|
||||
if cfg == nil {
|
||||
return nil
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"testing"
|
||||
)
|
||||
@@ -148,3 +150,64 @@ func TestClaudeBuildArgs_BackendMetadata(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadMinimalEnvSettings(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
t.Setenv("HOME", home)
|
||||
t.Setenv("USERPROFILE", home)
|
||||
|
||||
t.Run("missing file returns empty", func(t *testing.T) {
|
||||
if got := loadMinimalEnvSettings(); len(got) != 0 {
|
||||
t.Fatalf("got %v, want empty", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("valid env returns string map", func(t *testing.T) {
|
||||
dir := filepath.Join(home, ".claude")
|
||||
if err := os.MkdirAll(dir, 0o755); err != nil {
|
||||
t.Fatalf("MkdirAll: %v", err)
|
||||
}
|
||||
path := filepath.Join(dir, "setting.json")
|
||||
data := []byte(`{"env":{"ANTHROPIC_API_KEY":"secret","FOO":"bar"}}`)
|
||||
if err := os.WriteFile(path, data, 0o600); err != nil {
|
||||
t.Fatalf("WriteFile: %v", err)
|
||||
}
|
||||
|
||||
got := loadMinimalEnvSettings()
|
||||
if got["ANTHROPIC_API_KEY"] != "secret" || got["FOO"] != "bar" {
|
||||
t.Fatalf("got %v, want keys present", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("non-string values are ignored", func(t *testing.T) {
|
||||
dir := filepath.Join(home, ".claude")
|
||||
path := filepath.Join(dir, "setting.json")
|
||||
data := []byte(`{"env":{"GOOD":"ok","BAD":123,"ALSO_BAD":true}}`)
|
||||
if err := os.WriteFile(path, data, 0o600); err != nil {
|
||||
t.Fatalf("WriteFile: %v", err)
|
||||
}
|
||||
|
||||
got := loadMinimalEnvSettings()
|
||||
if got["GOOD"] != "ok" {
|
||||
t.Fatalf("got %v, want GOOD=ok", got)
|
||||
}
|
||||
if _, ok := got["BAD"]; ok {
|
||||
t.Fatalf("got %v, want BAD omitted", got)
|
||||
}
|
||||
if _, ok := got["ALSO_BAD"]; ok {
|
||||
t.Fatalf("got %v, want ALSO_BAD omitted", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("oversized file returns empty", func(t *testing.T) {
|
||||
dir := filepath.Join(home, ".claude")
|
||||
path := filepath.Join(dir, "setting.json")
|
||||
data := bytes.Repeat([]byte("a"), maxClaudeSettingsBytes+1)
|
||||
if err := os.WriteFile(path, data, 0o600); err != nil {
|
||||
t.Fatalf("WriteFile: %v", err)
|
||||
}
|
||||
if got := loadMinimalEnvSettings(); len(got) != 0 {
|
||||
t.Fatalf("got %v, want empty", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -26,6 +26,7 @@ type commandRunner interface {
|
||||
StdinPipe() (io.WriteCloser, error)
|
||||
SetStderr(io.Writer)
|
||||
SetDir(string)
|
||||
SetEnv(env map[string]string)
|
||||
Process() processHandle
|
||||
}
|
||||
|
||||
@@ -81,6 +82,52 @@ func (r *realCmd) SetDir(dir string) {
|
||||
}
|
||||
}
|
||||
|
||||
func (r *realCmd) SetEnv(env map[string]string) {
|
||||
if r == nil || r.cmd == nil || len(env) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
merged := make(map[string]string, len(env)+len(os.Environ()))
|
||||
for _, kv := range os.Environ() {
|
||||
if kv == "" {
|
||||
continue
|
||||
}
|
||||
idx := strings.IndexByte(kv, '=')
|
||||
if idx <= 0 {
|
||||
continue
|
||||
}
|
||||
merged[kv[:idx]] = kv[idx+1:]
|
||||
}
|
||||
for _, kv := range r.cmd.Env {
|
||||
if kv == "" {
|
||||
continue
|
||||
}
|
||||
idx := strings.IndexByte(kv, '=')
|
||||
if idx <= 0 {
|
||||
continue
|
||||
}
|
||||
merged[kv[:idx]] = kv[idx+1:]
|
||||
}
|
||||
for k, v := range env {
|
||||
if strings.TrimSpace(k) == "" {
|
||||
continue
|
||||
}
|
||||
merged[k] = v
|
||||
}
|
||||
|
||||
keys := make([]string, 0, len(merged))
|
||||
for k := range merged {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
|
||||
out := make([]string, 0, len(keys))
|
||||
for _, k := range keys {
|
||||
out = append(out, k+"="+merged[k])
|
||||
}
|
||||
r.cmd.Env = out
|
||||
}
|
||||
|
||||
func (r *realCmd) Process() processHandle {
|
||||
if r == nil || r.cmd == nil || r.cmd.Process == nil {
|
||||
return nil
|
||||
@@ -701,6 +748,12 @@ func runCodexTaskWithContext(parentCtx context.Context, taskSpec TaskSpec, backe
|
||||
|
||||
cmd := newCommandRunner(ctx, commandName, codexArgs...)
|
||||
|
||||
if cfg.Backend == "claude" {
|
||||
if env := loadMinimalEnvSettings(); len(env) > 0 {
|
||||
cmd.SetEnv(env)
|
||||
}
|
||||
}
|
||||
|
||||
// For backends that don't support -C flag (claude, gemini), set working directory via cmd.Dir
|
||||
// Codex passes workdir via -C flag, so we skip setting Dir for it to avoid conflicts
|
||||
if cfg.Mode != "resume" && commandName != "codex" && cfg.WorkDir != "" {
|
||||
|
||||
@@ -87,6 +87,7 @@ type execFakeRunner struct {
|
||||
process processHandle
|
||||
stdin io.WriteCloser
|
||||
dir string
|
||||
env map[string]string
|
||||
waitErr error
|
||||
waitDelay time.Duration
|
||||
startErr error
|
||||
@@ -129,6 +130,17 @@ func (f *execFakeRunner) StdinPipe() (io.WriteCloser, error) {
|
||||
}
|
||||
func (f *execFakeRunner) SetStderr(io.Writer) {}
|
||||
func (f *execFakeRunner) SetDir(dir string) { f.dir = dir }
|
||||
func (f *execFakeRunner) SetEnv(env map[string]string) {
|
||||
if len(env) == 0 {
|
||||
return
|
||||
}
|
||||
if f.env == nil {
|
||||
f.env = make(map[string]string, len(env))
|
||||
}
|
||||
for k, v := range env {
|
||||
f.env[k] = v
|
||||
}
|
||||
}
|
||||
func (f *execFakeRunner) Process() processHandle {
|
||||
if f.process != nil {
|
||||
return f.process
|
||||
|
||||
@@ -255,6 +255,10 @@ func (d *drainBlockingCmd) SetDir(dir string) {
|
||||
d.inner.SetDir(dir)
|
||||
}
|
||||
|
||||
func (d *drainBlockingCmd) SetEnv(env map[string]string) {
|
||||
d.inner.SetEnv(env)
|
||||
}
|
||||
|
||||
func (d *drainBlockingCmd) Process() processHandle {
|
||||
return d.inner.Process()
|
||||
}
|
||||
@@ -387,6 +391,8 @@ type fakeCmd struct {
|
||||
|
||||
stderr io.Writer
|
||||
|
||||
env map[string]string
|
||||
|
||||
waitDelay time.Duration
|
||||
waitErr error
|
||||
startErr error
|
||||
@@ -511,6 +517,20 @@ func (f *fakeCmd) SetStderr(w io.Writer) {
|
||||
|
||||
func (f *fakeCmd) SetDir(string) {}
|
||||
|
||||
func (f *fakeCmd) SetEnv(env map[string]string) {
|
||||
if len(env) == 0 {
|
||||
return
|
||||
}
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
if f.env == nil {
|
||||
f.env = make(map[string]string, len(env))
|
||||
}
|
||||
for k, v := range env {
|
||||
f.env[k] = v
|
||||
}
|
||||
}
|
||||
|
||||
func (f *fakeCmd) Process() processHandle {
|
||||
if f == nil {
|
||||
return nil
|
||||
@@ -1549,11 +1569,11 @@ func TestBackendBuildArgs_ClaudeBackend(t *testing.T) {
|
||||
got := backend.BuildArgs(cfg, "todo")
|
||||
want := []string{"-p", "--setting-sources", "", "--output-format", "stream-json", "--verbose", "todo"}
|
||||
if len(got) != len(want) {
|
||||
t.Fatalf("length mismatch")
|
||||
t.Fatalf("args length=%d, want %d: %v", len(got), len(want), got)
|
||||
}
|
||||
for i := range want {
|
||||
if got[i] != want[i] {
|
||||
t.Fatalf("index %d got %s want %s", i, got[i], want[i])
|
||||
t.Fatalf("index %d got %q want %q (args=%v)", i, got[i], want[i], got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1568,19 +1588,15 @@ func TestClaudeBackendBuildArgs_OutputValidation(t *testing.T) {
|
||||
target := "ensure-flags"
|
||||
|
||||
args := backend.BuildArgs(cfg, target)
|
||||
expectedPrefix := []string{"-p", "--setting-sources", "", "--output-format", "stream-json", "--verbose"}
|
||||
|
||||
if len(args) != len(expectedPrefix)+1 {
|
||||
t.Fatalf("args length=%d, want %d", len(args), len(expectedPrefix)+1)
|
||||
want := []string{"-p", "--setting-sources", "", "--output-format", "stream-json", "--verbose", target}
|
||||
if len(args) != len(want) {
|
||||
t.Fatalf("args length=%d, want %d: %v", len(args), len(want), args)
|
||||
}
|
||||
for i, val := range expectedPrefix {
|
||||
if args[i] != val {
|
||||
t.Fatalf("args[%d]=%q, want %q", i, args[i], val)
|
||||
for i := range want {
|
||||
if args[i] != want[i] {
|
||||
t.Fatalf("index %d got %q want %q (args=%v)", i, args[i], want[i], args)
|
||||
}
|
||||
}
|
||||
if args[len(args)-1] != target {
|
||||
t.Fatalf("last arg=%q, want target %q", args[len(args)-1], target)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBackendBuildArgs_GeminiBackend(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user