fix: address Codex round 7 — path traversal + dispatch check
Some checks failed
CI / test (push) Failing after 2s
Some checks failed
CI / test (push) Failing after 2s
High/Security: sanitise input.Repo via filepath.Base to prevent path traversal in workspace prep (../escape from CODE_PATH). High/Security: sanitise repo.Repo from API response in syncRepos to prevent path traversal via crafted checkin responses. Medium: dispatchFixFromQueue now returns error, review_queue checks success before recording fix_dispatched. Known issues updated with async bridge provider findings. Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
parent
013396bf91
commit
40d2b0db16
4 changed files with 32 additions and 7 deletions
|
|
@ -21,6 +21,11 @@ trade-offs or enhancement requests, not bugs.
|
|||
- `defaultBranch` falls back to `main`/`master` when `origin/HEAD` unavailable. Acceptable — covers 99% of repos.
|
||||
- `CODE_PATH` interpreted differently by `syncRepos` (repo root) vs rest of tooling (`CODE_PATH/core`). Known inconsistency.
|
||||
|
||||
## Async Bridge Returns (brain/provider.go)
|
||||
|
||||
- `provider.go:247` — recall HTTP handler forwards to bridge but returns empty `RecallOutput`. Results arrive async via WebSocket — by design for the IDE bridge path.
|
||||
- `provider.go:297` — list HTTP handler same pattern. Only affects bridge-mode clients, not DirectSubsystem.
|
||||
|
||||
## Compile Issues
|
||||
|
||||
- `pkg/setup` doesn't compile — calls `lib.RenderFile`, `lib.ListDirTemplates`, `lib.ExtractDir` which don't exist yet. Package is not imported by anything.
|
||||
|
|
|
|||
|
|
@ -169,8 +169,12 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques
|
|||
|
||||
out := PrepOutput{WorkspaceDir: wsDir}
|
||||
|
||||
// Source repo path
|
||||
repoPath := filepath.Join(s.codePath, "core", input.Repo)
|
||||
// Source repo path — sanitise to prevent path traversal
|
||||
repoName := filepath.Base(input.Repo) // strips ../ and absolute paths
|
||||
if repoName == "." || repoName == ".." || repoName == "" {
|
||||
return nil, PrepOutput{}, coreerr.E("prep", "invalid repo name: "+input.Repo, nil)
|
||||
}
|
||||
repoPath := filepath.Join(s.codePath, "core", repoName)
|
||||
|
||||
// 1. Clone repo into src/ and create feature branch
|
||||
srcDir := filepath.Join(wsDir, "src")
|
||||
|
|
|
|||
|
|
@ -231,8 +231,12 @@ func (s *PrepSubsystem) reviewRepo(ctx context.Context, repoDir, repo, reviewer
|
|||
"Commit: fix(coderabbit): address review findings\n\nFindings summary (%d issues):\n%s",
|
||||
result.Findings, truncate(output, 1500))
|
||||
|
||||
s.dispatchFixFromQueue(ctx, repo, task)
|
||||
result.Action = "fix_dispatched"
|
||||
if err := s.dispatchFixFromQueue(ctx, repo, task); err != nil {
|
||||
result.Action = "fix_dispatch_failed"
|
||||
result.Detail = err.Error()
|
||||
} else {
|
||||
result.Action = "fix_dispatched"
|
||||
}
|
||||
}
|
||||
|
||||
return result
|
||||
|
|
@ -263,14 +267,21 @@ func (s *PrepSubsystem) pushAndMerge(ctx context.Context, repoDir, repo string)
|
|||
}
|
||||
|
||||
// dispatchFixFromQueue dispatches an opus agent to fix CodeRabbit findings.
|
||||
func (s *PrepSubsystem) dispatchFixFromQueue(ctx context.Context, repo, task string) {
|
||||
func (s *PrepSubsystem) dispatchFixFromQueue(ctx context.Context, repo, task string) error {
|
||||
// Use the dispatch system — creates workspace, spawns agent
|
||||
input := DispatchInput{
|
||||
Repo: repo,
|
||||
Task: task,
|
||||
Agent: "claude:opus",
|
||||
}
|
||||
s.dispatch(ctx, nil, input)
|
||||
_, out, err := s.dispatch(ctx, nil, input)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !out.Success {
|
||||
return coreerr.E("dispatchFixFromQueue", "dispatch failed for "+repo, nil)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// countFindings estimates the number of findings in CodeRabbit output.
|
||||
|
|
|
|||
|
|
@ -94,7 +94,12 @@ func (m *Subsystem) syncRepos() string {
|
|||
|
||||
var pulled []string
|
||||
for _, repo := range checkin.Changed {
|
||||
repoDir := filepath.Join(basePath, repo.Repo)
|
||||
// Sanitise repo name to prevent path traversal from API response
|
||||
repoName := filepath.Base(repo.Repo)
|
||||
if repoName == "." || repoName == ".." || repoName == "" {
|
||||
continue
|
||||
}
|
||||
repoDir := filepath.Join(basePath, repoName)
|
||||
if _, err := os.Stat(repoDir); err != nil {
|
||||
continue
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue