fix: harden codex home boundaries
This commit is contained in:
@@ -11,6 +11,12 @@ type Config struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func DefaultConfig() Config {
|
func DefaultConfig() Config {
|
||||||
|
if codexHome := os.Getenv("CODEX_HOME"); codexHome != "" {
|
||||||
|
return Config{
|
||||||
|
CodexHome: codexHome,
|
||||||
|
HTTPAddr: "127.0.0.1:18083",
|
||||||
|
}
|
||||||
|
}
|
||||||
home, err := os.UserHomeDir()
|
home, err := os.UserHomeDir()
|
||||||
if err != nil {
|
if err != nil {
|
||||||
home = "."
|
home = "."
|
||||||
|
|||||||
33
internal/app/config_test.go
Normal file
33
internal/app/config_test.go
Normal file
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -2,6 +2,7 @@ package codexhome
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
@@ -31,9 +32,22 @@ func ResolveInside(home string, rel string) (string, error) {
|
|||||||
if IsForbidden(candidate, cleanHome) {
|
if IsForbidden(candidate, cleanHome) {
|
||||||
return "", ErrForbiddenPath
|
return "", ErrForbiddenPath
|
||||||
}
|
}
|
||||||
|
if err := rejectSymlinkEscape(cleanHome, candidate); err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
return candidate, nil
|
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 {
|
func IsForbidden(path string, home string) bool {
|
||||||
cleanHome, err := filepath.Abs(home)
|
cleanHome, err := filepath.Abs(home)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -47,9 +61,73 @@ func IsForbidden(path string, home string) bool {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
rel = filepath.ToSlash(rel)
|
rel = strings.ToLower(filepath.ToSlash(rel))
|
||||||
forbidden := map[string]bool{
|
forbidden := map[string]bool{
|
||||||
"auth.json": true,
|
"auth.json": true,
|
||||||
}
|
}
|
||||||
return forbidden[rel]
|
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)))
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package codexhome
|
package codexhome
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"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) {
|
func TestIsForbiddenPathBlocksAuthJSON(t *testing.T) {
|
||||||
home := filepath.Join(t.TempDir(), ".codex")
|
home := filepath.Join(t.TempDir(), ".codex")
|
||||||
path := filepath.Join(home, "auth.json")
|
path := filepath.Join(home, "auth.json")
|
||||||
@@ -32,3 +52,42 @@ func TestIsForbiddenPathBlocksAuthJSON(t *testing.T) {
|
|||||||
t.Fatal("auth.json must be forbidden")
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
12
progress.md
12
progress.md
@@ -7,6 +7,7 @@
|
|||||||
| 2026-05-25 | 0 | coding agent | 创建文件化计划和项目基线 | 完成并通过规格审查 |
|
| 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 | 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 | coding agent | 创建 Go 后端骨架和 Codex home 路径边界 | 已完成;未读取真实 `.codex` 数据文件 |
|
||||||
|
| 2026-05-25 | 1 | review loop | 代码质量审查发现 symlink 绕过、敏感文件大小写、操作域 resolver、`CODEX_HOME` override 问题 | 已按 TDD 修复,并通过最终门禁 |
|
||||||
|
|
||||||
## Test Results
|
## 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 | `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 diff --check` | PASS | 无 whitespace error |
|
||||||
| 2026-05-25 | `git status --short` | PASS | 仅本阶段文件变更和新增 |
|
| 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
|
## Bug Loop
|
||||||
|
|
||||||
| Phase | Bug | Fix Attempt | Retest Result |
|
| 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 |
|
||||||
|
|||||||
@@ -29,3 +29,4 @@
|
|||||||
|
|
||||||
| Time | Phase | Error | Attempt | Resolution |
|
| Time | Phase | Error | Attempt | Resolution |
|
||||||
| --- | --- | --- | --- | --- |
|
| --- | --- | --- | --- | --- |
|
||||||
|
| 2026-05-25 | 1 | 代码质量审查发现 symlink 边界绕过、敏感文件大小写匹配、缺少操作域 resolver、`CODEX_HOME` 未生效 | TDD 补充失败测试后修复 `codexhome` 和 `app` | 已通过最终验证 |
|
||||||
|
|||||||
Reference in New Issue
Block a user