Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ security:
require_scanners: false # true = block stories when applicable scanners are missing/failed (strict coverage) instead of degrading gracefully
auto_learn: true # grow the knowledge base from confirmed high+ findings
kb_path: "" # default <state_dir>/security/knowledge.json
strict_shell_commands: false # true = config-supplied commands (qa criteria, autoresearch metric) also reject | ; && || & > < >> 2>&1 — use command_list for multi-step
billing:
default_rate: 150.0
currency: USD
Expand Down Expand Up @@ -600,7 +601,7 @@ Closing summary (24 PRs merged across the bulletproofing pass):
49. ~~Ghost API error body redaction + SetTemplateFlag parameterized + schema-evolution + VXD_SHELL trust-boundary doc~~ — DONE (PR #67).
50. ~~YAML criterion path containment + sql_query_returns read-only gate (sqlsafety package split out of cli)~~ — DONE (PR #68).
51. ~~Prompt file 0o644 → 0o600 (registry.go + tmux_runner.go)~~ — DONE (this commit). Prompt content carrying DSNs / WAVE_CONTEXT / acceptance criteria no longer readable by non-owner users on a shared dispatch host.
52. **YAML pipe/semicolon caveat** — DOCUMENTED. `ValidateConfigShellCommand` blocks command substitution but deliberately allows `|`, `;`, `&&` for legitimate multi-step QA commands. An operator who copy-pastes a malicious vxd.yaml can still chain `; curl evil` — this is a documented operator trust boundary, not an oversight. The blocklist is one of three layers; the others are: (a) commands run only when the operator explicitly invokes a requirement that triggers QA, (b) the dashboard auth gate prevents remote requirement submission.
52. ~~YAML pipe/semicolon caveat~~ — CLOSED by opt-in strict mode (P0-03, 2026-07-16). Default mode still blocks only command substitution and deliberately allows `|`, `;`, `&&` for legitimate multi-step QA commands (documented operator trust boundary, backward compatible). New `security.strict_shell_commands: true` (recommended for SaaS-hosted/multi-tenant deploys) also rejects `|`, `;`, `&&`, `||`, `&`, `>`, `<`, `>>`, `2>&1` in config-supplied commands, enforced at BOTH load time (`config.Validate` → `config.ValidateShellCommand`, single source; engine + autoresearch delegate) and runtime (`EvaluateCriteriaWithMode`, `MetricHarness.StrictShellCommands`). Multi-step work is expressed via the new `command_list` criterion field (entries run sequentially, all must pass, mutually exclusive with `command`); the SP5 db-criteria fields (`command`, `sql`, `expected_rows`, `schema_baseline`) are now also mapped from vxd.yaml `qa.success_criteria` into the engine (previously a dangling wire — only the 6 basic kinds validated). Tests: `TestValidateShellCommand_StrictMode` (16 cases), `TestValidateShellCommand_StrictModeViaConfigValidate`, `TestConfigLoad_CommandListAsAlternative`, `TestEvaluate_MigrationSucceeds_CommandListRunsSequentially`, `TestEvaluate_MigrationSucceeds_StrictShellMode`.
53. ~~Errcheck cleanup + lint job blocking~~ — DONE. `golangci-lint` now reports **0 issues** across the project (`-default standard`, ~5 minute timeout). 44 silent event-store / projection-store `Append`/`Project` failures across `internal/cli` + `internal/engine` now log with full story-ID context; 15 dangerous `f.Write`/`db.Exec`/artifact-store sites now return wrapped errors; best-effort cleanup sites carry explicit `_ =` discards with one-line rationale; `.golangci.yml` excludes benign noise (`fmt.Fprint*` to stdout, `(io.Closer).Close`, HTTP body close, tabwriter Flush) and widens the test-file exemption to cover all linters. The `lint` job in `.github/workflows/ci.yml` lost its `continue-on-error: true` — it is now a blocking gate.
56. ~~CLI IO-seam refactor + coverage uplift~~ — DONE. Introduced `auditDirOverride` (improve.go) and `stateDirOverride` (autoresearch.go) package-level test seams so the production code path still reads CWD / `VXD_STATE_DIR` / `$HOME` but tests can swap directories with no `chdir`. Removed the older `withChdir` helper from `improve_commands_test.go` (chdir broke parallel-test isolation). New test files: `autoresearch_io_test.go` (loadConfigForAutoresearch / openEventStore / countWinsLosses), `backup_logs_test.go` (runBackup end-to-end + runLogs missing-file & stream paths), `db_helpers_test.go` (findDBByNameOrID, isTerminal, dbProviderFor, dockerProviderFor failure paths), `db_subcommands_test.go` (every `vxd db <sub>` CLI command drives its early-error path with devdb disabled), `devdb_provider_test.go` (newDevDBProvider covers null/docker/ghost/unknown branches), `resume_helpers_test.go` (pickRuntime priority + fallback, newDevDBLifecycle disabled/docker/bad-ghost paths, runDevDBOrphanRecovery no-op guard, runResume entry-point wiring), `extra_commands_test.go` (Execute, autoresearch stop/evolve/start/hypotheses/status), `req_helpers_test.go` (buildPlanningClient routing per provider). cli coverage: **58.3% → 72.9%** (+14.6 points). Falls short of 80% target; remaining gap requires fakes for docker/gh/claude CLI, tracked as a separate follow-up.

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ Run `vxd init` to generate `vxd.yaml` with sensible defaults, then customize:
| `runtimes` | Map of named CLI runtime definitions — command, args, supported models, and idle/permission detection patterns | Includes built-in entries for `claude-code`, `codex`, `gemini`, `swe-agent`; each supports optional `runner: docker\|ssh` |
| `billing` | Hourly consulting rate, currency, Fibonacci-to-hours range mapping, and LLM cost accounting mode | `default_rate: 150.0`, `currency: USD`, `llm_costs.mode: subscription` |
| `qa` | Declarative success criteria evaluated after each story (output_contains, file_exists, file_contains, exit_code_zero, etc.); `disable_pre_merge_verify` (turn off the per-story pre-merge build/test gate); and the requirement-completion gate — `disable_completion_gate` (turn off) + `completion_fix_cycles` (auto-fix attempts against a red composed mainline before blocking; `0`→default 2, negative→hard gate). The completion gate verifies the merged mainline and emits `REQ_BLOCKED` instead of `REQ_COMPLETED` when it cannot make the build/tests green. | No criteria by default; standard lint/build/test always run; `disable_pre_merge_verify: false`, `disable_completion_gate: false`, `completion_fix_cycles: 2` |
| `security` | Security agent: per-story pre-merge security gate (scanners + LLM threat-model review against a growable OWASP/CWE knowledge base). `disable_gate` (turn the gate off), `gate_severity` (build-pausing block threshold — default `critical` so only leaked secrets/confirmed injection pause a build; tighten to `high` for stricter gating), `require_scanners` (strict coverage: block a story when applicable scanners are missing/failed instead of degrading gracefully), `auto_learn` (grow the KB from confirmed findings), `kb_path` (KB location). Standalone audits via `vxd security scan` report high/medium too. | `disable_gate: false`, `gate_severity: critical`, `require_scanners: false`, `auto_learn: true`, `kb_path: <state_dir>/security/knowledge.json` |
| `security` | Security agent: per-story pre-merge security gate (scanners + LLM threat-model review against a growable OWASP/CWE knowledge base). `disable_gate` (turn the gate off), `gate_severity` (build-pausing block threshold — default `critical` so only leaked secrets/confirmed injection pause a build; tighten to `high` for stricter gating), `require_scanners` (strict coverage: block a story when applicable scanners are missing/failed instead of degrading gracefully), `auto_learn` (grow the KB from confirmed findings), `kb_path` (KB location), `strict_shell_commands` (reject pipes/`;`/`&&`/redirection in config-supplied commands — QA success-criteria commands and the autoresearch metric command; multi-step work uses `command_list` instead; recommended for SaaS-hosted/multi-tenant deploys). Standalone audits via `vxd security scan` report high/medium too. | `disable_gate: false`, `gate_severity: critical`, `require_scanners: false`, `auto_learn: true`, `kb_path: <state_dir>/security/knowledge.json`, `strict_shell_commands: false` |
| `sla` | Per-Fibonacci-point maximum story duration in minutes; `auto_escalate` promotes breached stories to the next tier | `1pt→60m`, `2pt→120m`, `3pt→240m`, `5pt→480m`, `8pt→960m`, `13pt→1920m`; `auto_escalate: false` |
| `secrets` | Secrets provider: `env` (default, reads from environment) or `vault` (HashiCorp Vault KV v2) | `provider: env`; Vault settings: `vault_mount: secret`, `vault_path: vxd` |
| `notify` | Outbound Slack webhook URL and per-event triggers. A configured webhook always sends `PIPELINE_STALLED` (human-intervention signal); `notify_on_sla` additionally sends `STORY_SLA_BREACHED`; `notify_on_complete` additionally sends terminal requirement outcomes (`REQ_COMPLETED` / `REQ_BLOCKED`) | Disabled by default (empty `slack_webhook_url`); both flags `false` |
Expand Down
22 changes: 9 additions & 13 deletions internal/autoresearch/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,11 @@ import (
// re-compiled this regex on every call.
var metricLastFloatRe = regexp.MustCompile(`-?\d+(?:\.\d+)?`)

// validateMetricCommand rejects YAML-supplied metric commands containing
// command-substitution patterns. Mirrors engine.ValidateConfigShell-
// Command but avoids the cross-package import (engine depends on this
// package transitively in some paths). Kept here as a small duplicate
// rather than introduce an import cycle risk.
func validateMetricCommand(cmd string) error {
for _, seq := range []string{"$(", "`", "$(("} {
if strings.Contains(cmd, seq) {
return fmt.Errorf("config-supplied command contains %q; rewrite to avoid embedded expressions", seq)
}
}
return nil
// validateMetricCommand rejects YAML-supplied metric commands via the
// canonical config.ValidateShellCommand (command substitution always;
// pipes/chaining/redirection too when security.strict_shell_commands is on).
func validateMetricCommand(cmd string, strict bool) error {
return config.ValidateShellCommand(cmd, strict)
}

// Tiebreaker is the LLM-based judge invoked when two scores fall within
Expand All @@ -47,6 +40,9 @@ type Tiebreaker interface {
type MetricHarness struct {
Metric config.AutoresearchMetric
Tiebreaker Tiebreaker
// StrictShellCommands mirrors security.strict_shell_commands for the
// metric command validation.
StrictShellCommands bool
}

// Measure runs the metric command in the given worktree, returning a Score.
Expand All @@ -56,7 +52,7 @@ func (h *MetricHarness) Measure(ctx context.Context, worktree string, baseline f
if h.Metric.Command == "" {
return Score{}, errors.New("metric command is empty")
}
if err := validateMetricCommand(h.Metric.Command); err != nil {
if err := validateMetricCommand(h.Metric.Command, h.StrictShellCommands); err != nil {
return Score{}, fmt.Errorf("metric command rejected: %w", err)
}
cmd := shellexec.CommandContext(ctx, h.Metric.Command)
Expand Down
3 changes: 2 additions & 1 deletion internal/cli/autoresearch.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ func buildLiveCoordinator(repoDir string, cfg config.Config) (*autoresearch.Coor
}

metric := &autoresearch.MetricHarness{
Metric: cfg.Autoresearch.Metric,
Metric: cfg.Autoresearch.Metric,
StrictShellCommands: cfg.Security.StrictShellCommands,
Tiebreaker: &autoresearch.LLMTiebreaker{
Client: ll,
Model: model,
Expand Down
14 changes: 10 additions & 4 deletions internal/cli/resume.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,12 +688,18 @@ func buildQAConfig(cfg config.Config, projectDir, repoDir string) engine.QAConfi
}
for _, sc := range cfg.QA.SuccessCriteria {
qaCfg.SuccessCriteria = append(qaCfg.SuccessCriteria, engine.Criterion{
Kind: engine.CriterionKind(sc.Kind),
Value: sc.Value,
Path: sc.Path,
Message: sc.Message,
Kind: engine.CriterionKind(sc.Kind),
Value: sc.Value,
Path: sc.Path,
Message: sc.Message,
Command: sc.Command,
CommandList: sc.CommandList,
SQL: sc.SQL,
ExpectedRows: sc.ExpectedRows,
SchemaBaseline: sc.SchemaBaseline,
})
}
qaCfg.StrictShellCommands = cfg.Security.StrictShellCommands
return qaCfg
}

Expand Down
43 changes: 42 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,14 @@ type SecurityConfig struct {
// KBPath overrides where the knowledge base persists. Empty ⇒
// <state_dir>/security/knowledge.json.
KBPath string `yaml:"kb_path,omitempty"`
// StrictShellCommands hardens ValidateShellCommand for config-supplied
// command strings (qa.success_criteria commands, autoresearch metric
// command): in addition to command substitution, it rejects pipes,
// `;`/`&&`/`||` chaining, background `&`, and redirection. Multi-step
// work is expressed via `command_list` instead. Default false for
// backward compatibility — single-operator installs legitimately pipe
// QA output. SaaS-hosted / multi-tenant deploys should set true.
StrictShellCommands bool `yaml:"strict_shell_commands,omitempty"`
}

// SuccessCriterion defines a declarative QA check.
Expand All @@ -258,6 +266,21 @@ type SuccessCriterion struct {
Value string `yaml:"value,omitempty"`
Path string `yaml:"path,omitempty"`
Message string `yaml:"message,omitempty"`

// Command is the shell command for migration_succeeds. Subject to
// ValidateShellCommand in the mode selected by
// security.strict_shell_commands.
Command string `yaml:"command,omitempty"`
// CommandList is the strict-mode-friendly alternative to Command:
// VXD runs the entries sequentially (all must succeed), so multi-step
// work needs no shell chaining metacharacters in the YAML. Mutually
// exclusive with Command.
CommandList []string `yaml:"command_list,omitempty"`
// SQL, ExpectedRows, SchemaBaseline configure the DB-touching criteria
// (sql_query_returns, schema_changed) shipped in SP5.
SQL string `yaml:"sql,omitempty"`
ExpectedRows *int `yaml:"expected_rows,omitempty"`
SchemaBaseline string `yaml:"schema_baseline,omitempty"`
}

// AutoresearchConfig configures the per-repo autoresearch experiment harness.
Expand Down Expand Up @@ -576,11 +599,29 @@ func (c Config) Validate() error {
"output_contains": true, "output_not_contains": true,
"file_exists": true, "file_contains": true,
"file_not_empty": true, "exit_code_zero": true,
"migration_succeeds": true, "schema_changed": true, "sql_query_returns": true,
}
for i, sc := range c.QA.SuccessCriteria {
if !validCriterionKinds[sc.Kind] {
return fmt.Errorf("qa.success_criteria[%d].kind must be one of output_contains, output_not_contains, file_exists, file_contains, file_not_empty, exit_code_zero; got %q", i, sc.Kind)
return fmt.Errorf("qa.success_criteria[%d].kind must be one of output_contains, output_not_contains, file_exists, file_contains, file_not_empty, exit_code_zero, migration_succeeds, schema_changed, sql_query_returns; got %q", i, sc.Kind)
}
if sc.Command != "" && len(sc.CommandList) > 0 {
return fmt.Errorf("qa.success_criteria[%d]: command and command_list are mutually exclusive", i)
}
// Fail fast at load time on commands the runtime would reject —
// a copy-pasted hostile vxd.yaml should not survive `vxd config
// validate`, let alone reach dispatch.
if err := ValidateShellCommand(sc.Command, c.Security.StrictShellCommands); err != nil {
return fmt.Errorf("qa.success_criteria[%d].command: %w", i, err)
}
for j, entry := range sc.CommandList {
if err := ValidateShellCommand(entry, c.Security.StrictShellCommands); err != nil {
return fmt.Errorf("qa.success_criteria[%d].command_list[%d]: %w", i, j, err)
}
}
}
if err := ValidateShellCommand(c.Autoresearch.Metric.Command, c.Security.StrictShellCommands); err != nil {
return fmt.Errorf("autoresearch.metric.command: %w", err)
}

if err := c.Autoresearch.validate(); err != nil {
Expand Down
65 changes: 65 additions & 0 deletions internal/config/shell_command.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package config

import (
"fmt"
"strings"
)

// shellSubstitutionPatterns are rejected in EVERY mode: command substitution
// is the one construct that turns a static YAML command string into arbitrary
// nested code execution.
var shellSubstitutionPatterns = []struct{ seq, name string }{
{"$((", "arithmetic expansion"}, // checked before $( so the message is precise
{"$(", "POSIX command substitution `$(...)`"},
{"`", "backtick command substitution"},
}

// shellChainPatterns are rejected only in strict mode
// (security.strict_shell_commands: true). Legitimate multi-step commands can
// be expressed with the `command_list` criterion field instead — VXD chains
// the entries itself, so the YAML never needs shell metacharacters.
//
// Multi-character sequences are listed before their single-character prefixes
// so error messages name the most specific construct.
var shellChainPatterns = []struct{ seq, name string }{
{"2>&1", "stderr redirection `2>&1`"},
{"&&", "AND chaining `&&`"},
{"||", "OR chaining `||`"},
{">>", "append redirection `>>`"},
{"|", "pipe `|`"},
{";", "statement chaining `;`"},
{"&", "background execution `&`"},
{">", "output redirection `>`"},
{"<", "input redirection `<`"},
}

// ValidateShellCommand rejects YAML-supplied shell-command strings that could
// smuggle extra commands past the operator. The threat model is an operator
// who pastes a vxd.yaml from a hostile or untrusted source — these command
// strings flow straight to `sh -c`.
//
// Non-strict (default): only command substitution is rejected; pipes and
// chains stay allowed because legitimate QA commands use them
// (`go test ./... | grep PASS`). Strict (security.strict_shell_commands:
// true — recommended for SaaS-hosted / multi-tenant deploys): pipes,
// chaining, background execution, and redirection are rejected too; use
// `command_list` to express multi-step work.
func ValidateShellCommand(cmd string, strict bool) error {
if cmd == "" {
return nil
}
for _, p := range shellSubstitutionPatterns {
if strings.Contains(cmd, p.seq) {
return fmt.Errorf("config-supplied command contains %s; rewrite to avoid embedded expressions", p.name)
}
}
if !strict {
return nil
}
for _, p := range shellChainPatterns {
if strings.Contains(cmd, p.seq) {
return fmt.Errorf("config-supplied command contains %s, rejected by security.strict_shell_commands; use command_list to express multi-step commands", p.name)
}
}
return nil
}
Loading
Loading