From 96d340b38bdf350ff9a6b12e26d7e4b73834160a Mon Sep 17 00:00:00 2001 From: xgopilot Date: Fri, 17 Apr 2026 07:46:54 +0000 Subject: [PATCH 1/4] refactor(runtime,tools): remove max-loop residue and harden permission/webfetch security Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com> --- internal/app/bootstrap_test.go | 3 +- internal/runtime/controlplane/decider.go | 9 +- internal/runtime/controlplane/decider_test.go | 23 +----- internal/runtime/controlplane/stop_reason.go | 2 - internal/runtime/run_lifecycle.go | 6 -- .../runtime/runtime_branch_coverage_test.go | 25 ------ internal/runtime/runtime_limits.go | 4 - internal/tools/manager.go | 6 +- internal/tools/manager_test.go | 34 ++++++-- internal/tools/webfetch/tool.go | 82 +++++++++++++++++++ internal/tools/webfetch/tool_test.go | 55 ++++++++++++- 11 files changed, 169 insertions(+), 80 deletions(-) delete mode 100644 internal/runtime/runtime_limits.go diff --git a/internal/app/bootstrap_test.go b/internal/app/bootstrap_test.go index 7127b3e1..7b451150 100644 --- a/internal/app/bootstrap_test.go +++ b/internal/app/bootstrap_test.go @@ -27,6 +27,7 @@ import ( "neo-code/internal/skills" "neo-code/internal/tools" "neo-code/internal/tools/mcp" + "neo-code/internal/tools/webfetch" "neo-code/internal/tui" ) @@ -167,7 +168,7 @@ func TestBuildToolRegistryUsesWebFetchConfig(t *testing.T) { t.Fatalf("marshal args: %v", err) } - result, execErr := tool.Execute(context.Background(), tools.ToolCallInput{ + result, execErr := tool.Execute(webfetch.WithUnsafeBypassTargetValidation(context.Background()), tools.ToolCallInput{ Name: "webfetch", Arguments: args, }) diff --git a/internal/runtime/controlplane/decider.go b/internal/runtime/controlplane/decider.go index 641bc6cb..4fbe7a61 100644 --- a/internal/runtime/controlplane/decider.go +++ b/internal/runtime/controlplane/decider.go @@ -9,22 +9,15 @@ import ( // StopInput 汇总停止决议所需的信号（可多信号并存，由 DecideStopReason 按优先级表决）。 type StopInput struct { ContextCanceled bool - MaxLoopsReached bool RunError error Success bool } -// DecideStopReason 按固定优先级返回唯一 StopReason：取消 > 达到轮数上限 > 错误 > 成功。 +// DecideStopReason 按固定优先级返回唯一 StopReason：取消 > 错误 > 成功。 func DecideStopReason(in StopInput) (StopReason, string) { if in.ContextCanceled { return StopReasonCanceled, "" } - if in.MaxLoopsReached { - if in.RunError != nil { - return StopReasonMaxLoops, strings.TrimSpace(in.RunError.Error()) - } - return StopReasonMaxLoops, "runtime: max loop reached" - } if in.RunError != nil { if errors.Is(in.RunError, context.Canceled) { return StopReasonCanceled, "" diff --git a/internal/runtime/controlplane/decider_test.go b/internal/runtime/controlplane/decider_test.go index 1f41b8e4..2aab317e 100644 --- a/internal/runtime/controlplane/decider_test.go +++ b/internal/runtime/controlplane/decider_test.go @@ -16,24 +16,15 @@ func TestDecideStopReasonPriority(t *testing.T) { reason StopReason }{ { - name: "canceled_wins_over_max_loops", + name: "canceled_wins_over_error", in: StopInput{ ContextCanceled: true, - MaxLoopsReached: true, RunError: errSample, }, reason: StopReasonCanceled, }, { - name: "max_loops_wins_over_error", - in: StopInput{ - MaxLoopsReached: true, - RunError: errSample, - }, - reason: StopReasonMaxLoops, - }, - { - name: "error_when_no_max_loop_flag", + name: "error", in: StopInput{ RunError: errSample, }, @@ -70,15 +61,7 @@ func TestDecideStopReasonPriority(t *testing.T) { func TestDecideStopReasonDetails(t *testing.T) { t.Parallel() - reason, detail := DecideStopReason(StopInput{MaxLoopsReached: true}) - if reason != StopReasonMaxLoops { - t.Fatalf("reason = %q, want %q", reason, StopReasonMaxLoops) - } - if detail != "runtime: max loop reached" { - t.Fatalf("detail = %q, want default max-loop detail", detail) - } - - reason, detail = DecideStopReason(StopInput{}) + reason, detail := DecideStopReason(StopInput{}) if reason != StopReasonError { t.Fatalf("reason = %q, want %q", reason, StopReasonError) } diff --git a/internal/runtime/controlplane/stop_reason.go b/internal/runtime/controlplane/stop_reason.go index d08b6cf6..ff51454b 100644 --- a/internal/runtime/controlplane/stop_reason.go +++ b/internal/runtime/controlplane/stop_reason.go @@ -6,8 +6,6 @@ type StopReason string const ( // StopReasonSuccess 表示助手正常结束（无待执行工具调用）。 StopReasonSuccess StopReason = "success" - // StopReasonMaxLoops 表示达到配置的最大推理轮数。 - StopReasonMaxLoops StopReason = "max_loops" // StopReasonError 表示不可恢复的运行时或 provider 错误。 StopReasonError StopReason = "error" // StopReasonCanceled 表示运行上下文被取消（含用户中断）。 diff --git a/internal/runtime/run_lifecycle.go b/internal/runtime/run_lifecycle.go index d82d71b1..62406eee 100644 --- a/internal/runtime/run_lifecycle.go +++ b/internal/runtime/run_lifecycle.go @@ -12,9 +12,6 @@ import ( "neo-code/internal/runtime/controlplane" ) -// ErrMaxLoopReached 表示达到配置的最大推理轮数，用于停止原因分类与测试断言。 -var ErrMaxLoopReached = errors.New("runtime: max loop reached") - // ErrNoProgressStreakLimit 表示循环内连续多次未取得进展，触发死循环拦截。 var ErrNoProgressStreakLimit = errors.New("runtime: no progress streak limit reached") @@ -57,9 +54,6 @@ func (s *Service) emitRunTermination(ctx context.Context, input UserInput, state switch { case errors.Is(err, context.Canceled): in.ContextCanceled = true - case errors.Is(err, ErrMaxLoopReached): - in.MaxLoopsReached = true - in.RunError = err default: in.RunError = err } diff --git a/internal/runtime/runtime_branch_coverage_test.go b/internal/runtime/runtime_branch_coverage_test.go index 318adbaf..eb4a85a5 100644 --- a/internal/runtime/runtime_branch_coverage_test.go +++ b/internal/runtime/runtime_branch_coverage_test.go @@ -103,31 +103,6 @@ func TestTransitionRunPhaseNoopBranches(t *testing.T) { } } -func TestEmitRunTerminationMaxLoopsBranch(t *testing.T) { - t.Parallel() - - service := &Service{events: make(chan RuntimeEvent, 4)} - state := newRunState("run-max-loops", newRuntimeSession("session-max-loops")) - service.emitRunTermination( - context.Background(), - UserInput{RunID: "run-max-loops", SessionID: state.session.ID}, - &state, - ErrMaxLoopReached, - ) - - events := collectRuntimeEvents(service.Events()) - if len(events) != 1 || events[0].Type != EventStopReasonDecided { - t.Fatalf("expected one stop_reason_decided event, got %+v", events) - } - payload, ok := events[0].Payload.(StopReasonDecidedPayload) - if !ok { - t.Fatalf("expected StopReasonDecidedPayload, got %#v", events[0].Payload) - } - if payload.Reason != controlplane.StopReasonMaxLoops { - t.Fatalf("reason = %q, want %q", payload.Reason, controlplane.StopReasonMaxLoops) - } -} - func TestCloneMessagesReturnsNilForEmptyInput(t *testing.T) { t.Parallel() diff --git a/internal/runtime/runtime_limits.go b/internal/runtime/runtime_limits.go deleted file mode 100644 index 3cab9213..00000000 --- a/internal/runtime/runtime_limits.go +++ /dev/null @@ -1,4 +0,0 @@ -package runtime - -// defaultMaxLoops 定义兼容旧运行循环逻辑时的默认最大轮数。 -const defaultMaxLoops = 8 diff --git a/internal/tools/manager.go b/internal/tools/manager.go index 424e68be..dc485d9b 100644 --- a/internal/tools/manager.go +++ b/internal/tools/manager.go @@ -173,11 +173,7 @@ func NewManager(executor Executor, engine security.PermissionEngine, sandbox Wor return nil, errors.New("tools: executor is nil") } if engine == nil { - defaultEngine, err := security.NewStaticGateway(security.DecisionAllow, nil) - if err != nil { - return nil, err - } - engine = defaultEngine + return nil, errors.New("tools: permission engine is nil") } if sandbox == nil { sandbox = NoopWorkspaceSandbox{} diff --git a/internal/tools/manager_test.go b/internal/tools/manager_test.go index 1645d83c..ebbcde60 100644 --- a/internal/tools/manager_test.go +++ b/internal/tools/manager_test.go @@ -71,12 +71,21 @@ func (s *stubSandbox) Check(ctx context.Context, action security.Action) (*secur return s.plan, s.err } +func mustAllowEngine(t *testing.T) security.PermissionEngine { + t.Helper() + engine, err := security.NewStaticGateway(security.DecisionAllow, nil) + if err != nil { + t.Fatalf("new static gateway: %v", err) + } + return engine +} + func TestDefaultManagerListAvailableSpecs(t *testing.T) { t.Parallel() registry := NewRegistry() registry.Register(&managerStubTool{name: "bash"}) - manager, err := NewManager(registry, nil, nil) + manager, err := NewManager(registry, mustAllowEngine(t), nil) if err != nil { t.Fatalf("new manager: %v", err) } @@ -105,7 +114,7 @@ func TestDefaultManagerMicroCompactPolicy(t *testing.T) { t.Run("executor without policy support defaults to compact", func(t *testing.T) { t.Parallel() - manager, err := NewManager(executorWithoutMicroCompactPolicy{}, nil, nil) + manager, err := NewManager(executorWithoutMicroCompactPolicy{}, mustAllowEngine(t), nil) if err != nil { t.Fatalf("new manager: %v", err) } @@ -120,7 +129,7 @@ func TestDefaultManagerMicroCompactPolicy(t *testing.T) { registry := NewRegistry() registry.Register(&managerStubTool{name: "preserve_tool", policy: MicroCompactPolicyPreserveHistory}) - manager, err := NewManager(registry, nil, nil) + manager, err := NewManager(registry, mustAllowEngine(t), nil) if err != nil { t.Fatalf("new manager: %v", err) } @@ -150,7 +159,7 @@ func TestDefaultManagerListAvailableSpecsBoundaries(t *testing.T) { manager: func() *DefaultManager { registry := NewRegistry() registry.Register(&managerStubTool{name: "bash"}) - manager, _ := NewManager(registry, nil, nil) + manager, _ := NewManager(registry, mustAllowEngine(t), nil) return manager }(), ctx: func() context.Context { @@ -331,7 +340,7 @@ func TestDefaultManagerExecuteBoundaries(t *testing.T) { manager: func() *DefaultManager { registry := NewRegistry() registry.Register(&managerStubTool{name: "custom_tool"}) - manager, _ := NewManager(registry, nil, nil) + manager, _ := NewManager(registry, mustAllowEngine(t), nil) return manager }(), input: ToolCallInput{Name: "custom_tool"}, @@ -342,7 +351,7 @@ func TestDefaultManagerExecuteBoundaries(t *testing.T) { manager: func() *DefaultManager { registry := NewRegistry() registry.Register(&managerStubTool{name: "bash"}) - manager, _ := NewManager(registry, nil, nil) + manager, _ := NewManager(registry, mustAllowEngine(t), nil) return manager }(), input: ToolCallInput{Name: "bash", Arguments: []byte(`{"command":"echo hi"}`)}, @@ -537,6 +546,17 @@ func TestNewManagerRejectsNilExecutor(t *testing.T) { } } +func TestNewManagerRejectsNilPermissionEngine(t *testing.T) { + t.Parallel() + + registry := NewRegistry() + registry.Register(&managerStubTool{name: "bash"}) + manager, err := NewManager(registry, nil, nil) + if err == nil || !strings.Contains(err.Error(), "permission engine is nil") { + t.Fatalf("expected nil engine error, got manager=%v err=%v", manager, err) + } +} + func TestManagerPermissionHelperBranches(t *testing.T) { t.Parallel() @@ -1832,7 +1852,7 @@ func TestDefaultManagerCapabilitySignerHelpers(t *testing.T) { registry := NewRegistry() registry.Register(&managerStubTool{name: "filesystem_read_file", content: "ok"}) - manager, err := NewManager(registry, nil, nil) + manager, err := NewManager(registry, mustAllowEngine(t), nil) if err != nil { t.Fatalf("new manager: %v", err) } diff --git a/internal/tools/webfetch/tool.go b/internal/tools/webfetch/tool.go index b468b666..284d46ef 100644 --- a/internal/tools/webfetch/tool.go +++ b/internal/tools/webfetch/tool.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "mime" + "net" "net/http" "net/url" "strings" @@ -55,6 +56,18 @@ type responseData struct { Truncated bool } +type validationContextKey string + +const bypassTargetValidationKey validationContextKey = "webfetch_bypass_target_validation" + +// WithUnsafeBypassTargetValidation 返回跳过目标地址安全校验的上下文，仅用于受控测试场景。 +func WithUnsafeBypassTargetValidation(ctx context.Context) context.Context { + if ctx == nil { + ctx = context.Background() + } + return context.WithValue(ctx, bypassTargetValidationKey, true) +} + // New creates a webfetch tool with bounded responses and content-type filtering. func New(cfg Config) *Tool { normalized := normalizeConfig(cfg) @@ -112,6 +125,12 @@ func (t *Tool) Execute(ctx context.Context, call tools.ToolCallInput) (tools.Too result := t.newErrorResult(responseData{URL: strings.TrimSpace(in.URL)}, reasonInvalidURL, err.Error()) return result, fmt.Errorf("webfetch: validate url: %w", err) } + if !bypassTargetValidation(ctx) { + if err := validateFetchTarget(ctx, targetURL); err != nil { + result := t.newErrorResult(responseData{URL: targetURL}, reasonInvalidURL, err.Error()) + return result, fmt.Errorf("webfetch: validate target: %w", err) + } + } resp, err := t.fetch(ctx, targetURL) if err != nil { @@ -150,6 +169,69 @@ func validateURL(raw string) (string, error) { return parsed.String(), nil } +// validateFetchTarget 校验目标主机是否命中本地或内网地址，防止 SSRF 访问敏感网络。 +func validateFetchTarget(ctx context.Context, target string) error { + parsed, err := url.Parse(target) + if err != nil { + return err + } + + hostname := strings.TrimSpace(parsed.Hostname()) + if hostname == "" { + return fmt.Errorf("%s: url host is empty", toolName) + } + if isLocalHostName(hostname) { + return fmt.Errorf("%s: target host is blocked", toolName) + } + if ip := net.ParseIP(hostname); ip != nil { + if isBlockedIP(ip) { + return fmt.Errorf("%s: target host is blocked", toolName) + } + return nil + } + + lookupCtx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + records, err := net.DefaultResolver.LookupIPAddr(lookupCtx, hostname) + if err != nil { + return fmt.Errorf("%s: resolve host: %w", toolName, err) + } + if len(records) == 0 { + return fmt.Errorf("%s: resolve host: empty result", toolName) + } + for _, record := range records { + if isBlockedIP(record.IP) { + return fmt.Errorf("%s: target host is blocked", toolName) + } + } + return nil +} + +// bypassTargetValidation 判断当前上下文是否显式跳过目标地址安全校验，仅供包内测试使用。 +func bypassTargetValidation(ctx context.Context) bool { + if ctx == nil { + return false + } + enabled, ok := ctx.Value(bypassTargetValidationKey).(bool) + return ok && enabled +} + +// isLocalHostName 判断主机名是否属于本地回环域名变体。 +func isLocalHostName(host string) bool { + normalized := strings.ToLower(strings.TrimSpace(host)) + return normalized == "localhost" || strings.HasSuffix(normalized, ".localhost") +} + +// isBlockedIP 判断 IP 是否属于回环、链路本地、私网或其他不应被 webfetch 访问的网段。 +func isBlockedIP(ip net.IP) bool { + return ip.IsLoopback() || + ip.IsLinkLocalUnicast() || + ip.IsLinkLocalMulticast() || + ip.IsPrivate() || + ip.IsMulticast() || + ip.IsUnspecified() +} + func (t *Tool) fetch(ctx context.Context, targetURL string) (*http.Response, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, targetURL, nil) if err != nil { diff --git a/internal/tools/webfetch/tool_test.go b/internal/tools/webfetch/tool_test.go index c4670ab7..6ae44859 100644 --- a/internal/tools/webfetch/tool_test.go +++ b/internal/tools/webfetch/tool_test.go @@ -14,6 +14,10 @@ import ( "neo-code/internal/tools" ) +func executionContextAllowLoopback() context.Context { + return WithUnsafeBypassTargetValidation(context.Background()) +} + func TestToolExecute(t *testing.T) { t.Parallel() @@ -281,7 +285,7 @@ func TestToolExecute(t *testing.T) { raw = data } - result, execErr := tool.Execute(context.Background(), tools.ToolCallInput{ + result, execErr := tool.Execute(executionContextAllowLoopback(), tools.ToolCallInput{ Name: tool.Name(), Arguments: raw, }) @@ -334,7 +338,7 @@ func TestToolDefaultsAndContentTypeFallback(t *testing.T) { t.Fatalf("marshal args: %v", err) } - result, execErr := tool.Execute(context.Background(), tools.ToolCallInput{ + result, execErr := tool.Execute(executionContextAllowLoopback(), tools.ToolCallInput{ Name: tool.Name(), Arguments: args, }) @@ -379,3 +383,50 @@ func TestHTMLHelpers(t *testing.T) { t.Fatalf("expected title text content, got %q", textContent(titleNode)) } } + +func TestToolExecuteBlocksLocalAndPrivateTargets(t *testing.T) { + t.Parallel() + + tool := New(Config{ + Timeout: 2 * time.Second, + MaxResponseBytes: config.DefaultWebFetchMaxResponseBytes, + SupportedContentTypes: config.DefaultWebFetchSupportedContentTypes(), + }) + + tests := []struct { + name string + url string + }{ + {name: "localhost", url: "http://localhost/internal"}, + {name: "localhost subdomain", url: "http://api.localhost/internal"}, + {name: "loopback ipv4", url: "http://127.0.0.1/internal"}, + {name: "link local metadata", url: "http://169.254.169.254/latest/meta-data"}, + {name: "private network", url: "http://10.1.2.3/internal"}, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + raw, err := json.Marshal(map[string]string{"url": tc.url}) + if err != nil { + t.Fatalf("marshal args: %v", err) + } + + result, execErr := tool.Execute(context.Background(), tools.ToolCallInput{ + Name: tool.Name(), + Arguments: raw, + }) + if execErr == nil || !strings.Contains(execErr.Error(), "target host is blocked") { + t.Fatalf("expected blocked target error, got %v", execErr) + } + if !result.IsError { + t.Fatalf("expected error result for blocked target") + } + if !strings.Contains(result.Content, "reason: "+reasonInvalidURL) { + t.Fatalf("expected invalid url reason, got %q", result.Content) + } + }) + } +} From 33d39a7dfeae07e95adb765730d0f55e55917c77 Mon Sep 17 00:00:00 2001 From: xgopilot Date: Fri, 17 Apr 2026 08:44:24 +0000 Subject: [PATCH 2/4] fix(webfetch): validate target IP on actual dial path Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com> --- internal/tools/webfetch/tool.go | 55 +++++++++++++++++++++++++++- internal/tools/webfetch/tool_test.go | 20 ++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/internal/tools/webfetch/tool.go b/internal/tools/webfetch/tool.go index 284d46ef..cc0e7097 100644 --- a/internal/tools/webfetch/tool.go +++ b/internal/tools/webfetch/tool.go @@ -80,8 +80,19 @@ func New(cfg Config) *Tool { // newHTTPClient 创建禁止自动重定向的客户端，避免跨域重定向绕过上层网络权限校验。 func newHTTPClient(timeout time.Duration) *http.Client { + transport := http.DefaultTransport.(*http.Transport).Clone() + dialer := &net.Dialer{Timeout: timeout} + transport.DialContext = func(ctx context.Context, network, address string) (net.Conn, error) { + dialAddress, err := resolveDialAddress(ctx, address) + if err != nil { + return nil, err + } + return dialer.DialContext(ctx, network, dialAddress) + } + return &http.Client{ - Timeout: timeout, + Timeout: timeout, + Transport: transport, CheckRedirect: func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }, @@ -207,6 +218,48 @@ func validateFetchTarget(ctx context.Context, target string) error { return nil } +// resolveDialAddress 在真实拨号前校验并收敛最终目标地址，避免 DNS 重绑定导致的 TOCTOU 绕过。 +func resolveDialAddress(ctx context.Context, address string) (string, error) { + host, port, err := net.SplitHostPort(address) + if err != nil { + return "", fmt.Errorf("%s: invalid dial address", toolName) + } + host = strings.TrimSpace(host) + if host == "" { + return "", fmt.Errorf("%s: url host is empty", toolName) + } + if bypassTargetValidation(ctx) { + return address, nil + } + if isLocalHostName(host) { + return "", fmt.Errorf("%s: target host is blocked", toolName) + } + + if ip := net.ParseIP(host); ip != nil { + if isBlockedIP(ip) { + return "", fmt.Errorf("%s: target host is blocked", toolName) + } + return net.JoinHostPort(ip.String(), port), nil + } + + lookupCtx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + records, err := net.DefaultResolver.LookupIPAddr(lookupCtx, host) + if err != nil { + return "", fmt.Errorf("%s: resolve host: %w", toolName, err) + } + for _, record := range records { + if isBlockedIP(record.IP) { + continue + } + return net.JoinHostPort(record.IP.String(), port), nil + } + if len(records) == 0 { + return "", fmt.Errorf("%s: resolve host: empty result", toolName) + } + return "", fmt.Errorf("%s: target host is blocked", toolName) +} + // bypassTargetValidation 判断当前上下文是否显式跳过目标地址安全校验，仅供包内测试使用。 func bypassTargetValidation(ctx context.Context) bool { if ctx == nil { diff --git a/internal/tools/webfetch/tool_test.go b/internal/tools/webfetch/tool_test.go index 6ae44859..58c79df6 100644 --- a/internal/tools/webfetch/tool_test.go +++ b/internal/tools/webfetch/tool_test.go @@ -430,3 +430,23 @@ func TestToolExecuteBlocksLocalAndPrivateTargets(t *testing.T) { }) } } + +func TestFetchBlocksLoopbackAtDial(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte("ok")) + })) + defer server.Close() + + tool := New(Config{ + Timeout: 2 * time.Second, + MaxResponseBytes: config.DefaultWebFetchMaxResponseBytes, + SupportedContentTypes: config.DefaultWebFetchSupportedContentTypes(), + }) + + _, err := tool.fetch(context.Background(), server.URL) + if err == nil || !strings.Contains(err.Error(), "target host is blocked") { + t.Fatalf("expected loopback dial to be blocked, got %v", err) + } +} From f6a811788c53381c8224aef11cc0576dc492c1d4 Mon Sep 17 00:00:00 2001 From: xgopilot Date: Fri, 17 Apr 2026 08:55:39 +0000 Subject: [PATCH 3/4] =?UTF-8?q?test(tools):=20=E4=BF=AE=E5=A4=8D=20manager?= =?UTF-8?q?=5Ftest=20=E7=BC=96=E8=AF=91=E9=94=99=E8=AF=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com> --- internal/tools/manager_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/tools/manager_test.go b/internal/tools/manager_test.go index 3f378b2f..c5fb06e1 100644 --- a/internal/tools/manager_test.go +++ b/internal/tools/manager_test.go @@ -114,7 +114,7 @@ func TestDefaultManagerMicroCompactPolicy(t *testing.T) { t.Run("executor without policy support defaults to compact", func(t *testing.T) { t.Parallel() - manager, err := NewManager(executorWithoutMicroCompactPolicy{}, mustAllowEngine(t), nil) + manager, err := NewManager(executorWithoutOptionalCompactFeatures{}, mustAllowEngine(t), nil) if err != nil { t.Fatalf("new manager: %v", err) } @@ -154,7 +154,7 @@ func TestDefaultManagerMicroCompactSummarizer(t *testing.T) { t.Run("executor without summarizer support returns nil", func(t *testing.T) { t.Parallel() - manager, err := NewManager(executorWithoutOptionalCompactFeatures{}, nil, nil) + manager, err := NewManager(executorWithoutOptionalCompactFeatures{}, mustAllowEngine(t), nil) if err != nil { t.Fatalf("new manager: %v", err) } @@ -171,7 +171,7 @@ func TestDefaultManagerMicroCompactSummarizer(t *testing.T) { return "summary:" + content }) - manager, err := NewManager(registry, nil, nil) + manager, err := NewManager(registry, mustAllowEngine(t), nil) if err != nil { t.Fatalf("new manager: %v", err) } From ca909c6c78164318cdda10da134cab4c2bf80216 Mon Sep 17 00:00:00 2001 From: xgopilot Date: Fri, 17 Apr 2026 09:14:28 +0000 Subject: [PATCH 4/4] fix(webfetch): remove unsafe bypass API and add dial fallback Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: phantom5099 <245659304+phantom5099@users.noreply.github.com> --- internal/app/bootstrap_test.go | 34 +++------- internal/tools/webfetch/tool.go | 71 +++++++++++++------- internal/tools/webfetch/tool_test.go | 98 +++++++++++++++++++++++++++- 3 files changed, 153 insertions(+), 50 deletions(-) diff --git a/internal/app/bootstrap_test.go b/internal/app/bootstrap_test.go index 7b451150..ad84cb48 100644 --- a/internal/app/bootstrap_test.go +++ b/internal/app/bootstrap_test.go @@ -8,10 +8,9 @@ import ( "errors" "fmt" "io" - "net/http" - "net/http/httptest" "os" "path/filepath" + "reflect" "strconv" "strings" "testing" @@ -27,7 +26,6 @@ import ( "neo-code/internal/skills" "neo-code/internal/tools" "neo-code/internal/tools/mcp" - "neo-code/internal/tools/webfetch" "neo-code/internal/tui" ) @@ -141,12 +139,6 @@ func TestBuildRuntimeRejectsUnsupportedSelectedProviderDriverOnStartup(t *testin func TestBuildToolRegistryUsesWebFetchConfig(t *testing.T) { t.Parallel() - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "text/plain") - _, _ = w.Write([]byte("1234567890")) - })) - defer server.Close() - cfg := config.StaticDefaults().Clone() cfg.Workdir = t.TempDir() cfg.Tools.WebFetch.MaxResponseBytes = 4 @@ -163,23 +155,17 @@ func TestBuildToolRegistryUsesWebFetchConfig(t *testing.T) { t.Fatalf("registry.Get(webfetch) error = %v", err) } - args, err := json.Marshal(map[string]string{"url": server.URL}) - if err != nil { - t.Fatalf("marshal args: %v", err) - } - - result, execErr := tool.Execute(webfetch.WithUnsafeBypassTargetValidation(context.Background()), tools.ToolCallInput{ - Name: "webfetch", - Arguments: args, - }) - if execErr != nil { - t.Fatalf("webfetch execute error = %v", execErr) + concrete := reflect.ValueOf(tool) + if concrete.Kind() != reflect.Ptr { + t.Fatalf("expected pointer tool, got %T", tool) } - if truncated, ok := result.Metadata["truncated"].(bool); !ok || !truncated { - t.Fatalf("expected truncated metadata, got %+v", result.Metadata) + cfgField := concrete.Elem().FieldByName("cfg") + if !cfgField.IsValid() { + t.Fatalf("expected webfetch tool cfg field") } - if result.Content == "" { - t.Fatalf("expected formatted webfetch content") + maxBytesField := cfgField.FieldByName("MaxResponseBytes") + if maxBytesField.Int() != 4 { + t.Fatalf("expected MaxResponseBytes=4, got %d", maxBytesField.Int()) } } diff --git a/internal/tools/webfetch/tool.go b/internal/tools/webfetch/tool.go index cc0e7097..3b353902 100644 --- a/internal/tools/webfetch/tool.go +++ b/internal/tools/webfetch/tool.go @@ -60,12 +60,8 @@ type validationContextKey string const bypassTargetValidationKey validationContextKey = "webfetch_bypass_target_validation" -// WithUnsafeBypassTargetValidation 返回跳过目标地址安全校验的上下文，仅用于受控测试场景。 -func WithUnsafeBypassTargetValidation(ctx context.Context) context.Context { - if ctx == nil { - ctx = context.Background() - } - return context.WithValue(ctx, bypassTargetValidationKey, true) +var lookupIPAddr = func(ctx context.Context, host string) ([]net.IPAddr, error) { + return net.DefaultResolver.LookupIPAddr(ctx, host) } // New creates a webfetch tool with bounded responses and content-type filtering. @@ -83,11 +79,11 @@ func newHTTPClient(timeout time.Duration) *http.Client { transport := http.DefaultTransport.(*http.Transport).Clone() dialer := &net.Dialer{Timeout: timeout} transport.DialContext = func(ctx context.Context, network, address string) (net.Conn, error) { - dialAddress, err := resolveDialAddress(ctx, address) + dialAddresses, err := resolveDialAddresses(ctx, address) if err != nil { return nil, err } - return dialer.DialContext(ctx, network, dialAddress) + return dialFirstReachable(ctx, dialer.DialContext, network, dialAddresses) } return &http.Client{ @@ -203,7 +199,7 @@ func validateFetchTarget(ctx context.Context, target string) error { lookupCtx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - records, err := net.DefaultResolver.LookupIPAddr(lookupCtx, hostname) + records, err := lookupIPAddr(lookupCtx, hostname) if err != nil { return fmt.Errorf("%s: resolve host: %w", toolName, err) } @@ -218,46 +214,71 @@ func validateFetchTarget(ctx context.Context, target string) error { return nil } -// resolveDialAddress 在真实拨号前校验并收敛最终目标地址，避免 DNS 重绑定导致的 TOCTOU 绕过。 -func resolveDialAddress(ctx context.Context, address string) (string, error) { +// resolveDialAddresses 在真实拨号前校验并收敛可用目标地址集合，避免 DNS 重绑定导致的 TOCTOU 绕过。 +func resolveDialAddresses(ctx context.Context, address string) ([]string, error) { host, port, err := net.SplitHostPort(address) if err != nil { - return "", fmt.Errorf("%s: invalid dial address", toolName) + return nil, fmt.Errorf("%s: invalid dial address", toolName) } host = strings.TrimSpace(host) if host == "" { - return "", fmt.Errorf("%s: url host is empty", toolName) + return nil, fmt.Errorf("%s: url host is empty", toolName) } if bypassTargetValidation(ctx) { - return address, nil + return []string{address}, nil } if isLocalHostName(host) { - return "", fmt.Errorf("%s: target host is blocked", toolName) + return nil, fmt.Errorf("%s: target host is blocked", toolName) } if ip := net.ParseIP(host); ip != nil { if isBlockedIP(ip) { - return "", fmt.Errorf("%s: target host is blocked", toolName) + return nil, fmt.Errorf("%s: target host is blocked", toolName) } - return net.JoinHostPort(ip.String(), port), nil + return []string{net.JoinHostPort(ip.String(), port)}, nil } lookupCtx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - records, err := net.DefaultResolver.LookupIPAddr(lookupCtx, host) + records, err := lookupIPAddr(lookupCtx, host) if err != nil { - return "", fmt.Errorf("%s: resolve host: %w", toolName, err) + return nil, fmt.Errorf("%s: resolve host: %w", toolName, err) + } + if len(records) == 0 { + return nil, fmt.Errorf("%s: resolve host: empty result", toolName) } + + allowed := make([]string, 0, len(records)) for _, record := range records { - if isBlockedIP(record.IP) { - continue + if !isBlockedIP(record.IP) { + allowed = append(allowed, net.JoinHostPort(record.IP.String(), port)) } - return net.JoinHostPort(record.IP.String(), port), nil } - if len(records) == 0 { - return "", fmt.Errorf("%s: resolve host: empty result", toolName) + if len(allowed) == 0 { + return nil, fmt.Errorf("%s: target host is blocked", toolName) + } + return allowed, nil +} + +type dialContextFunc func(ctx context.Context, network, address string) (net.Conn, error) + +// dialFirstReachable 依次尝试多个候选地址，命中首个可连通地址后立即返回，全部失败时返回最后一次错误。 +func dialFirstReachable(ctx context.Context, dialFn dialContextFunc, network string, addresses []string) (net.Conn, error) { + if len(addresses) == 0 { + return nil, fmt.Errorf("%s: invalid dial address", toolName) + } + var lastErr error + for _, address := range addresses { + conn, err := dialFn(ctx, network, address) + if err == nil { + return conn, nil + } + lastErr = err + } + if lastErr != nil { + return nil, lastErr } - return "", fmt.Errorf("%s: target host is blocked", toolName) + return nil, fmt.Errorf("%s: invalid dial address", toolName) } // bypassTargetValidation 判断当前上下文是否显式跳过目标地址安全校验，仅供包内测试使用。 diff --git a/internal/tools/webfetch/tool_test.go b/internal/tools/webfetch/tool_test.go index 58c79df6..ce06d82a 100644 --- a/internal/tools/webfetch/tool_test.go +++ b/internal/tools/webfetch/tool_test.go @@ -3,6 +3,8 @@ package webfetch import ( "context" "encoding/json" + "errors" + "net" "net/http" "net/http/httptest" "strings" @@ -15,7 +17,7 @@ import ( ) func executionContextAllowLoopback() context.Context { - return WithUnsafeBypassTargetValidation(context.Background()) + return context.WithValue(context.Background(), bypassTargetValidationKey, true) } func TestToolExecute(t *testing.T) { @@ -450,3 +452,97 @@ func TestFetchBlocksLoopbackAtDial(t *testing.T) { t.Fatalf("expected loopback dial to be blocked, got %v", err) } } + +func TestResolveDialAddressesPrefersAllowedSet(t *testing.T) { + originalLookup := lookupIPAddr + t.Cleanup(func() { + lookupIPAddr = originalLookup + }) + lookupIPAddr = func(ctx context.Context, host string) ([]net.IPAddr, error) { + return []net.IPAddr{ + {IP: net.ParseIP("203.0.113.1")}, + {IP: net.ParseIP("127.0.0.1")}, + {IP: net.ParseIP("198.51.100.2")}, + }, nil + } + + addresses, err := resolveDialAddresses(context.Background(), "example.com:443") + if err != nil { + t.Fatalf("resolveDialAddresses() error = %v", err) + } + if len(addresses) != 2 { + t.Fatalf("expected 2 allowed addresses, got %v", addresses) + } + if addresses[0] != "203.0.113.1:443" || addresses[1] != "198.51.100.2:443" { + t.Fatalf("unexpected addresses order: %v", addresses) + } +} + +func TestResolveDialAddressesBlockedOrEmpty(t *testing.T) { + t.Run("empty dns result", func(t *testing.T) { + originalLookup := lookupIPAddr + t.Cleanup(func() { + lookupIPAddr = originalLookup + }) + lookupIPAddr = func(ctx context.Context, host string) ([]net.IPAddr, error) { + return nil, nil + } + + _, err := resolveDialAddresses(context.Background(), "example.com:80") + if err == nil || !strings.Contains(err.Error(), "empty result") { + t.Fatalf("expected empty result error, got %v", err) + } + }) + + t.Run("all blocked", func(t *testing.T) { + originalLookup := lookupIPAddr + t.Cleanup(func() { + lookupIPAddr = originalLookup + }) + lookupIPAddr = func(ctx context.Context, host string) ([]net.IPAddr, error) { + return []net.IPAddr{{IP: net.ParseIP("127.0.0.1")}}, nil + } + + _, err := resolveDialAddresses(context.Background(), "example.com:80") + if err == nil || !strings.Contains(err.Error(), "target host is blocked") { + t.Fatalf("expected blocked target error, got %v", err) + } + }) +} + +func TestDialFirstReachable(t *testing.T) { + t.Parallel() + + t.Run("fallback to later address", func(t *testing.T) { + var called []string + connA, connB := net.Pipe() + t.Cleanup(func() { + _ = connA.Close() + _ = connB.Close() + }) + + conn, err := dialFirstReachable(context.Background(), func(ctx context.Context, network, address string) (net.Conn, error) { + called = append(called, address) + if address != "198.51.100.2:443" { + return nil, errors.New("dial failed") + } + return connA, nil + }, "tcp", []string{"203.0.113.1:443", "192.0.2.1:443", "198.51.100.2:443"}) + if err != nil { + t.Fatalf("dialFirstReachable() error = %v", err) + } + _ = conn.Close() + if strings.Join(called, ",") != "203.0.113.1:443,192.0.2.1:443,198.51.100.2:443" { + t.Fatalf("unexpected dial sequence: %v", called) + } + }) + + t.Run("return last error", func(t *testing.T) { + _, err := dialFirstReachable(context.Background(), func(ctx context.Context, network, address string) (net.Conn, error) { + return nil, errors.New("last dial error") + }, "tcp", []string{"198.51.100.2:443"}) + if err == nil || !strings.Contains(err.Error(), "last dial error") { + t.Fatalf("expected last dial error, got %v", err) + } + }) +}