fix!: deny percent-encoded request paths (#53)

AGENTS.md

@@ -37,17 +37,17 @@ deploy/ — Docker Compose + integration tests
 - Zero external deps where possible (Go: yaml.v3, Rust: tokio/hyper/serde/clap, TS: yaml)
 
 ## Test Coverage
-- Go: 103 unit tests (main/listener: 24, policy: 10, middleware: 29, proxy: 36, audit: 4)
-- Rust: 140 unit tests (main/listener: 23, policy: 15, middleware: 50, proxy: 42, handler: 4, audit: 4, transport: 2)
-- TypeScript: 157 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 41, proxy: 29, policy: 10, handler: 6, shutdown: 5, transport: 5, audit: 4)
-- Integration, per implementation: 36 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose)
-- Quint: `make test-spec` runs the `spec/listener.qnt` `run` tests (instances `listener_locked`, `listener_unlocked`) and the `spec/router.qnt` `run` tests (instances `router`, `router_pre48`)
+- Go: 107 unit tests (main/listener: 24, policy: 10, middleware: 29, proxy: 40, audit: 4)
+- Rust: 144 unit tests (main/listener: 23, policy: 15, middleware: 50, proxy: 43, handler: 7, audit: 4, transport: 2)
+- TypeScript: 161 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 41, proxy: 30, policy: 10, handler: 9, shutdown: 5, transport: 5, audit: 4)
+- Integration, per implementation: 39 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose)
+- Quint: `make test-spec` runs the `spec/listener.qnt` `run` tests (instances `listener_locked`, `listener_unlocked`) and the `spec/router.qnt` `run` tests (instances `router`, `router_pre48`, `router_pre53`)
 
 ## Test Conventions
 - Go: stdlib `testing` package, `go test ./...`
 - Rust: `#[cfg(test)]` inline modules, `cargo test`
 - TypeScript: `node:test` framework, `npm run build && node --test dist/*.test.js`
