diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cc4b07..fdb3aea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Give scanner commands run against OpenClaw plugin targets a 30-minute budget (was 20 minutes); skill, file and URL targets keep 20 minutes. The judge and overview budgets are unchanged. - Refresh Node setup to 7.1.0, use Node 24.22.2 for CI and docs, and Node 26.13.0 for release packaging. - **Breaking for plugin targets:** add the plugin-only `aig-mcp` scanner (Tencent A.I.G `aig-mcp-scan` 0.2.0, patched in the runtime image to allow only confined read-only tools, refuse model-chosen MCP destinations and fail unusable reports) and the profile field `pluginScanners`. The `clawhub` profile now runs only `aig-mcp` for OpenClaw plugins, over a bounded copy without `node_modules`; skills keep `skillspector` and `aig`. Consumers that read `scanners.skillspector` or `scanners.aig` for plugin runs must read `scanners["aig-mcp"]` instead. Host runs need `AIG_MCP_SCAN_ALLOW_HOST_EXEC=1` ([#149](https://github.com/openclaw/clawscan/pull/149)). diff --git a/docs/scanner-runtime.md b/docs/scanner-runtime.md index 529d463..e2154fc 100644 --- a/docs/scanner-runtime.md +++ b/docs/scanner-runtime.md @@ -315,7 +315,8 @@ not detached or given a fresh request deadline. Python cannot forcibly terminate an uncooperative thread/finalizer. Process budgets belong to the caller: ClawHub's external consumer supplies a 900-second process budget plus a 60-second cleanup allowance. The public CLI instead uses the external-command runner's configured -timeout (20 minutes by default), cancelling the direct command via Go's +timeout (20 minutes per scanner by default; 30 minutes for scanners run against +an OpenClaw plugin target, which is audited as one whole artifact), cancelling the direct command via Go's `exec.CommandContext` without a separate 60-second cleanup phase. That cancellation does not guarantee process-tree termination or a strict wall-clock bound on pipe cleanup. diff --git a/docs/scanners.md b/docs/scanners.md index 55afbcc..900f384 100644 --- a/docs/scanners.md +++ b/docs/scanners.md @@ -383,6 +383,11 @@ release with this profile change therefore needs the runtime image published first and any host worker updated before its ClawScan pin moves; until then, plugin scans fail closed rather than running without `aig-mcp`. +Each scanner command against a plugin target gets a 30-minute budget, instead +of the 20 minutes used for skills, because the whole plugin is audited as one +artifact. A run that hits the budget is cancelled and fails like any other +command failure. + Failures are never reported as clean scans. `aig-mcp-scan` exits 0 and writes SARIF marked `scanNote: complete` even when its audit did not finish, so the adapter also reads its output. Any of these marks `aig-mcp` failed with the diff --git a/internal/runner/aig_mcp_scanner.go b/internal/runner/aig_mcp_scanner.go index 47b8cef..6ea718a 100644 --- a/internal/runner/aig_mcp_scanner.go +++ b/internal/runner/aig_mcp_scanner.go @@ -156,7 +156,7 @@ func (runner ExternalScannerRunner) runAIGMCP(target string, startedAt string) ( fullCommand := append([]string{aigMCPCommand}, args...) timeout := runner.Timeout if timeout == 0 { - timeout = 20 * time.Minute + timeout = pluginScannerTimeout } output, runErr := runner.CommandRunner.Run(aigMCPCommand, args, workDir, timeout) exitCode := gateEligibleExitCode(output.ExitCode) diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 2ccd52f..6ec5745 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -603,7 +603,7 @@ func Run(opts Options, ctx RunContext) (artifact Artifact, runErr error) { Registry: registryForOptions(opts), SandboxMode: scannerSandboxMode, SkillSpectorCommand: ctx.SkillSpectorCommand, - Timeout: 20 * time.Minute, + Timeout: scannerTimeoutForTarget(target.kind), } } artifact.Sandbox = sandbox @@ -2250,6 +2250,25 @@ type ExternalScannerRunner struct { Timeout time.Duration } +const ( + // defaultScannerTimeout bounds one scanner command for skill, file and URL + // targets. + defaultScannerTimeout = 20 * time.Minute + // pluginScannerTimeout bounds one scanner command for an OpenClaw plugin + // target. A plugin is audited as one whole artifact (compiled output, + // embedded skills and hooks), so it gets a larger budget. + pluginScannerTimeout = 30 * time.Minute +) + +// scannerTimeoutForTarget returns the per-scanner command budget for a target +// kind. Only plugin targets differ; every other kind keeps the default. +func scannerTimeoutForTarget(kind string) time.Duration { + if kind == targetKindPlugin { + return pluginScannerTimeout + } + return defaultScannerTimeout +} + func (runner ExternalScannerRunner) RunScanner(name string, target string, startedAt string) (ScannerResult, error) { if err := workContext(runner.Context).Err(); err != nil { return ScannerResult{}, err diff --git a/internal/runner/scanner_timeout_test.go b/internal/runner/scanner_timeout_test.go new file mode 100644 index 0000000..3fc9a83 --- /dev/null +++ b/internal/runner/scanner_timeout_test.go @@ -0,0 +1,129 @@ +package runner + +import ( + "os" + "path/filepath" + "testing" + "time" +) + +// timeoutRecordingRunner records each command's timeout and answers like the +// scanners' fixtures, so the runner's per-target budget is observable. +type timeoutRecordingRunner struct { + timeouts map[string]time.Duration +} + +func (runner *timeoutRecordingRunner) Run(command string, args []string, cwd string, timeout time.Duration) (CommandOutput, error) { + runner.timeouts[command] = timeout + if output := argValue(args, "-o"); output != "" { + if !filepath.IsAbs(output) { + output = filepath.Join(cwd, output) + } + if err := os.WriteFile(output, []byte(aigMCPSARIF), 0o644); err != nil { + return CommandOutput{}, err + } + } + return CommandOutput{Stdout: `{"findings":[]}`}, nil +} + +func scannerTimeouts(t *testing.T, target string, scanners ...string) map[string]time.Duration { + t.Helper() + args := []string{target, "--sandbox", "off"} + for _, scanner := range scanners { + args = append(args, "--scanner", scanner) + } + opts, err := ParseArgs(args) + if err != nil { + t.Fatal(err) + } + commands := &timeoutRecordingRunner{timeouts: map[string]time.Duration{}} + if _, err := Run(opts, RunContext{Env: withAIGMCPHostExec(map[string]string{"LLM_API_KEY": "k"}), CommandRunner: commands}); err != nil { + t.Fatal(err) + } + return commands.timeouts +} + +func TestPluginScannersGetThirtyMinutesAndSkillScannersKeepTwenty(t *testing.T) { + plugin := scannerTimeouts(t, createAIGMCPTestPlugin(t), "aig-mcp") + if got := plugin[aigMCPCommand]; got != 30*time.Minute { + t.Fatalf("plugin aig-mcp-scan timeout = %s, want 30m0s", got) + } + + skill := scannerTimeouts(t, createAIGTestSkill(t), "aig") + if got := skill["aig-skill-scan"]; got != 20*time.Minute { + t.Fatalf("skill aig-skill-scan timeout = %s, want 20m0s", got) + } +} + +func TestScannerTimeoutDependsOnlyOnTargetKind(t *testing.T) { + for kind, want := range map[string]time.Duration{ + targetKindPlugin: 30 * time.Minute, + targetKindSkill: 20 * time.Minute, + targetKindURL: 20 * time.Minute, + } { + if got := scannerTimeoutForTarget(kind); got != want { + t.Fatalf("%s timeout = %s, want %s", kind, got, want) + } + } + // A directly constructed runner without a budget still gives aig-mcp the + // plugin budget, since it only ever scans plugins. + commands := &timeoutRecordingRunner{timeouts: map[string]time.Duration{}} + runner := ExternalScannerRunner{CommandRunner: commands, Env: withAIGMCPHostExec(map[string]string{"LLM_API_KEY": "k"})} + if _, err := runner.runAIGMCP(createAIGMCPTestPlugin(t), "now"); err != nil { + t.Fatal(err) + } + if got := commands.timeouts[aigMCPCommand]; got != 30*time.Minute { + t.Fatalf("default aig-mcp-scan timeout = %s, want 30m0s", got) + } +} + +// TestProfilePluginSelectionUsesPluginBudget follows the clawhub profile's +// shape: skill scanners in Scanners, aig-mcp in PluginScanners. +func TestProfilePluginSelectionUsesPluginBudget(t *testing.T) { + run := func(target string) map[string]time.Duration { + opts, err := ParseArgs([]string{target, "--scanner", "aig", "--sandbox", "off"}) + if err != nil { + t.Fatal(err) + } + opts.PluginScanners = []string{aigMCPScannerID} + commands := &timeoutRecordingRunner{timeouts: map[string]time.Duration{}} + if _, err := Run(opts, RunContext{Env: withAIGMCPHostExec(map[string]string{"LLM_API_KEY": "k"}), CommandRunner: commands}); err != nil { + t.Fatal(err) + } + return commands.timeouts + } + if got := run(createAIGMCPTestPlugin(t)); got[aigMCPCommand] != 30*time.Minute || len(got) != 1 { + t.Fatalf("plugin run timeouts = %v, want only aig-mcp-scan at 30m0s", got) + } + if got := run(createAIGTestSkill(t)); got["aig-skill-scan"] != 20*time.Minute || len(got) != 1 { + t.Fatalf("skill run timeouts = %v, want only aig-skill-scan at 20m0s", got) + } +} + +// TestDockerSandboxPassesPluginBudgetToTheHost checks the default sandboxed +// path: the docker invocation carries the plugin budget. +func TestDockerSandboxPassesPluginBudgetToTheHost(t *testing.T) { + opts, err := ParseArgs([]string{createAIGMCPTestPlugin(t), "--scanner", "aig-mcp"}) + if err != nil { + t.Fatal(err) + } + host := &dockerTimeoutRecorder{} + if _, err := Run(opts, RunContext{ + Env: map[string]string{"LLM_API_KEY": "k"}, + HostCommandRunner: host, + DockerAvailability: func() error { return nil }, + }); err != nil { + t.Fatal(err) + } + if host.timeout != 30*time.Minute { + t.Fatalf("docker command timeout = %s, want 30m0s", host.timeout) + } +} + +type dockerTimeoutRecorder struct{ timeout time.Duration } + +func (recorder *dockerTimeoutRecorder) Run(command string, args []string, cwd string, timeout time.Duration) (CommandOutput, error) { + recorder.timeout = timeout + workDir := argValue(args, "-w") + return CommandOutput{}, os.WriteFile(filepath.Join(workDir, argValue(args, "-o")), []byte(aigMCPSARIF), 0o644) +}