From 2e5daaa515b06eea6c7f02e6917a0b166986bf9d Mon Sep 17 00:00:00 2001 From: Mohammad Parvin Date: Wed, 15 Jul 2026 10:36:02 +0330 Subject: [PATCH 1/4] Implement audit roadmap: security hardening, reliability, and CI. Close permission bypasses for all tools, fix Docker argv isolation and goal resume, add tests and DevOps docs, and stop tracking personal config.yaml. Co-authored-by: Cursor --- .github/workflows/ci.yml | 31 +++ .gitignore | 5 +- AGENTS.md | 87 ++++++++ CONTRIBUTING.md | 28 +++ Dockerfile | 27 +++ SECURITY.md | 36 ++++ cmd/octa-agent/main.go | 359 ++++++++++++++++++++++++++++++++ cmd/octa-agentd/main.go | 182 ++++++++++++++++ config.example.yaml | 9 + config.yaml | 35 ---- go.mod | 4 +- pkg/browser/server.go | 46 ++-- pkg/config/browser.go | 22 ++ pkg/config/config.go | 48 ++++- pkg/config/path_test.go | 92 ++++++++ pkg/engine/checkpoint.go | 6 +- pkg/engine/engine.go | 131 ++++++++---- pkg/engine/prompts.go | 36 +++- pkg/engine/runner.go | 89 ++++++-- pkg/engine/step.go | 11 + pkg/engine/step_test.go | 25 +++ pkg/evaluator/evaluator.go | 4 +- pkg/evaluator/evaluator_test.go | 40 ++++ pkg/isolation/docker.go | 16 +- pkg/isolation/docker_test.go | 24 +++ pkg/permission/manager.go | 103 ++++++++- pkg/permission/manager_test.go | 113 ++++++++++ pkg/permission/normalize.go | 13 ++ pkg/permission/shellquote.go | 8 + pkg/permission/url.go | 99 +++++++++ pkg/planner/planner.go | 46 ++-- pkg/planner/project.go | 50 +++++ pkg/planner/project_test.go | 36 ++++ pkg/planner/replan.go | 2 +- pkg/plugin/registry.go | 12 +- pkg/storage/sqlite.go | 46 ++-- pkg/storage/storage.go | 1 + pkg/tools/ISSUE.md | 54 ----- pkg/tools/command.go | 17 +- pkg/tools/command_test.go | 29 +++ pkg/tools/filesystem.go | 9 +- pkg/tools/filesystem_test.go | 66 ++++++ pkg/tools/git.go | 5 + pkg/tools/ssh.go | 41 ++-- 44 files changed, 1875 insertions(+), 268 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 AGENTS.md create mode 100644 CONTRIBUTING.md create mode 100644 Dockerfile create mode 100644 SECURITY.md create mode 100644 cmd/octa-agent/main.go create mode 100644 cmd/octa-agentd/main.go delete mode 100644 config.yaml create mode 100644 pkg/config/browser.go create mode 100644 pkg/config/path_test.go create mode 100644 pkg/engine/step_test.go create mode 100644 pkg/evaluator/evaluator_test.go create mode 100644 pkg/permission/manager_test.go create mode 100644 pkg/permission/normalize.go create mode 100644 pkg/permission/shellquote.go create mode 100644 pkg/permission/url.go create mode 100644 pkg/planner/project.go create mode 100644 pkg/planner/project_test.go delete mode 100644 pkg/tools/ISSUE.md create mode 100644 pkg/tools/command_test.go create mode 100644 pkg/tools/filesystem_test.go diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..0a8f4a3 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,31 @@ +name: CI + +on: + push: + branches: [master, main] + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version: "1.23" + + - name: Install build deps (CGO sqlite) + run: sudo apt-get update && sudo apt-get install -y gcc libc6-dev + + - name: Download dependencies + run: make deps + + - name: Vet + run: make vet + + - name: Test + run: make test + + - name: Build + run: make build diff --git a/.gitignore b/.gitignore index 9c2e55b..5f38ddb 100644 --- a/.gitignore +++ b/.gitignore @@ -4,8 +4,8 @@ bin/ *.dll *.so *.dylib -octa-agentd -octa-agent +/octa-agentd +/octa-agent # Test binary, built with `go test -c` *.test @@ -35,6 +35,7 @@ Thumbs.db *.db *.sqlite *.sqlite3 +config.yaml data/ logs/ tmp/ diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..01085d3 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,87 @@ +# AGENTS.md + +This file provides guidance to WARP (warp.dev) when working with code in this repository. + +## Core development commands + +### Setup and build +- `make deps` — download and tidy Go modules. +- `make build` — build both binaries into `bin/` (`octa-agentd`, `octa-agent`). +- `make build-daemon` / `make build-cli` — build one binary. +- `make install` — install both binaries via `go install`. + +### Run locally +- `./bin/octa-agentd` — start daemon (polls and processes queued goals). +- `./bin/octa-agentd --browser-port 8765` — start daemon with browser automation enabled on a specific port. +- `./bin/octa-agent init` — create `~/.config/octaai/config.yaml`. +- `./bin/octa-agent goal ""` — submit a goal. +- `./bin/octa-agent status` — list goal states and progress. +- `./bin/octa-agent logs ` — inspect execution logs and task outcomes. +- `./bin/octa-agent approvals` / `approve ` / `deny ` — handle human approval gates. + +### Test, lint, and validation +- `make test` — run all tests (`go test -v ./...`). +- `make test-coverage` — run tests with coverage report (`coverage.out`, `coverage.html`). +- `go test -v ./pkg/workflow -run TestName` — run a single test in one package. +- `make lint` — run `golangci-lint run` (requires `golangci-lint` installed). +- `make fmt` and `make vet` — baseline formatting and static checks. + +## High-level architecture + +### Runtime shape (daemon + CLI + state store) +- `cmd/octa-agent/main.go`: CLI for creating goals, checking status/logs, and resolving approvals. +- `cmd/octa-agentd/main.go`: long-running daemon that loads config/LLM/tools, polls goals every 5 seconds, resumes interrupted `EXECUTING`/`EVALUATING` goals, and processes goals concurrently. +- `pkg/storage/sqlite.go`: central persistence layer — tables: `goals`, `tasks`, `execution_steps`, `logs`, `checkpoints`, `approvals`. SQLite is the source of truth for resumability. Default path: `~/.config/octaai/state.db`. + +### Goal state machine +Goals move through: `IDLE → PLANNING → EXECUTING → EVALUATING → COMPLETED / FAILED`, with optional states `RETRYING` (recoverable failure triggers replanning), `WAITING_FOR_APPROVAL` (permission gate), and `BLOCKED`. Valid transitions are enforced by `engine.CanTransition()` in `pkg/engine/state.go`. + +### Goal execution pipeline +- `pkg/agent/agent.go`: thin wrapper around the engine. +- `pkg/engine/engine.go`: drives the state machine loop (default: 50 max loops, 500ms pause per loop): + 1. Planner creates 1–2 template tasks (project setup + LLM-driven execution task); the LLM chooses concrete tool calls per step inside `executeLLMTask`. + 2. Engine runs ready tasks respecting dependencies; parallel up to `MaxParallel=3` when enabled. + 3. Every tool call produces a persisted `ExecutionStep` record (pending → running → completed/failed) with retry count bounded by `engine.max_retries` (default 3). + 4. Evaluator chain runs after each step; can signal retry or fatal failure. + 5. On recoverable failure with `EnableReplan=true`, `replan.go` adds one corrective task. +- `pkg/engine/runner.go`: executes tool calls with per-step timeout (default 5 min) + permission checks + optional Docker isolation (argv-safe). +- `pkg/engine/checkpoint.go`: checkpoints saved during execution; daemon resumes interrupted goals on restart. + +### Evaluator chain +`pkg/evaluator/evaluator.go` runs four evaluators in sequence per step: +1. **ToolResultEvaluator** — basic success/failure from `ToolResult.Success`. +2. **BuildResultEvaluator** — detects compile/build error patterns in command output. +3. **TestResultEvaluator** — detects test failure patterns. +4. **GoalCompletionEvaluator** — heuristic check based on step outcomes (not a separate LLM call). + +Each returns one of: `success`, `partial_success`, `retry_required`, `fatal_failure` (defined in `pkg/execution/types.go`). + +### Planner +`pkg/planner/planner.go` uses a lightweight template planner (1–2 tasks) with an LLM yes/no project-name prompt. Actual tool selection happens per-step in the engine. `pkg/workflow` provides DAG validation utilities used in tests only. + +### Extensibility and tool loading +- `pkg/plugin/registry.go` + `pkg/plugin/builtin.go`: plugin system wires capability groups into tool registry at daemon startup. +- Default plugins: `coding` (filesystem, command, git), `devops` (ssh, http), `browser` (only when `--browser-port` is passed). +- Adding a new tool: implement `tools.Tool` interface (Name/Schema/Execute), register it in the relevant plugin's `Load()`, and add a `CheckTool` case in `pkg/permission/manager.go`. + +### Safety, approvals, and isolation +- `pkg/permission/manager.go`: enforces policy for all six tools — `Allow`, `Deny`, or `RequireApproval`. Command `cwd`, git paths, HTTP URLs (SSRF guard), browser domains, and SSH always gated. +- `pkg/approval/service.go`: persists pending approvals; resolved interactively via `octa-agent approve/deny `. +- `pkg/isolation/docker.go`: wraps tool args for Docker execution when `isolation.require_docker_for` lists the tool type. + +### LLM providers +Configured via `llm.provider` in config. Supported values: `ollama` (default, no API key needed), `openai`, `claude`. The provider interface (`pkg/llm/provider.go`) exposes `Complete(ctx, messages)` — all planners and evaluators go through this single interface. + +### Memory +- `pkg/memory/manager.go`: appends facts learned during execution to a per-goal store. +- `pkg/memory/semantic.go`: vector-based retrieval providing relevant past facts to the planner as context, improving replanning quality. + +### Configuration that affects behavior +- Primary config: `~/.config/octaai/config.yaml` (created by `octa-agent init`). +- Key sections in `pkg/config/config.go`: + - `llm`: provider / model / base_url / api_key / temperature / max_tokens + - `safety`: allow_paths, allow_http_hosts, deny_commands, require_confirmation_for + - `engine`: max_loops, max_retries, enable_replan, enable_parallel + - `isolation`: enabled, docker (image/network/memory/cpu limits), require_docker_for + - `browser`: enabled, port, token, browser_domains + - `storage`: type, path (defaults to `~/.config/octaai/state.db`) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..8365505 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,28 @@ +# Contributing to OctaAI + +Thank you for contributing. This project is a Go CLI + daemon agent for autonomous goal execution. + +## Development setup + +```bash +make deps +make build +make test +``` + +## Code style + +- Run `make fmt` and `make vet` before opening a PR. +- Keep changes focused; match existing package layout and naming. +- Add tests for safety-critical behavior (`pkg/permission`, `pkg/tools`). + +## Pull requests + +1. Fork and create a feature branch. +2. Ensure `make test` passes locally. +3. Describe behavior changes and security impact when touching tools or permissions. +4. Do not commit personal `config.yaml` files or SQLite databases. + +## Reporting security issues + +See [SECURITY.md](SECURITY.md). Do not open public issues for exploitable vulnerabilities. diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..bf4b7e1 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,27 @@ +FROM golang:1.23-bookworm AS builder + +RUN apt-get update && apt-get install -y gcc libc6-dev && rm -rf /var/lib/apt/lists/* + +WORKDIR /src +COPY go.mod go.sum ./ +RUN go mod download + +COPY . . +ENV CGO_ENABLED=1 +RUN make build + +FROM debian:bookworm-slim + +RUN apt-get update && apt-get install -y ca-certificates git openssh-client && rm -rf /var/lib/apt/lists/* + +COPY --from=builder /src/bin/octa-agentd /usr/local/bin/octa-agentd +COPY --from=builder /src/bin/octa-agent /usr/local/bin/octa-agent + +RUN useradd -m -u 1000 octa +USER octa +WORKDIR /home/octa + +ENV HOME=/home/octa +VOLUME ["/home/octa/.config/octaai"] + +ENTRYPOINT ["octa-agentd"] diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..148165f --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,36 @@ +# Security Policy + +OctaAI executes shell commands, filesystem operations, SSH, HTTP requests, and browser automation based on LLM output. Treat it as a privileged local agent. + +## Supported versions + +Security fixes are applied on the `master` branch. + +## Reporting a vulnerability + +Please report security issues privately to the repository maintainer rather than opening a public GitHub issue. + +Include: + +- Description of the vulnerability and impact +- Steps to reproduce +- Suggested fix (if any) + +## Security configuration + +Review `config.example.yaml` before running the daemon: + +- `safety.allow_paths` — filesystem and command `cwd` boundaries +- `safety.deny_commands` — blocked shell command patterns (normalized matching) +- `safety.require_confirmation_for` — commands requiring human approval +- `safety.allow_http_hosts` — optional HTTP host allowlist for `http` and `git clone` +- `browser.token` — required WebSocket token when browser automation is enabled +- `browser.browser_domains` — allowed navigation domains for the browser tool +- `ssh.known_hosts_file` — required for SSH host-key verification + +## Threat model notes + +- The permission manager gates all six built-in tools. +- Docker isolation is opt-in and should be validated before relying on it in production. +- Browser WebSocket connections require a token and restricted origins. +- HTTP tool requests block private/link-local addresses by default (SSRF mitigation). diff --git a/cmd/octa-agent/main.go b/cmd/octa-agent/main.go new file mode 100644 index 0000000..05c9954 --- /dev/null +++ b/cmd/octa-agent/main.go @@ -0,0 +1,359 @@ +package main + +import ( + "flag" + "fmt" + "log" + "os" + "strings" + "text/tabwriter" + "time" + + "github.com/mparvin/octaai/pkg/approval" + "github.com/mparvin/octaai/pkg/config" + "github.com/mparvin/octaai/pkg/planner" + "github.com/mparvin/octaai/pkg/storage" +) + +func main() { + if len(os.Args) < 2 { + printUsage() + os.Exit(1) + } + + command := os.Args[1] + + cfg, err := config.LoadConfig(config.ConfigPath()) + if err != nil { + log.Fatalf("Failed to load config: %v", err) + } + + store, err := storage.NewSQLiteStorage(cfg.Storage.Path) + if err != nil { + log.Fatalf("Failed to create storage: %v", err) + } + defer store.Close() + + switch command { + case "goal": + handleGoal(store, os.Args[2:]) + case "status": + handleStatus(store) + case "list": + handleList(store) + case "logs": + handleLogs(store, os.Args[2:]) + case "approvals": + handleApprovals(store) + case "approve": + handleApprove(store, os.Args[2:]) + case "deny": + handleDeny(store, os.Args[2:]) + case "init": + handleInit(cfg) + default: + fmt.Printf("Unknown command: %s\n", command) + printUsage() + os.Exit(1) + } +} + +func printUsage() { + fmt.Println("OctaAI Agent CLI") + fmt.Println() + fmt.Println("Usage:") + fmt.Println(" octa-agent goal [flags] Submit a new goal") + fmt.Println(" octa-agent list List all goal IDs") + fmt.Println(" octa-agent status Show goals status") + fmt.Println(" octa-agent logs Show logs for a goal") + fmt.Println(" octa-agent approvals List pending approvals") + fmt.Println(" octa-agent approve Approve a pending action") + fmt.Println(" octa-agent deny Deny a pending action") + fmt.Println(" octa-agent init Initialize configuration") + fmt.Println() + fmt.Println("Examples:") + fmt.Println(" octa-agent goal --project github-list \"Create a Python CLI for GitHub repos\"") + fmt.Println(" octa-agent approvals") + fmt.Println(" octa-agent approve approval_1234567890") +} + +func handleGoal(store storage.Storage, args []string) { + fs := flag.NewFlagSet("goal", flag.ExitOnError) + projectName := fs.String("project", "", "Project directory name under projects_root") + fs.StringVar(projectName, "p", "", "Project directory name (shorthand)") + if err := fs.Parse(args); err != nil { + log.Fatalf("Failed to parse goal flags: %v", err) + } + + remaining := fs.Args() + if len(remaining) == 0 { + fmt.Println("Error: Goal description is required") + fmt.Println("Usage: octa-agent goal [--project NAME] ") + os.Exit(1) + } + + description := strings.Join(remaining, " ") + sanitizedProject := planner.SanitizeProjectName(*projectName) + if *projectName != "" && sanitizedProject == "" { + fmt.Println("Error: Invalid project name") + os.Exit(1) + } + + goal := &storage.Goal{ + ID: fmt.Sprintf("goal_%d", time.Now().Unix()), + Description: description, + ProjectName: sanitizedProject, + State: storage.StateIdle, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + } + + if err := store.CreateGoal(goal); err != nil { + log.Fatalf("Failed to create goal: %v", err) + } + + fmt.Printf("✓ Goal created: %s\n", goal.ID) + fmt.Printf(" Description: %s\n", goal.Description) + if goal.ProjectName != "" { + fmt.Printf(" Project: %s\n", goal.ProjectName) + } + fmt.Println() + fmt.Println("The goal will be processed by the agent daemon.") + fmt.Println("Run 'octa-agent status' to check progress.") +} + +func handleList(store storage.Storage) { + goals, err := store.ListGoals() + if err != nil { + log.Fatalf("Failed to list goals: %v", err) + } + + if len(goals) == 0 { + fmt.Println("No goals found.") + return + } + + for _, goal := range goals { + fmt.Println(goal.ID) + } +} + +func handleStatus(store storage.Storage) { + goals, err := store.ListGoals() + if err != nil { + log.Fatalf("Failed to list goals: %v", err) + } + + if len(goals) == 0 { + fmt.Println("No goals found.") + fmt.Println("Create a goal with: octa-agent goal \"\"") + return + } + + fmt.Println("Goals Status:") + fmt.Println() + + w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(w, "ID\tSTATE\tDESCRIPTION\tCREATED") + fmt.Fprintln(w, "──\t─────\t───────────\t───────") + + for _, goal := range goals { + desc := goal.Description + if len(desc) > 50 { + desc = desc[:47] + "..." + } + fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", + goal.ID, + goal.State, + desc, + goal.CreatedAt.Format("2006-01-02 15:04"), + ) + } + + w.Flush() + for _, goal := range goals { + if goal.ProjectName != "" { + fmt.Printf(" %s → project: %s\n", goal.ID, goal.ProjectName) + } + } + fmt.Println() + + for _, goal := range goals { + if goal.State == storage.StatePlanning || goal.State == storage.StateExecuting || + goal.State == storage.StateEvaluating || goal.State == storage.StateRetrying { + fmt.Printf("\n📍 Active: %s\n", goal.ID) + fmt.Printf(" State: %s\n", goal.State) + + tasks, err := store.GetTasksByGoal(goal.ID) + if err == nil && len(tasks) > 0 { + completed := 0 + for _, task := range tasks { + if task.Status == "completed" { + completed++ + } + } + fmt.Printf(" Progress: %d/%d tasks completed\n", completed, len(tasks)) + } + } + if goal.State == storage.StateWaitingForApproval { + fmt.Printf("\n⚠ Awaiting approval: %s\n", goal.ID) + fmt.Println(" Run 'octa-agent approvals' to review pending actions.") + } + } + + for _, goal := range goals { + if goal.State == storage.StateCompleted { + fmt.Printf("\n✓ Completed: %s\n", goal.ID) + fmt.Printf(" %s\n", goal.Result) + } else if goal.State == storage.StateFailed { + fmt.Printf("\n✗ Failed: %s\n", goal.ID) + fmt.Printf(" Error: %s\n", goal.Error) + } + } +} + +func handleApprovals(store storage.Storage) { + svc := approval.NewService(store) + reqs, err := svc.ListPending() + if err != nil { + log.Fatalf("Failed to list approvals: %v", err) + } + + if len(reqs) == 0 { + fmt.Println("No pending approvals.") + return + } + + fmt.Println("Pending Approvals:") + fmt.Println() + w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(w, "ID\tGOAL\tTOOL\tREASON\tCREATED") + fmt.Fprintln(w, "──\t────\t────\t──────\t───────") + + for _, req := range reqs { + reason := req.Reason + if len(reason) > 40 { + reason = reason[:37] + "..." + } + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", + req.ID, req.GoalID, req.ToolName, reason, + req.CreatedAt.Format("2006-01-02 15:04"), + ) + } + w.Flush() + fmt.Println() + fmt.Println("Use: octa-agent approve or octa-agent deny ") +} + +func handleApprove(store storage.Storage, args []string) { + if len(args) == 0 { + fmt.Println("Usage: octa-agent approve ") + os.Exit(1) + } + svc := approval.NewService(store) + req, err := svc.Approve(args[0]) + if err != nil { + log.Fatalf("Failed to approve: %v", err) + } + fmt.Printf("✓ Approved: %s\n", req.ID) + fmt.Printf(" Goal %s will resume on next daemon cycle.\n", req.GoalID) +} + +func handleDeny(store storage.Storage, args []string) { + if len(args) == 0 { + fmt.Println("Usage: octa-agent deny ") + os.Exit(1) + } + svc := approval.NewService(store) + req, err := svc.Deny(args[0]) + if err != nil { + log.Fatalf("Failed to deny: %v", err) + } + fmt.Printf("✗ Denied: %s\n", req.ID) + fmt.Printf(" Goal %s marked as failed.\n", req.GoalID) +} + +func handleLogs(store storage.Storage, args []string) { + if len(args) == 0 { + fmt.Println("Error: Goal ID is required") + fmt.Println("Usage: octa-agent logs ") + os.Exit(1) + } + + goalID := args[0] + + goal, err := store.GetGoal(goalID) + if err != nil { + log.Fatalf("Failed to get goal: %v", err) + } + + fmt.Printf("Logs for Goal: %s\n", goal.ID) + fmt.Printf("Description: %s\n", goal.Description) + fmt.Printf("State: %s\n", goal.State) + fmt.Println() + + logs, err := store.GetLogsByGoal(goalID) + if err != nil { + log.Fatalf("Failed to get logs: %v", err) + } + + if len(logs) == 0 { + fmt.Println("No logs available for this goal.") + return + } + + for _, l := range logs { + timestamp := l.CreatedAt.Format("15:04:05") + fmt.Printf("[%s] %s: %s\n", timestamp, l.Level, l.Message) + if l.Data != "" { + fmt.Printf(" Data: %s\n", l.Data) + } + } + + fmt.Println() + fmt.Println("Tasks:") + tasks, err := store.GetTasksByGoal(goalID) + if err == nil { + for i, task := range tasks { + status := "⏳" + if task.Status == "completed" { + status = "✓" + } else if task.Status == "failed" { + status = "✗" + } + fmt.Printf(" %d. %s %s - %s\n", i+1, status, task.Status, task.Description) + if task.Error != "" { + fmt.Printf(" Error: %s\n", task.Error) + } + } + } +} + +func handleInit(cfg *config.Config) { + configPath := config.ConfigPath() + + if _, err := os.Stat(configPath); err == nil { + fmt.Printf("Configuration file already exists at: %s\n", configPath) + fmt.Print("Overwrite? (y/N): ") + var response string + _, _ = fmt.Scanln(&response) + if response != "y" && response != "Y" { + fmt.Println("Initialization cancelled.") + return + } + } + + if err := config.SaveConfig(cfg, configPath); err != nil { + log.Fatalf("Failed to save config: %v", err) + } + + fmt.Printf("✓ Configuration initialized at: %s\n", configPath) + fmt.Println() + fmt.Println("Default settings:") + fmt.Printf(" Projects Root: %s\n", cfg.ProjectsRoot) + fmt.Printf(" LLM Provider: %s\n", cfg.LLM.Provider) + fmt.Printf(" LLM Model: %s\n", cfg.LLM.Model) + fmt.Println() + fmt.Println("Edit the configuration file to customize settings.") + fmt.Println("Then start the agent daemon with: octa-agentd") +} diff --git a/cmd/octa-agentd/main.go b/cmd/octa-agentd/main.go new file mode 100644 index 0000000..5452051 --- /dev/null +++ b/cmd/octa-agentd/main.go @@ -0,0 +1,182 @@ +package main + +import ( + "context" + "flag" + "fmt" + "log" + "os" + "os/signal" + "sync" + "syscall" + "time" + + "github.com/mparvin/octaai/pkg/agent" + "github.com/mparvin/octaai/pkg/browser" + "github.com/mparvin/octaai/pkg/config" + "github.com/mparvin/octaai/pkg/llm" + "github.com/mparvin/octaai/pkg/plugin" + "github.com/mparvin/octaai/pkg/storage" + "github.com/mparvin/octaai/pkg/tools" +) + +func isProcessableState(state storage.State) bool { + switch state { + case storage.StateIdle, storage.StatePlanning, storage.StateRetrying, + storage.StateExecuting, storage.StateEvaluating: + return true + default: + return false + } +} + +func main() { + browserPort := flag.Int("browser-port", 0, "Override browser WebSocket port (default: from config, usually 8765)") + flag.Parse() + + fmt.Println("OctaAI Agent Daemon - Starting...") + + cfg, err := config.LoadConfig(config.ConfigPath()) + if err != nil { + log.Fatalf("Failed to load config: %v", err) + } + + if *browserPort > 0 { + cfg.Browser.Port = *browserPort + cfg.Browser.Enabled = true + } + + fmt.Printf("Using LLM: %s (%s)\n", cfg.LLM.Provider, cfg.LLM.Model) + fmt.Printf("Projects root: %s\n", cfg.ProjectsRoot) + fmt.Printf("Storage: %s\n", cfg.Storage.Path) + + llmProvider, err := llm.NewProvider(&cfg.LLM) + if err != nil { + log.Fatalf("Failed to create LLM provider: %v", err) + } + + store, err := storage.NewSQLiteStorage(cfg.Storage.Path) + if err != nil { + log.Fatalf("Failed to create storage: %v", err) + } + defer store.Close() + + var browserServer *browser.Server + if cfg.Browser.Enabled { + if _, err := config.EnsureBrowserToken(cfg); err != nil { + log.Fatalf("Failed to configure browser token: %v", err) + } + addr := fmt.Sprintf("localhost:%d", cfg.Browser.Port) + browserServer = browser.NewServer(addr, cfg.Browser.Token) + go func() { + if err := browserServer.Start(); err != nil { + log.Printf("Browser WebSocket server error: %v", err) + } + }() + fmt.Printf("Browser automation enabled on port %d\n", cfg.Browser.Port) + } + + pluginRegistry := plugin.NewRegistry() + for _, p := range plugin.DefaultPlugins(browserServer) { + pluginRegistry.Register(p) + } + + toolRegistry := tools.NewRegistry() + if err := pluginRegistry.LoadAll(toolRegistry, cfg); err != nil { + log.Fatalf("Failed to load plugins: %v", err) + } + + fmt.Printf("Registered %d tools via %d plugins\n", len(toolRegistry.List()), len(pluginRegistry.List())) + + ag := agent.NewAgent(cfg, llmProvider, toolRegistry, store) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + sigChan := make(chan os.Signal, 1) + signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) + + fmt.Println("Agent daemon is running. Waiting for goals...") + fmt.Println("Press Ctrl+C to stop") + + ticker := time.NewTicker(5 * time.Second) + defer ticker.Stop() + + var active sync.WaitGroup + processing := make(map[string]struct{}) + var processingMu sync.Mutex + + processGoal := func(goalID string) { + processingMu.Lock() + if _, exists := processing[goalID]; exists { + processingMu.Unlock() + return + } + processing[goalID] = struct{}{} + processingMu.Unlock() + + active.Add(1) + go func() { + defer active.Done() + defer func() { + processingMu.Lock() + delete(processing, goalID) + processingMu.Unlock() + }() + + fmt.Printf("\n=== Processing Goal: %s ===\n", goalID) + if err := ag.ProcessGoal(ctx, goalID); err != nil { + if ctx.Err() != nil { + log.Printf("Goal %s interrupted: %v", goalID, err) + return + } + log.Printf("Error processing goal %s: %v", goalID, err) + } else { + fmt.Printf("=== Goal %s completed ===\n", goalID) + } + }() + } + + pollGoals := func() { + goals, err := store.ListGoals() + if err != nil { + log.Printf("Error listing goals: %v", err) + return + } + for _, goal := range goals { + if isProcessableState(goal.State) { + processGoal(goal.ID) + } + } + } + + pollGoals() + + for { + select { + case <-sigChan: + fmt.Println("\nShutting down...") + cancel() + done := make(chan struct{}) + go func() { + active.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(10 * time.Second): + log.Println("Shutdown timeout; exiting with goals still running") + } + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer shutdownCancel() + _ = pluginRegistry.Shutdown(shutdownCtx) + if browserServer != nil { + _ = browserServer.Stop(shutdownCtx) + } + return + + case <-ticker.C: + pollGoals() + } + } +} diff --git a/config.example.yaml b/config.example.yaml index d2c6898..b699c6a 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -15,6 +15,8 @@ llm: safety: allow_paths: - "/home/user/Projects" + # Optional HTTP host allowlist for git clone and http tool (empty = public hosts only, private IPs blocked) + allow_http_hosts: [] deny_commands: - "rm -rf /" - ":(){ :|:& };:" @@ -54,3 +56,10 @@ engine: max_retries: 3 enable_replan: true enable_parallel: true + +browser: + enabled: false + port: 8765 + # token is required when enabled; daemon generates one if empty + token: "" + browser_domains: [] diff --git a/config.yaml b/config.yaml deleted file mode 100644 index cfebe0c..0000000 --- a/config.yaml +++ /dev/null @@ -1,35 +0,0 @@ -projects_root: "/home/mparvin/MyGit/MadProjects" - -llm: - # Provider options: "ollama", "openai", "claude" - provider: "ollama" - model: "deepseek-coder-v2:16b" - base_url: "http://localhost:11434" - # For OpenAI: set OPENAI_API_KEY env var or uncomment: - # api_key: "sk-..." - # For Claude: set ANTHROPIC_API_KEY env var or uncomment: - # api_key: "sk-ant-..." - temperature: 0.3 - max_tokens: 4096 - -safety: - allow_paths: - - "/home/mparvin/MyGit/MadProjects" - deny_commands: - - "rm -rf /" - - ":(){ :|:& };:" - - "mkfs" - - "dd if=/dev/zero" - require_confirmation_for: - - "apt upgrade" - - "dnf upgrade" - - "yum upgrade" - -ssh: - default_port: 22 - known_hosts_file: "~/.ssh/known_hosts" - default_key_path: "~/.ssh/id_rsa" - -storage: - type: "sqlite" - path: "~/.config/octaai/state.db" diff --git a/go.mod b/go.mod index db6fafb..d7b7e0f 100644 --- a/go.mod +++ b/go.mod @@ -3,14 +3,14 @@ module github.com/mparvin/octaai go 1.23 require ( + github.com/google/uuid v1.5.0 + github.com/gorilla/websocket v1.5.1 github.com/mattn/go-sqlite3 v1.14.22 golang.org/x/crypto v0.25.0 gopkg.in/yaml.v3 v3.0.1 ) require ( - github.com/google/uuid v1.5.0 // indirect - github.com/gorilla/websocket v1.5.1 // indirect golang.org/x/net v0.21.0 // indirect golang.org/x/sys v0.22.0 // indirect ) diff --git a/pkg/browser/server.go b/pkg/browser/server.go index 47de6c2..6368fbc 100644 --- a/pkg/browser/server.go +++ b/pkg/browser/server.go @@ -6,6 +6,7 @@ import ( "fmt" "log" "net/http" + "strings" "sync" "time" @@ -24,7 +25,29 @@ type Server struct { server *http.Server } -// NewServer creates a new WebSocket server for browser connections +// allowedOrigin reports whether a WebSocket Origin header is permitted. +func allowedOrigin(origin string) bool { + if origin == "" { + return true + } + allowedPrefixes := []string{ + "moz-extension://", + "chrome-extension://", + "http://localhost", + "http://127.0.0.1", + "https://localhost", + "https://127.0.0.1", + } + for _, prefix := range allowedPrefixes { + if strings.HasPrefix(origin, prefix) { + return true + } + } + return false +} + +// NewServer creates a new WebSocket server for browser connections. +// token must be non-empty; callers should generate one before starting the server. func NewServer(addr string, token string) *Server { s := &Server{ addr: addr, @@ -32,8 +55,7 @@ func NewServer(addr string, token string) *Server { clients: make(map[string]*Client), upgrader: websocket.Upgrader{ CheckOrigin: func(r *http.Request) bool { - // Allow connections from browser extensions - return true + return allowedOrigin(r.Header.Get("Origin")) }, }, } @@ -52,6 +74,9 @@ func NewServer(addr string, token string) *Server { // Start starts the WebSocket server func (s *Server) Start() error { + if s.token == "" { + return fmt.Errorf("browser WebSocket token is required") + } log.Printf("Starting browser WebSocket server on %s", s.addr) return s.server.ListenAndServe() } @@ -60,7 +85,6 @@ func (s *Server) Start() error { func (s *Server) Stop(ctx context.Context) error { log.Println("Stopping browser WebSocket server...") - // Close all client connections s.clientsMu.Lock() for _, client := range s.clients { client.Close() @@ -72,22 +96,19 @@ func (s *Server) Stop(ctx context.Context) error { // handleWebSocket handles WebSocket connection requests func (s *Server) handleWebSocket(w http.ResponseWriter, r *http.Request) { - // Check authentication token authToken := r.URL.Query().Get("token") - if s.token != "" && authToken != s.token { + if authToken != s.token { http.Error(w, "Unauthorized", http.StatusUnauthorized) - log.Printf("Rejected connection: invalid token") + log.Printf("Rejected connection: invalid or missing token") return } - // Upgrade connection to WebSocket conn, err := s.upgrader.Upgrade(w, r, nil) if err != nil { log.Printf("Failed to upgrade connection: %v", err) return } - // Create new client clientID := uuid.New().String() client := NewClient(clientID, conn) @@ -97,7 +118,6 @@ func (s *Server) handleWebSocket(w http.ResponseWriter, r *http.Request) { log.Printf("Browser connected: %s (total: %d)", clientID, len(s.clients)) - // Handle client connection go s.handleClient(client) } @@ -111,10 +131,8 @@ func (s *Server) handleClient(client *Client) { log.Printf("Browser disconnected: %s (remaining: %d)", client.ID, len(s.clients)) }() - // Start ping routine go client.pingRoutine() - // Read messages from browser for { var response BrowserResponse err := client.conn.ReadJSON(&response) @@ -125,7 +143,6 @@ func (s *Server) handleClient(client *Client) { break } - // Route response to waiting command client.mu.RLock() respChan, exists := client.pendingCmds[response.ID] client.mu.RUnlock() @@ -154,7 +171,7 @@ func (s *Server) handleHealth(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(status) + _ = json.NewEncoder(w).Encode(status) } // SendCommand sends a command to a specific browser client @@ -179,7 +196,6 @@ func (s *Server) SendCommandToAny(cmd *BrowserCommand, timeout time.Duration) (* return nil, fmt.Errorf("no browser connected") } - // Get first available client for _, client := range s.clients { return client.SendCommand(cmd, timeout) } diff --git a/pkg/config/browser.go b/pkg/config/browser.go new file mode 100644 index 0000000..c8818d7 --- /dev/null +++ b/pkg/config/browser.go @@ -0,0 +1,22 @@ +package config + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "log" +) + +// EnsureBrowserToken generates a random WebSocket token when browser automation is enabled. +func EnsureBrowserToken(cfg *Config) (string, error) { + if cfg.Browser.Token != "" { + return cfg.Browser.Token, nil + } + buf := make([]byte, 32) + if _, err := rand.Read(buf); err != nil { + return "", fmt.Errorf("failed to generate browser token: %w", err) + } + cfg.Browser.Token = hex.EncodeToString(buf) + log.Printf("Generated browser WebSocket token; add to config: browser.token: %q", cfg.Browser.Token) + return cfg.Browser.Token, nil +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 2c50072..0c0d80c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "gopkg.in/yaml.v3" ) @@ -33,6 +34,7 @@ type LLMConfig struct { // SafetyConfig defines safety constraints type SafetyConfig struct { AllowPaths []string `yaml:"allow_paths"` + AllowHTTPHosts []string `yaml:"allow_http_hosts"` DenyCommands []string `yaml:"deny_commands"` RequireConfirmationFor []string `yaml:"require_confirmation_for"` } @@ -172,11 +174,14 @@ func LoadConfig(path string) (*Config, error) { return nil, fmt.Errorf("failed to parse config file: %w", err) } - cfg.ProjectsRoot = os.ExpandEnv(cfg.ProjectsRoot) - cfg.SSH.KnownHostsFile = os.ExpandEnv(cfg.SSH.KnownHostsFile) - cfg.SSH.DefaultKeyPath = os.ExpandEnv(cfg.SSH.DefaultKeyPath) - cfg.Storage.Path = os.ExpandEnv(cfg.Storage.Path) - cfg.Isolation.Docker.WorkdirMount = os.ExpandEnv(cfg.Isolation.Docker.WorkdirMount) + cfg.ProjectsRoot = ExpandPath(cfg.ProjectsRoot) + for i, p := range cfg.Safety.AllowPaths { + cfg.Safety.AllowPaths[i] = ExpandPath(p) + } + cfg.SSH.KnownHostsFile = ExpandPath(cfg.SSH.KnownHostsFile) + cfg.SSH.DefaultKeyPath = ExpandPath(cfg.SSH.DefaultKeyPath) + cfg.Storage.Path = ExpandPath(cfg.Storage.Path) + cfg.Isolation.Docker.WorkdirMount = ExpandPath(cfg.Isolation.Docker.WorkdirMount) if cfg.Isolation.Docker.WorkdirMount == "" { cfg.Isolation.Docker.WorkdirMount = cfg.ProjectsRoot @@ -215,6 +220,39 @@ func SaveConfig(cfg *Config, path string) error { return nil } +// ExpandPath expands environment variables and a leading ~ using the HOME variable. +func ExpandPath(path string) string { + path = os.ExpandEnv(path) + if path == "" { + return path + } + + home := os.Getenv("HOME") + if home == "" { + home, _ = os.UserHomeDir() + } + if home == "" { + return path + } + + if path == "~" { + return home + } + if strings.HasPrefix(path, "~/") { + return filepath.Join(home, path[2:]) + } + return path +} + +// ResolveProjectPath expands ~ and env vars, then resolves relative paths against projects_root. +func ResolveProjectPath(cfg *Config, path string) string { + path = ExpandPath(path) + if filepath.IsAbs(path) { + return path + } + return filepath.Join(cfg.ProjectsRoot, path) +} + // ConfigPath returns the default configuration file path func ConfigPath() string { homeDir, _ := os.UserHomeDir() diff --git a/pkg/config/path_test.go b/pkg/config/path_test.go new file mode 100644 index 0000000..4c59132 --- /dev/null +++ b/pkg/config/path_test.go @@ -0,0 +1,92 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestExpandPath(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Skip("no home directory") + } + + tests := []struct { + in, want string + }{ + {"~", home}, + {"~/Projects", filepath.Join(home, "Projects")}, + {"/abs/path", "/abs/path"}, + } + + for _, tc := range tests { + got := ExpandPath(tc.in) + if got != tc.want { + t.Errorf("ExpandPath(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestResolveProjectPath(t *testing.T) { + root := "/home/user/projects" + cfg := &Config{ProjectsRoot: root} + + got := ResolveProjectPath(cfg, "myapp/main.py") + want := filepath.Join(root, "myapp/main.py") + if got != want { + t.Errorf("ResolveProjectPath() = %q, want %q", got, want) + } + + got = ResolveProjectPath(cfg, "/abs/outside.py") + if got != "/abs/outside.py" { + t.Errorf("absolute path = %q, want /abs/outside.py", got) + } +} + +func TestLoadConfigExpandsTildePaths(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Skip("no home directory") + } + + cfg := DefaultConfig() + cfg.Storage.Path = "~/.config/octaai/state.db" + cfg.Safety.AllowPaths = []string{"~/Projects"} + + cfg.ProjectsRoot = ExpandPath(cfg.ProjectsRoot) + for i, p := range cfg.Safety.AllowPaths { + cfg.Safety.AllowPaths[i] = ExpandPath(p) + } + cfg.Storage.Path = ExpandPath(cfg.Storage.Path) + + wantStorage := filepath.Join(home, ".config", "octaai", "state.db") + if cfg.Storage.Path != wantStorage { + t.Errorf("storage path = %q, want %q", cfg.Storage.Path, wantStorage) + } + if cfg.Safety.AllowPaths[0] != filepath.Join(home, "Projects") { + t.Errorf("allow path = %q", cfg.Safety.AllowPaths[0]) + } +} + +func TestRelativePathAllowedUnderProjectsRoot(t *testing.T) { + cfg := &Config{ + ProjectsRoot: "/home/user/MadProjects", + Safety: SafetyConfig{ + AllowPaths: []string{"/home/user/MadProjects"}, + }, + } + + full := ResolveProjectPath(cfg, "project/app.py") + absPath, err := filepath.Abs(full) + if err != nil { + t.Fatal(err) + } + + allowedAbs, _ := filepath.Abs(cfg.Safety.AllowPaths[0]) + allowed := absPath == allowedAbs || strings.HasPrefix(absPath, allowedAbs+string(filepath.Separator)) + if !allowed { + t.Errorf("project/app.py should be allowed under projects root, got %q", absPath) + } +} diff --git a/pkg/engine/checkpoint.go b/pkg/engine/checkpoint.go index a267317..86023ca 100644 --- a/pkg/engine/checkpoint.go +++ b/pkg/engine/checkpoint.go @@ -17,10 +17,10 @@ type Checkpoint struct { // CheckpointPayload holds serialized execution context. type CheckpointPayload struct { - GoalState GoalState `json:"goal_state"` - StepIDs []string `json:"step_ids"` + GoalState GoalState `json:"goal_state"` + StepIDs []string `json:"step_ids"` Memory map[string]string `json:"memory,omitempty"` - PlanVersion int `json:"plan_version"` + PlanVersion int `json:"plan_version"` } // NewCheckpoint creates a checkpoint from current engine state. diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 7ece8a2..506ec91 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -70,7 +70,7 @@ func NewEngine( mem := memory.NewManager(store) logger := observability.NewLogger(store) perm := permission.NewManager(cfg, store) - pl := planner.NewLLMPlanner(llmProvider, toolRegistry, mem) + pl := planner.NewLLMPlanner(llmProvider, toolRegistry, mem, cfg.Engine.MaxRetries) ecfg := DefaultEngineConfig() if cfg.Engine.MaxLoops > 0 { @@ -110,6 +110,13 @@ func (e *Engine) ProcessGoal(ctx context.Context, goalID string) error { return fmt.Errorf("failed to get goal: %w", err) } + switch goal.State { + case StateCompleted, StateFailed: + return nil + case StateWaitingForApproval, StateBlocked: + return fmt.Errorf("goal %s is in state %s and cannot be processed", goalID, goal.State) + } + e.logger.RecordTimeline(observability.TimelineEvent{ Timestamp: time.Now(), GoalID: goalID, @@ -117,12 +124,24 @@ func (e *Engine) ProcessGoal(ctx context.Context, goalID string) error { Detail: goal.Description, }) - if err := e.transitionGoal(goal, StatePlanning); err != nil { - return err + existingTasks, _ := e.store.GetTasksByGoal(goalID) + for i := range existingTasks { + if existingTasks[i].Status == "running" { + existingTasks[i].Status = "pending" + existingTasks[i].UpdatedAt = time.Now() + _ = e.store.UpdateTask(&existingTasks[i]) + } } - existingTasks, _ := e.store.GetTasksByGoal(goalID) if len(existingTasks) == 0 { + if goal.State == StateExecuting || goal.State == StateEvaluating { + return e.failGoal(goal, fmt.Errorf("no tasks found for goal in state %s", goal.State)) + } + if goal.State != StatePlanning { + if err := e.transitionGoal(goal, StatePlanning); err != nil { + return err + } + } memCtx := e.memory.SearchContext(goalID, goal.Description, 10) plan, err := e.planner.Plan(ctx, goal, memCtx) if err != nil { @@ -138,9 +157,28 @@ func (e *Engine) ProcessGoal(ctx context.Context, goalID string) error { e.logger.Info(goalID, fmt.Sprintf("Resuming with %d existing task(s)", len(existingTasks)), nil) } - if err := e.transitionGoal(goal, StateExecuting); err != nil { - return err + if goal.State == StateEvaluating { + retry, err := e.runGoalEvaluation(ctx, goal) + if err != nil { + return err + } + if retry { + e.loopCount = 0 + goto executionLoop + } + return nil + } + + if goal.State == StateRetrying { + if err := e.transitionGoal(goal, StateExecuting); err != nil { + return err + } + } else if goal.State != StateExecuting { + if err := e.transitionGoal(goal, StateExecuting); err != nil { + return err + } } + e.persistCheckpoint(goalID, goal.State, e.loopCount) executionLoop: for e.loopCount < e.engineCfg.MaxLoops { @@ -177,6 +215,8 @@ executionLoop: e.logger.Error(goalID, fmt.Sprintf("Task execution error: %v", err), nil) } + e.persistCheckpoint(goalID, goal.State, e.loopCount) + select { case <-ctx.Done(): return ctx.Err() @@ -188,9 +228,25 @@ executionLoop: return e.failGoal(goal, fmt.Errorf("exceeded maximum loop count")) } - if err := e.transitionGoal(goal, StateEvaluating); err != nil { + retry, err := e.runGoalEvaluation(ctx, goal) + if err != nil { return err } + if retry { + e.loopCount = 0 + goto executionLoop + } + return nil +} + +func (e *Engine) runGoalEvaluation(ctx context.Context, goal *storage.Goal) (bool, error) { + goalID := goal.ID + if goal.State != StateEvaluating { + if err := e.transitionGoal(goal, StateEvaluating); err != nil { + return false, err + } + } + e.persistCheckpoint(goalID, goal.State, e.loopCount) steps, _ := e.store.GetStepsByGoal(goalID) stepPtrs := make([]*execution.StepView, len(steps)) @@ -200,14 +256,14 @@ executionLoop: verdict, err := e.evaluator.EvaluateGoal(ctx, goal.Description, stepPtrs) if err != nil { - return e.failGoal(goal, err) + return false, e.failGoal(goal, err) } switch verdict.Result { case evaluator.RetryRequired: if e.engineCfg.EnableReplan { if err := e.transitionGoal(goal, StateRetrying); err != nil { - return err + return false, err } tasks, _ := e.store.GetTasksByGoal(goalID) newTasks, replanErr := e.planner.Replan(ctx, goal, verdict.Detail, e.memory.SearchContext(goalID, goal.Description, 5), tasks) @@ -217,16 +273,15 @@ executionLoop: } e.logger.Info(goalID, fmt.Sprintf("Replanned with %d new task(s)", len(newTasks)), nil) if err := e.transitionGoal(goal, StateExecuting); err != nil { - return err + return false, err } - e.loopCount = 0 - goto executionLoop + return true, nil } e.logger.Warn(goalID, fmt.Sprintf("Replan failed: %v", replanErr), nil) } - return e.failGoal(goal, fmt.Errorf("retry required: %s", verdict.Detail)) + return false, e.failGoal(goal, fmt.Errorf("retry required: %s", verdict.Detail)) case evaluator.Fatal: - return e.failGoal(goal, fmt.Errorf("evaluation failed: %s", verdict.Detail)) + return false, e.failGoal(goal, fmt.Errorf("evaluation failed: %s", verdict.Detail)) } now := time.Now() @@ -240,7 +295,13 @@ executionLoop: Event: "goal_completed", Detail: goal.Result, }) - return e.store.UpdateGoal(goal) + return false, e.store.UpdateGoal(goal) +} + +func (e *Engine) persistCheckpoint(goalID string, state GoalState, stepIndex int) { + if _, err := e.SaveCheckpoint(goalID, state, stepIndex); err != nil { + e.logger.Warn(goalID, fmt.Sprintf("checkpoint save failed: %v", err), nil) + } } func (e *Engine) transitionGoal(goal *storage.Goal, to GoalState) error { @@ -253,7 +314,7 @@ func (e *Engine) transitionGoal(goal *storage.Goal, to GoalState) error { Timestamp: time.Now(), GoalID: goal.ID, Event: "state_transition", - Detail: fmt.Sprintf("%s", to), + Detail: string(to), }) return e.store.UpdateGoal(goal) } @@ -378,14 +439,18 @@ func (e *Engine) executeToolTask(ctx context.Context, goal *storage.Goal, task * return e.handleStepFailure(task, step, output, err) } - verdict, _ := e.evaluator.Evaluate(ctx, step.View()) + return e.finalizeStepExecution(ctx, goal, task, step, output, nil) +} + +func (e *Engine) finalizeStepExecution(ctx context.Context, goal *storage.Goal, task *storage.Task, step *ExecutionStep, output *StepOutput, resultOverride *string) error { stepOutput := output if stepOutput == nil { stepOutput = &StepOutput{} } + + verdict, _ := e.evaluator.Evaluate(ctx, step.ViewWithOutput(stepOutput)) step.MarkCompleted(stepOutput, ValidationResult(verdict.Result), verdict.Detail) _ = e.store.UpdateStep(stepToStorage(step)) - e.memory.RecordStepOutcome(goal.ID, step.ID, task.Description, stepOutput.Output, stepOutput.Success) if verdict.Result == evaluator.RetryRequired && task.Attempts < task.MaxAttempts { @@ -399,7 +464,12 @@ func (e *Engine) executeToolTask(ctx context.Context, goal *storage.Goal, task * } task.Status = "completed" - task.Result = stepOutput.Output + if resultOverride != nil { + task.Result = *resultOverride + } else { + task.Result = stepOutput.Output + } + task.Error = "" task.UpdatedAt = time.Now() return e.store.UpdateTask(task) } @@ -459,28 +529,7 @@ func (e *Engine) executeLLMTask(ctx context.Context, goal *storage.Goal, task *s return e.handleStepFailure(task, step, output, err) } - verdict, _ := e.evaluator.Evaluate(ctx, step.View()) - if output == nil { - output = &StepOutput{} - } - step.MarkCompleted(output, ValidationResult(verdict.Result), verdict.Detail) - _ = e.store.UpdateStep(stepToStorage(step)) - e.memory.RecordStepOutcome(goal.ID, step.ID, task.Description, output.Output, output.Success) - - if verdict.Result == evaluator.RetryRequired && task.Attempts < task.MaxAttempts { - task.Status = "pending" - task.Error = verdict.Detail - task.UpdatedAt = time.Now() - return e.store.UpdateTask(task) - } - if verdict.Result == evaluator.Fatal || !output.Success { - return e.failTask(task, fmt.Errorf("%s", verdict.Detail)) - } - - task.Result = output.Output - task.Status = "completed" - task.UpdatedAt = time.Now() - return e.store.UpdateTask(task) + return e.finalizeStepExecution(ctx, goal, task, step, output, nil) } func (e *Engine) handleStepFailure(task *storage.Task, step *ExecutionStep, output *StepOutput, err error) error { diff --git a/pkg/engine/prompts.go b/pkg/engine/prompts.go index 12e4a95..2a4b7a1 100644 --- a/pkg/engine/prompts.go +++ b/pkg/engine/prompts.go @@ -9,7 +9,7 @@ import ( ) func (e *Engine) getSystemPrompt() string { - return `You are an autonomous software engineering agent. You can: + return fmt.Sprintf(`You are an autonomous software engineering agent. You can: - Create and modify files - Run commands and programs - Use Git operations @@ -17,8 +17,23 @@ func (e *Engine) getSystemPrompt() string { - Make HTTP requests - Control a browser via the browser tool +All file paths and command working directories are relative to the projects root: %s +Do not use ~, $HOME, or absolute paths outside the projects root. +Create a subdirectory for each new project (e.g. "my-project/script.py", cwd "my-project"). + +When the goal asks to develop, create, or write a program/script/application, use the filesystem +tool to write complete source code files. Do NOT use the http tool to call external APIs instead +of writing code — implement the logic in source files the user can run locally. + Always respond with specific, actionable tool calls. -When executing, choose the right tool and provide complete arguments.` +When executing, choose the right tool and provide complete arguments.`, e.cfg.ProjectsRoot) +} + +func projectContext(goal *storage.Goal) string { + if goal.ProjectName == "" { + return "" + } + return fmt.Sprintf("\nProject directory: %s (create all files under this subdirectory)\n", goal.ProjectName) } func (e *Engine) buildTaskExecutionPrompt(goal *storage.Goal, task *storage.Task) string { @@ -33,22 +48,25 @@ func (e *Engine) buildTaskExecutionPrompt(goal *storage.Goal, task *storage.Task return fmt.Sprintf(`Goal: %s Task: %s +Projects root: %s%s %s Tools:%s INSTRUCTIONS: 1. Analyze the task and choose the appropriate tool -2. For creating files, use "filesystem" with action "write_file", providing the full file content -3. You MUST respond with ONLY a JSON object - no explanations, no markdown -4. The JSON MUST have exactly two keys: "tool" and "args" +2. For creating/writing programs or scripts, use "filesystem" with action "write_file" and provide complete source code +3. Do NOT use the "http" tool when the goal is to develop code — write the source files instead +4. All paths are relative to the projects root above (e.g. "my-project/main.py", not "~/" or absolute paths) +5. You MUST respond with ONLY a JSON object - no explanations, no markdown +6. The JSON MUST have exactly two keys: "tool" and "args" -EXAMPLE - Creating a file: -{"tool": "filesystem", "args": {"action": "write_file", "path": "project/app.py", "content": "print('hello')"}} +EXAMPLE - Creating a Python script: +{"tool": "filesystem", "args": {"action": "write_file", "path": "github-list/main.py", "content": "#!/usr/bin/env python3\\nimport sys\\n..."}} EXAMPLE - Running a command: -{"tool": "command", "args": {"command": "ls -la", "cwd": "project"}} +{"tool": "command", "args": {"command": "python main.py octocat", "cwd": "github-list"}} -Your JSON response:`, goal.Description, task.Description, contextInfo, toolsList) +Your JSON response:`, goal.Description, task.Description, e.cfg.ProjectsRoot, projectContext(goal), contextInfo, toolsList) } func formatToolSchemas(registry *tools.Registry) string { diff --git a/pkg/engine/runner.go b/pkg/engine/runner.go index a449542..82ce882 100644 --- a/pkg/engine/runner.go +++ b/pkg/engine/runner.go @@ -3,6 +3,7 @@ package engine import ( "context" "fmt" + "os/exec" "sync" "time" @@ -45,15 +46,7 @@ func NewRunner(cfg *config.Config, registry *tools.Registry, perm *permission.Ma func (r *Runner) Run(ctx context.Context, toolName string, args map[string]interface{}, cfg RunConfig) (*StepOutput, error) { span := r.logger.StartSpan(cfg.GoalID, fmt.Sprintf("tool:%s", toolName), observability.Fields{"step_id": cfg.StepID}) - execArgs := args - if wrapped, ok, err := r.docker.WrapArgs(toolName, args); err != nil { - r.logger.EndSpan(span) - return nil, err - } else if ok { - execArgs = wrapped - } - - check := r.permission.CheckTool(cfg.GoalID, cfg.TaskID, toolName, execArgs) + check := r.permission.CheckTool(cfg.GoalID, cfg.TaskID, toolName, args) switch check.Decision { case permission.DecisionDeny: r.logger.EndSpan(span) @@ -63,10 +56,12 @@ func (r *Runner) Run(ctx context.Context, toolName string, args map[string]inter return nil, fmt.Errorf("approval_required: %s", check.Reason) } - tool, ok := r.tools.Get(toolName) - if !ok { + execArgs := args + if wrapped, ok, err := r.docker.WrapArgs(toolName, args); err != nil { r.logger.EndSpan(span) - return nil, fmt.Errorf("tool not found: %s", toolName) + return nil, err + } else if ok { + execArgs = wrapped } runCtx := ctx @@ -76,7 +71,19 @@ func (r *Runner) Run(ctx context.Context, toolName string, args map[string]inter defer cancel() } - result, err := tool.Execute(runCtx, execArgs) + var result *tools.ToolResult + var err error + + if argv, ok := dockerArgv(execArgs); ok { + result, err = r.runDockerArgv(runCtx, argv) + } else { + tool, found := r.tools.Get(toolName) + if !found { + r.logger.EndSpan(span) + return nil, fmt.Errorf("tool not found: %s", toolName) + } + result, err = tool.Execute(runCtx, execArgs) + } r.logger.EndSpan(span) if err != nil { @@ -90,6 +97,11 @@ func (r *Runner) Run(ctx context.Context, toolName string, args map[string]inter } if data, ok := result.Data.(map[string]interface{}); ok { out.Data = data + } else if result.Data != nil { + r.logger.Warn(cfg.GoalID, "tool returned non-map Data payload", observability.Fields{ + "tool": toolName, + "step": cfg.StepID, + }) } if execArgs["_isolated"] == true { if out.Data == nil { @@ -101,6 +113,57 @@ func (r *Runner) Run(ctx context.Context, toolName string, args map[string]inter return out, nil } +func dockerArgv(args map[string]interface{}) ([]string, bool) { + raw, ok := args["_docker_argv"] + if !ok { + return nil, false + } + switch v := raw.(type) { + case []string: + return v, true + case []interface{}: + argv := make([]string, 0, len(v)) + for _, item := range v { + if s, ok := item.(string); ok { + argv = append(argv, s) + } + } + if len(argv) > 0 { + return argv, true + } + } + return nil, false +} + +func (r *Runner) runDockerArgv(ctx context.Context, argv []string) (*tools.ToolResult, error) { + if len(argv) == 0 { + return nil, fmt.Errorf("empty docker argv") + } + cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) + output, err := cmd.CombinedOutput() + + exitCode := 0 + if err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + exitCode = exitErr.ExitCode() + } else { + return &tools.ToolResult{ + Success: false, + Error: err.Error(), + }, nil + } + } + + return &tools.ToolResult{ + Success: exitCode == 0, + Output: string(output), + Data: map[string]interface{}{ + "exit_code": exitCode, + "isolated": true, + }, + }, nil +} + // RunParallel executes multiple tool calls concurrently with a limit. func (r *Runner) RunParallel(ctx context.Context, calls []parallelCall, maxParallel int) ([]parallelResult, error) { if maxParallel <= 0 { diff --git a/pkg/engine/step.go b/pkg/engine/step.go index c6f1e4c..84e65b8 100644 --- a/pkg/engine/step.go +++ b/pkg/engine/step.go @@ -128,3 +128,14 @@ func (s *ExecutionStep) View() *execution.StepView { } return view } + +// ViewWithOutput returns a snapshot including runtime output not yet stored on the step. +func (s *ExecutionStep) ViewWithOutput(output *StepOutput) *execution.StepView { + view := s.View() + if output != nil { + view.Output = output.Output + view.Error = output.Error + view.Success = output.Success + } + return view +} diff --git a/pkg/engine/step_test.go b/pkg/engine/step_test.go new file mode 100644 index 0000000..17e9d9f --- /dev/null +++ b/pkg/engine/step_test.go @@ -0,0 +1,25 @@ +package engine + +import ( + "testing" +) + +func TestViewWithOutputIncludesRuntimeResult(t *testing.T) { + step := NewExecutionStep("step_1", "goal_1", "write file", StepInput{ToolName: "filesystem"}) + view := step.ViewWithOutput(&StepOutput{ + Success: true, + Output: "Wrote 20 bytes", + }) + + if !view.Success { + t.Fatal("expected Success=true in view") + } + if view.Output != "Wrote 20 bytes" { + t.Fatalf("expected output in view, got %q", view.Output) + } + + emptyView := step.View() + if emptyView.Success || emptyView.Output != "" { + t.Fatal("View() without stored output should not report success") + } +} diff --git a/pkg/evaluator/evaluator.go b/pkg/evaluator/evaluator.go index 2f47e69..b01429f 100644 --- a/pkg/evaluator/evaluator.go +++ b/pkg/evaluator/evaluator.go @@ -28,8 +28,8 @@ type Evaluator interface { // CompositeEvaluator runs multiple evaluators in sequence. type CompositeEvaluator struct { - evaluators []Evaluator - goalEvaluators []GoalEvaluator + evaluators []Evaluator + goalEvaluators []GoalEvaluator } // NewComposite creates an evaluator chain. diff --git a/pkg/evaluator/evaluator_test.go b/pkg/evaluator/evaluator_test.go new file mode 100644 index 0000000..bcd9fde --- /dev/null +++ b/pkg/evaluator/evaluator_test.go @@ -0,0 +1,40 @@ +package evaluator + +import ( + "context" + "testing" + + "github.com/mparvin/octaai/pkg/execution" +) + +func TestToolResultEvaluatorSuccessfulStep(t *testing.T) { + ev := &ToolResultEvaluator{} + step := &execution.StepView{ + Success: true, + Output: "Wrote 20 bytes to: /tmp/foo.py", + ToolName: "filesystem", + } + + verdict, err := ev.Evaluate(context.Background(), step) + if err != nil { + t.Fatal(err) + } + if verdict.Result != Success { + t.Fatalf("expected success, got %q (%s)", verdict.Result, verdict.Detail) + } +} + +func TestToolResultEvaluatorEmptyUnsuccessfulStep(t *testing.T) { + ev := &ToolResultEvaluator{} + step := &execution.StepView{ + Success: false, + } + + verdict, err := ev.Evaluate(context.Background(), step) + if err != nil { + t.Fatal(err) + } + if verdict.Result != RetryRequired || verdict.Detail != "no output recorded" { + t.Fatalf("expected retry for empty step, got %q (%s)", verdict.Result, verdict.Detail) + } +} diff --git a/pkg/isolation/docker.go b/pkg/isolation/docker.go index 36ec161..50e7c78 100644 --- a/pkg/isolation/docker.go +++ b/pkg/isolation/docker.go @@ -3,7 +3,7 @@ package isolation import ( "fmt" "os/exec" - "strings" + "path/filepath" "github.com/mparvin/octaai/pkg/config" ) @@ -40,7 +40,7 @@ func (d *Docker) ShouldIsolate(toolName string) bool { return false } -// WrapCommand builds a docker run invocation for a shell command. +// WrapCommand builds a docker run invocation argv for a shell command. func (d *Docker) WrapCommand(cwd, command string) ([]string, error) { if command == "" { return nil, fmt.Errorf("empty command") @@ -54,7 +54,10 @@ func (d *Docker) WrapCommand(cwd, command string) ([]string, error) { containerWorkdir := "/workspace" if cwd != "" { - containerWorkdir = "/workspace/" + strings.TrimPrefix(strings.TrimPrefix(cwd, mount), "/") + resolved := config.ResolveProjectPath(d.cfg, cwd) + if rel, err := filepath.Rel(mount, resolved); err == nil && rel != "." { + containerWorkdir = "/workspace/" + filepath.ToSlash(rel) + } } args := []string{"run", "--rm"} @@ -78,7 +81,7 @@ func (d *Docker) WrapCommand(cwd, command string) ([]string, error) { return append([]string{"docker"}, args...), nil } -// WrapArgs rewrites command tool args to execute inside Docker when applicable. +// WrapArgs annotates command tool args for Docker isolation without destroying argv boundaries. func (d *Docker) WrapArgs(toolName string, args map[string]interface{}) (map[string]interface{}, bool, error) { if !d.ShouldIsolate(toolName) { return args, false, nil @@ -95,12 +98,11 @@ func (d *Docker) WrapArgs(toolName string, args map[string]interface{}) (map[str return nil, false, err } - wrapped := make(map[string]interface{}, len(args)) + wrapped := make(map[string]interface{}, len(args)+2) for k, v := range args { wrapped[k] = v } - wrapped["command"] = strings.Join(dockerCmd, " ") - wrapped["cwd"] = d.cfg.ProjectsRoot + wrapped["_docker_argv"] = dockerCmd wrapped["_isolated"] = true return wrapped, true, nil diff --git a/pkg/isolation/docker_test.go b/pkg/isolation/docker_test.go index 5f70208..7a28b59 100644 --- a/pkg/isolation/docker_test.go +++ b/pkg/isolation/docker_test.go @@ -46,6 +46,30 @@ func TestShouldIsolate(t *testing.T) { } } +func TestWrapArgsPreservesArgv(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Isolation.Enabled = true + cfg.Isolation.Docker.Enabled = true + + d := NewDocker(cfg) + args := map[string]interface{}{ + "cwd": "myapp", + "command": "go test ./...", + } + wrapped, ok, err := d.WrapArgs("command", args) + if err != nil || !ok { + t.Fatalf("expected wrapped args, got ok=%v err=%v", ok, err) + } + argv, exists := wrapped["_docker_argv"].([]string) + if !exists || len(argv) == 0 || argv[0] != "docker" { + t.Fatalf("expected docker argv slice, got %v", wrapped["_docker_argv"]) + } + rewritten, ok := wrapped["command"].(string) + if !ok || rewritten != "go test ./..." { + t.Fatalf("expected original command preserved, got %v", wrapped["command"]) + } +} + func contains(s, sub string) bool { return len(s) >= len(sub) && (s == sub || len(sub) == 0 || indexOf(s, sub) >= 0) } diff --git a/pkg/permission/manager.go b/pkg/permission/manager.go index 49d2580..0b730e0 100644 --- a/pkg/permission/manager.go +++ b/pkg/permission/manager.go @@ -2,6 +2,7 @@ package permission import ( "fmt" + "path/filepath" "strings" "github.com/mparvin/octaai/pkg/approval" @@ -26,8 +27,8 @@ type CheckResult struct { // Manager enforces safety policies before tool execution. type Manager struct { - cfg *config.Config - store storage.Storage + cfg *config.Config + store storage.Storage approvals *approval.Service } @@ -45,8 +46,22 @@ func (m *Manager) CheckPath(path string) CheckResult { if len(m.cfg.Safety.AllowPaths) == 0 { return CheckResult{Decision: DecisionAllow} } + + fullPath := config.ResolveProjectPath(m.cfg, path) + absPath, err := filepath.Abs(fullPath) + if err != nil { + return CheckResult{ + Decision: DecisionDeny, + Reason: fmt.Sprintf("invalid path %q: %v", path, err), + } + } + for _, allowed := range m.cfg.Safety.AllowPaths { - if strings.HasPrefix(path, allowed) || strings.HasPrefix(allowed, path) { + allowedAbs, err := filepath.Abs(allowed) + if err != nil { + continue + } + if absPath == allowedAbs || strings.HasPrefix(absPath, allowedAbs+string(filepath.Separator)) { return CheckResult{Decision: DecisionAllow} } } @@ -58,8 +73,10 @@ func (m *Manager) CheckPath(path string) CheckResult { // CheckCommand verifies a shell command against deny/require lists. func (m *Manager) CheckCommand(command string) CheckResult { + normalized := NormalizeCommand(command) + for _, denied := range m.cfg.Safety.DenyCommands { - if strings.Contains(command, denied) { + if strings.Contains(normalized, NormalizeCommand(denied)) { return CheckResult{ Decision: DecisionDeny, Reason: fmt.Sprintf("command matches denied pattern: %q", denied), @@ -67,7 +84,7 @@ func (m *Manager) CheckCommand(command string) CheckResult { } } for _, confirm := range m.cfg.Safety.RequireConfirmationFor { - if strings.Contains(command, confirm) { + if strings.Contains(normalized, NormalizeCommand(confirm)) { return CheckResult{ Decision: DecisionRequireApproval, Reason: fmt.Sprintf("command requires approval: %q", confirm), @@ -88,19 +105,91 @@ func (m *Manager) CheckTool(goalID, taskID, toolName string, args map[string]int switch toolName { case "command": cmd, _ := args["command"].(string) - return m.CheckCommand(cmd) + if result := m.CheckCommand(cmd); result.Decision != DecisionAllow { + return result + } + if cwd, ok := args["cwd"].(string); ok && cwd != "" { + return m.CheckPath(cwd) + } + return CheckResult{Decision: DecisionAllow} + case "filesystem": path, _ := args["path"].(string) if path != "" { return m.CheckPath(path) } + + case "git": + path, _ := args["path"].(string) + if path == "" { + return CheckResult{Decision: DecisionDeny, Reason: "git path is required"} + } + if result := m.CheckPath(path); result.Decision != DecisionAllow { + return result + } + action, _ := args["action"].(string) + if action == "push" { + return CheckResult{ + Decision: DecisionRequireApproval, + Reason: "git push requires approval", + } + } + if action == "clone" { + url, _ := args["url"].(string) + if strings.HasPrefix(strings.TrimSpace(url), "-") { + return CheckResult{ + Decision: DecisionDeny, + Reason: "git clone URL cannot start with '-'", + } + } + if result := m.CheckURL(url); result.Decision != DecisionAllow { + return result + } + } + return CheckResult{Decision: DecisionAllow} + + case "http": + url, _ := args["url"].(string) + if url == "" { + return CheckResult{Decision: DecisionDeny, Reason: "http url is required"} + } + return m.CheckURL(url) + + case "browser": + action, _ := args["action"].(string) + if action == "execute" { + return CheckResult{ + Decision: DecisionRequireApproval, + Reason: "browser JavaScript execution requires approval", + } + } + if action == "screenshot" { + if outputPath, ok := args["output_path"].(string); ok && outputPath != "" { + return m.CheckPath(outputPath) + } + } + if action == "navigate" { + url, _ := args["url"].(string) + if url == "" { + return CheckResult{Decision: DecisionDeny, Reason: "browser navigate requires url"} + } + return m.CheckBrowserDomain(url) + } + return CheckResult{Decision: DecisionAllow} + case "ssh": + if localPath, ok := args["local_path"].(string); ok && localPath != "" { + if result := m.CheckPath(localPath); result.Decision != DecisionAllow { + return result + } + } return CheckResult{ Decision: DecisionRequireApproval, Reason: "remote SSH execution requires approval", } } - return CheckResult{Decision: DecisionAllow} + + return CheckResult{Decision: DecisionDeny, Reason: fmt.Sprintf("unknown tool: %s", toolName)} } // Approvals exposes the approval service. diff --git a/pkg/permission/manager_test.go b/pkg/permission/manager_test.go new file mode 100644 index 0000000..6d9937e --- /dev/null +++ b/pkg/permission/manager_test.go @@ -0,0 +1,113 @@ +package permission + +import ( + "testing" + + "github.com/mparvin/octaai/pkg/config" +) + +func testConfig() *config.Config { + cfg := config.DefaultConfig() + cfg.ProjectsRoot = "/home/user/Projects" + cfg.Safety.AllowPaths = []string{"/home/user/Projects"} + cfg.Safety.DenyCommands = []string{"rm -rf /"} + cfg.Safety.RequireConfirmationFor = []string{"apt upgrade"} + cfg.Browser.BrowserDomains = []string{"example.com"} + return cfg +} + +func TestNormalizeCommand(t *testing.T) { + if got := NormalizeCommand(" RM -rf / "); got != "rm -rf /" { + t.Fatalf("expected normalized command, got %q", got) + } +} + +func TestCheckCommandDenyBypass(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckCommand("RM -rf /") + if result.Decision != DecisionDeny { + t.Fatalf("expected deny, got %s", result.Decision) + } +} + +func TestCheckCommandCwd(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckTool("", "", "command", map[string]interface{}{ + "command": "ls", + "cwd": "/etc", + }) + if result.Decision != DecisionDeny { + t.Fatalf("expected deny for cwd outside allow_paths, got %s (%s)", result.Decision, result.Reason) + } +} + +func TestCheckGitPath(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckTool("", "", "git", map[string]interface{}{ + "action": "init", + "path": "/tmp/repo", + }) + if result.Decision != DecisionDeny { + t.Fatalf("expected deny for git path outside allow_paths, got %s", result.Decision) + } +} + +func TestCheckGitPushRequiresApproval(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckTool("", "", "git", map[string]interface{}{ + "action": "push", + "path": "myapp", + }) + if result.Decision != DecisionRequireApproval { + t.Fatalf("expected require_approval, got %s", result.Decision) + } +} + +func TestCheckHTTPBlocksLocalhost(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckTool("", "", "http", map[string]interface{}{ + "method": "GET", + "url": "http://localhost:8080/admin", + }) + if result.Decision != DecisionDeny { + t.Fatalf("expected deny for localhost SSRF, got %s", result.Decision) + } +} + +func TestCheckBrowserExecuteRequiresApproval(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckTool("", "", "browser", map[string]interface{}{ + "action": "execute", + "script": "alert(1)", + }) + if result.Decision != DecisionRequireApproval { + t.Fatalf("expected require_approval, got %s", result.Decision) + } +} + +func TestCheckBrowserDomain(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckTool("", "", "browser", map[string]interface{}{ + "action": "navigate", + "url": "https://evil.com/page", + }) + if result.Decision != DecisionDeny { + t.Fatalf("expected deny for disallowed browser domain, got %s", result.Decision) + } + + allowed := m.CheckTool("", "", "browser", map[string]interface{}{ + "action": "navigate", + "url": "https://www.example.com/page", + }) + if allowed.Decision != DecisionAllow { + t.Fatalf("expected allow for example.com, got %s (%s)", allowed.Decision, allowed.Reason) + } +} + +func TestUnknownToolDenied(t *testing.T) { + m := NewManager(testConfig(), nil) + result := m.CheckTool("", "", "unknown_tool", nil) + if result.Decision != DecisionDeny { + t.Fatalf("expected deny for unknown tool, got %s", result.Decision) + } +} diff --git a/pkg/permission/normalize.go b/pkg/permission/normalize.go new file mode 100644 index 0000000..e2c268c --- /dev/null +++ b/pkg/permission/normalize.go @@ -0,0 +1,13 @@ +package permission + +import ( + "regexp" + "strings" +) + +var multiSpace = regexp.MustCompile(`\s+`) + +// NormalizeCommand collapses whitespace and lowercases a shell command for policy matching. +func NormalizeCommand(cmd string) string { + return strings.ToLower(multiSpace.ReplaceAllString(strings.TrimSpace(cmd), " ")) +} diff --git a/pkg/permission/shellquote.go b/pkg/permission/shellquote.go new file mode 100644 index 0000000..d61009c --- /dev/null +++ b/pkg/permission/shellquote.go @@ -0,0 +1,8 @@ +package permission + +import "strings" + +// ShellQuote wraps a string in single quotes for safe remote shell use. +func ShellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", "'\"'\"'") + "'" +} diff --git a/pkg/permission/url.go b/pkg/permission/url.go new file mode 100644 index 0000000..ee781ee --- /dev/null +++ b/pkg/permission/url.go @@ -0,0 +1,99 @@ +package permission + +import ( + "fmt" + "net" + "net/url" + "strings" +) + +func isBlockedIP(ip net.IP) bool { + if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() { + return true + } + if ip4 := ip.To4(); ip4 != nil { + return ip4[0] == 169 && ip4[1] == 254 + } + return false +} + +// CheckURL validates an HTTP(S) URL against SSRF rules. +func (m *Manager) CheckURL(rawURL string) CheckResult { + u, err := url.Parse(rawURL) + if err != nil || u.Scheme == "" || u.Host == "" { + return CheckResult{ + Decision: DecisionDeny, + Reason: fmt.Sprintf("invalid URL: %q", rawURL), + } + } + + scheme := strings.ToLower(u.Scheme) + if scheme != "http" && scheme != "https" { + return CheckResult{ + Decision: DecisionDeny, + Reason: fmt.Sprintf("unsupported URL scheme: %s", scheme), + } + } + + host := strings.ToLower(u.Hostname()) + if host == "localhost" || strings.HasSuffix(host, ".localhost") { + return CheckResult{ + Decision: DecisionDeny, + Reason: "localhost URLs are not allowed", + } + } + + if ip := net.ParseIP(host); ip != nil && isBlockedIP(ip) { + return CheckResult{ + Decision: DecisionDeny, + Reason: "private or link-local URLs are not allowed", + } + } + + if len(m.cfg.Safety.AllowHTTPHosts) > 0 { + allowed := false + for _, pattern := range m.cfg.Safety.AllowHTTPHosts { + pattern = strings.ToLower(pattern) + if host == pattern || strings.HasSuffix(host, "."+pattern) { + allowed = true + break + } + } + if !allowed { + return CheckResult{ + Decision: DecisionDeny, + Reason: fmt.Sprintf("host %q is not in allow_http_hosts", host), + } + } + } + + return CheckResult{Decision: DecisionAllow} +} + +// CheckBrowserDomain validates a browser navigation URL against browser_domains. +func (m *Manager) CheckBrowserDomain(rawURL string) CheckResult { + if len(m.cfg.Browser.BrowserDomains) == 0 { + return CheckResult{Decision: DecisionAllow} + } + + u, err := url.Parse(rawURL) + if err != nil || u.Host == "" { + return CheckResult{ + Decision: DecisionDeny, + Reason: fmt.Sprintf("invalid browser URL: %q", rawURL), + } + } + + host := strings.ToLower(u.Hostname()) + for _, domain := range m.cfg.Browser.BrowserDomains { + domain = strings.ToLower(domain) + if host == domain || strings.HasSuffix(host, "."+domain) { + return CheckResult{Decision: DecisionAllow} + } + } + + return CheckResult{ + Decision: DecisionDeny, + Reason: fmt.Sprintf("browser domain %q is not allowed", host), + } +} diff --git a/pkg/planner/planner.go b/pkg/planner/planner.go index dea4469..9396194 100644 --- a/pkg/planner/planner.go +++ b/pkg/planner/planner.go @@ -15,9 +15,9 @@ import ( // Plan is the output of the planner. type Plan struct { - ProjectName string `json:"project_name,omitempty"` - NeedsProject bool `json:"needs_project"` - Tasks []*storage.Task `json:"tasks"` + ProjectName string `json:"project_name,omitempty"` + NeedsProject bool `json:"needs_project"` + Tasks []*storage.Task `json:"tasks"` } // Planner creates actionable task plans from goals. @@ -28,19 +28,25 @@ type Planner interface { // LLMPlanner uses an LLM to decompose goals into tasks. type LLMPlanner struct { - llm llm.Provider - tools *tools.Registry - memory *memory.Manager + llm llm.Provider + tools *tools.Registry + memory *memory.Manager + maxAttempts int } // NewLLMPlanner creates an LLM-backed planner. -func NewLLMPlanner(provider llm.Provider, registry *tools.Registry, mem *memory.Manager) *LLMPlanner { - return &LLMPlanner{llm: provider, tools: registry, memory: mem} +func NewLLMPlanner(provider llm.Provider, registry *tools.Registry, mem *memory.Manager, maxAttempts int) *LLMPlanner { + if maxAttempts <= 0 { + maxAttempts = 3 + } + return &LLMPlanner{llm: provider, tools: registry, memory: mem, maxAttempts: maxAttempts} } // Plan generates tasks for a goal. func (p *LLMPlanner) Plan(ctx context.Context, goal *storage.Goal, memoryContext string) (*Plan, error) { - projectName, needsProject := p.extractProjectInfo(ctx, goal) + llmName, llmNeeds := p.extractProjectInfo(ctx, goal) + needsProject := goalNeedsProject(goal.Description, goal.ProjectName) || llmNeeds + projectName, needsProject := resolveProjectName(goal.Description, goal.ProjectName, llmName, needsProject) now := time.Now() plan := &Plan{ @@ -58,7 +64,7 @@ func (p *LLMPlanner) Plan(ctx context.Context, goal *storage.Goal, memoryContext ToolArgs: map[string]interface{}{"action": "create_directory", "path": projectName}, CreatedAt: now, UpdatedAt: now, - MaxAttempts: 3, + MaxAttempts: p.maxAttempts, Dependencies: []string{}, }) plan.Tasks = append(plan.Tasks, &storage.Task{ @@ -68,9 +74,20 @@ func (p *LLMPlanner) Plan(ctx context.Context, goal *storage.Goal, memoryContext Status: "pending", CreatedAt: now, UpdatedAt: now, - MaxAttempts: 3, + MaxAttempts: p.maxAttempts, Dependencies: []string{fmt.Sprintf("task_%s_1", goal.ID)}, }) + } else if needsProject { + plan.Tasks = append(plan.Tasks, &storage.Task{ + ID: fmt.Sprintf("task_%s_1", goal.ID), + GoalID: goal.ID, + Description: fmt.Sprintf("Complete goal: %s", goal.Description), + Status: "pending", + CreatedAt: now, + UpdatedAt: now, + MaxAttempts: p.maxAttempts, + Dependencies: []string{}, + }) } else { plan.Tasks = append(plan.Tasks, &storage.Task{ ID: fmt.Sprintf("task_%s_1", goal.ID), @@ -79,7 +96,7 @@ func (p *LLMPlanner) Plan(ctx context.Context, goal *storage.Goal, memoryContext Status: "pending", CreatedAt: now, UpdatedAt: now, - MaxAttempts: 3, + MaxAttempts: p.maxAttempts, Dependencies: []string{}, }) } @@ -87,6 +104,9 @@ func (p *LLMPlanner) Plan(ctx context.Context, goal *storage.Goal, memoryContext if memoryContext != "" { p.memory.Remember(goal.ID, "plan_context", memoryContext, "planner") } + if projectName != "" { + p.memory.Remember(goal.ID, "project_name", projectName, "planner") + } return plan, nil } @@ -118,7 +138,7 @@ PROJECT_NAME: `, goal.Description) if len(parts) >= 2 { name := strings.TrimSpace(parts[1]) if name != "NONE" && name != "" { - name = strings.ToLower(strings.ReplaceAll(name, " ", "-")) + name = SanitizeProjectName(name) return name, true } } diff --git a/pkg/planner/project.go b/pkg/planner/project.go new file mode 100644 index 0000000..5e4d9d3 --- /dev/null +++ b/pkg/planner/project.go @@ -0,0 +1,50 @@ +package planner + +import ( + "regexp" + "strings" +) + +var projectNamePattern = regexp.MustCompile(`[^a-z0-9_-]+`) + +// SanitizeProjectName normalizes a user- or LLM-provided directory name. +func SanitizeProjectName(name string) string { + name = strings.TrimSpace(strings.ToLower(name)) + name = strings.ReplaceAll(name, " ", "-") + name = projectNamePattern.ReplaceAllString(name, "") + name = strings.Trim(name, "-_") + return name +} + +func goalNeedsProject(goalDescription, explicitName string) bool { + if explicitName != "" { + return true + } + lower := strings.ToLower(goalDescription) + keywords := []string{ + "script", "application", "program", "project", "service", + "tool", "cli", "app ", " app", "library", "package", "module", + } + for _, keyword := range keywords { + if strings.Contains(lower, keyword) { + return true + } + } + return false +} + +func resolveProjectName(goalDescription, explicitName, llmName string, needsProject bool) (string, bool) { + if explicitName != "" { + name := SanitizeProjectName(explicitName) + if name != "" { + return name, true + } + } + if !needsProject { + return "", false + } + if llmName != "" { + return llmName, true + } + return "", false +} diff --git a/pkg/planner/project_test.go b/pkg/planner/project_test.go new file mode 100644 index 0000000..17bb6f8 --- /dev/null +++ b/pkg/planner/project_test.go @@ -0,0 +1,36 @@ +package planner + +import "testing" + +func TestSanitizeProjectName(t *testing.T) { + tests := map[string]string{ + "github-list": "github-list", + "GitHub List": "github-list", + " my_app ": "my_app", + "foo@bar!": "foobar", + } + for input, want := range tests { + if got := SanitizeProjectName(input); got != want { + t.Errorf("SanitizeProjectName(%q) = %q, want %q", input, got, want) + } + } +} + +func TestGoalNeedsProject(t *testing.T) { + if !goalNeedsProject("Develop a python script for GitHub", "") { + t.Fatal("expected script goal to need a project") + } + if goalNeedsProject("fix typo in README", "") { + t.Fatal("expected simple edit to skip project") + } + if !goalNeedsProject("anything", "my-app") { + t.Fatal("explicit project name should require project") + } +} + +func TestResolveProjectNameExplicit(t *testing.T) { + name, needs := resolveProjectName("write code", "GitHub-List", "", false) + if !needs || name != "github-list" { + t.Fatalf("resolveProjectName() = (%q, %v)", name, needs) + } +} diff --git a/pkg/planner/replan.go b/pkg/planner/replan.go index 9cbbff5..bba50d7 100644 --- a/pkg/planner/replan.go +++ b/pkg/planner/replan.go @@ -60,7 +60,7 @@ Respond with ONLY the task description (one line, no markdown).`, goal.Descripti Status: "pending", CreatedAt: now, UpdatedAt: now, - MaxAttempts: 3, + MaxAttempts: p.maxAttempts, Dependencies: deps, } diff --git a/pkg/plugin/registry.go b/pkg/plugin/registry.go index cf74e60..7a3abac 100644 --- a/pkg/plugin/registry.go +++ b/pkg/plugin/registry.go @@ -11,12 +11,12 @@ import ( type Capability string const ( - CapabilityCoding Capability = "coding" - CapabilityDevOps Capability = "devops" - CapabilityRedTeam Capability = "redteam" - CapabilityK8s Capability = "kubernetes" - CapabilityBrowser Capability = "browser" - CapabilitySSH Capability = "ssh" + CapabilityCoding Capability = "coding" + CapabilityDevOps Capability = "devops" + CapabilityRedTeam Capability = "redteam" + CapabilityK8s Capability = "kubernetes" + CapabilityBrowser Capability = "browser" + CapabilitySSH Capability = "ssh" ) // Plugin registers tools and capabilities with the core engine. diff --git a/pkg/storage/sqlite.go b/pkg/storage/sqlite.go index 113985d..22d9b1a 100644 --- a/pkg/storage/sqlite.go +++ b/pkg/storage/sqlite.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "strings" _ "github.com/mattn/go-sqlite3" ) @@ -135,17 +136,29 @@ func (s *SQLiteStorage) initSchema() error { ` _, err := s.db.Exec(schema) - return err + if err != nil { + return err + } + return s.migrateSchema() +} + +func (s *SQLiteStorage) migrateSchema() error { + _, err := s.db.Exec(`ALTER TABLE goals ADD COLUMN project_name TEXT DEFAULT ''`) + if err != nil && !strings.Contains(strings.ToLower(err.Error()), "duplicate column") { + return err + } + return nil } // CreateGoal implements Storage.CreateGoal func (s *SQLiteStorage) CreateGoal(goal *Goal) error { - query := `INSERT INTO goals (id, description, state, created_at, updated_at, completed_at, result, error) - VALUES (?, ?, ?, ?, ?, ?, ?, ?)` + query := `INSERT INTO goals (id, description, project_name, state, created_at, updated_at, completed_at, result, error) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)` _, err := s.db.Exec(query, goal.ID, goal.Description, + goal.ProjectName, goal.State, goal.CreatedAt, goal.UpdatedAt, @@ -158,13 +171,14 @@ func (s *SQLiteStorage) CreateGoal(goal *Goal) error { // GetGoal implements Storage.GetGoal func (s *SQLiteStorage) GetGoal(id string) (*Goal, error) { - query := `SELECT id, description, state, created_at, updated_at, completed_at, result, error + query := `SELECT id, description, project_name, state, created_at, updated_at, completed_at, result, error FROM goals WHERE id = ?` goal := &Goal{} err := s.db.QueryRow(query, id).Scan( &goal.ID, &goal.Description, + &goal.ProjectName, &goal.State, &goal.CreatedAt, &goal.UpdatedAt, @@ -181,11 +195,12 @@ func (s *SQLiteStorage) GetGoal(id string) (*Goal, error) { // UpdateGoal implements Storage.UpdateGoal func (s *SQLiteStorage) UpdateGoal(goal *Goal) error { - query := `UPDATE goals SET description = ?, state = ?, updated_at = ?, + query := `UPDATE goals SET description = ?, project_name = ?, state = ?, updated_at = ?, completed_at = ?, result = ?, error = ? WHERE id = ?` _, err := s.db.Exec(query, goal.Description, + goal.ProjectName, goal.State, goal.UpdatedAt, goal.CompletedAt, @@ -198,7 +213,7 @@ func (s *SQLiteStorage) UpdateGoal(goal *Goal) error { // ListGoals implements Storage.ListGoals func (s *SQLiteStorage) ListGoals() ([]*Goal, error) { - query := `SELECT id, description, state, created_at, updated_at, completed_at, result, error + query := `SELECT id, description, project_name, state, created_at, updated_at, completed_at, result, error FROM goals ORDER BY created_at DESC` rows, err := s.db.Query(query) @@ -213,6 +228,7 @@ func (s *SQLiteStorage) ListGoals() ([]*Goal, error) { err := rows.Scan( &goal.ID, &goal.Description, + &goal.ProjectName, &goal.State, &goal.CreatedAt, &goal.UpdatedAt, @@ -234,7 +250,7 @@ func (s *SQLiteStorage) CreateTask(task *Task) error { depsJSON, _ := json.Marshal(task.Dependencies) argsJSON, _ := json.Marshal(task.ToolArgs) - query := `INSERT INTO tasks (id, goal_id, description, status, dependencies, tool_name, + query := `INSERT INTO tasks (id, goal_id, description, status, dependencies, tool_name, tool_args, result, error, created_at, updated_at, attempts, max_attempts) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` @@ -258,7 +274,7 @@ func (s *SQLiteStorage) CreateTask(task *Task) error { // GetTask implements Storage.GetTask func (s *SQLiteStorage) GetTask(id string) (*Task, error) { - query := `SELECT id, goal_id, description, status, dependencies, tool_name, tool_args, + query := `SELECT id, goal_id, description, status, dependencies, tool_name, tool_args, result, error, created_at, updated_at, attempts, max_attempts FROM tasks WHERE id = ?` @@ -285,8 +301,8 @@ func (s *SQLiteStorage) GetTask(id string) (*Task, error) { return nil, err } - json.Unmarshal([]byte(depsJSON), &task.Dependencies) - json.Unmarshal([]byte(argsJSON), &task.ToolArgs) + _ = json.Unmarshal([]byte(depsJSON), &task.Dependencies) + _ = json.Unmarshal([]byte(argsJSON), &task.ToolArgs) return task, nil } @@ -296,8 +312,8 @@ func (s *SQLiteStorage) UpdateTask(task *Task) error { depsJSON, _ := json.Marshal(task.Dependencies) argsJSON, _ := json.Marshal(task.ToolArgs) - query := `UPDATE tasks SET description = ?, status = ?, dependencies = ?, - tool_name = ?, tool_args = ?, result = ?, error = ?, updated_at = ?, + query := `UPDATE tasks SET description = ?, status = ?, dependencies = ?, + tool_name = ?, tool_args = ?, result = ?, error = ?, updated_at = ?, attempts = ?, max_attempts = ? WHERE id = ?` _, err := s.db.Exec(query, @@ -318,7 +334,7 @@ func (s *SQLiteStorage) UpdateTask(task *Task) error { // GetTasksByGoal implements Storage.GetTasksByGoal func (s *SQLiteStorage) GetTasksByGoal(goalID string) ([]Task, error) { - query := `SELECT id, goal_id, description, status, dependencies, tool_name, tool_args, + query := `SELECT id, goal_id, description, status, dependencies, tool_name, tool_args, result, error, created_at, updated_at, attempts, max_attempts FROM tasks WHERE goal_id = ? ORDER BY created_at ASC` @@ -352,8 +368,8 @@ func (s *SQLiteStorage) GetTasksByGoal(goalID string) ([]Task, error) { return nil, err } - json.Unmarshal([]byte(depsJSON), &task.Dependencies) - json.Unmarshal([]byte(argsJSON), &task.ToolArgs) + _ = json.Unmarshal([]byte(depsJSON), &task.Dependencies) + _ = json.Unmarshal([]byte(argsJSON), &task.ToolArgs) tasks = append(tasks, task) } diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index bab658c..91fbb74 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -23,6 +23,7 @@ const ( type Goal struct { ID string `json:"id"` Description string `json:"description"` + ProjectName string `json:"project_name,omitempty"` State State `json:"state"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` diff --git a/pkg/tools/ISSUE.md b/pkg/tools/ISSUE.md deleted file mode 100644 index eafae4c..0000000 --- a/pkg/tools/ISSUE.md +++ /dev/null @@ -1,54 +0,0 @@ -# OctaAI agent Issues - -## Issue: Agent creates empty directory but doesn't write files (FIXED) - -### Symptoms -``` -(base) ➜ OctaAI git:(feature/browser-automation) ✗ ./bin/octa-agentd -OctaAI Agent Daemon - Starting... -Using LLM: ollama (dolphin-mistral:7b) -Browser automation enabled on port 8765 -Registered 6 tools -2025/12/25 03:14:29 Starting browser WebSocket server on localhost:8765 -Agent daemon is running. Waiting for goals... -Press Ctrl+C to stop - -=== Processing Goal: goal_1766619913 === -Description: Write a docker compose that setups traefik on port 80 and 443 -=== Goal goal_1766619913 completed === -``` - -``` -(base) ➜ traefik-docker-compose ls -lha -total 8.0K -drwxr-xr-x 2 mparvin mparvin 4.0K Dec 25 03:15 . -drwxr-xr-x 9 mparvin mparvin 4.0K Dec 25 03:15 .. -``` - -### Root Causes Identified - -1. **No retry on parse/tool failures**: When `parseToolCall` failed or tool was not found, the task was immediately marked as "failed" without any retry attempts. - -2. **allDone logic bug**: A task with status "failed" but `attempts < maxAttempts` was not being counted as pending work. This caused the main loop to exit prematurely thinking all tasks were done. - -3. **Prompt clarity**: The task execution prompt wasn't clear enough for smaller LLM models like dolphin-mistral:7b, leading to malformed JSON responses. - -### Fixes Applied - -1. **Added retry logic for parse failures** (`agent.go` lines ~356-375): - - When parsing fails or tool is not found, task is now set to "pending" if retries remain - - Task is only marked "failed" after exhausting all attempts - -2. **Fixed allDone detection** (`agent.go` lines ~83-105): - - Failed tasks with remaining retries are now reset to "pending" - - These tasks correctly prevent premature goal completion - -3. **Improved parseToolCall** (`agent.go` lines ~760-815): - - Now accepts alternative key names: `tool_name`, `toolName`, `name` for tool - - Accepts `arguments`, `params`, `parameters`, `tool_args` for args - - Added better debug logging to see what LLM returns - -4. **Simplified task execution prompt** (`agent.go` lines ~700-730): - - Made prompt more concise and direct - - Added clearer examples for common operations - - Emphasized JSON-only response requirement \ No newline at end of file diff --git a/pkg/tools/command.go b/pkg/tools/command.go index c422b9b..964eb98 100644 --- a/pkg/tools/command.go +++ b/pkg/tools/command.go @@ -8,6 +8,7 @@ import ( "time" "github.com/mparvin/octaai/pkg/config" + "github.com/mparvin/octaai/pkg/permission" ) // CommandTool executes shell commands @@ -62,7 +63,6 @@ func (t *CommandTool) Execute(ctx context.Context, args map[string]interface{}) return nil, fmt.Errorf("command is required") } - // Check if command is denied if t.isCommandDenied(cmdStr) { return &ToolResult{ Success: false, @@ -70,27 +70,22 @@ func (t *CommandTool) Execute(ctx context.Context, args map[string]interface{}) }, nil } - // Parse timeout timeout := 300 * time.Second if timeoutVal, ok := args["timeout"].(float64); ok { timeout = time.Duration(timeoutVal) * time.Second } - // Create context with timeout cmdCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - // Parse command parts := strings.Fields(cmdStr) if len(parts) == 0 { return nil, fmt.Errorf("empty command") } - // Create command cmd := exec.CommandContext(cmdCtx, parts[0], parts[1:]...) cmd.Dir = t.resolvePath(cwd) - // Execute command output, err := cmd.CombinedOutput() exitCode := 0 @@ -105,10 +100,8 @@ func (t *CommandTool) Execute(ctx context.Context, args map[string]interface{}) } } - success := exitCode == 0 - return &ToolResult{ - Success: success, + Success: exitCode == 0, Output: string(output), Data: map[string]interface{}{ "exit_code": exitCode, @@ -119,13 +112,13 @@ func (t *CommandTool) Execute(ctx context.Context, args map[string]interface{}) } func (t *CommandTool) resolvePath(path string) string { - // Similar to filesystem tool - return path + return config.ResolveProjectPath(t.cfg, path) } func (t *CommandTool) isCommandDenied(cmdStr string) bool { + normalized := permission.NormalizeCommand(cmdStr) for _, denied := range t.cfg.Safety.DenyCommands { - if strings.Contains(cmdStr, denied) { + if strings.Contains(normalized, permission.NormalizeCommand(denied)) { return true } } diff --git a/pkg/tools/command_test.go b/pkg/tools/command_test.go new file mode 100644 index 0000000..e9b4f8f --- /dev/null +++ b/pkg/tools/command_test.go @@ -0,0 +1,29 @@ +package tools + +import ( + "testing" + + "github.com/mparvin/octaai/pkg/config" +) + +func testCommandConfig() *config.Config { + cfg := config.DefaultConfig() + cfg.ProjectsRoot = "/home/user/Projects" + cfg.Safety.AllowPaths = []string{"/home/user/Projects"} + cfg.Safety.DenyCommands = []string{"rm -rf /"} + return cfg +} + +func TestCommandToolDenyBypass(t *testing.T) { + tool := NewCommandTool(testCommandConfig()) + if !tool.isCommandDenied("RM -rf /") { + t.Fatal("expected denied command with whitespace/case bypass attempt") + } +} + +func TestCommandToolAllowsSafeCommand(t *testing.T) { + tool := NewCommandTool(testCommandConfig()) + if tool.isCommandDenied("ls -la") { + t.Fatal("expected ls -la to be allowed") + } +} diff --git a/pkg/tools/filesystem.go b/pkg/tools/filesystem.go index 26ab62d..fb1f3e5 100644 --- a/pkg/tools/filesystem.go +++ b/pkg/tools/filesystem.go @@ -64,7 +64,7 @@ func (t *FilesystemTool) Execute(ctx context.Context, args map[string]interface{ } // Resolve path - fullPath := t.resolvePath(pathStr) + fullPath := config.ResolveProjectPath(t.cfg, pathStr) // Check if path is allowed if !t.isPathAllowed(fullPath) { @@ -92,13 +92,6 @@ func (t *FilesystemTool) Execute(ctx context.Context, args map[string]interface{ } } -func (t *FilesystemTool) resolvePath(path string) string { - if filepath.IsAbs(path) { - return path - } - return filepath.Join(t.cfg.ProjectsRoot, path) -} - func (t *FilesystemTool) isPathAllowed(path string) bool { absPath, err := filepath.Abs(path) if err != nil { diff --git a/pkg/tools/filesystem_test.go b/pkg/tools/filesystem_test.go new file mode 100644 index 0000000..d88b0d7 --- /dev/null +++ b/pkg/tools/filesystem_test.go @@ -0,0 +1,66 @@ +package tools + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/mparvin/octaai/pkg/config" +) + +func testFilesystemConfig(root string) *config.Config { + cfg := config.DefaultConfig() + cfg.ProjectsRoot = root + cfg.Safety.AllowPaths = []string{root} + return cfg +} + +func TestFilesystemToolPathAllowance(t *testing.T) { + root := t.TempDir() + tool := NewFilesystemTool(testFilesystemConfig(root)) + + allowed := filepath.Join(root, "app") + if !tool.isPathAllowed(allowed) { + t.Fatal("expected path inside projects root to be allowed") + } + + outside := filepath.Join(root, "..", "outside") + absOutside, _ := filepath.Abs(outside) + if tool.isPathAllowed(absOutside) { + t.Fatal("expected path outside projects root to be denied") + } +} + +func TestFilesystemWriteAndRead(t *testing.T) { + root := t.TempDir() + tool := NewFilesystemTool(testFilesystemConfig(root)) + + writeResult, err := tool.Execute(context.Background(), map[string]interface{}{ + "action": "write_file", + "path": "hello.txt", + "content": "hello world", + }) + if err != nil { + t.Fatalf("write failed: %v", err) + } + if !writeResult.Success { + t.Fatalf("write unsuccessful: %s", writeResult.Error) + } + + readResult, err := tool.Execute(context.Background(), map[string]interface{}{ + "action": "read_file", + "path": "hello.txt", + }) + if err != nil { + t.Fatalf("read failed: %v", err) + } + if readResult.Output != "hello world" { + t.Fatalf("unexpected content: %q", readResult.Output) + } + + fullPath := filepath.Join(root, "hello.txt") + if _, err := os.Stat(fullPath); err != nil { + t.Fatalf("file not created on disk: %v", err) + } +} diff --git a/pkg/tools/git.go b/pkg/tools/git.go index 5d1dc6a..f77ceb8 100644 --- a/pkg/tools/git.go +++ b/pkg/tools/git.go @@ -107,6 +107,8 @@ func (t *GitTool) clone(ctx context.Context, url, destPath string) (*ToolResult, return nil, fmt.Errorf("url is required for clone") } + destPath = config.ResolveProjectPath(t.cfg, destPath) + // Ensure parent directory exists parentDir := filepath.Dir(destPath) if err := os.MkdirAll(parentDir, 0755); err != nil { @@ -133,6 +135,7 @@ func (t *GitTool) clone(ctx context.Context, url, destPath string) (*ToolResult, } func (t *GitTool) init(ctx context.Context, path string) (*ToolResult, error) { + path = config.ResolveProjectPath(t.cfg, path) if err := os.MkdirAll(path, 0755); err != nil { return &ToolResult{ Success: false, @@ -158,6 +161,7 @@ func (t *GitTool) init(ctx context.Context, path string) (*ToolResult, error) { } func (t *GitTool) commitAll(ctx context.Context, path, message string) (*ToolResult, error) { + path = config.ResolveProjectPath(t.cfg, path) if message == "" { message = "Automated commit by OctaAI" } @@ -191,6 +195,7 @@ func (t *GitTool) commitAll(ctx context.Context, path, message string) (*ToolRes } func (t *GitTool) push(ctx context.Context, path, remote, branch string) (*ToolResult, error) { + path = config.ResolveProjectPath(t.cfg, path) cmd := exec.CommandContext(ctx, "git", "push", remote, branch) cmd.Dir = path output, err := cmd.CombinedOutput() diff --git a/pkg/tools/ssh.go b/pkg/tools/ssh.go index 12d2c1f..9d73a8d 100644 --- a/pkg/tools/ssh.go +++ b/pkg/tools/ssh.go @@ -8,7 +8,9 @@ import ( "time" "github.com/mparvin/octaai/pkg/config" + "github.com/mparvin/octaai/pkg/permission" "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/knownhosts" ) // SSHTool provides SSH operations for remote servers @@ -104,7 +106,6 @@ func (t *SSHTool) Execute(ctx context.Context, args map[string]interface{}) (*To port = int(portVal) } - // Create SSH client client, err := t.createSSHClient(host, port, user, args) if err != nil { return &ToolResult{ @@ -134,12 +135,10 @@ func (t *SSHTool) Execute(ctx context.Context, args map[string]interface{}) (*To func (t *SSHTool) createSSHClient(host string, port int, user string, args map[string]interface{}) (*ssh.Client, error) { var authMethods []ssh.AuthMethod - // Try password authentication if password, ok := args["password"].(string); ok && password != "" { authMethods = append(authMethods, ssh.Password(password)) } - // Try key-based authentication keyPath, ok := args["key_path"].(string) if !ok || keyPath == "" { keyPath = t.cfg.SSH.DefaultKeyPath @@ -159,15 +158,20 @@ func (t *SSHTool) createSSHClient(host string, port int, user string, args map[s return nil, fmt.Errorf("no authentication method available") } - config := &ssh.ClientConfig{ + hostKeyCallback, err := t.hostKeyCallback() + if err != nil { + return nil, err + } + + clientConfig := &ssh.ClientConfig{ User: user, Auth: authMethods, - HostKeyCallback: ssh.InsecureIgnoreHostKey(), // TODO: Implement proper host key checking + HostKeyCallback: hostKeyCallback, Timeout: 30 * time.Second, } addr := fmt.Sprintf("%s:%d", host, port) - client, err := ssh.Dial("tcp", addr, config) + client, err := ssh.Dial("tcp", addr, clientConfig) if err != nil { return nil, fmt.Errorf("failed to connect: %w", err) } @@ -175,6 +179,14 @@ func (t *SSHTool) createSSHClient(host string, port int, user string, args map[s return client, nil } +func (t *SSHTool) hostKeyCallback() (ssh.HostKeyCallback, error) { + knownHostsFile := t.cfg.SSH.KnownHostsFile + if knownHostsFile == "" { + return nil, fmt.Errorf("SSH known_hosts file is not configured") + } + return knownhosts.New(knownHostsFile) +} + func (t *SSHTool) execCommand(client *ssh.Client, command string) (*ToolResult, error) { if command == "" { return nil, fmt.Errorf("command is required") @@ -212,7 +224,6 @@ func (t *SSHTool) uploadFile(client *ssh.Client, localPath, remotePath string) ( return nil, fmt.Errorf("local_path and remote_path are required") } - // Read local file content, err := os.ReadFile(localPath) if err != nil { return &ToolResult{ @@ -221,9 +232,8 @@ func (t *SSHTool) uploadFile(client *ssh.Client, localPath, remotePath string) ( }, nil } - // Create remote directory if needed remoteDir := filepath.Dir(remotePath) - mkdirCmd := fmt.Sprintf("mkdir -p %s", remoteDir) + mkdirCmd := fmt.Sprintf("mkdir -p %s", permission.ShellQuote(remoteDir)) session, err := client.NewSession() if err != nil { @@ -232,10 +242,9 @@ func (t *SSHTool) uploadFile(client *ssh.Client, localPath, remotePath string) ( Error: err.Error(), }, nil } - session.Run(mkdirCmd) + _ = session.Run(mkdirCmd) session.Close() - // Upload file using SCP protocol session, err = client.NewSession() if err != nil { return &ToolResult{ @@ -245,8 +254,7 @@ func (t *SSHTool) uploadFile(client *ssh.Client, localPath, remotePath string) ( } defer session.Close() - // Simple approach: write content via shell redirection - uploadCmd := fmt.Sprintf("cat > %s", remotePath) + uploadCmd := fmt.Sprintf("cat > %s", permission.ShellQuote(remotePath)) stdin, err := session.StdinPipe() if err != nil { return &ToolResult{ @@ -262,7 +270,7 @@ func (t *SSHTool) uploadFile(client *ssh.Client, localPath, remotePath string) ( }, nil } - stdin.Write(content) + _, _ = stdin.Write(content) stdin.Close() if err := session.Wait(); err != nil { @@ -292,8 +300,7 @@ func (t *SSHTool) downloadFile(client *ssh.Client, remotePath, localPath string) } defer session.Close() - // Read remote file content - content, err := session.Output(fmt.Sprintf("cat %s", remotePath)) + content, err := session.Output(fmt.Sprintf("cat %s", permission.ShellQuote(remotePath))) if err != nil { return &ToolResult{ Success: false, @@ -301,7 +308,6 @@ func (t *SSHTool) downloadFile(client *ssh.Client, remotePath, localPath string) }, nil } - // Ensure local directory exists localDir := filepath.Dir(localPath) if err := os.MkdirAll(localDir, 0755); err != nil { return &ToolResult{ @@ -310,7 +316,6 @@ func (t *SSHTool) downloadFile(client *ssh.Client, remotePath, localPath string) }, nil } - // Write to local file if err := os.WriteFile(localPath, content, 0644); err != nil { return &ToolResult{ Success: false, From d3727bb56c7371daedf1746f695aa516e9a54344 Mon Sep 17 00:00:00 2001 From: Mohammad Parvin Date: Wed, 15 Jul 2026 13:02:11 +0330 Subject: [PATCH 2/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pkg/permission/url.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/permission/url.go b/pkg/permission/url.go index ee781ee..648b47b 100644 --- a/pkg/permission/url.go +++ b/pkg/permission/url.go @@ -8,7 +8,7 @@ import ( ) func isBlockedIP(ip net.IP) bool { - if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() { + if ip.IsLoopback() || ip.IsUnspecified() || ip.IsMulticast() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() { return true } if ip4 := ip.To4(); ip4 != nil { From 6bbfaa6ced8db4afc80d0a3ee1c7905a5944f9a6 Mon Sep 17 00:00:00 2001 From: Mohammad Parvin Date: Wed, 15 Jul 2026 13:02:24 +0330 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pkg/permission/url.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/permission/url.go b/pkg/permission/url.go index 648b47b..26ee8ee 100644 --- a/pkg/permission/url.go +++ b/pkg/permission/url.go @@ -43,13 +43,21 @@ func (m *Manager) CheckURL(rawURL string) CheckResult { } } - if ip := net.ParseIP(host); ip != nil && isBlockedIP(ip) { - return CheckResult{ - Decision: DecisionDeny, - Reason: "private or link-local URLs are not allowed", + if ip := net.ParseIP(host); ip != nil { + if isBlockedIP(ip) { + return CheckResult{Decision: DecisionDeny, Reason: "private or link-local URLs are not allowed"} + } + } else { + ips, err := net.LookupIP(host) + if err != nil { + return CheckResult{Decision: DecisionDeny, Reason: fmt.Sprintf("failed to resolve host %q: %v", host, err)} + } + for _, resolved := range ips { + if isBlockedIP(resolved) { + return CheckResult{Decision: DecisionDeny, Reason: "private or link-local URLs are not allowed"} + } } } - if len(m.cfg.Safety.AllowHTTPHosts) > 0 { allowed := false for _, pattern := range m.cfg.Safety.AllowHTTPHosts { From 741df8e5afe91d662bd27d8156b087c0a8440992 Mon Sep 17 00:00:00 2001 From: Mohammad Parvin Date: Wed, 15 Jul 2026 13:02:48 +0330 Subject: [PATCH 4/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pkg/permission/url.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/permission/url.go b/pkg/permission/url.go index 26ee8ee..677dabb 100644 --- a/pkg/permission/url.go +++ b/pkg/permission/url.go @@ -85,13 +85,18 @@ func (m *Manager) CheckBrowserDomain(rawURL string) CheckResult { } u, err := url.Parse(rawURL) - if err != nil || u.Host == "" { + if err != nil || u.Scheme == "" || u.Host == "" { return CheckResult{ Decision: DecisionDeny, Reason: fmt.Sprintf("invalid browser URL: %q", rawURL), } } - + if scheme := strings.ToLower(u.Scheme); scheme != "http" && scheme != "https" { + return CheckResult{ + Decision: DecisionDeny, + Reason: fmt.Sprintf("unsupported browser URL scheme: %s", scheme), + } + } host := strings.ToLower(u.Hostname()) for _, domain := range m.cfg.Browser.BrowserDomains { domain = strings.ToLower(domain)