fix: validate agent toml boundaries

This commit is contained in:
Yoilun
2026-05-25 17:58:22 +08:00
parent fee920a895
commit 425e11c444
4 changed files with 124 additions and 0 deletions

View File

@@ -61,6 +61,15 @@ func (s Store) readOne(fileName string) AgentDefinition {
def.ParseError = err.Error() def.ParseError = err.Error()
return def return def
} }
if info, err := os.Lstat(safePath); err != nil {
def.ParseStatus = "invalid"
def.ParseError = err.Error()
return def
} else if info.Mode()&os.ModeSymlink != 0 {
def.ParseStatus = "invalid"
def.ParseError = codexhome.ErrForbiddenPath.Error()
return def
}
info, statErr := os.Stat(safePath) info, statErr := os.Stat(safePath)
if statErr == nil { if statErr == nil {
def.ModifiedAt = info.ModTime() def.ModifiedAt = info.ModTime()
@@ -109,6 +118,12 @@ func parseSimpleTOML(input string) (map[string]string, error) {
if key == "" { if key == "" {
return values, fmt.Errorf("第 %d 行缺少字段名", lineNumber) return values, fmt.Errorf("第 %d 行缺少字段名", lineNumber)
} }
if !isValidBareKey(key) {
return values, fmt.Errorf("第 %d 行包含无效字段名", lineNumber)
}
if _, exists := values[key]; exists {
return values, fmt.Errorf("第 %d 行重复字段名 %q", lineNumber, key)
}
value, err := parseTOMLString(raw, scanner) value, err := parseTOMLString(raw, scanner)
if err != nil { if err != nil {
@@ -122,6 +137,25 @@ func parseSimpleTOML(input string) (map[string]string, error) {
return values, nil return values, nil
} }
func isValidBareKey(key string) bool {
for _, char := range key {
if char >= 'a' && char <= 'z' {
continue
}
if char >= 'A' && char <= 'Z' {
continue
}
if char >= '0' && char <= '9' {
continue
}
if char == '_' || char == '-' {
continue
}
return false
}
return true
}
func parseTOMLString(raw string, scanner *bufio.Scanner) (string, error) { func parseTOMLString(raw string, scanner *bufio.Scanner) (string, error) {
if strings.HasPrefix(raw, `"""`) { if strings.HasPrefix(raw, `"""`) {
block := strings.TrimPrefix(raw, `"""`) block := strings.TrimPrefix(raw, `"""`)

View File

@@ -72,6 +72,54 @@ func TestListAgentsReportsParseError(t *testing.T) {
} }
} }
func TestListAgentsReportsDuplicateKeyAsParseError(t *testing.T) {
root := t.TempDir()
agentsDir := filepath.Join(root, "agents")
if err := os.MkdirAll(agentsDir, 0o755); err != nil {
t.Fatal(err)
}
content := "name = \"a\"\nname = \"b\"\n"
if err := os.WriteFile(filepath.Join(agentsDir, "duplicate.toml"), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
store := Store{CodexHome: root}
got, err := store.List()
if err != nil {
t.Fatalf("List should return parse status, not fatal error: %v", err)
}
if len(got) != 1 {
t.Fatalf("agent count = %d, want 1", len(got))
}
if got[0].ParseStatus != "invalid" || got[0].ParseError == "" {
t.Fatalf("expected duplicate key to be invalid, got %#v", got[0])
}
}
func TestListAgentsReportsInvalidKeyAsParseError(t *testing.T) {
root := t.TempDir()
agentsDir := filepath.Join(root, "agents")
if err := os.MkdirAll(agentsDir, 0o755); err != nil {
t.Fatal(err)
}
content := "bad key = \"value\"\n"
if err := os.WriteFile(filepath.Join(agentsDir, "bad-key.toml"), []byte(content), 0o644); err != nil {
t.Fatal(err)
}
store := Store{CodexHome: root}
got, err := store.List()
if err != nil {
t.Fatalf("List should return parse status, not fatal error: %v", err)
}
if len(got) != 1 {
t.Fatalf("agent count = %d, want 1", len(got))
}
if got[0].ParseStatus != "invalid" || got[0].ParseError == "" {
t.Fatalf("expected invalid key to be invalid, got %#v", got[0])
}
}
func TestListAgentsRejectsSensitiveSymlinkTargets(t *testing.T) { func TestListAgentsRejectsSensitiveSymlinkTargets(t *testing.T) {
root := t.TempDir() root := t.TempDir()
agentsDir := filepath.Join(root, "agents") agentsDir := filepath.Join(root, "agents")
@@ -100,3 +148,34 @@ func TestListAgentsRejectsSensitiveSymlinkTargets(t *testing.T) {
t.Fatalf("sensitive file content leaked: %#v", got[0]) t.Fatalf("sensitive file content leaked: %#v", got[0])
} }
} }
func TestListAgentsRejectsSymlinkToNonAgentToml(t *testing.T) {
root := t.TempDir()
agentsDir := filepath.Join(root, "agents")
if err := os.MkdirAll(agentsDir, 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(root, "config.toml"), []byte(`name = "project-secret"`), 0o600); err != nil {
t.Fatal(err)
}
if err := os.Symlink("../config.toml", filepath.Join(agentsDir, "leak.toml")); err != nil {
t.Fatal(err)
}
store := Store{CodexHome: root}
got, err := store.List()
if err != nil {
t.Fatalf("List should report unsafe files per item, not fatal error: %v", err)
}
if len(got) != 1 {
t.Fatalf("agent count = %d, want 1", len(got))
}
if got[0].ParseStatus != "invalid" {
t.Fatalf("expected non-agent symlink target to be invalid, got %#v", got[0])
}
if strings.Contains(got[0].Name, "project-secret") ||
strings.Contains(got[0].Description, "project-secret") ||
strings.Contains(got[0].ParseError, "project-secret") {
t.Fatalf("non-agent TOML content leaked: %#v", got[0])
}
}

View File

@@ -11,6 +11,8 @@
| 2026-05-25 | 1 | review loop | 规格复审发现 `ResolveAgentTOML` 可经 `agents/demo.toml -> ../auth.json` symlink 绕过 forbidden 检查 | 已按 TDD 修复,并通过最终门禁 | | 2026-05-25 | 1 | review loop | 规格复审发现 `ResolveAgentTOML` 可经 `agents/demo.toml -> ../auth.json` symlink 绕过 forbidden 检查 | 已按 TDD 修复,并通过最终门禁 |
| 2026-05-25 | planning | main agent | 修正 task_plan.md 阶段命名,与实施计划 Task 2-7 对齐 | 下一阶段明确为 Agent TOML 只读读取 | | 2026-05-25 | planning | main agent | 修正 task_plan.md 阶段命名,与实施计划 Task 2-7 对齐 | 下一阶段明确为 Agent TOML 只读读取 |
| 2026-05-25 | 2 | coding agent | TDD 实现 Agent TOML 只读读取和 `/api/agents` | 完成;提交 `feat: read codex agent definitions` | | 2026-05-25 | 2 | coding agent | TDD 实现 Agent TOML 只读读取和 `/api/agents` | 完成;提交 `feat: read codex agent definitions` |
| 2026-05-25 | 2 | spec review | 规格审查未通过:重复键 TOML 可 validagent symlink 可读取 root `config.toml` | coding agent 按 blocking 范围修复 |
| 2026-05-25 | 2 | coding agent | TDD 修复 agent TOML parser 和 symlink 边界 | 完成;提交 `fix: validate agent toml boundaries` |
## Test Results ## Test Results
@@ -44,6 +46,12 @@
| 2026-05-25 | `go test ./...` | PASS | Required verification | | 2026-05-25 | `go test ./...` | PASS | Required verification |
| 2026-05-25 | `git diff --check` | PASS | Required verification | | 2026-05-25 | `git diff --check` | PASS | Required verification |
| 2026-05-25 | `git status --short` | PASS | Required verificationPhase 2 文件待提交 | | 2026-05-25 | `git status --short` | PASS | Required verificationPhase 2 文件待提交 |
| 2026-05-25 | `go test ./internal/agents` | FAIL | TDD 红灯duplicate key、invalid key、`agents/leak.toml -> ../config.toml` 均被错误报告为 valid/泄漏内容 |
| 2026-05-25 | `go test ./internal/agents` | PASS | duplicate key 和 invalid key 返回 invalidagent TOML symlink 被拒绝且不读取非 agent TOML |
| 2026-05-25 | `go test ./internal/codexhome` | PASS | Required verification |
| 2026-05-25 | `go test ./...` | PASS | Required verification |
| 2026-05-25 | `git diff --check` | PASS | Required verification |
| 2026-05-25 | `git status --short` | PASS | Required verificationPhase 2 review fix 文件待提交 |
## Bug Loop ## Bug Loop
@@ -54,3 +62,5 @@
| 1 | 缺少操作域 resolver通用 `ResolveInside` 容易误用 | 新增 `ResolveAgentTOML`,只允许 `agents/` 直属 `.toml` 文件名 | `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 | | 1 | `docs/project.md` 记录 `CODEX_HOME` 但默认配置未读取 | `DefaultConfig` 增加 `CODEX_HOME` 非空 override | `go test ./internal/app` PASS |
| 1 | `ResolveAgentTOML` 可通过 `agents/*.toml` symlink 指向 root `auth.json` 绕过 forbidden 检查 | 在 symlink 解析后对 evaluated final target 再执行 forbidden 检查 | `go test ./internal/codexhome` PASS | | 1 | `ResolveAgentTOML` 可通过 `agents/*.toml` symlink 指向 root `auth.json` 绕过 forbidden 检查 | 在 symlink 解析后对 evaluated final target 再执行 forbidden 检查 | `go test ./internal/codexhome` PASS |
| 2 | Agent TOML parser 对重复键使用 map 覆盖,且未校验 bare key | 增加 duplicate key 和 invalid key 检测,遇到 malformed TOML 返回单条 invalid | `go test ./internal/agents` PASS |
| 2 | Agent symlink 只校验最终路径在 Codex home 内,可读取 root `config.toml` | 在 agent store 层拒绝 `.toml` symlink避免读取非 agent TOML 内容 | `go test ./internal/agents` PASS |

View File

@@ -32,3 +32,4 @@
| --- | --- | --- | --- | --- | | --- | --- | --- | --- | --- |
| 2026-05-25 | 1 | 代码质量审查发现 symlink 边界绕过、敏感文件大小写匹配、缺少操作域 resolver、`CODEX_HOME` 未生效 | TDD 补充失败测试后修复 `codexhome``app` | 已通过最终验证 | | 2026-05-25 | 1 | 代码质量审查发现 symlink 边界绕过、敏感文件大小写匹配、缺少操作域 resolver、`CODEX_HOME` 未生效 | TDD 补充失败测试后修复 `codexhome``app` | 已通过最终验证 |
| 2026-05-25 | 1 | 规格复审发现 `ResolveAgentTOML` 可通过 `agents/*.toml` symlink 指向 root `auth.json` 绕过 forbidden 检查 | TDD 补充 symlink-to-auth 测试后检查 resolved final target | 已通过最终验证 | | 2026-05-25 | 1 | 规格复审发现 `ResolveAgentTOML` 可通过 `agents/*.toml` symlink 指向 root `auth.json` 绕过 forbidden 检查 | TDD 补充 symlink-to-auth 测试后检查 resolved final target | 已通过最终验证 |
| 2026-05-25 | 2 | 规格审查发现重复键 TOML 被报告为 valid且 agent symlink 可读取 Codex home 内非 agent TOML | TDD 补充 duplicate key、invalid key、symlink-to-config 测试后修复 parser 和 agent store 边界 | 已通过最终验证 |