-- Integration: `make test-integration` (36 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose
+- Integration: `make test-integration` (39 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose
 
 ## Contribution Workflow
 

Makefile

@@ -84,6 +84,7 @@ test-spec:
 	$(QUINT) test $(LISTENER_SPEC) --main=listener_unlocked
 	$(QUINT) test $(ROUTER_SPEC) --main=router
 	$(QUINT) test $(ROUTER_SPEC) --main=router_pre48
+	$(QUINT) test $(ROUTER_SPEC) --main=router_pre53
 
 verify-ts:
 	$(QUINT) run $(SPEC) --max-steps=50 --invariants allInvariants --backend typescript

README.md

@@ -100,9 +100,9 @@ All three implementations expose the same API surface, share the same [Quint spe
 
 | Language | Directory | Tests | Stack |
 |----------|-----------|-------|-------|
-| Go | [go/](go/) | 103 unit + 36 integration | stdlib net/http + yaml.v3 |
-| Rust | [rs/](rs/) | 140 unit | tokio, hyper, serde, clap |
-| TypeScript | [ts/](ts/) | 157 unit (1 skipped) | Node 22 ESM, built-in http |
+| Go | [go/](go/) | 107 unit + 39 integration | stdlib net/http + yaml.v3 |
+| Rust | [rs/](rs/) | 144 unit | tokio, hyper, serde, clap |
+| TypeScript | [ts/](ts/) | 161 unit (1 skipped) | Node 22 ESM, built-in http |
 
 ### Build All
 
@@ -204,6 +204,7 @@ docker pull attacker/malware:latest  # denied: image not in allowlist
 
 | HTTP Method | Path | Action |
 |-------------|------|--------|
+| Any | Path containing `%` | **DENIED** (see below) |
 | POST | `/containers/create` | Validated by middleware chain |
 | POST | `/containers/{name}/start\|stop\|restart\|kill\|wait\|pause\|unpause` | Allowed on known containers |
 | DELETE | `/containers/{name}` | Allowed on known containers |
@@ -213,9 +214,11 @@ docker pull attacker/malware:latest  # denied: image not in allowlist
 | POST | `/auth` | **DENIED** |
 | POST | `/build` | **DENIED** |
 | POST | `/commit` | **DENIED** |
-| GET/HEAD | Any | Allowed (read-only) |
+| GET/HEAD | Any path without `%` | Allowed (read-only) |
 | Other | Other | **DENIED** |
 
+Any request whose path contains a percent-encoded byte (`%`) is denied with 403 for every method, GET and HEAD included, because the daemon decodes the path before routing. The query string is not inspected, so filters such as `docker ps --filter …` still work. The Go implementation also denies paths that contain raw characters it must re-encode, such as non-ASCII bytes or `{`; the Docker CLI never sends these. A consequence is that networks whose names need percent-encoding (for example a space or `%`) cannot be inspected by name through the proxy; inspecting them by ID still works, and other network operations are denied regardless.
+
 ## Configuration
 
 ### CLI Flags

deploy/test.sh

@@ -353,6 +353,22 @@ else
   FAIL=$((FAIL+1))
 fi
 
+# #53: percent-encoded paths. The daemon decodes the path before it routes,
+# so the proxy and the daemon could read the same request differently; any %
+# in the path is denied. curl sends the path as written (it does not decode
+# %XX, and there are no dot segments to squash). The query string is never
+# inspected: an encoded filter, as `docker ps --filter` sends, still passes.
+S=$(delete_status "$PROXY/containers/no-such%20x")
+check "DELETE /containers/no-such%20x -> 403 (percent-encoded path)" "403" "$S"
+
+S=$(delete_status "$PROXY/containers/%2F")
+check "DELETE /containers/%2F -> 403 (percent-encoded path)" "403" "$S"
+
+# The handler tests prove the query reaches the daemon unchanged; this check
+# proves an encoded query is not denied.
+S=$(get_status "$PROXY/v1.45/containers/json?filters=%7B%22status%22%3A%5B%22running%22%5D%7D")
+check "GET /v1.45/containers/json?filters=<encoded> -> 200 (query not inspected)" "200" "$S"
+
 # ─── Summary ──────────────────────────────────────────
 
 echo ""

go/internal/proxy/handler.go

@@ -50,11 +50,13 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	route := h.router.Route(r.Method, r.URL.Path, bodyJSON)
+	// Route on the raw path: r.URL.Path is already decoded and would hide %-escapes (#53).
+	path := r.URL.EscapedPath()
+	route := h.router.Route(r.Method, path, bodyJSON)
 
 	extra := map[string]interface{}{
 		"method": r.Method,
-		"path":   r.URL.Path,
+		"path":   path,
 	}
 	if route.Service != "" {
 		extra["service"] = route.Service
@@ -64,7 +66,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if route.Action == ActionDeny {
-		slog.Warn("denied", "method", r.Method, "path", r.URL.Path, "reason", route.DenyMsg)
+		slog.Warn("denied", "method", r.Method, "path", path, "reason", route.DenyMsg)
 		h.auditLog.Deny(r.Method, r.RequestURI, route.DenyMsg, extra)
 		http.Error(w, route.DenyMsg, http.StatusForbidden)
 		return
@@ -73,7 +75,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if route.Action == ActionCreateContainer && route.Policy != nil && bodyJSON != nil {
 		result := h.chain.Execute(r, route.Policy, bodyJSON)
 		if !result.Allowed {
-			slog.Warn("denied by middleware", "method", r.Method, "path", r.URL.Path, "reason", result.Reason)
+			slog.Warn("denied by middleware", "method", r.Method, "path", path, "reason", result.Reason)
 			h.auditLog.Deny(r.Method, r.RequestURI, result.Reason, extra)
 			http.Error(w, result.Reason, http.StatusForbidden)
 			return
@@ -89,7 +91,7 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		r.Body = io.NopCloser(bytes.NewReader(body))
 	}
 
-	slog.Info("allowed", "method", r.Method, "path", r.URL.Path)
+	slog.Info("allowed", "method", r.Method, "path", path)
 	h.auditLog.Allow(r.Method, r.RequestURI, "request allowed", extra)
 
 	h.transport.ServeHTTP(w, r)

go/internal/proxy/handler_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/ChainSafe/docker-socket-policy/go/internal/audit"
@@ -84,6 +85,81 @@ func TestHandler_DeniesRoute(t *testing.T) {
 	}
 }
 
+// newPercentRequest builds a request from a raw target and checks that the
+// escaped path really carries the % (#53).
+func newPercentRequest(t *testing.T, method, target string) *http.Request {
+	t.Helper()
+	req := httptest.NewRequest(method, target, nil)
+	if !strings.Contains(req.URL.EscapedPath(), "%") {
+		t.Fatalf("precondition: EscapedPath() = %q has no %%", req.URL.EscapedPath())
+	}
+	return req
+}
+
+// #53: before the fix the Go handler routed on the decoded r.URL.Path
+// ("foo bar"), so a router-only fix would never see the '%' and would let
+// this through. The handler must route on the escaped path.
+func TestHandler_DeniesPercentEncodedName(t *testing.T) {
+	rec := &recorderTransport{}
+	h := newTestHandler(t, nil, rec)
+
+	req := newPercentRequest(t, "DELETE", "/containers/foo%20bar")
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, req)
+
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("expected 403, got %d", w.Code)
+	}
+	if !strings.Contains(w.Body.String(), "percent-encoded") {
+		t.Fatalf("expected a percent-encoded deny reason, got %q", w.Body.String())
+	}
+	if rec.lastRequest != nil {
+		t.Fatal("expected no forward for a percent-encoded path")
+	}
+}
+
+// #53: the daemon reads %2F as a request about "/".
+func TestHandler_DeniesPercentEncodedSlash(t *testing.T) {
+	rec := &recorderTransport{}
+	h := newTestHandler(t, nil, rec)
+
+	req := newPercentRequest(t, "DELETE", "/containers/%2F")
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, req)
+
+	if w.Code != http.StatusForbidden {
+		t.Fatalf("expected 403, got %d", w.Code)
+	}
+	if !strings.Contains(w.Body.String(), "percent-encoded") {
+		t.Fatalf("expected a percent-encoded deny reason, got %q", w.Body.String())
+	}
+	if rec.lastRequest != nil {
+		t.Fatal("expected no forward for a percent-encoded path")
+	}
+}
+
+// #53: the rule looks at the path only; an encoded query string is routine
+// (docker ps --filter) and must reach the daemon unchanged.
+func TestHandler_ForwardsPercentEncodedQuery(t *testing.T) {
+	rec := &recorderTransport{}
+	h := newTestHandler(t, nil, rec)
+
+	const query = "filters=%7B%22status%22%3A%5B%22running%22%5D%7D"
+	req := httptest.NewRequest("GET", "/containers/json?"+query, nil)
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	if rec.lastRequest == nil {
+		t.Fatal("expected request to be forwarded")
+	}
+	if got := rec.lastRequest.URL.RawQuery; got != query {
+		t.Fatalf("forwarded query = %q, want %q", got, query)
+	}
+}
+
 func TestHandler_CreateContainerValid(t *testing.T) {
 	rec := &recorderTransport{}
 	h := newTestHandler(t, map[string]string{

go/internal/proxy/router.go

@@ -34,6 +34,11 @@ func NewRouter(manager *policy.Manager) *Router {
 }
 
 func (r *Router) Route(method, path string, body map[string]interface{}) *RouteResult {
+	// The daemon decodes the path before routing, so deny any escape (#53).
+	if strings.Contains(path, "%") {
+		return &RouteResult{Action: ActionDeny, DenyMsg: "percent-encoded path not allowed"}
+	}
+
 	path = stripAPIVersion(path)
 
 	if path == "/_ping" || path == "/version" || path == "/info" || strings.HasPrefix(path, "/events") {

go/internal/proxy/router_test.go

@@ -3,6 +3,7 @@ package proxy
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/ChainSafe/docker-socket-policy/go/internal/policy"
@@ -467,6 +468,68 @@ allowed_image_prefixes:
 	}
 }
 
+// TestRoutePercentEncodedPaths is the cross-language parity guard for #53.
+//
+// The daemon percent-decodes the path before it routes, so any % in the path
+// lets the proxy and the daemon read the same request differently. A path
+// containing % is denied for every method. Paths are given raw, as the handler
+// passes r.URL.EscapedPath(). Rows mirror the percent* runs in spec/router.qnt.
+func TestRoutePercentEncodedPaths(t *testing.T) {
+	m := newTestManager(t, map[string]string{
+		"beacon.yaml": `
+service_name: beacon
+allowed_image_prefixes:
+  - chainsafe/lodestar
+`,
+	})
+	r := NewRouter(m)
+
+	tests := []struct {
+		method string
+		path   string
+		want   Action
+	}{
+		// percentSlashDeleteDeniedTest (#53)
+		{"DELETE", "/containers/%2F", ActionDeny},
+		// percentLowerSlashDeleteDeniedTest (#53)
+		{"DELETE", "/containers/%2f", ActionDeny},
+		// percentReservedDeleteDeniedTest (#53): %6A%73%6F%6E decodes to json.
+		{"DELETE", "/containers/%6A%73%6F%6E", ActionDeny},
+		// percentSubpathStartDeniedTest (#53): a pin; raw routing already denies it.
+		// It discriminates only in Go's handler before #53, which decoded first;
+		// the foo%20bar handler test covers that.
+		{"POST", "/containers/beacon%2Fstart", ActionDeny},
+		// percentNameStartDeniedTest (#53)
+		{"POST", "/containers/%2F/start", ActionDeny},
+		// percentGetDeniedTest (#53)
+		{"GET", "/containers/%2F", ActionDeny},
+		// #53, language-only: HEAD is outside the model's METHODS, but the rule
+		// covers every method.
+		{"HEAD", "/containers/%2F", ActionDeny},
+		// #53: the check runs on the versioned path too.
+		{"DELETE", "/v1.45/containers/foo%25", ActionDeny},
+		// #53: an encoded version prefix is not stripped.
+		{"DELETE", "/v%31/containers/foo", ActionDeny},
+		// #53: network names that need escaping are denied, GET included.
+		{"GET", "/networks/a%20b", ActionDeny},
+		// #53 control: a plain name is still routed as a container.
+		{"DELETE", "/containers/foo", ActionAllow},
+	}
+	for _, tt := range tests {
+		t.Run(tt.method+" "+tt.path, func(t *testing.T) {
+			got := r.Route(tt.method, tt.path, nil)
+			if got.Action != tt.want {
+				t.Fatalf("Route(%s, %s) = %v, want %v (deny msg: %q)",
+					tt.method, tt.path, got.Action, tt.want, got.DenyMsg)
+			}
+			if tt.want == ActionDeny && !strings.Contains(got.DenyMsg, "percent-encoded") {
+				t.Fatalf("Route(%s, %s) deny msg = %q, want it to contain %q",
+					tt.method, tt.path, got.DenyMsg, "percent-encoded")
+			}
+		})
+	}
+}
+
 func TestExtractContainerNameSkipsEmptySegment(t *testing.T) {
 	for _, path := range []string{"/containers/", "/containers//start"} {
 		if got := extractContainerName(path); got != "" {

rs/src/handler.rs

@@ -366,4 +366,79 @@ mod tests {
         let resp = handler.handle(req).await;
         assert_eq!(resp.status(), StatusCode::FORBIDDEN);
     }
+
+    struct UriRecordingTransport {
+        captured_uri: Arc<std::sync::Mutex<Option<hyper::Uri>>>,
+    }
+
+    #[async_trait]
+    impl Transport for UriRecordingTransport {
+        async fn forward(
+            &self,
+            req: Request<Full<Bytes>>,
+        ) -> Result<Response<Full<Bytes>>, TransportError> {
+            *self.captured_uri.lock().unwrap() = Some(req.uri().clone());
+            Ok(Response::builder()
+                .status(StatusCode::OK)
+                .body(Full::new(Bytes::new()))
+                .unwrap())
+        }
+    }
+
+    fn make_uri_recording_handler() -> (Handler, Arc<std::sync::Mutex<Option<hyper::Uri>>>) {
+        let manager = Manager::from_map(std::collections::HashMap::new());
+        let router = Arc::new(Router::new(manager));
+        let chain = Chain::new(false);
+        let audit = AuditLogger::new("/dev/null").unwrap();
+        let captured_uri = Arc::new(std::sync::Mutex::new(None));
+        let transport = UriRecordingTransport { captured_uri: captured_uri.clone() };
+        (Handler::new(router, chain, audit, Box::new(transport)), captured_uri)
+    }
+
+    /// #53: Rust routed on the raw name and forwarded it; the daemon decodes
+    /// it to "foo bar" and acts on that container.
+    #[tokio::test]
+    async fn test_handler_denies_percent_encoded_name() {
+        let (handler, captured_uri) = make_uri_recording_handler();
+        let req = Request::delete("http://localhost/containers/foo%20bar")
+            .body(Full::new(Bytes::new()))
+            .unwrap();
+        let resp = handler.handle(req).await;
+        assert_eq!(resp.status(), StatusCode::FORBIDDEN);
+        let body = resp.into_body().collect().await.unwrap().to_bytes();
+        let body = String::from_utf8_lossy(&body);
+        assert!(body.contains("percent-encoded"), "deny reason = {:?}", body);
+        assert!(captured_uri.lock().unwrap().is_none(), "expected no forward");
+    }
+
+    /// #53: the daemon reads %2F as a request about "/".
+    #[tokio::test]
+    async fn test_handler_denies_percent_encoded_slash() {
+        let (handler, captured_uri) = make_uri_recording_handler();
+        let req = Request::delete("http://localhost/containers/%2F")
+            .body(Full::new(Bytes::new()))
+            .unwrap();
+        let resp = handler.handle(req).await;
+        assert_eq!(resp.status(), StatusCode::FORBIDDEN);
+        let body = resp.into_body().collect().await.unwrap().to_bytes();
+        let body = String::from_utf8_lossy(&body);
+        assert!(body.contains("percent-encoded"), "deny reason = {:?}", body);
+        assert!(captured_uri.lock().unwrap().is_none(), "expected no forward");
+    }
+
+    /// #53: the rule looks at the path only; an encoded query string is
+    /// routine (docker ps --filter) and must reach the daemon unchanged.
+    #[tokio::test]
+    async fn test_handler_forwards_percent_encoded_query() {
+        let (handler, captured_uri) = make_uri_recording_handler();
+        let query = "filters=%7B%22status%22%3A%5B%22running%22%5D%7D";
+        let req = Request::get(format!("http://localhost/containers/json?{}", query))
+            .body(Full::new(Bytes::new()))
+            .unwrap();
+        let resp = handler.handle(req).await;
+        assert_eq!(resp.status(), StatusCode::OK);
+        let captured = captured_uri.lock().unwrap();
+        let uri = captured.as_ref().expect("expected request to be forwarded");
+        assert_eq!(uri.query(), Some(query));
+    }
 }