From dc8b06f961a197b8a1cd0bc288f3a3f934713cc0 Mon Sep 17 00:00:00 2001 From: Yoilun Date: Mon, 25 May 2026 16:21:25 +0800 Subject: [PATCH] fix: harden codex home boundaries --- internal/app/config.go | 6 +++ internal/app/config_test.go | 33 +++++++++++++ internal/codexhome/bounds.go | 80 ++++++++++++++++++++++++++++++- internal/codexhome/bounds_test.go | 59 +++++++++++++++++++++++ progress.md | 12 +++++ task_plan.md | 1 + 6 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 internal/app/config_test.go diff --git a/internal/app/config.go b/internal/app/config.go index 90919ad..e0d256c 100644 --- a/internal/app/config.go +++ b/internal/app/config.go @@ -11,6 +11,12 @@ type Config struct { } func DefaultConfig() Config { + if codexHome := os.Getenv("CODEX_HOME"); codexHome != "" { + return Config{ + CodexHome: codexHome, + HTTPAddr: "127.0.0.1:18083", + } + } home, err := os.UserHomeDir() if err != nil { home = "." diff --git a/internal/app/config_test.go b/internal/app/config_test.go new file mode 100644 index 0000000..5021c9c --- /dev/null +++ b/internal/app/config_test.go @@ -0,0 +1,33 @@ +package app + +import ( + "os" + "path/filepath" + "testing" +) + +func TestDefaultConfigUsesCODEXHomeOverride(t *testing.T) { + override := filepath.Join(t.TempDir(), "custom-codex") + t.Setenv("CODEX_HOME", override) + + cfg := DefaultConfig() + + if cfg.CodexHome != override { + t.Fatalf("CodexHome = %q, want %q", cfg.CodexHome, override) + } +} + +func TestDefaultConfigFallsBackToUserCodexHome(t *testing.T) { + t.Setenv("CODEX_HOME", "") + home, err := os.UserHomeDir() + if err != nil { + t.Fatal(err) + } + + cfg := DefaultConfig() + + want := filepath.Join(home, ".codex") + if cfg.CodexHome != want { + t.Fatalf("CodexHome = %q, want %q", cfg.CodexHome, want) + } +} diff --git a/internal/codexhome/bounds.go b/internal/codexhome/bounds.go index 78dc820..effd1e5 100644 --- a/internal/codexhome/bounds.go +++ b/internal/codexhome/bounds.go @@ -2,6 +2,7 @@ package codexhome import ( "errors" + "os" "path/filepath" "strings" ) @@ -31,9 +32,22 @@ func ResolveInside(home string, rel string) (string, error) { if IsForbidden(candidate, cleanHome) { return "", ErrForbiddenPath } + if err := rejectSymlinkEscape(cleanHome, candidate); err != nil { + return "", err + } return candidate, nil } +func ResolveAgentTOML(home string, fileName string) (string, error) { + if filepath.IsAbs(fileName) || filepath.Dir(fileName) != "." || fileName == "" { + return "", ErrOutsideCodexHome + } + if !strings.HasSuffix(strings.ToLower(fileName), ".toml") { + return "", ErrForbiddenPath + } + return ResolveInside(home, filepath.Join("agents", fileName)) +} + func IsForbidden(path string, home string) bool { cleanHome, err := filepath.Abs(home) if err != nil { @@ -47,9 +61,73 @@ func IsForbidden(path string, home string) bool { if err != nil { return true } - rel = filepath.ToSlash(rel) + rel = strings.ToLower(filepath.ToSlash(rel)) forbidden := map[string]bool{ "auth.json": true, } return forbidden[rel] } + +func rejectSymlinkEscape(home string, candidate string) error { + evaluatedHome, err := evalExistingPath(home) + if err != nil { + return err + } + rel, err := filepath.Rel(home, candidate) + if err != nil { + return err + } + if rel == "." { + return nil + } + currentLexical := home + currentEvaluated := evaluatedHome + for _, part := range strings.Split(rel, string(filepath.Separator)) { + currentLexical = filepath.Join(currentLexical, part) + currentEvaluated = filepath.Join(currentEvaluated, part) + info, err := os.Lstat(currentLexical) + if err != nil { + if os.IsNotExist(err) { + continue + } + return err + } + if info.Mode()&os.ModeSymlink != 0 { + currentEvaluated, err = filepath.EvalSymlinks(currentLexical) + if err != nil { + return ErrOutsideCodexHome + } + } + if !isInside(evaluatedHome, currentEvaluated) { + return ErrOutsideCodexHome + } + } + return nil +} + +func evalExistingPath(path string) (string, error) { + evaluated, err := filepath.EvalSymlinks(path) + if err == nil { + return evaluated, nil + } + if !os.IsNotExist(err) { + return "", err + } + parent := filepath.Dir(path) + if parent == path { + return filepath.Abs(path) + } + evaluatedParent, err := evalExistingPath(parent) + if err != nil { + return "", err + } + return filepath.Join(evaluatedParent, filepath.Base(path)), nil +} + +func isInside(home string, path string) bool { + rel, err := filepath.Rel(home, path) + if err != nil { + return false + } + return rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))) +} diff --git a/internal/codexhome/bounds_test.go b/internal/codexhome/bounds_test.go index ca286dd..e6b17a1 100644 --- a/internal/codexhome/bounds_test.go +++ b/internal/codexhome/bounds_test.go @@ -1,6 +1,7 @@ package codexhome import ( + "os" "path/filepath" "testing" ) @@ -25,6 +26,25 @@ func TestResolveInsideCodexHomeRejectsTraversal(t *testing.T) { } } +func TestResolveInsideCodexHomeRejectsSymlinkEscape(t *testing.T) { + root := t.TempDir() + home := filepath.Join(root, ".codex") + external := filepath.Join(root, "external") + if err := os.MkdirAll(home, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(external, 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(external, filepath.Join(home, "agents")); err != nil { + t.Fatal(err) + } + _, err := ResolveInside(home, "agents/x.toml") + if err == nil { + t.Fatal("expected symlink escape to be rejected") + } +} + func TestIsForbiddenPathBlocksAuthJSON(t *testing.T) { home := filepath.Join(t.TempDir(), ".codex") path := filepath.Join(home, "auth.json") @@ -32,3 +52,42 @@ func TestIsForbiddenPathBlocksAuthJSON(t *testing.T) { t.Fatal("auth.json must be forbidden") } } + +func TestResolveInsideCodexHomeBlocksAuthJSONCaseInsensitive(t *testing.T) { + home := filepath.Join(t.TempDir(), ".codex") + _, err := ResolveInside(home, "AUTH.JSON") + if err == nil { + t.Fatal("AUTH.JSON must be forbidden") + } +} + +func TestResolveAgentTOMLAllowsDirectAgentToml(t *testing.T) { + home := filepath.Join(t.TempDir(), ".codex") + got, err := ResolveAgentTOML(home, "product-manager.toml") + if err != nil { + t.Fatalf("ResolveAgentTOML returned error: %v", err) + } + want := filepath.Join(home, "agents", "product-manager.toml") + if got != want { + t.Fatalf("path mismatch: got %q want %q", got, want) + } +} + +func TestResolveAgentTOMLRejectsUnsafeNames(t *testing.T) { + tests := []string{ + "../auth.json", + "auth.json", + "sessions/demo.jsonl", + "nested/demo.toml", + "demo.txt", + } + home := filepath.Join(t.TempDir(), ".codex") + for _, tt := range tests { + t.Run(tt, func(t *testing.T) { + _, err := ResolveAgentTOML(home, tt) + if err == nil { + t.Fatalf("expected %q to be rejected", tt) + } + }) + } +} diff --git a/progress.md b/progress.md index b3b0686..99e5822 100644 --- a/progress.md +++ b/progress.md @@ -7,6 +7,7 @@ | 2026-05-25 | 0 | coding agent | 创建文件化计划和项目基线 | 完成并通过规格审查 | | 2026-05-25 | 0 | review loop | 质量审查发现 docs/project.md 架构语气和 task_plan.md Phase 0 状态问题 | 已修复：改为目标架构语气，并将 Phase 0 标记为 complete | | 2026-05-25 | 1 | coding agent | 创建 Go 后端骨架和 Codex home 路径边界 | 已完成；未读取真实 `.codex` 数据文件 | +| 2026-05-25 | 1 | review loop | 代码质量审查发现 symlink 绕过、敏感文件大小写、操作域 resolver、`CODEX_HOME` override 问题 | 已按 TDD 修复，并通过最终门禁 | ## Test Results @@ -19,8 +20,19 @@ | 2026-05-25 | `curl http://127.0.0.1:18083/api/health` | PASS_WITH_ESCALATION | 普通 sandbox localhost 请求失败；提升权限后返回 `{"status":"ok"}` | | 2026-05-25 | `git diff --check` | PASS | 无 whitespace error | | 2026-05-25 | `git status --short` | PASS | 仅本阶段文件变更和新增 | +| 2026-05-25 | `go test ./internal/codexhome` | FAIL | TDD 红灯：新增 `ResolveAgentTOML` 测试后 API 未实现 | +| 2026-05-25 | `go test ./internal/app` | FAIL | TDD 红灯：`CODEX_HOME` override 未生效 | +| 2026-05-25 | `go test ./internal/app` | PASS | `CODEX_HOME` override 和默认 fallback 测试通过 | +| 2026-05-25 | `go test ./internal/codexhome` | PASS | symlink escape、大小写敏感文件、agent TOML scoped resolver 测试通过 | +| 2026-05-25 | `go test ./...` | PASS | 全量 Go 测试通过 | +| 2026-05-25 | `git diff --check` | PASS | 无 whitespace error | +| 2026-05-25 | `git status --short` | PASS | 仅本轮 Phase 1 修复文件变更 | ## Bug Loop | Phase | Bug | Fix Attempt | Retest Result | | --- | --- | --- | --- | +| 1 | `ResolveInside` 可被 `.codex/agents` symlink 指向外部目录绕过 | 检查已存在路径组件，发现 symlink 后使用 `EvalSymlinks` 并确认仍在 evaluated Codex home 内 | `go test ./internal/codexhome` PASS | +| 1 | `AUTH.JSON` 等大小写变体未被敏感文件 denylist 拦截 | 对敏感根文件相对路径做 case-insensitive 匹配 | `go test ./internal/codexhome` PASS | +| 1 | 缺少操作域 resolver，通用 `ResolveInside` 容易误用 | 新增 `ResolveAgentTOML`，只允许 `agents/` 直属 `.toml` 文件名 | `go test ./internal/codexhome` PASS | +| 1 | `docs/project.md` 记录 `CODEX_HOME` 但默认配置未读取 | `DefaultConfig` 增加 `CODEX_HOME` 非空 override | `go test ./internal/app` PASS | diff --git a/task_plan.md b/task_plan.md index 59c5e15..75dbdd7 100644 --- a/task_plan.md +++ b/task_plan.md @@ -29,3 +29,4 @@ | Time | Phase | Error | Attempt | Resolution | | --- | --- | --- | --- | --- | +| 2026-05-25 | 1 | 代码质量审查发现 symlink 边界绕过、敏感文件大小写匹配、缺少操作域 resolver、`CODEX_HOME` 未生效 | TDD 补充失败测试后修复 `codexhome` 和 `app` | 已通过最终验证 |