From d512cde21a9c14072307a7fc682f66a0d472f503 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 16:31:04 +0200 Subject: [PATCH 1/3] Keep URL-encoded diagnostic path parameters intact --- .changeset/mcp-encoded-api-paths.md | 5 +++++ go/internal/appproto/agentapi.go | 6 ++++++ go/internal/appproto/agentapi_test.go | 24 ++++++++++++++++++++++++ go/protocol/agentapi.go | 22 ++++++++++++++++++---- go/protocol/agentapi_test.go | 4 ++-- 5 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 .changeset/mcp-encoded-api-paths.md diff --git a/.changeset/mcp-encoded-api-paths.md b/.changeset/mcp-encoded-api-paths.md new file mode 100644 index 000000000..d79e2a1d5 --- /dev/null +++ b/.changeset/mcp-encoded-api-paths.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Allow MCP diagnostic reads for URL-encoded driver names, including spaces, slashes and percent signs. diff --git a/go/internal/appproto/agentapi.go b/go/internal/appproto/agentapi.go index 80ca8c669..cc67e02ba 100644 --- a/go/internal/appproto/agentapi.go +++ b/go/internal/appproto/agentapi.go @@ -3,6 +3,7 @@ package appproto import ( "context" "net/http" + "net/url" "sync" "github.com/srcfl/ftw/go/protocol" @@ -55,6 +56,11 @@ func (h *Handler) onAPIRead(env Envelope) error { return } req := buildAPIRequest(ctx, caller, request) + // API paths use ordinary URL encoding. Keep RawPath so an escaped + // slash remains inside one driver-name segment in Go's ServeMux. + target, _ := url.ParseRequestURI(read.Path) + req.URL.Path, req.URL.RawPath = target.Path, target.RawPath + req.RequestURI = req.URL.RequestURI() writer := &apiWriter{h: h, id: id, ctx: ctx, max: protocol.AgentAPIMaxBytes, free: free, header: http.Header{}, buf: make([]byte, 0, APIChunkBytes)} writer.recheck = func() *ErrorBody { if h.isAgent() { diff --git a/go/internal/appproto/agentapi_test.go b/go/internal/appproto/agentapi_test.go index fda30d823..67bb7f084 100644 --- a/go/internal/appproto/agentapi_test.go +++ b/go/internal/appproto/agentapi_test.go @@ -1,7 +1,9 @@ package appproto import ( + "encoding/json" "net/http" + "net/url" "testing" "time" @@ -86,3 +88,25 @@ func TestRevocationStopsADiagnosticReadBeforeItsAnswer(t *testing.T) { t.Fatal("a revoked read returned data") } } + +func TestDiagnosticReadsKeepEncodedDriverNamesInOnePathSegment(t *testing.T) { + for _, name := range []string{"EV charger", "EV charger/garage", "battery%name", "/garage", "EV//charger", `battery\garage`} { + t.Run(name, func(t *testing.T) { + r := newAgentRig(t) + row, _ := r.grant(apiauth.RoleAgent, []string{protocol.ScopeDiagRun}, nil, protocol.Caps{}, time.Hour, protocol.Principal{}) + h, rec := r.agentSession(row) + t.Cleanup(h.Close) + mux := http.NewServeMux() + mux.HandleFunc("GET /api/drivers/{name}/logs", func(w http.ResponseWriter, request *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]string{"driver": request.PathValue("name")}) + }) + h.cfg.API = &diagnosticGateway{read: mux.ServeHTTP} + deliver(t, h, protocol.MsgAPIRead, ptrU32(1), protocol.APIRead{Path: "/api/drivers/" + url.PathEscape(name) + "/logs"}) + end := body[APIEnd](t, waitFor(t, rec, MsgAPIEnd)) + var answer map[string]string + if end.Truncated || json.Unmarshal(apiChunks(rec), &answer) != nil || answer["driver"] != name { + t.Fatalf("driver name changed: %s", apiChunks(rec)) + } + }) + } +} diff --git a/go/protocol/agentapi.go b/go/protocol/agentapi.go index 9a070da47..b30b5cd9b 100644 --- a/go/protocol/agentapi.go +++ b/go/protocol/agentapi.go @@ -1,6 +1,7 @@ package protocol import ( + "net/url" "path" "strings" "unicode" @@ -10,7 +11,8 @@ import ( // Offset and MaxBytes select a byte page. IfSHA256 prevents joining pages // from different answers. The answer uses api.head/chunk/end, carrying JSON. type APIRead struct { - Path string `cbor:"path"` + Path string `cbor:"path" + "net/url"` Query map[string]string `cbor:"query,omitempty"` MaxBytes int64 `cbor:"maxBytes,omitempty"` Offset int64 `cbor:"offset,omitempty"` @@ -33,7 +35,8 @@ const ( // AgentAPIRoute describes a route registered by the connected Core version. // Query is guidance; the existing HTTP handler owns parameter validation. type AgentAPIRoute struct { - Path string `json:"path"` + Path string `json:"path" + "net/url"` Scope string `json:"scope"` Description string `json:"description"` Query map[string]string `json:"query,omitempty"` @@ -42,6 +45,17 @@ type AgentAPIRoute struct { // ValidAgentAPIPath validates syntax only. Core's router decides whether the // path is a read; Cloud does not keep a second list of the box's API. func ValidAgentAPIPath(value string) bool { - return strings.HasPrefix(value, "/api/") && path.Clean(value) == value && - !strings.ContainsAny(value, "?%#\\") && strings.IndexFunc(value, unicode.IsControl) < 0 + target, err := url.ParseRequestURI(value) + if err != nil || !strings.HasPrefix(value, "/api/") || path.Clean(value) != value || + target.RawQuery != "" || target.ForceQuery || strings.ContainsAny(value, "?#\\") || + strings.IndexFunc(target.Path, unicode.IsControl) >= 0 { + return false + } + for _, part := range strings.Split(value, "/") { + decoded, _ := url.PathUnescape(part) + if decoded == "." || decoded == ".." { + return false + } + } + return true } diff --git a/go/protocol/agentapi_test.go b/go/protocol/agentapi_test.go index 934dd9982..5cb3cb02d 100644 --- a/go/protocol/agentapi_test.go +++ b/go/protocol/agentapi_test.go @@ -33,12 +33,12 @@ func TestDiagnosticPageVector(t *testing.T) { } } func TestDiagnosticPathSyntaxDoesNotKeepAnAPIList(t *testing.T) { - for _, value := range []string{"/api/future/read", "/api/drivers/charger-192.168.1.9/source"} { + for _, value := range []string{"/api/future/read", "/api/drivers/charger-192.168.1.9/source", "/api/drivers/EV%20charger%2Fgarage/logs", "/api/drivers/battery%25name"} { if !ValidAgentAPIPath(value) { t.Errorf("valid path refused: %s", value) } } - for _, value := range []string{"/status", "/api/../config", "/api/status?x=1", "/api/status#x", "/api/a%2fb", "/api/a\\b", "/api/a\n"} { + for _, value := range []string{"/status", "/api/../config", "/api/status?x=1", "/api/status#x", "/api/%2e%2e/config", "/api/a%0Ab", "/api/a%ZZ", "/api/a\\b", "/api/a\n"} { if ValidAgentAPIPath(value) { t.Errorf("invalid path accepted: %q", value) } From 0dda6cc81ae1dfd3165aa735119e0f0a12252d6b Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 16:33:12 +0200 Subject: [PATCH 2/3] Keep diagnostic path struct tags unchanged --- go/protocol/agentapi.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/go/protocol/agentapi.go b/go/protocol/agentapi.go index b30b5cd9b..65cc3c648 100644 --- a/go/protocol/agentapi.go +++ b/go/protocol/agentapi.go @@ -11,8 +11,7 @@ import ( // Offset and MaxBytes select a byte page. IfSHA256 prevents joining pages // from different answers. The answer uses api.head/chunk/end, carrying JSON. type APIRead struct { - Path string `cbor:"path" - "net/url"` + Path string `cbor:"path"` Query map[string]string `cbor:"query,omitempty"` MaxBytes int64 `cbor:"maxBytes,omitempty"` Offset int64 `cbor:"offset,omitempty"` @@ -35,8 +34,7 @@ const ( // AgentAPIRoute describes a route registered by the connected Core version. // Query is guidance; the existing HTTP handler owns parameter validation. type AgentAPIRoute struct { - Path string `json:"path" - "net/url"` + Path string `json:"path"` Scope string `json:"scope"` Description string `json:"description"` Query map[string]string `json:"query,omitempty"` From 0546e9d76dea56958b75442f13b4ca110462bd8f Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Sun, 18 Oct 2026 16:41:06 +0200 Subject: [PATCH 3/3] Give diagnostic exports their own five-minute call lifetime --- .changeset/mcp-encoded-api-paths.md | 2 +- go/internal/api/api_agent_reads.go | 1 + go/internal/appproto/agentapi.go | 2 +- go/internal/appproto/agentapi_test.go | 4 ++++ go/protocol/agentapi.go | 6 ++++-- 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.changeset/mcp-encoded-api-paths.md b/.changeset/mcp-encoded-api-paths.md index d79e2a1d5..cb7443a0c 100644 --- a/.changeset/mcp-encoded-api-paths.md +++ b/.changeset/mcp-encoded-api-paths.md @@ -2,4 +2,4 @@ "ftw": patch --- -Allow MCP diagnostic reads for URL-encoded driver names, including spaces, slashes and percent signs. +Keep URL-encoded driver names intact in MCP diagnostic reads and allow large exports five minutes to finish. diff --git a/go/internal/api/api_agent_reads.go b/go/internal/api/api_agent_reads.go index 7f790d6e7..5931f652b 100644 --- a/go/internal/api/api_agent_reads.go +++ b/go/internal/api/api_agent_reads.go @@ -29,6 +29,7 @@ func (s *Server) ServeAgentRead(w http.ResponseWriter, request *http.Request, re if request.URL.Path == protocol.AgentAPICatalogPath { writeJSON(w, http.StatusOK, map[string]any{ "routes": s.agentReadCatalog(), "requiredScope": protocol.ScopeDiagRun, + "timeoutMs": protocol.AgentAPIReadTimeout.Milliseconds(), "maxPageBytes": protocol.AgentAPIPageBytes, "troubleshooting": s.agentDebugContext(), "queryPolicy": "Use the existing API's parameters. Query hints are not a restriction.", diff --git a/go/internal/appproto/agentapi.go b/go/internal/appproto/agentapi.go index cc67e02ba..5b5f355f7 100644 --- a/go/internal/appproto/agentapi.go +++ b/go/internal/appproto/agentapi.go @@ -38,7 +38,7 @@ func (h *Handler) onAPIRead(env Envelope) error { var once sync.Once free := func() { once.Do(func() { h.apiBusy.Store(false) }) } defer free() - ctx, cancel := context.WithTimeout(h.ctx, apiHandlerTimeout) + ctx, cancel := context.WithTimeout(h.ctx, protocol.AgentAPIReadTimeout) defer cancel() if h.isAgent() { _, refusal, end := h.agentGate() diff --git a/go/internal/appproto/agentapi_test.go b/go/internal/appproto/agentapi_test.go index 67bb7f084..e0a6cf03a 100644 --- a/go/internal/appproto/agentapi_test.go +++ b/go/internal/appproto/agentapi_test.go @@ -34,6 +34,10 @@ func TestAnAgentDiagnosticReadNeedsOnlyItsDiagnosticGrant(t *testing.T) { called := make(chan struct{}, 1) h.cfg.API = &diagnosticGateway{stubAPI: stubAPI{facts: apiauth.RouteFacts{Tier: apiauth.TierRead}}, read: func(w http.ResponseWriter, request *http.Request) { called <- struct{}{} + deadline, ok := request.Context().Deadline() + if !ok || time.Until(deadline) < time.Minute { + t.Error("diagnostic export inherited the short status-read deadline") + } if request.Method != http.MethodGet || request.URL.Path != "/api/energy/history" { t.Error("wrong diagnostic request") } diff --git a/go/protocol/agentapi.go b/go/protocol/agentapi.go index 65cc3c648..0941fffca 100644 --- a/go/protocol/agentapi.go +++ b/go/protocol/agentapi.go @@ -4,6 +4,7 @@ import ( "net/url" "path" "strings" + "time" "unicode" ) @@ -26,8 +27,9 @@ func DecodeAPIRead(body []byte) (APIRead, error) { } const ( - AgentAPIMaxBytes = 1 << 20 // Complete JSON wire answer, including escaped text. - AgentAPIPageBytes = 128 << 10 // Source bytes per page; not a total data limit. + AgentAPIReadTimeout = 5 * time.Minute // Large exports may need more time than a status read. + AgentAPIMaxBytes = 1 << 20 // Complete JSON wire answer, including escaped text. + AgentAPIPageBytes = 128 << 10 // Source bytes per page; not a total data limit. AgentAPICatalogPath = "/api/diagnostics/catalog" )