diff --git a/CHANGELOG.md b/CHANGELOG.md index ba853a3..8fa6ba5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,21 @@ All notable changes to Rook, following [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and [Semantic Versioning](https://semver.org/). -## [0.6.0] - unreleased +## [0.6.1] - 2026-08-24 + +### Changed + +- **"Backend" is now "provider", to match zot.** Rook and zot are configured the same way but spoke different dialects: rook called a named model-provider connection a *backend*, zot called it a *provider*, and where zot's connection names its implementation with `driver:`, rook's used `provider:`. They now agree on zot's vocabulary. The config keys `default_backend` → `default_provider` and `backends:` → `providers:`; the inner `provider:` field (the implementation) → `driver:`; the flag `--backend` → `--provider`; and the env var `ROOK_DEFAULT_BACKEND` → `ROOK_DEFAULT_PROVIDER`. This is a breaking config change - a config file or script using the old keys must be updated - but it makes moving between the two tools one vocabulary instead of two. + +### Added + +- **The engine tuning knobs, and per-model capability overrides, to match zot.** rook's `agent:` config exposed only `model` and `max_iterations`; the rest of the engine's controls were hardcoded or unreachable. It now carries the same knobs zot does - `max_settles`, `max_calls`, `max_time`, `max_tokens`, `max_tool_output`, `max_continuations`, `max_recoveries`, `max_cycles`, `max_empties`, `limit_checkpoints`, `context_strategy`, and the `compact_*` trio - each optional, zero using the engine default, and validated at load. A model entry also takes `context` (correct a too-large assumed context window for a small endpoint) and `vision` (say a model can be shown images, so the agent is offered the view tool - previously rook never enabled vision at all). rook cannot import zot's internal model catalogue, so there is no auto-detection; these are explicit operator overrides, which is the part that matters for custom endpoints. + +### Fixed + +- **A provider's API key never reaches a custom endpoint it was not written for, and never reaches the agent's shell.** A built-in provider's conventional key (e.g. `OPENAI_API_KEY`) was seeded as its credential purely from its name. Two consequences are now closed: (1) when a built-in provider is pointed at a custom `base_url`, its ambient key is no longer adopted as the connection's credential, so a key scoped to the provider's real host is never forwarded to a URL from the config; and (2) `ScrubProviderSecrets` now unsets every built-in provider's conventional variable by name - not only those whose value was resolved into config - so a provider key exported in the environment is stripped before the agent runs even when that provider has a `base_url` set. For an offensive-security tool whose agent runs commands against targets, both keep a provider credential out of reach of those commands. + +## [0.6.0] - 2026-08-24 ### Added diff --git a/README.md b/README.md index a96ac01..d854741 100644 --- a/README.md +++ b/README.md @@ -190,14 +190,14 @@ Or clone and build with the provided `Makefile`: make build # → ./rook ``` -## Backends +## Providers -A run targets a **backend** - the provider Rook talks to. Rook speaks to each one -directly over the OpenAI-compatible API; there is no gateway and no account in -between, so all you need is a provider key. Pick a backend with `--backend`, or -set `default_backend` in config. +A run targets a **provider** - the model provider Rook talks to. Rook speaks to +each one directly over the OpenAI-compatible API; there is no gateway and no +account in between, so all you need is a provider key. Pick a provider with +`--provider`, or set `default_provider` in config. -| Backend | Endpoint | Credential from | +| Provider | Endpoint | Credential from | | ------------ | -------------------------------- | -------------------- | | `zai` | `https://api.z.ai/api/paas/v4` | `ZAI_API_KEY` | | `openai` | `https://api.openai.com/v1` | `OPENAI_API_KEY` | @@ -215,7 +215,7 @@ set `default_backend` in config. Rook defaults to **`zai`** running **`glm-5.2`** - a strong open model for bug-hunting work: large context for reading codebases, and permissive for -offensive tasks. The model must be one the chosen backend serves. +offensive tasks. The model must be one the chosen provider serves. The common case is one exported variable and nothing else: @@ -231,26 +231,26 @@ Switch provider with a flag: ```bash export OPENAI_API_KEY="sk-..." -rook --backend openai --model gpt-5 "…" +rook --provider openai --model gpt-5 "…" ``` For sensitive material that must not leave the machine, a local model is the -right choice - and the one backend that never sends data off-host: +right choice - and the one provider that never sends data off-host: ```bash -rook --backend ollama --model llama-4 "…" +rook --provider ollama --model llama-4 "…" ``` ### Any other provider -Anything that speaks the OpenAI-compatible API works. Name a backend, give it a +Anything that speaks the OpenAI-compatible API works. Name a provider, give it a base URL and a key: ```yaml -default_backend: mygateway -backends: +default_provider: mygateway +providers: mygateway: - provider: custom + driver: custom base_url: https://gateway.internal.example.com/v1 api_key: '$GATEWAY_KEY' ``` @@ -269,15 +269,24 @@ rook config path # print the config file location The file lives at `~/.config/rook/config.yaml` (override with `$ROOK_CONFIG` or `--config`). Every scalar has a matching `ROOK_*` env var (`agent.model` → -`ROOK_AGENT_MODEL`, `default_backend` → `ROOK_DEFAULT_BACKEND`). A backend's key +`ROOK_AGENT_MODEL`, `default_provider` → `ROOK_DEFAULT_PROVIDER`). A provider's key comes from its provider's conventional variable or `api_key` in the file, which may be a literal or a `$VAR` reference. A developer build also reads a `.env` from the working directory - a released one does not (see [Development](#development)). See [configs/rook.example.yaml](configs/rook.example.yaml). -Rook strips the resolved backend credential from the environment before the -agent runs, so the commands it executes against a target cannot read it. +Under `agent:` the run's engine knobs mirror zot's - `max_settles`, `max_calls`, +`max_time`, `max_tokens`, `max_tool_output`, the recovery and cycle caps, +`limit_checkpoints`, `context_strategy` and the `compact_*` trio - each optional, +zero using the engine default. A model entry additionally takes `context` (a +smaller real context window than the model's card) and `vision` (this model can +be shown images). See [configs/rook.example.yaml](configs/rook.example.yaml). + +Rook strips the resolved provider credential from the environment before the +agent runs, so the commands it executes against a target cannot read it. A +built-in provider's conventional key is withheld once you set a custom +`base_url`, so a key scoped to one host is never forwarded to another. ## Files & directories @@ -307,7 +316,7 @@ recent active run. ## Usage ```bash -export ZAI_API_KEY="sk-..." # or --backend openai with OPENAI_API_KEY, etc. +export ZAI_API_KEY="sk-..." # or --provider openai with OPENAI_API_KEY, etc. # Write an objective, then run it rook new "Gain access to the target network and map paths to domain admin" @@ -329,7 +338,7 @@ does not (see [Development](#development)). | Flag | Default | Description | | ------------------ | ---------------------------- | -------------------------------------------------- | -| `--backend` | `zai` | Backend to target: any provider, or one named in config | +| `--provider` | `zai` | Model provider to run against, or one named in config | | `--config` | `~/.config/rook/config.yaml` | Path to the config file (or `$ROOK_CONFIG`) | | `--model` | `glm-5.2` | Model the agent reasons with (overrides config) | | `--max-iterations` | `10000` | Maximum agent iterations before a forced stop | diff --git a/VERSION b/VERSION index a918a2a..ee6cdce 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.6.0 +0.6.1 diff --git a/cmd/rook/main.go b/cmd/rook/main.go index 41d2bee..79e7ab4 100644 --- a/cmd/rook/main.go +++ b/cmd/rook/main.go @@ -25,7 +25,7 @@ // # every run writes artifacts (status + events) and a ledger receipt // rook .rook/objectives/hunt.yaml // -// rook config # edit the config (backend, model, key) +// rook config # edit the config (provider, model, key) // rook version // // Configuration is layered: built-in defaults < config file < ROOK_* env vars < @@ -89,7 +89,7 @@ func run() error { flags := pflag.NewFlagSet("rook", pflag.ContinueOnError) configPath := flags.String("config", "", "path to the config file (default: $ROOK_CONFIG or ~/.config/rook/config.yaml)") - backend := flags.String("backend", "", "backend to target: a provider such as zai (default), openai, anthropic, groq, ollama, or a backend named in the config") + provider := flags.String("provider", "", "model provider to run against: zai (default), openai, anthropic, groq, ollama, or a provider named in the config") model := flags.String("model", "", "model the agent reasons with (overrides config)") dir := flags.String("dir", ".", "working directory the agent investigates: the objective runs against this tree") maxIter := flags.Int("max-iterations", 0, "maximum agent iterations before forced stop (overrides config)") @@ -149,8 +149,8 @@ func run() error { } // CLI flags win over file and env. - if *backend != "" { - cfg.DefaultBackend = *backend + if *provider != "" { + cfg.DefaultProvider = *provider } if *model != "" { cfg.Agent.Model = *model @@ -239,10 +239,10 @@ func run() error { return fmt.Errorf("cannot enter --dir %q: %w", *dir, err) } - // Strip backend credentials from the environment before the agent runs, so + // Strip provider credentials from the environment before the agent runs, so // the commands it executes against a target cannot read them. The resolved // key is still handed to the client below. - config.ScrubBackendSecrets(cfg) + config.ScrubProviderSecrets(cfg) ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) defer stop() @@ -356,12 +356,15 @@ func (r runRunner) execute(o objective.Objective, quitOnDone bool) error { } code, outcome, err := agent.Run(r.ctx, agent.Config{ - Provider: r.selected.Provider, + Driver: r.selected.Driver, APIKey: r.selected.APIKey, BaseURL: r.selected.BaseURL, Model: r.selected.Model, - Backend: r.cfg.DefaultBackend, + Provider: r.cfg.DefaultProvider, MaxIterations: r.selected.MaxIterations, + ContextWindow: r.selected.ContextWindow, + Vision: r.selected.Vision, + Tuning: r.cfg.Agent, Objective: o, Verbose: r.verbose, RunDir: r.runDir, @@ -534,7 +537,7 @@ func printVersion() { // editConfig ensures the config file exists - seeding it from the embedded // template on first run - and opens it in the user's editor. This is the setup -// path: configure the backend, model and provider key by editing the file. +// path: configure the provider, model and API key by editing the file. func editConfig() error { path := config.DefaultConfigPath() diff --git a/cmd/rook/main_test.go b/cmd/rook/main_test.go index 1a36f67..54b5197 100644 --- a/cmd/rook/main_test.go +++ b/cmd/rook/main_test.go @@ -640,7 +640,7 @@ func TestRunNoObjectivesErrors(t *testing.T) { devNull.Close() }() - // Provide a config so the backend resolves, then fail on objectives. + // Provide a config so the provider resolves, then fail on objectives. // Actually, a bare `rook` with no config file and no API key will fail at // cfg.Validate or cfg.Selected first. We want the "no objectives" error, // so provide a valid config. diff --git a/configs/rook.example.yaml b/configs/rook.example.yaml index 39f42ee..c59fa60 100644 --- a/configs/rook.example.yaml +++ b/configs/rook.example.yaml @@ -9,10 +9,10 @@ # field's path upper-cased with dots as underscores. Env vars override the file. # agent.model -> ROOK_AGENT_MODEL # agent.max_iterations -> ROOK_AGENT_MAX_ITERATIONS -# default_backend -> ROOK_DEFAULT_BACKEND +# default_provider -> ROOK_DEFAULT_PROVIDER # # Credentials: put the key DIRECTLY in this file, or reference an environment -# variable with $VAR - either works. Each built-in backend also reads its +# variable with $VAR - either works. Each built-in provider also reads its # provider's conventional variable, so exporting that is usually enough: # openai -> OPENAI_API_KEY anthropic -> ANTHROPIC_API_KEY # groq -> GROQ_API_KEY mistral -> MISTRAL_API_KEY @@ -28,14 +28,31 @@ agent: # codebases, and permissive for offensive tasks: glm-5.2 (default), kimi-k3, # deepseek-v4-flash. # - # Whatever you pick, make sure default_backend below actually serves it. A - # model and a backend that cannot talk to each other fail as a provider error + # Whatever you pick, make sure default_provider below names a driver that serves it. A + # model and provider that cannot talk to each other fail as a provider error # rather than a configuration one, which is much harder to read. model: 'glm-5.2' # Safety cap on plan/act/observe cycles before the agent is forced to stop. max_iterations: 1000000 + # Engine tuning (all optional; unset uses the built-in default). These mirror + # zot's, so a run is shaped the same way in either tool. + # max_settles: 20 # nudges to record an outcome before "unsettled" + # max_calls: 0 # cap on total tool calls (0 = unbounded) + # max_time: '2h' # wall-clock cap ('30m', '2h', '90s'); empty = none + # max_tokens: 0 # cap on a single response's output (0 = unbounded) + # max_tool_output: 0 # bytes a tool result may return before truncation + # max_continuations: 0 # consecutive recovery attempts before giving up + # max_recoveries: 0 # total recovery attempts across the run + # max_cycles: 0 # nudges out of a detected repetition + # max_empties: 0 # consecutive empty turns before bailing + # limit_checkpoints: [50, 80, 90] # % of a bounded limit to warn at ([] = off) + # context_strategy: compact # 'compact' (summarise) or 'truncate' (drop oldest) + # compact_min_tokens: 0 + # compact_min_messages: 0 + # compact_trigger_ratio: 0.9 # fraction of the window at which compaction fires + # Run artifacts. Every run writes status.json (live state) and events.jsonl (an # append-only log) into //. This is telemetry - what the desktop # status widget reads - and is separate from the workspace the agent works in. @@ -50,15 +67,15 @@ agent: # update_check: # disabled: true -# Backends are the providers a run can target. Rook talks to each one directly +# Providers are the model-provider connections a run can target. Rook talks to each one directly # over the OpenAI-compatible API - there is no gateway and no account in between. -# Pick one per run with --backend; otherwise default_backend is used. +# Pick one per run with --provider; otherwise default_provider is used. # -# A backend named after a provider needs no further configuration: the name is -# the provider, and the credential comes from its conventional variable. -default_backend: zai +# A provider connection named after a driver needs no further configuration: the +# name is the driver, and the credential comes from its conventional variable. +default_provider: zai -backends: +providers: # The default, paired with the default model above: glm-5.2 served by Z.AI. # The common case is nothing here at all, and ZAI_API_KEY in the environment. zai: @@ -77,9 +94,9 @@ backends: # base_url: 'http://localhost:11434/v1' # Anything else that speaks the OpenAI-compatible API. A custom endpoint needs - # its own key and, unless it is a known provider, an explicit provider: custom. + # its own key and, unless it is a known provider, an explicit driver: custom. # mygateway: - # provider: custom + # driver: custom # base_url: 'https://gateway.internal.example.com/v1' # api_key: '$GATEWAY_KEY' # @@ -89,6 +106,8 @@ backends: # models: # fast: # model: 'gpt-5-mini' - # provider: 'openai' + # driver: 'openai' # api_key: '$OPENAI_API_KEY' # max_iterations: 200 + # context: 128000 # real context window, if the endpoint's is smaller + # vision: true # this model can be shown images (offers the view tool) diff --git a/image/desktop/README.md b/image/desktop/README.md index ef2bb6b..5d03146 100644 --- a/image/desktop/README.md +++ b/image/desktop/README.md @@ -75,10 +75,10 @@ substrate version for reproducible builds. ## The API key Rook talks straight to a model provider, so a run needs that provider's key. -Rook defaults to the `zai` backend running `glm-5.2`, which reads `ZAI_API_KEY`. +Rook defaults to the `zai` provider running `glm-5.2`, which reads `ZAI_API_KEY`. Setup is the built-in **`rook config`**: it seeds `~/.config/rook/config.yaml` from the template on first run and opens it in `$EDITOR`, where you set the -backend, model and `api_key`. The file lives on the persistent `~/.config/rook` +provider, model and `api_key`. The file lives on the persistent `~/.config/rook` volume and rook reads it directly. The panel shows `ROOK · RUN rook config` until a key is present. diff --git a/internal/agent/runner.go b/internal/agent/runner.go index cf14a27..7bf21cb 100644 --- a/internal/agent/runner.go +++ b/internal/agent/runner.go @@ -44,23 +44,36 @@ var rookTheme = tui.Theme{ // Config controls a single autonomous run. type Config struct { - // Provider names the model provider to call: "openai", "anthropic", "zai" - // and so on, or "custom" with a BaseURL for anything else that speaks the - // OpenAI-compatible API. - Provider string + // Driver names the provider implementation to call: "openai", "anthropic", + // "zai" and so on, or "custom" with a BaseURL for anything else that speaks + // the OpenAI-compatible API. + Driver string // APIKey is the provider credential. APIKey string // BaseURL overrides the provider's default endpoint. Required for a custom - // provider, ignored otherwise unless a gateway needs it. + // driver, ignored otherwise unless a gateway needs it. BaseURL string // Model is the model the agent reasons with, as the provider names it. Model string - // Backend is the name of the backend the run targets (zai, openai, …), shown - // in the viewer's header. Presentation only; the client is already resolved. - Backend string + // Provider is the name of the provider connection the run targets (zai, + // openai, …), shown in the viewer's header. Presentation only; the client is + // already resolved. + Provider string // MaxIterations bounds how many tool-using turns the agent may take // before it is forced to stop. MaxIterations int + // Tuning carries the engine knobs a run is shaped by - the settle, call, + // time, token, recovery, cycle and compaction bounds. Its Model and + // MaxIterations are ignored here (those are already resolved above); the + // rest map straight onto the engine's run options. Zero fields use the + // engine defaults, so a bare Config still runs. + Tuning config.Agent + // ContextWindow overrides the model's total context window, in tokens. Zero + // uses the engine default. + ContextWindow int + // Vision offers the agent a tool for looking at images, for a model the + // operator has said can be shown them. + Vision bool // Objective is the mission brief the run is dispatched from. Its task text // (objective + success criteria + rules of engagement) is placed in the // system prompt. @@ -117,7 +130,7 @@ func Run(ctx context.Context, cfg Config) (int, tui.Outcome, error) { // which endpoint and which tokenizer the engine uses, so it has to be known // before the conversation starts. client, err := agent.NewClient(agent.ClientOptions{ - Provider: cfg.Provider, + Provider: cfg.Driver, Model: cfg.Model, APIKey: cfg.APIKey, BaseURL: cfg.BaseURL, @@ -132,6 +145,8 @@ func Run(ctx context.Context, cfg Config) (int, tui.Outcome, error) { // cloned onto disk - one tool, one verb, the path the only difference. tools := agent.DefaultToolsFor(agent.ToolOptions{ EmbeddedSkills: skillsResult.EmbeddedContents(), + MaxOutput: cfg.Tuning.MaxToolOutput, + Vision: cfg.Vision, }) // The task is the durable objective and goes into the system prompt; the @@ -140,18 +155,48 @@ func Run(ctx context.Context, cfg Config) (int, tui.Outcome, error) { // never summarised and always ordered first. task := cfg.Objective.Task() + // max_time was validated at load, so a parse error here would be a bug; + // treat it as unbounded rather than failing a run that already passed. + maxDuration, _ := cfg.Tuning.MaxDuration() + + // Settle mode: a run ends only when the agent records an outcome, never + // because its prose happened to sound conclusive. Rook is unattended by + // design - nobody is watching to judge whether "I have finished the audit" + // actually means finished - so an unambiguous ending matters more here than + // almost anywhere. The config may raise or lower it; unset keeps Rook's + // generous default. + maxSettles := cfg.Tuning.MaxSettles + if maxSettles == 0 { + maxSettles = defaultMaxSettles + } + opts := agent.ExecuteWithToolsOptions{ Instructions: instructions + "\n\n## Your objective\n\n" + task, Tools: tools, Skills: loader.Skills, MaxIterations: cfg.MaxIterations, + MaxSettles: maxSettles, + + MaxCalls: cfg.Tuning.MaxCalls, + MaxContinuations: cfg.Tuning.MaxContinuations, + MaxRecoveries: cfg.Tuning.MaxRecoveries, + MaxCycles: cfg.Tuning.MaxCycles, + MaxEmpties: cfg.Tuning.MaxEmpties, + MaxDuration: maxDuration, + LimitCheckpoints: cfg.Tuning.LimitCheckpoints, + + // Empty is the default (compact); the agent layer resolves the string. + ContextStrategy: cfg.Tuning.ContextStrategy, + CompactMinTokens: cfg.Tuning.CompactMinTokens, + CompactMinMessages: cfg.Tuning.CompactMinMessages, + CompactTriggerRatio: cfg.Tuning.CompactTriggerRatio, + ContextWindow: cfg.ContextWindow, + } - // Settle mode: a run ends only when the agent records an outcome, never - // because its prose happened to sound conclusive. Rook is unattended by - // design - nobody is watching to judge whether "I have finished the - // audit" actually means finished - so an unambiguous ending matters more - // here than almost anywhere. A positive value enables it. - MaxSettles: defaultMaxSettles, + // MaxTokens is a pointer so "unset" (provider decides) is distinct from a + // deliberate cap; a positive config value caps a single response. + if cfg.Tuning.MaxTokens > 0 { + opts.MaxTokens = &cfg.Tuning.MaxTokens } // A resumed run replays the earlier conversation, so the agent picks up with @@ -214,7 +259,7 @@ func Run(ctx context.Context, cfg Config) (int, tui.Outcome, error) { meta := session.Meta{ Task: task, Model: cfg.Model, - Provider: cfg.Backend, + Provider: cfg.Provider, Driver: client.Provider(), Workdir: "", } @@ -257,7 +302,7 @@ func Run(ctx context.Context, cfg Config) (int, tui.Outcome, error) { Task: task, Title: cfg.Objective.DisplayTitle(), Model: cfg.Model, - Provider: cfg.Backend, + Provider: cfg.Provider, Workdir: workdir, Plain: cfg.Verbose, Theme: rookTheme, diff --git a/internal/config/config.go b/internal/config/config.go index 51ab589..1819c4d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "strings" + "time" "gopkg.in/yaml.v3" ) @@ -17,21 +18,21 @@ import ( // glm-5.2 is a strong open model well suited to autonomous security work: large // context for reading codebases during source audits, solid tool use, and it is // open/permissive for offensive-security tasks. The model must be one the -// selected backend actually serves, which is why the default backend below is -// the provider that serves this one. +// selected provider actually serves, which is why the default provider below is +// the one that serves this one. const DefaultModel = "glm-5.2" // DefaultMaxIterations bounds how many tool-using turns the agent may take // before it is forced to stop. const DefaultMaxIterations = 10000 -// DefaultBackend is the backend a run targets when --backend and config do not -// select one. +// DefaultProvider is the provider a run targets when --provider and config do +// not select one. // // @note it has to serve DefaultModel. A default pair that cannot talk to each // other is worse than no default, because the failure arrives as a provider // error rather than as a configuration one. -const DefaultBackend = "zai" +const DefaultProvider = "zai" // Config is the fully-resolved Rook configuration. type Config struct { @@ -40,12 +41,12 @@ type Config struct { // per-run subdirectory with status.json and events.jsonl). Empty uses the // built-in default, $XDG_STATE_HOME/rook/runs. RunDir string `yaml:"run_dir"` - // DefaultBackend is the backend used when --backend is not given. - DefaultBackend string `yaml:"default_backend"` - // Backends are the named providers a run can target. Rook ships with one per - // provider it knows; a config file can override their credential or - // endpoint, or add custom model entries. - Backends map[string]Backend `yaml:"backends"` + // DefaultProvider is the provider used when --provider is not given. + DefaultProvider string `yaml:"default_provider"` + // Providers are the named model-provider connections a run can target. Rook + // ships with one for each provider it knows; a config file can override their + // credential or endpoint, or add custom model entries. + Providers map[string]ProviderConfig `yaml:"providers"` // UpdateCheck controls the one call a run makes that is not to a provider: a // lookup of the latest release on GitHub, so an out-of-date binary can say // so. The zero value checks; disable it for an air-gapped or locked-down @@ -62,35 +63,102 @@ type UpdateCheck struct { Disabled bool `yaml:"disabled"` } -// Agent holds the knobs that shape an autonomous run. +// Agent holds the knobs that shape an autonomous run. Most mirror zot's, so a +// run is tuned the same way in either tool; each is optional and zero uses the +// engine's built-in default. type Agent struct { - // Model is the model name driving the agent, resolved against the backend. + // Model is the model name driving the agent, resolved against the provider. Model string `yaml:"model"` // MaxIterations caps how many plan/act/observe cycles the agent may run // before it is forced to stop. MaxIterations int `yaml:"max_iterations"` + // MaxSettles bounds how many times the agent is nudged to record an outcome + // (call _success or _failure) before the run is surfaced as unsettled. Zero + // uses Rook's built-in default, which is generous because a security run + // legitimately writes a long final report before it settles. + MaxSettles int `yaml:"max_settles"` + // MaxCalls caps total tool calls across a run, independently of iterations. + // Zero is unbounded - only max_iterations is a finite default. + MaxCalls int `yaml:"max_calls"` + // MaxTime caps the wall-clock time of a run, as a duration string ("30m", + // "2h", "90s"). Empty is unbounded. + MaxTime string `yaml:"max_time"` + // MaxTokens caps the output tokens of a single model response. Zero is + // unbounded - the model produces its full output. + MaxTokens int `yaml:"max_tokens"` + // MaxToolOutput caps the bytes a single tool result may return before it is + // truncated. Zero uses the built-in default; lower it for a small-context + // endpoint where one large result can overflow the request. + MaxToolOutput int `yaml:"max_tool_output"` + // MaxContinuations caps CONSECUTIVE recovery attempts (a truncated response + // or a retriable error) with no good turn between them. Zero uses the default. + MaxContinuations int `yaml:"max_continuations"` + // MaxRecoveries caps recovery attempts across a whole run, however spaced. + // Zero uses the default. + MaxRecoveries int `yaml:"max_recoveries"` + // MaxCycles is how many times the loop nudges the model out of a detected + // repetition before giving up. Zero uses the default. + MaxCycles int `yaml:"max_cycles"` + // MaxEmpties caps consecutive empty turns before the run bails. Zero uses + // the default. + MaxEmpties int `yaml:"max_empties"` + // LimitCheckpoints are the percentages of a bounded limit at which the model + // is told it is approaching that limit, so it can pace itself. Nil uses the + // default (50, 80, 90); an explicit empty list turns the notices off. + LimitCheckpoints []int `yaml:"limit_checkpoints"` + // ContextStrategy decides what happens as the conversation approaches the + // model's context window: "compact" summarises older history into a + // checkpoint (an extra model call, higher fidelity), "truncate" drops the + // oldest messages to fit. Empty uses the default, "compact". + ContextStrategy string `yaml:"context_strategy"` + // CompactMinTokens, CompactMinMessages and CompactTriggerRatio tune when the + // compact strategy fires. Zero uses the built-in default for each; + // CompactTriggerRatio must be within (0, 1]. + CompactMinTokens int `yaml:"compact_min_tokens"` + CompactMinMessages int `yaml:"compact_min_messages"` + CompactTriggerRatio float64 `yaml:"compact_trigger_ratio"` } -// Backend is a provider Rook can run against. Every provider authenticates with -// a Bearer credential. -type Backend struct { - // Provider names the model provider this backend talks to: "openai", - // "anthropic", "zai" and so on. Empty infers it from the backend's own name, - // so a backend called "groq" needs no further configuration. - Provider string `yaml:"provider"` +// MaxDuration parses Agent.MaxTime into a duration. Empty is zero (unbounded); +// a malformed or negative value is an error so a typo is caught at load. +func (a Agent) MaxDuration() (time.Duration, error) { + value := strings.TrimSpace(a.MaxTime) + if value == "" { + return 0, nil + } + + d, err := time.ParseDuration(value) + if err != nil { + return 0, fmt.Errorf("%q is not a duration (use forms like \"30m\", \"2h\", \"90s\")", a.MaxTime) + } + + if d < 0 { + return 0, fmt.Errorf("%q is negative", a.MaxTime) + } + + return d, nil +} + +// ProviderConfig is a named model-provider connection Rook can run against. +// Every provider authenticates with a Bearer credential. +type ProviderConfig struct { + // Driver names the provider implementation this connection uses: "openai", + // "anthropic", "zai" and so on. Empty infers it from the connection's own + // name, so a provider called "groq" needs no further configuration. + Driver string `yaml:"driver"` // BaseURL overrides the API endpoint. Empty uses the built-in default. - // Required for a "custom" provider. + // Required for a "custom" driver. BaseURL string `yaml:"base_url"` // APIKey is the provider credential. Supports "$ENV_VAR" references, so no // secret need be written to disk; for a built-in it defaults from the // provider's conventional variable. APIKey string `yaml:"api_key"` - // Models holds custom, named model configurations for this backend. When a + // Models holds custom, named model configurations for this provider. When a // run's model name matches a key here, that entry's settings take priority. Models map[string]ModelConfig `yaml:"models"` } -// ModelConfig is a custom model definition under a backend. Any field set here +// ModelConfig is a custom model definition under a provider. Any field set here // overrides the run's defaults when the model is selected. type ModelConfig struct { // Model is the underlying model id to send. Lets a custom name alias a real @@ -98,23 +166,33 @@ type ModelConfig struct { Model string `yaml:"model"` // MaxIterations overrides the global iteration cap for this model. MaxIterations int `yaml:"max_iterations"` - // Provider overrides the backend's provider for this model, so one gateway - // entry can front several. - Provider string `yaml:"provider"` - // APIKey is this model's own credential, overriding the backend's. Supports + // Driver overrides the provider's driver for this model, so one gateway + // connection can front several implementations. + Driver string `yaml:"driver"` + // APIKey is this model's own credential, overriding the provider's. Supports // "$ENV_VAR". APIKey string `yaml:"api_key"` + // Context overrides the model's total context window, in tokens. The escape + // hatch for a serving endpoint whose real ceiling is smaller than the + // model's card - an uncatalogued model is assumed large, so a small upstream + // rejects the request before compaction fires. Zero uses the engine default. + Context int `yaml:"context"` + // Vision says whether this model can be shown images. Off by default; set it + // true for a model served by an endpoint Rook cannot ask (there is no + // catalogue lookup here), so the agent is offered the tool for looking. A + // pointer so "not stated" and "stated false" are distinct. + Vision *bool `yaml:"vision"` } -// builtinBackends are the providers Rook ships with, each seeded from its -// provider's conventional environment variable so exporting that one variable is -// all a run needs. +// builtinProviders are the providers Rook ships with, each seeded from its +// conventional environment variable so exporting that one variable is all a run +// needs. // // The endpoints themselves live in the engine, which is what actually calls // them; duplicating the URLs here would give two places for them to drift. // Ollama is deliberately included: a local model is the right default for // security work on material that must not leave the machine. -var builtinBackends = map[string]struct { +var builtinProviders = map[string]struct { secretEnv string // the provider's conventional credential variable }{ "openai": {secretEnv: "OPENAI_API_KEY"}, @@ -132,14 +210,14 @@ var builtinBackends = map[string]struct { "ollama": {}, } -// BackendProvider returns the provider a backend talks to, inferring it from the -// backend's own name when nothing says otherwise. -func BackendProvider(name string, backend Backend) string { - if p := strings.TrimSpace(backend.Provider); p != "" { - return p +// ProviderDriver returns the driver a provider connection uses, inferring it +// from the connection's own name when nothing says otherwise. +func ProviderDriver(name string, provider ProviderConfig) string { + if d := strings.TrimSpace(provider.Driver); d != "" { + return d } - if _, ok := builtinBackends[name]; ok { + if _, ok := builtinProviders[name]; ok { return name } @@ -153,7 +231,7 @@ func Defaults() Config { Model: DefaultModel, MaxIterations: DefaultMaxIterations, }, - DefaultBackend: DefaultBackend, + DefaultProvider: DefaultProvider, } } @@ -184,51 +262,61 @@ func Load(path string) (Config, error) { return cfg, err } - resolveBackends(&cfg) + resolveProviders(&cfg) - if cfg.DefaultBackend == "" { - cfg.DefaultBackend = DefaultBackend + if cfg.DefaultProvider == "" { + cfg.DefaultProvider = DefaultProvider } return cfg, nil } -// resolveBackends ensures the built-in backends exist and resolves every +// resolveProviders ensures the built-in providers exist and resolves every // credential: a config "$ENV_VAR" reference first, then the provider's // conventional environment variable as a fallback. // // The endpoint is left empty for a built-in. The engine knows each provider's // URL, so filling one in here would create a second copy to drift. -func resolveBackends(cfg *Config) { - if cfg.Backends == nil { - cfg.Backends = map[string]Backend{} +func resolveProviders(cfg *Config) { + if cfg.Providers == nil { + cfg.Providers = map[string]ProviderConfig{} } - for name := range builtinBackends { - if _, ok := cfg.Backends[name]; !ok { - cfg.Backends[name] = Backend{} + for name := range builtinProviders { + if _, ok := cfg.Providers[name]; !ok { + cfg.Providers[name] = ProviderConfig{} } } - for name, b := range cfg.Backends { - builtin, isBuiltin := builtinBackends[name] + for name, p := range cfg.Providers { + builtin, isBuiltin := builtinProviders[name] + + // Whether a custom endpoint was typed for this connection. A built-in + // provider's conventional key is scoped to its own host; forwarding it + // to a URL somebody put in the config is how a provider credential ends + // up in someone else's logs - and for an offensive-security tool aimed + // at endpoints that may be adversarial, that leak matters more than the + // convenience. So the ambient fallback is withheld once base_url is set. + overridden := p.BaseURL != "" - b.APIKey = resolveSecret(b.APIKey) + p.APIKey = resolveSecret(p.APIKey) // Exporting the provider's own variable is enough on its own, which is - // what makes a run possible with no config file at all. - if b.APIKey == "" && isBuiltin && builtin.secretEnv != "" { - b.APIKey = strings.TrimSpace(os.Getenv(builtin.secretEnv)) + // what makes a run possible with no config file at all - but not once + // base_url has been overridden, when the connection must carry a key + // written for it. + if p.APIKey == "" && isBuiltin && builtin.secretEnv != "" && !overridden { + p.APIKey = strings.TrimSpace(os.Getenv(builtin.secretEnv)) } - for mName, mc := range b.Models { + for mName, mc := range p.Models { if mc.APIKey != "" { mc.APIKey = resolveSecret(mc.APIKey) - b.Models[mName] = mc + p.Models[mName] = mc } } - cfg.Backends[name] = b + cfg.Providers[name] = p } } @@ -243,77 +331,88 @@ func resolveSecret(v string) string { return v } -// Selected resolves the default backend into the provider, endpoint, credential, +// Selected resolves the default provider into the driver, endpoint, credential, // model and iteration cap a run uses, applying any custom model definition. // -// It is the one place a backend choice turns into concrete client settings, so +// It is the one place a provider choice turns into concrete client settings, so // a misconfiguration is reported here - before a request is made - rather than // as a provider error mid-run. func (c Config) Selected() (Selection, error) { - b, ok := c.Backends[c.DefaultBackend] + p, ok := c.Providers[c.DefaultProvider] if !ok { - return Selection{}, fmt.Errorf("backend %q is not configured", c.DefaultBackend) + return Selection{}, fmt.Errorf("provider %q is not configured", c.DefaultProvider) } selection := Selection{ - Provider: BackendProvider(c.DefaultBackend, b), - BaseURL: b.BaseURL, - APIKey: b.APIKey, + Driver: ProviderDriver(c.DefaultProvider, p), + BaseURL: p.BaseURL, + APIKey: p.APIKey, Model: c.Agent.Model, MaxIterations: c.Agent.MaxIterations, } - if mc, ok := b.Models[selection.Model]; ok { + if mc, ok := p.Models[selection.Model]; ok { if mc.Model != "" { selection.Model = mc.Model } if mc.MaxIterations > 0 { selection.MaxIterations = mc.MaxIterations } - if mc.Provider != "" { - selection.Provider = mc.Provider + if mc.Driver != "" { + selection.Driver = mc.Driver } if mc.APIKey != "" { selection.APIKey = mc.APIKey } + if mc.Context > 0 { + selection.ContextWindow = mc.Context + } + if mc.Vision != nil { + selection.Vision = *mc.Vision + } } - if selection.Provider == "" { + if selection.Driver == "" { return Selection{}, fmt.Errorf( - "backend %q does not name a model provider (set provider: on the backend or the model)", - c.DefaultBackend) + "provider %q does not name a driver (set driver: on the provider or the model)", + c.DefaultProvider) } // Ollama is local and unauthenticated; everything else needs a key, and // saying so now beats a 401 halfway through a run. - if selection.APIKey == "" && selection.Provider != "ollama" { + if selection.APIKey == "" && selection.Driver != "ollama" { return Selection{}, fmt.Errorf( - "no API key for backend %q (set %s in the environment, or api_key in config)", - c.DefaultBackend, secretEnvName(c.DefaultBackend)) + "no API key for provider %q (set %s in the environment, or api_key in config)", + c.DefaultProvider, secretEnvName(c.DefaultProvider)) } return selection, nil } -// Selection is a resolved backend choice: everything a run needs to build its -// client. +// Selection is a resolved provider choice: everything a run needs to build its +// client. Driver is the provider implementation the engine talks to. type Selection struct { - Provider string + Driver string BaseURL string APIKey string Model string MaxIterations int + // ContextWindow overrides the model's total context window, in tokens. Zero + // uses the engine default. + ContextWindow int + // Vision says whether the model may be shown images, offering the view tool. + Vision bool } -func secretEnvName(backend string) string { - if b, ok := builtinBackends[backend]; ok && b.secretEnv != "" { - return b.secretEnv +func secretEnvName(provider string) string { + if p, ok := builtinProviders[provider]; ok && p.secretEnv != "" { + return p.secretEnv } return "its credential" } -// ScrubBackendSecrets removes every resolved backend credential, backend-level +// ScrubProviderSecrets removes every resolved provider credential, provider-level // and per-model, from the process environment. // // Config keeps the resolved values for the client, while shell commands the @@ -321,16 +420,31 @@ func secretEnvName(backend string) string { // tools: an offensive-security agent runs commands against targets, and a // provider key in the environment of one of those commands is a key that can // leave with it. -func ScrubBackendSecrets(cfg Config) { +func ScrubProviderSecrets(cfg Config) { + // Every built-in provider's conventional credential variable is unset by + // name, whichever provider the run actually uses and whether or not its key + // was adopted into config. A connection with an overridden base_url + // deliberately does not adopt its ambient key (see resolveProviders), so a + // value-only scrub would miss it and leave the variable readable to the + // commands the agent runs against a target - which is the exact leak this + // scrub exists to prevent. + for _, builtin := range builtinProviders { + if builtin.secretEnv != "" { + _ = os.Unsetenv(builtin.secretEnv) + } + } + + // Then scrub by value, to catch a credential carried under a non-conventional + // variable name - a `$VAR` reference or a custom provider's own key. secrets := map[string]bool{} add := func(v string) { if v != "" { secrets[v] = true } } - for _, backend := range cfg.Backends { - add(backend.APIKey) - for _, mc := range backend.Models { + for _, provider := range cfg.Providers { + add(provider.APIKey) + for _, mc := range provider.Models { add(mc.APIKey) } } @@ -346,6 +460,12 @@ func ScrubBackendSecrets(cfg Config) { } } +// Context-overflow strategies. Empty means StrategyCompact. +const ( + StrategyCompact = "compact" + StrategyTruncate = "truncate" +) + // Validate checks the fully-merged configuration. func (c Config) Validate() error { if strings.TrimSpace(c.Agent.Model) == "" { @@ -354,8 +474,28 @@ func (c Config) Validate() error { if c.Agent.MaxIterations <= 0 { return fmt.Errorf("agent.max_iterations must be a positive number") } - if _, ok := c.Backends[c.DefaultBackend]; !ok { - return fmt.Errorf("default backend %q is not configured", c.DefaultBackend) + if _, err := c.Agent.MaxDuration(); err != nil { + return fmt.Errorf("agent.max_time: %w", err) + } + for _, p := range c.Agent.LimitCheckpoints { + if p < 1 || p > 99 { + return fmt.Errorf("agent.limit_checkpoints: %d is out of range (each must be 1-99)", p) + } + } + switch c.Agent.ContextStrategy { + case "", StrategyCompact, StrategyTruncate: + default: + return fmt.Errorf("agent.context_strategy: %q is not valid (use %q or %q)", + c.Agent.ContextStrategy, StrategyCompact, StrategyTruncate) + } + if r := c.Agent.CompactTriggerRatio; r != 0 && (r <= 0 || r > 1) { + return fmt.Errorf("agent.compact_trigger_ratio: %g is out of range (must be within (0, 1])", r) + } + if c.Agent.CompactMinTokens < 0 || c.Agent.CompactMinMessages < 0 { + return fmt.Errorf("agent.compact_min_tokens / compact_min_messages must not be negative") + } + if _, ok := c.Providers[c.DefaultProvider]; !ok { + return fmt.Errorf("default provider %q is not configured", c.DefaultProvider) } return nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index d989ca8..7f956c3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -31,8 +31,8 @@ func isolate(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", t.TempDir()) t.Setenv("ROOK_CONFIG", "") - for name := range builtinBackends { - if env := builtinBackends[name].secretEnv; env != "" { + for name := range builtinProviders { + if env := builtinProviders[name].secretEnv; env != "" { t.Setenv(env, "") } } @@ -54,7 +54,7 @@ func TestAProviderKeyAloneIsEnough(t *testing.T) { t.Fatalf("Selected: %v", err) } - if selection.Provider != "zai" || selection.APIKey != "sk-zai" { + if selection.Driver != "zai" || selection.APIKey != "sk-zai" { t.Errorf("selection = %+v", selection) } @@ -63,25 +63,25 @@ func TestAProviderKeyAloneIsEnough(t *testing.T) { } } -// The default model must be one the default backend actually serves. A pair +// The default model must be one the default provider actually serves. A pair // that cannot talk to each other fails as a provider error mid-run rather than // as a configuration error before it starts. func TestTheDefaultPairAgrees(t *testing.T) { - if _, ok := builtinBackends[DefaultBackend]; !ok { - t.Fatalf("the default backend %q is not built in", DefaultBackend) + if _, ok := builtinProviders[DefaultProvider]; !ok { + t.Fatalf("the default provider %q is not built in", DefaultProvider) } // glm-5.2 is Z.AI's model; if either default moves, the other has to follow - if DefaultBackend != "zai" || DefaultModel != "glm-5.2" { + if DefaultProvider != "zai" || DefaultModel != "glm-5.2" { t.Errorf("defaults are %s/%s - check they still serve each other", - DefaultBackend, DefaultModel) + DefaultProvider, DefaultModel) } } -// The built-in backends are exactly the providers Rook speaks to. Pinning the +// The built-in providers are exactly the providers Rook speaks to. Pinning the // whole set catches an accidental addition and an accidental removal with one // assertion. -func TestBuiltinBackendsAreExactlyTheProviders(t *testing.T) { +func TestBuiltinProvidersAreExactlyTheDrivers(t *testing.T) { isolate(t) cfg, err := Load("") @@ -89,9 +89,9 @@ func TestBuiltinBackendsAreExactlyTheProviders(t *testing.T) { t.Fatalf("Load: %v", err) } - seeded := make([]string, 0, len(cfg.Backends)) + seeded := make([]string, 0, len(cfg.Providers)) - for name := range cfg.Backends { + for name := range cfg.Providers { seeded = append(seeded, name) } @@ -103,35 +103,35 @@ func TestBuiltinBackendsAreExactlyTheProviders(t *testing.T) { } if !reflect.DeepEqual(seeded, want) { - t.Errorf("built-in backends:\n got %v\nwant %v", seeded, want) + t.Errorf("built-in providers:\n got %v\nwant %v", seeded, want) } for _, name := range seeded { - if BackendProvider(name, cfg.Backends[name]) == "" { - t.Errorf("backend %q names no provider", name) + if ProviderDriver(name, cfg.Providers[name]) == "" { + t.Errorf("provider %q resolves to no driver", name) } } } -// A backend named after a provider needs no further configuration - the name is -// the provider. -func TestTheBackendNameInfersTheProvider(t *testing.T) { +// A provider connection named after a driver needs no further configuration - +// the name is the driver. +func TestTheProviderNameInfersTheDriver(t *testing.T) { tests := []struct { - name string - backend Backend - want string + name string + provider ProviderConfig + want string }{ {name: "groq", want: "groq"}, {name: "openai", want: "openai"}, - {name: "mygateway", backend: Backend{Provider: "custom"}, want: "custom"}, + {name: "mygateway", provider: ProviderConfig{Driver: "custom"}, want: "custom"}, {name: "mygateway", want: ""}, - {name: "openai", backend: Backend{Provider: "anthropic"}, want: "anthropic"}, + {name: "openai", provider: ProviderConfig{Driver: "anthropic"}, want: "anthropic"}, } for _, test := range tests { - if got := BackendProvider(test.name, test.backend); got != test.want { - t.Errorf("BackendProvider(%q, %+v) = %q, want %q", - test.name, test.backend, got, test.want) + if got := ProviderDriver(test.name, test.provider); got != test.want { + t.Errorf("ProviderDriver(%q, %+v) = %q, want %q", + test.name, test.provider, got, test.want) } } } @@ -148,7 +148,7 @@ func TestAMissingKeyIsReportedUpFront(t *testing.T) { _, err = cfg.Selected() if err == nil { - t.Fatal("a backend with no credential must be rejected") + t.Fatal("a provider with no credential must be rejected") } // the message has to name the variable to export @@ -158,7 +158,7 @@ func TestAMissingKeyIsReportedUpFront(t *testing.T) { } // Ollama is local and unauthenticated. Demanding a key would make the one -// backend that never sends data off the machine the hardest to use - which is +// provider that never sends data off the machine the hardest to use - which is // backwards for security work on sensitive material. func TestOllamaNeedsNoKey(t *testing.T) { isolate(t) @@ -168,20 +168,20 @@ func TestOllamaNeedsNoKey(t *testing.T) { t.Fatalf("Load: %v", err) } - cfg.DefaultBackend = "ollama" + cfg.DefaultProvider = "ollama" selection, err := cfg.Selected() if err != nil { t.Fatalf("Selected: %v", err) } - if selection.Provider != "ollama" { - t.Errorf("provider = %q, want ollama", selection.Provider) + if selection.Driver != "ollama" { + t.Errorf("provider = %q, want ollama", selection.Driver) } } -// A backend that names no provider cannot resolve, and says so. -func TestAnUnknownBackendIsRejected(t *testing.T) { +// A provider connection that names no driver cannot resolve, and says so. +func TestAnUnknownProviderIsRejected(t *testing.T) { isolate(t) cfg, err := Load("") @@ -189,17 +189,17 @@ func TestAnUnknownBackendIsRejected(t *testing.T) { t.Fatalf("Load: %v", err) } - cfg.DefaultBackend = "nowhere" + cfg.DefaultProvider = "nowhere" if _, err := cfg.Selected(); err == nil { - t.Fatal("an unconfigured backend must be rejected") + t.Fatal("an unconfigured provider must be rejected") } - cfg.Backends["nowhere"] = Backend{APIKey: "sk-test"} + cfg.Providers["nowhere"] = ProviderConfig{APIKey: "sk-test"} _, err = cfg.Selected() if err == nil { - t.Fatal("a backend naming no provider must be rejected") + t.Fatal("a provider naming no driver must be rejected") } if !strings.Contains(err.Error(), "provider") { @@ -213,8 +213,8 @@ func TestAnEnvReferenceIsExpanded(t *testing.T) { t.Setenv("MY_PROVIDER_KEY", "sk-from-env") path := writeConfig(t, ` -default_backend: openai -backends: +default_provider: openai +providers: openai: api_key: '$MY_PROVIDER_KEY' `) @@ -224,12 +224,12 @@ backends: t.Fatalf("Load: %v", err) } - if got := cfg.Backends["openai"].APIKey; got != "sk-from-env" { + if got := cfg.Providers["openai"].APIKey; got != "sk-from-env" { t.Errorf("key = %q, want the expanded value", got) } path = writeConfig(t, ` -backends: +providers: openai: api_key: '${MY_PROVIDER_KEY}' `) @@ -239,7 +239,7 @@ backends: t.Fatalf("Load: %v", err) } - if got := cfg.Backends["openai"].APIKey; got != "sk-from-env" { + if got := cfg.Providers["openai"].APIKey; got != "sk-from-env" { t.Errorf("braced key = %q", got) } } @@ -251,8 +251,8 @@ func TestAnUnsetEnvReferenceResolvesToNothing(t *testing.T) { t.Setenv("ROOK_TEST_UNSET", "") path := writeConfig(t, ` -default_backend: openai -backends: +default_provider: openai +providers: openai: api_key: '$ROOK_TEST_UNSET' `) @@ -262,7 +262,7 @@ backends: t.Fatalf("Load: %v", err) } - if got := cfg.Backends["openai"].APIKey; got != "" { + if got := cfg.Providers["openai"].APIKey; got != "" { t.Errorf("key = %q, want nothing", got) } } @@ -276,11 +276,11 @@ func TestEnvOverridesFile(t *testing.T) { path := writeConfig(t, ` agent: model: from-file -default_backend: openai +default_provider: openai `) t.Setenv("ROOK_AGENT_MODEL", "from-env") - t.Setenv("ROOK_DEFAULT_BACKEND", "groq") + t.Setenv("ROOK_DEFAULT_PROVIDER", "groq") cfg, err := Load(path) if err != nil { @@ -291,8 +291,8 @@ default_backend: openai t.Errorf("model = %q, want from-env", cfg.Agent.Model) } - if cfg.DefaultBackend != "groq" { - t.Errorf("default backend = %q, want groq", cfg.DefaultBackend) + if cfg.DefaultProvider != "groq" { + t.Errorf("default provider = %q, want groq", cfg.DefaultProvider) } } @@ -305,17 +305,17 @@ func TestCustomModelEntry(t *testing.T) { path := writeConfig(t, ` agent: model: fast -default_backend: mygateway -backends: +default_provider: mygateway +providers: mygateway: - provider: custom + driver: custom base_url: 'https://gateway.example.com/v1' api_key: sk-gateway models: fast: model: gpt-5 max_iterations: 50 - provider: openai + driver: openai api_key: $OPENAI_API_KEY `) @@ -337,27 +337,27 @@ backends: t.Errorf("max iterations = %d, want 50", selection.MaxIterations) } - if selection.Provider != "openai" || selection.APIKey != "sk-openai" { + if selection.Driver != "openai" || selection.APIKey != "sk-openai" { t.Errorf("the model's own provider and key must win: %+v", selection) } if selection.BaseURL != "https://gateway.example.com/v1" { - t.Errorf("base URL = %q, want the backend's", selection.BaseURL) + t.Errorf("base URL = %q, want the provider's", selection.BaseURL) } } // Scrubbing removes every resolved credential from the environment. An // offensive-security agent runs commands against targets, and a provider key in // one of those commands' environment is a key that can leave with it. -func TestScrubBackendSecrets(t *testing.T) { +func TestScrubProviderSecrets(t *testing.T) { isolate(t) t.Setenv("ZAI_API_KEY", "sk-zai") t.Setenv("OPENAI_API_KEY", "sk-openai") t.Setenv("ROOK_TEST_UNRELATED", "keep-me") path := writeConfig(t, ` -default_backend: zai -backends: +default_provider: zai +providers: zai: api_key: $ZAI_API_KEY models: @@ -370,7 +370,7 @@ backends: t.Fatalf("Load: %v", err) } - ScrubBackendSecrets(cfg) + ScrubProviderSecrets(cfg) if got := os.Getenv("ZAI_API_KEY"); got != "" { t.Errorf("ZAI_API_KEY survived scrubbing: %q", got) @@ -385,13 +385,13 @@ backends: } // the config keeps what the client needs - if cfg.Backends["zai"].APIKey != "sk-zai" { + if cfg.Providers["zai"].APIKey != "sk-zai" { t.Error("scrubbing must not empty the resolved config") } } // Validate catches a missing model, a non-positive iteration cap, and a default -// backend the config does not define - before any request reaches a provider. +// provider the config does not define - before any request reaches a provider. func TestValidateRejectsBadConfigs(t *testing.T) { isolate(t) t.Setenv("ZAI_API_KEY", "sk-zai") @@ -426,16 +426,16 @@ func TestValidateRejectsBadConfigs(t *testing.T) { } cfg.Agent.MaxIterations = goodIter - // Unknown default backend. - goodBackend := cfg.DefaultBackend - cfg.DefaultBackend = "nowhere" + // Unknown default provider. + goodProvider := cfg.DefaultProvider + cfg.DefaultProvider = "nowhere" if err := cfg.Validate(); err == nil { - t.Error("an unknown default backend must be rejected") + t.Error("an unknown default provider must be rejected") } - cfg.DefaultBackend = goodBackend + cfg.DefaultProvider = goodProvider } -// secretEnvName returns the conventional variable for a built-in backend, and a +// secretEnvName returns the conventional variable for a built-in provider, and a // generic fallback for one the tool does not know. func TestSecretEnvName(t *testing.T) { if got := secretEnvName("zai"); got != "ZAI_API_KEY" { @@ -498,7 +498,7 @@ func TestDefaultRunDir(t *testing.T) { func TestApplyEnvRejectsBadIntegers(t *testing.T) { isolate(t) - path := writeConfig(t, "default_backend: zai\n") + path := writeConfig(t, "default_provider: zai\n") t.Setenv("ZAI_API_KEY", "sk-zai") t.Setenv("ROOK_AGENT_MAX_ITERATIONS", "not-a-number") @@ -587,3 +587,164 @@ func TestConfigPathHomeFallback(t *testing.T) { t.Errorf("home fallback = %q", got) } } + +// A built-in provider's conventional env key is withheld once base_url is +// overridden: forwarding OPENAI_API_KEY to a URL someone typed is exactly the +// leak an offensive-security tool must not create. +func TestOverriddenBaseURLWithholdsTheAmbientKey(t *testing.T) { + isolate(t) + t.Setenv("OPENAI_API_KEY", "sk-openai") + + // No base_url: the conventional key seeds the connection. + cfg, err := Load(writeConfig(t, "default_provider: openai\nproviders:\n openai:\n")) + if err != nil { + t.Fatalf("Load: %v", err) + } + if got := cfg.Providers["openai"].APIKey; got != "sk-openai" { + t.Fatalf("without base_url the ambient key should seed the connection, got %q", got) + } + + // base_url set: the ambient key is withheld, so the connection has no key. + cfg, err = Load(writeConfig(t, "default_provider: openai\nproviders:\n openai:\n base_url: https://gw.example.com/v1\n")) + if err != nil { + t.Fatalf("Load: %v", err) + } + if got := cfg.Providers["openai"].APIKey; got != "" { + t.Errorf("an overridden base_url must not inherit OPENAI_API_KEY, got %q", got) + } +} + +// MaxDuration parses a duration string, treats empty as unbounded, and rejects +// a malformed or negative value. +func TestMaxDuration(t *testing.T) { + if d, err := (Agent{MaxTime: ""}).MaxDuration(); err != nil || d != 0 { + t.Errorf("empty = (%v, %v), want (0, nil)", d, err) + } + if d, err := (Agent{MaxTime: "30m"}).MaxDuration(); err != nil || d.Minutes() != 30 { + t.Errorf("30m = (%v, %v)", d, err) + } + if _, err := (Agent{MaxTime: "soon"}).MaxDuration(); err == nil { + t.Error("a malformed duration must be rejected") + } + if _, err := (Agent{MaxTime: "-5m"}).MaxDuration(); err == nil { + t.Error("a negative duration must be rejected") + } +} + +// Validate catches the new tuning knobs' bad values before a run starts. +func TestValidateRejectsBadTuning(t *testing.T) { + isolate(t) + t.Setenv("ZAI_API_KEY", "sk-zai") + + base := func(t *testing.T) Config { + cfg, err := Load("") + if err != nil { + t.Fatalf("Load: %v", err) + } + return cfg + } + + cfg := base(t) + cfg.Agent.ContextStrategy = "sideways" + if err := cfg.Validate(); err == nil { + t.Error("an unknown context_strategy must be rejected") + } + + cfg = base(t) + cfg.Agent.CompactTriggerRatio = 1.5 + if err := cfg.Validate(); err == nil { + t.Error("a compact_trigger_ratio above 1 must be rejected") + } + + cfg = base(t) + cfg.Agent.LimitCheckpoints = []int{50, 120} + if err := cfg.Validate(); err == nil { + t.Error("an out-of-range limit checkpoint must be rejected") + } + + cfg = base(t) + cfg.Agent.MaxTime = "nope" + if err := cfg.Validate(); err == nil { + t.Error("a malformed max_time must be rejected") + } + + // A run with valid tuning still passes. + cfg = base(t) + cfg.Agent.ContextStrategy = "truncate" + cfg.Agent.CompactTriggerRatio = 0.9 + cfg.Agent.LimitCheckpoints = []int{50, 90} + cfg.Agent.MaxTime = "2h" + if err := cfg.Validate(); err != nil { + t.Errorf("valid tuning must pass: %v", err) + } +} + +// A model entry's context and vision overrides resolve into the Selection, so a +// custom endpoint can state a ceiling and that it can be shown images. +func TestModelCapabilityOverridesResolve(t *testing.T) { + isolate(t) + t.Setenv("OPENAI_API_KEY", "sk-openai") + + yes := true + cfg, err := Load(writeConfig(t, ` +agent: + model: fast +default_provider: openai +providers: + openai: + models: + fast: {} +`)) + if err != nil { + t.Fatalf("Load: %v", err) + } + + // default: no override -> zero/false + if sel, _ := cfg.Selected(); sel.ContextWindow != 0 || sel.Vision { + t.Errorf("unset overrides should be zero/false, got %+v", sel) + } + + m := cfg.Providers["openai"].Models["fast"] + m.Context = 8000 + m.Vision = &yes + cfg.Providers["openai"].Models["fast"] = m + + sel, err := cfg.Selected() + if err != nil { + t.Fatalf("Selected: %v", err) + } + if sel.ContextWindow != 8000 || !sel.Vision { + t.Errorf("overrides did not resolve: %+v", sel) + } +} + +// A provider key must be scrubbed from the agent's environment even when that +// provider has an overridden base_url (so its ambient key was never adopted +// into config). Regression: the base_url scoping fix must not leave the key +// readable to the commands the agent runs against a target. +func TestScrubRemovesKeysOfOverriddenProviders(t *testing.T) { + isolate(t) + t.Setenv("ZAI_API_KEY", "sk-zai") + t.Setenv("OPENAI_API_KEY", "sk-openai") + + // Run zai, but the openai connection has a custom base_url (so it does not + // adopt OPENAI_API_KEY as its own credential). + cfg, err := Load(writeConfig(t, ` +default_provider: zai +providers: + openai: + base_url: https://gw.example.com/v1 +`)) + if err != nil { + t.Fatalf("Load: %v", err) + } + + ScrubProviderSecrets(cfg) + + if got := os.Getenv("OPENAI_API_KEY"); got != "" { + t.Errorf("OPENAI_API_KEY survived scrubbing despite base_url override: %q", got) + } + if got := os.Getenv("ZAI_API_KEY"); got != "" { + t.Errorf("the active provider key survived scrubbing: %q", got) + } +} diff --git a/internal/config/env.go b/internal/config/env.go index b452f84..7376813 100644 --- a/internal/config/env.go +++ b/internal/config/env.go @@ -10,9 +10,9 @@ import ( // applyEnv overrides scalar config fields from ROOK_* environment variables, // where the variable name is the field's yaml path upper-cased with dots as -// underscores (e.g. agent.model -> ROOK_AGENT_MODEL, default_backend -> -// ROOK_DEFAULT_BACKEND). Backend credentials are not set this way; they come -// from each backend's own environment variable (see builtinBackends). +// underscores (e.g. agent.model -> ROOK_AGENT_MODEL, default_provider -> +// ROOK_DEFAULT_PROVIDER). Provider credentials are not set this way; they come +// from each provider's own environment variable (see builtinProviders). func applyEnv(cfg *Config) error { return applyEnvStruct(reflect.ValueOf(cfg).Elem(), "ROOK") } @@ -36,7 +36,7 @@ func applyEnvStruct(v reflect.Value, prefix string) error { } continue } - // Maps (e.g. backends) are not settable via a scalar env var; configure + // Maps (e.g. providers) are not settable via a scalar env var; configure // them in the file. Skip rather than error. if fv.Kind() == reflect.Slice || fv.Kind() == reflect.Map { continue