feat(tools): make exec timeout configurable

This commit is contained in:
Goon committed 2026-05-21 16:44:43 +07:00
1 parent 81d96ae014
commit 04d2b4266e
12 files changed
+414 -8

No files matched your search

+1
View File
@@ -22,6 +22,7 @@ func builtinToolSeedData() []store.BuiltinToolDef {
// runtime
{Name: "exec", DisplayName: "Execute Command", Description: "Execute a shell command in the workspace and return stdout/stderr", Category: "runtime", Enabled: true,
Settings: json.RawMessage(`{"timeout_seconds":60}`),
Metadata: json.RawMessage(`{"config_hint":"Config → Tools → Exec Approval"}`),
},
{Name: "wait", DisplayName: "Wait", Description: "Pause the current agent tool sequence for a bounded number of milliseconds", Category: "runtime", Enabled: true},
+4 -2
View File
@@ -382,6 +382,8 @@ Merged settings map:
Merge is per-tool-name: tenant entry for `web_search` wins wholesale over global default — no deep field merge. Tools that do not read the settings map are unaffected.
`exec` reads `settings.timeout_seconds` for host command execution. The REST API validates `exec` settings as a JSON object with optional integer `timeout_seconds` in the `1..3600` range. Missing or invalid runtime values fall back to 60 seconds, while values above the maximum are clamped to 3600 seconds for defense in depth. Docker sandbox tool calls still use `sandbox_config.timeout_sec`; this setting only controls the host `exec` built-in.
**Secret vs non-secret split:**
- Non-secret config (provider priorities, limits, domain policies) → `builtin_tool_tenant_configs.settings`
- Secrets (API keys, tokens) → `config_secrets` table (AES-256-GCM encrypted, tenant-scoped)
@@ -390,11 +392,11 @@ Never put credentials in the settings JSON blob — backend does not validate th
**Cache invalidation:** settings changes propagate via pub/sub (tenant-scoped). Next agent turn re-resolves automatically.
**Tenant admin workflow:** Settings → Builtin Tools → gear icon → JSON editor → Save. Changes take effect on the next agent turn. "Reset to default" reverts to platform defaults.
**Tenant admin workflow:** Settings → Builtin Tools → gear icon → typed form when available, otherwise JSON editor → Save. Changes take effect on the next agent turn. "Reset to default" reverts to platform defaults.
**Master-scope guard:** Writes to global `builtin_tools` table require master tenant scope. Tenant admins use the `/tenant-config` endpoint. Same guard applies on the WS config methods.
Current adopters: `web_search`, `web_fetch`, `tts`, `create_image`, `read_image`, `create_audio`, `read_audio`, `knowledge_graph_search`.
Current adopters: `exec`, `web_search`, `web_fetch`, `tts`, `create_image`, `read_image`, `create_audio`, `read_audio`, `knowledge_graph_search`.
### Shell Deny-Groups (Runtime Config)
+21
View File
@@ -4,6 +4,27 @@ Significant changes, features, and fixes in reverse chronological order.
---
## 2026-05-21
### Tools: configurable exec timeout
**Features**
- Added `exec.settings.timeout_seconds` for configurable host command timeout through global and tenant built-in tool settings.
- Added dashboard controls for the `exec` timeout, defaulting missing settings to 60 seconds.
- Kept Docker sandbox execution on `sandbox_config.timeout_sec`; `exec.timeout_seconds` only affects the host built-in.
**Fixes**
- Added API validation for `exec` settings so invalid, zero, negative, fractional, or excessive timeout values are rejected before persistence.
**Tests**
- Added focused runtime and HTTP API regression coverage for configured `exec` timeouts and invalid settings.
- Verified web dashboard build with the typed `exec` settings form.
---
## 2026-05-20
### HTTP API contract hardening
+11
View File
@@ -13,6 +13,7 @@ import (
"github.com/nextlevelbuilder/goclaw/internal/i18n"
"github.com/nextlevelbuilder/goclaw/internal/permissions"
"github.com/nextlevelbuilder/goclaw/internal/store"
"github.com/nextlevelbuilder/goclaw/internal/tools"
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
)
@@ -266,6 +267,10 @@ func (h *BuiltinToolsHandler) handleUpdate(w http.ResponseWriter, r *http.Reques
if v, ok := updates["settings"]; ok {
// Extract secrets before saving settings
if settingsRaw, err := json.Marshal(v); err == nil {
if err := tools.ValidateExecSettingsJSON(name, settingsRaw, false); err != nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()})
return
}
cleaned := h.extractAndSaveSecrets(r.Context(), name, settingsRaw)
var cleanedMap any
if err2 := json.Unmarshal(cleaned, &cleanedMap); err2 == nil {
@@ -390,6 +395,12 @@ func (h *BuiltinToolsHandler) handleSetTenantConfig(w http.ResponseWriter, r *ht
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "settings must be a JSON object or null"})
return
}
if body.Settings != nil {
if err := tools.ValidateExecSettingsJSON(name, body.Settings, true); err != nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": err.Error()})
return
}
}
// Write enabled if provided (preserves settings column via column-list upsert).
if body.Enabled != nil {
@@ -217,10 +217,15 @@ func buildTenantCfgHandler(tcfg *stubTenantCfgStore, tid uuid.UUID) (*BuiltinToo
// mustDoPut wires a mux pattern so r.PathValue("name") resolves correctly.
func mustDo(t *testing.T, inject http.HandlerFunc, method, body string) *httptest.ResponseRecorder {
t.Helper()
return mustDoTool(t, inject, method, "web_search", body)
}
func mustDoTool(t *testing.T, inject http.HandlerFunc, method, toolName, body string) *httptest.ResponseRecorder {
t.Helper()
mux := http.NewServeMux()
mux.HandleFunc(method+" /v1/tools/builtin/{name}/tenant-config", inject)
req := httptest.NewRequest(method, "/v1/tools/builtin/web_search/tenant-config", strings.NewReader(body))
req := httptest.NewRequest(method, "/v1/tools/builtin/"+toolName+"/tenant-config", strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
mux.ServeHTTP(rec, req)
@@ -305,6 +310,67 @@ func TestPutTenantConfig_InvalidSettingsJSON_Returns400(t *testing.T) {
}
}
func TestPutTenantConfig_InvalidExecTimeoutSettings_Returns400(t *testing.T) {
tcfg := newStubTenantCfgStore()
_, inject := buildTenantCfgHandler(tcfg, uuid.New())
cases := []struct {
name string
body string
}{
{"non_number", `{"settings":{"timeout_seconds":"60"}}`},
{"zero", `{"settings":{"timeout_seconds":0}}`},
{"negative", `{"settings":{"timeout_seconds":-1}}`},
{"above_max", `{"settings":{"timeout_seconds":3601}}`},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
rec := mustDoTool(t, inject, http.MethodPut, "exec", tc.body)
if rec.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want 400; body=%s", rec.Code, rec.Body.String())
}
})
}
}
func TestBuiltinToolsUpdate_InvalidExecTimeoutSettings_Returns400(t *testing.T) {
recStore := &recordingBuiltinToolStore{}
h := &BuiltinToolsHandler{store: recStore}
mux := http.NewServeMux()
mux.HandleFunc("PUT /v1/tools/builtin/{name}", h.handleUpdate)
cases := []struct {
name string
body string
}{
{"non_object", `{"settings":[1,2]}`},
{"non_number", `{"settings":{"timeout_seconds":"60"}}`},
{"zero", `{"settings":{"timeout_seconds":0}}`},
{"negative", `{"settings":{"timeout_seconds":-1}}`},
{"above_max", `{"settings":{"timeout_seconds":3601}}`},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
req := httptest.NewRequest(http.MethodPut, "/v1/tools/builtin/exec", strings.NewReader(tc.body))
req.Header.Set("Content-Type", "application/json")
ctx := store.WithTenantID(req.Context(), store.MasterTenantID)
ctx = store.WithRole(ctx, "admin")
req = req.WithContext(ctx)
rr := httptest.NewRecorder()
mux.ServeHTTP(rr, req)
if rr.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want 400; body=%s", rr.Code, rr.Body.String())
}
})
}
if recStore.updateName != "" {
t.Fatalf("invalid settings reached store update for %q", recStore.updateName)
}
}
func TestPutTenantConfig_BackwardCompatEnabledOnly(t *testing.T) {
// Old clients send { enabled: bool } only — must still work.
tcfg := newStubTenantCfgStore()
+75 -4
View File
@@ -3,6 +3,7 @@ package tools
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
@@ -20,6 +21,12 @@ import (
"golang.org/x/text/unicode/norm"
)
const (
ExecDefaultTimeoutSeconds = 60
ExecMinTimeoutSeconds = 1
ExecMaxTimeoutSeconds = 3600
)
// Dangerous command patterns organized into configurable deny groups.
// Defense-in-depth: patterns complement Docker hardening (cap-drop ALL,
// no-new-privileges, pids-limit, memory limit).
@@ -93,7 +100,7 @@ func (t *ExecTool) EffectiveDenyGroupsForTest(ctx context.Context) map[string]bo
func NewExecTool(workspace string, restrict bool) *ExecTool {
return &ExecTool{
workspace: workspace,
timeout: 60 * time.Second,
timeout: time.Duration(ExecDefaultTimeoutSeconds) * time.Second,
restrict: restrict,
}
}
@@ -405,6 +412,69 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result {
return t.executeOnHost(ctx, command, cwd)
}
type execSettings struct {
TimeoutSeconds *int `json:"timeout_seconds"`
}
// ValidateExecSettingsJSON validates the public settings contract for the exec
// builtin. Other tools are ignored so their flexible JSON settings stay intact.
func ValidateExecSettingsJSON(toolName string, raw json.RawMessage, allowNull bool) error {
if toolName != "exec" {
return nil
}
seconds, ok, err := parseExecTimeoutSeconds(raw, allowNull)
if err != nil {
return err
}
if !ok {
return nil
}
if seconds > ExecMaxTimeoutSeconds {
return fmt.Errorf("timeout_seconds must be <= %d", ExecMaxTimeoutSeconds)
}
return nil
}
func parseExecTimeoutSeconds(raw json.RawMessage, allowNull bool) (int, bool, error) {
if len(raw) == 0 {
return 0, false, nil
}
trimmed := strings.TrimSpace(string(raw))
if trimmed == "null" {
if allowNull {
return 0, false, nil
}
return 0, false, errors.New("settings must be a JSON object")
}
var settings execSettings
if err := json.Unmarshal(raw, &settings); err != nil {
return 0, false, errors.New("settings must be a JSON object with numeric timeout_seconds")
}
if settings.TimeoutSeconds == nil {
return 0, false, nil
}
if *settings.TimeoutSeconds < ExecMinTimeoutSeconds {
return 0, true, fmt.Errorf("timeout_seconds must be >= %d", ExecMinTimeoutSeconds)
}
return *settings.TimeoutSeconds, true, nil
}
func (t *ExecTool) effectiveTimeout(ctx context.Context) time.Duration {
settings := BuiltinToolSettingsFromCtx(ctx)
if raw, ok := settings["exec"]; ok {
if seconds, hasValue, err := parseExecTimeoutSeconds(json.RawMessage(raw), false); err == nil && hasValue {
if seconds > ExecMaxTimeoutSeconds {
seconds = ExecMaxTimeoutSeconds
}
return time.Duration(seconds) * time.Second
}
}
if t.timeout > 0 {
return t.timeout
}
return time.Duration(ExecDefaultTimeoutSeconds) * time.Second
}
// matchesAny checks if a command matches any pattern in the list.
func matchesAny(command string, patterns []*regexp.Regexp) bool {
for _, p := range patterns {
@@ -428,7 +498,8 @@ func posixShellPath() (string, error) {
// ctx cancellation (e.g. agent abort) triggers SIGTERM → 3s grace → SIGKILL on the
// entire process group so forked children are also cleaned up (no orphans).
func (t *ExecTool) executeOnHost(ctx context.Context, command, cwd string) *Result {
ctx, cancel := context.WithTimeout(ctx, t.timeout)
timeout := t.effectiveTimeout(ctx)
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
// Use plain exec.Command (not CommandContext) so we control the kill sequence.
@@ -477,7 +548,7 @@ func (t *ExecTool) executeOnHost(ctx context.Context, command, cwd string) *Resu
select {
case err := <-done:
// Normal completion (success or non-zero exit).
return buildHostResult(err, stdout, stderr, ctx, t.timeout)
return buildHostResult(err, stdout, stderr, ctx, timeout)
case <-ctx.Done():
// Context cancelled or timed out — kill the process group gracefully then forcefully.
@@ -491,7 +562,7 @@ func (t *ExecTool) executeOnHost(ctx context.Context, command, cwd string) *Resu
<-done
}
if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return ErrorResult(fmt.Sprintf("command timed out after %s", t.timeout))
return ErrorResult(fmt.Sprintf("command timed out after %s", timeout))
}
return ErrorResult("command aborted")
}
@@ -0,0 +1,90 @@
package tools
import (
"context"
"strings"
"testing"
"time"
)
func TestExecToolEffectiveTimeoutFromSettings(t *testing.T) {
tool := NewExecTool(t.TempDir(), false)
cases := []struct {
name string
ctx context.Context
want time.Duration
}{
{
name: "global exec timeout",
ctx: WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
"exec": []byte(`{"timeout_seconds":120}`),
}),
want: 120 * time.Second,
},
{
name: "tenant overrides global",
ctx: WithTenantToolSettings(
WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
"exec": []byte(`{"timeout_seconds":120}`),
}),
BuiltinToolSettings{"exec": []byte(`{"timeout_seconds":30}`)},
),
want: 30 * time.Second,
},
{
name: "excessive timeout clamps",
ctx: WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
"exec": []byte(`{"timeout_seconds":999999}`),
}),
want: 3600 * time.Second,
},
{
name: "invalid json falls back",
ctx: WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
"exec": []byte(`{`),
}),
want: 60 * time.Second,
},
{
name: "missing timeout falls back",
ctx: WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
"exec": []byte(`{"other":true}`),
}),
want: 60 * time.Second,
},
{
name: "zero timeout falls back",
ctx: WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
"exec": []byte(`{"timeout_seconds":0}`),
}),
want: 60 * time.Second,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
if got := tool.effectiveTimeout(tc.ctx); got != tc.want {
t.Fatalf("effectiveTimeout() = %s, want %s", got, tc.want)
}
})
}
}
func TestExecToolExecuteUsesConfiguredHostTimeout(t *testing.T) {
tool := NewExecTool(t.TempDir(), false)
ctx := WithBuiltinToolSettings(context.Background(), BuiltinToolSettings{
"exec": []byte(`{"timeout_seconds":1}`),
})
result := tool.Execute(ctx, map[string]any{
"command": "sleep 2",
})
if !result.IsError {
t.Fatalf("expected timeout error, got ForLLM=%q", result.ForLLM)
}
if !strings.Contains(result.ForLLM, "command timed out after 1s") {
t.Fatalf("expected configured timeout in error, got %q", result.ForLLM)
}
}
+10
View File
@@ -143,6 +143,16 @@
"save": "Save",
"saving": "Saving..."
},
"execSettings": {
"title": "Exec Settings",
"description": "Configure the host command timeout for the built-in exec tool.",
"timeoutSeconds": "Timeout (seconds)",
"timeoutHint": "Allowed range: {{min}}-{{max}} seconds.",
"sandboxHint": "Docker sandbox runs use the agent sandbox timeout setting.",
"cancel": "Cancel",
"save": "Save",
"saving": "Saving..."
},
"searchChain": {
"title": "Web Search Provider Chain",
"description": "Drag to reorder search providers. First enabled provider with a valid API key is tried first.",
+10
View File
@@ -143,6 +143,16 @@
"save": "Lưu",
"saving": "Đang lưu..."
},
"execSettings": {
"title": "Cài đặt Exec",
"description": "Cấu hình thời gian chờ lệnh host cho công cụ exec tích hợp.",
"timeoutSeconds": "Thời gian chờ (giây)",
"timeoutHint": "Phạm vi cho phép: {{min}}-{{max}} giây.",
"sandboxHint": "Lần chạy Docker sandbox dùng cài đặt thời gian chờ sandbox của agent.",
"cancel": "Hủy",
"save": "Lưu",
"saving": "Đang lưu..."
},
"searchChain": {
"title": "Chuỗi nhà cung cấp tìm kiếm web",
"description": "Kéo để sắp xếp lại nhà cung cấp tìm kiếm. Nhà cung cấp đầu tiên được bật với API key hợp lệ sẽ được thử trước.",
+10
View File
@@ -57,6 +57,16 @@
"saving": "保存中...",
"title": "知识图谱设置"
},
"execSettings": {
"title": "Exec 设置",
"description": "配置内置 exec 工具的主机命令超时。",
"timeoutSeconds": "超时(秒)",
"timeoutHint": "允许范围:{{min}}-{{max}} 秒。",
"sandboxHint": "Docker sandbox 运行使用 Agent sandbox 超时设置。",
"cancel": "取消",
"save": "保存",
"saving": "保存中..."
},
"searchChain": {
"title": "网页搜索 Provider 链",
"description": "拖动以重新排序搜索 Provider。第一个已启用且具有有效 API Key 的 Provider 将被优先尝试。",
@@ -18,7 +18,9 @@ import { KGSettingsForm } from "./kg-settings-form";
import { WebFetchExtractorChainForm } from "./web-fetch-extractor-chain-form";
import { WebSearchChainForm } from "./web-search-chain-form";
import { SttProviderForm } from "./stt-provider-form";
import { ExecSettingsForm } from "./exec-settings-form";
const EXEC_TOOL = "exec";
const KG_TOOL = "knowledge_graph_search";
const WEB_FETCH_TOOL = "web_fetch";
const WEB_SEARCH_TOOL = "web_search";
@@ -61,6 +63,7 @@ export function BuiltinToolSettingsDialog({
const isWebFetch = tool?.name === WEB_FETCH_TOOL;
const isWebSearch = tool?.name === WEB_SEARCH_TOOL;
const isStt = tool?.name === STT_TOOL;
const isExec = tool?.name === EXEC_TOOL;
const wide = isMedia || isKG || isWebFetch || isWebSearch;
// Tenant-scope overlay: prefer the tenant override when present; fall back
@@ -83,7 +86,13 @@ export function BuiltinToolSettingsDialog({
<Dialog open={open} onOpenChange={onOpenChange}>
<DialogContent className={wide ? "sm:max-w-2xl" : "sm:max-w-md"}>
{modeBadge}
{isWebSearch && tool ? (
{isExec && tool ? (
<ExecSettingsForm
initialSettings={initialSettings}
onSave={(settings) => onSave(tool.name, settings).then(() => onOpenChange(false))}
onCancel={() => onOpenChange(false)}
/>
) : isWebSearch && tool ? (
<WebSearchChainForm
initialSettings={initialSettings}
secretsSet={tool.secrets_set}
@@ -0,0 +1,105 @@
import { useEffect, useState } from "react";
import { useTranslation } from "react-i18next";
import { Loader2 } from "lucide-react";
import {
DialogDescription,
DialogFooter,
DialogHeader,
DialogTitle,
} from "@/components/ui/dialog";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
import { Label } from "@/components/ui/label";
const DEFAULT_TIMEOUT_SECONDS = 60;
const MIN_TIMEOUT_SECONDS = 1;
const MAX_TIMEOUT_SECONDS = 3600;
interface Props {
initialSettings: Record<string, unknown>;
onSave: (settings: Record<string, unknown>) => Promise<void>;
onCancel: () => void;
}
function resolveTimeoutSeconds(settings: Record<string, unknown>): number {
const value = settings.timeout_seconds;
if (typeof value !== "number" || !Number.isFinite(value)) {
return DEFAULT_TIMEOUT_SECONDS;
}
if (value < MIN_TIMEOUT_SECONDS) {
return DEFAULT_TIMEOUT_SECONDS;
}
return Math.min(Math.trunc(value), MAX_TIMEOUT_SECONDS);
}
export function ExecSettingsForm({ initialSettings, onSave, onCancel }: Props) {
const { t } = useTranslation("tools");
const [timeoutSeconds, setTimeoutSeconds] = useState(() => resolveTimeoutSeconds(initialSettings));
const [saving, setSaving] = useState(false);
useEffect(() => {
setTimeoutSeconds(resolveTimeoutSeconds(initialSettings));
}, [initialSettings]);
const handleSave = async () => {
const nextTimeout = Math.min(
Math.max(Math.trunc(timeoutSeconds), MIN_TIMEOUT_SECONDS),
MAX_TIMEOUT_SECONDS,
);
setSaving(true);
try {
await onSave({ timeout_seconds: nextTimeout });
} catch {
// toast shown by hook
} finally {
setSaving(false);
}
};
return (
<>
<DialogHeader>
<DialogTitle>{t("builtin.execSettings.title")}</DialogTitle>
<DialogDescription>{t("builtin.execSettings.description")}</DialogDescription>
</DialogHeader>
<div className="space-y-4 py-2">
<div className="grid gap-1.5">
<Label htmlFor="exec-timeout-seconds" className="text-sm">
{t("builtin.execSettings.timeoutSeconds")}
</Label>
<Input
id="exec-timeout-seconds"
type="number"
min={MIN_TIMEOUT_SECONDS}
max={MAX_TIMEOUT_SECONDS}
step={1}
value={timeoutSeconds}
onChange={(e) => setTimeoutSeconds(Number(e.target.value) || DEFAULT_TIMEOUT_SECONDS)}
className="max-w-[140px] text-base md:text-sm"
/>
<p className="text-xs text-muted-foreground">
{t("builtin.execSettings.timeoutHint", {
min: MIN_TIMEOUT_SECONDS,
max: MAX_TIMEOUT_SECONDS,
})}
</p>
</div>
<p className="text-xs text-muted-foreground">
{t("builtin.execSettings.sandboxHint")}
</p>
</div>
<DialogFooter>
<Button variant="outline" onClick={onCancel}>
{t("builtin.execSettings.cancel")}
</Button>
<Button onClick={handleSave} disabled={saving}>
{saving && <Loader2 className="h-4 w-4 animate-spin" />}
{saving ? t("builtin.execSettings.saving") : t("builtin.execSettings.save")}
</Button>
</DialogFooter>
</>
);
}