Fix diagnostic path encoding and long export lifetimes

.changeset/mcp-encoded-api-paths.md

@@ -0,0 +1,5 @@
+---
+"ftw": patch
+---
+
+Keep URL-encoded driver names intact in MCP diagnostic reads and allow large exports five minutes to finish.

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.",

go/internal/appproto/agentapi.go

@@ -3,6 +3,7 @@ package appproto
 import (
 	"context"
 	"net/http"
+	"net/url"
 	"sync"
 
 	"github.com/srcfl/ftw/go/protocol"
@@ -37,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()
@@ -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() {

go/internal/appproto/agentapi_test.go

@@ -1,7 +1,9 @@
 package appproto
 
 import (
+	"encoding/json"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
@@ -32,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")
 				}
@@ -86,3 +92,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))
+			}
+		})
+	}
+}

go/protocol/agentapi.go

@@ -1,8 +1,10 @@
 package protocol
 
 import (
+	"net/url"
 	"path"
 	"strings"
+	"time"
 	"unicode"
 )
 
@@ -25,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"
 )
 
@@ -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
 }

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)
 		}