From 2cf208547e8b42c48f090595a44fbd8b447a1a96 Mon Sep 17 00:00:00 2001 From: Adrian Bienkowski Date: Mon, 28 Sep 2026 20:31:10 -0400 Subject: [PATCH 1/2] fix(proxy): exclude reserved path segments from Go container name extraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /containers/ is ambiguous: is usually a container name, but Docker also has reserved endpoints at that position — /containers/json lists containers and /containers/create creates one. Rust and TypeScript already excluded create, json and exec; Go did not. The consequence was not just a different verdict. Treating the reserved word as a container name sends the request down the lifecycle branch, where an unknown container falls through to Allow, so Go forwarded DELETE /containers/json to the daemon. Confirmed end to end: before the fix the integration suite gets 404 back from Docker itself, where Rust and TypeScript return 403 without ever forwarding. GET /containers/json is unaffected — with the reserved word excluded it reaches the GET/HEAD passthrough, so listing still works. Adds the same parity tests to all three languages plus three integration cases, as the issue suggested. Verified the tests fail without the fix: the Go unit tests report "create"/"json"/"exec" as container names, and the integration suite reports 404 instead of 403. Closes #24 --- deploy/test.sh | 19 ++++++++++ go/internal/proxy/router.go | 14 +++++++- go/internal/proxy/router_test.go | 62 ++++++++++++++++++++++++++++++++ rs/src/proxy.rs | 39 ++++++++++++++++++++ ts/src/proxy.test.ts | 24 +++++++++++++ 5 files changed, 157 insertions(+), 1 deletion(-) diff --git a/deploy/test.sh b/deploy/test.sh index 0f5ad65..4061645 100755 --- a/deploy/test.sh +++ b/deploy/test.sh @@ -68,6 +68,11 @@ post_empty() { -X POST -H "Content-Type: application/json" -d "" "$1" 2>/dev/null || true) echo "${out:-000}" } +delete_status() { + out=$(curl -s -o /dev/null -w '%{http_code}' $TIMEOUT --unix-socket "$PROXY_SOCK" \ + -X DELETE "$1" 2>/dev/null || true) + echo "${out:-000}" +} check() { desc="$1" @@ -296,6 +301,20 @@ check "POST /volumes/create -> 403" "403" "$S" S=$(post_empty "$PROXY/containers/test") check "PATCH /containers/test -> 403" "403" "$S" +# Reserved path segments (#24). /containers/json is the list endpoint, not a +# container called "json". Treating it as a container name sent the request +# down the lifecycle path, where an unknown container is allowed through, so Go +# allowed this while Rust and TypeScript denied it. +S=$(delete_status "$PROXY/containers/json") +check "DELETE /containers/json -> 403 (reserved, not a container)" "403" "$S" + +S=$(delete_status "$PROXY/containers/create") +check "DELETE /containers/create -> 403 (reserved, not a container)" "403" "$S" + +# Listing must still work: it reaches the GET/HEAD passthrough instead. +S=$(get_status "$PROXY/containers/json") +check "GET /containers/json -> 200 (still the list endpoint)" "200" "$S" + # ─── Summary ────────────────────────────────────────── echo "" diff --git a/go/internal/proxy/router.go b/go/internal/proxy/router.go index 42e7e1a..cb6767d 100644 --- a/go/internal/proxy/router.go +++ b/go/internal/proxy/router.go @@ -177,10 +177,22 @@ func matchEndpoint(path, resource, endpoint string) bool { return parts[0] == resource && parts[1] == endpoint } +// reservedContainerSegments are Docker endpoints that sit where a container +// name would: /containers/json lists, /containers/create creates. Treating one +// as a container name routes the request down the lifecycle path, where an +// unknown container is allowed through — so DELETE /containers/json would be +// allowed rather than denied. Rust (rs/src/proxy.rs) and TypeScript +// (ts/src/proxy.ts) exclude the same set. +var reservedContainerSegments = map[string]bool{ + "create": true, + "json": true, + "exec": true, +} + func extractContainerName(path string) string { path = strings.TrimPrefix(path, "/") parts := strings.Split(path, "/") - if len(parts) >= 2 && parts[0] == "containers" { + if len(parts) >= 2 && parts[0] == "containers" && !reservedContainerSegments[parts[1]] { return parts[1] } return "" diff --git a/go/internal/proxy/router_test.go b/go/internal/proxy/router_test.go index 0607005..b791863 100644 --- a/go/internal/proxy/router_test.go +++ b/go/internal/proxy/router_test.go @@ -293,3 +293,65 @@ func TestRouterDenyPostOnReadOnly(t *testing.T) { t.Fatalf("expected ActionDeny for POST on non-lifecycle path, got %v", result.Action) } } + +// TestRouterReservedPathSegments is the cross-language parity guard for #24. +// +// /containers/ is ambiguous: is usually a container name, but Docker +// also has reserved endpoints at that position (/containers/json to list, +// /containers/create to create). Treating a reserved word as a container name +// sends the request down the lifecycle path, where an unknown container is +// allowed through — so DELETE /containers/json was allowed in Go while Rust +// and TypeScript denied it. +// +// GET stays allowed either way: it reaches the GET/HEAD passthrough instead. +func TestRouterReservedPathSegments(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 + }{ + // Reserved: must not be mistaken for a container to remove. + {"DELETE", "/containers/json", ActionDeny}, + {"DELETE", "/containers/create", ActionDeny}, + // Listing and inspecting stay allowed via the GET/HEAD passthrough. + {"GET", "/containers/json", ActionAllow}, + // A real container name is still routed as a container. + {"DELETE", "/containers/mycontainer", ActionAllow}, + {"GET", "/containers/mycontainer", ActionAllow}, + // The reserved word as a *sub*-resource is a normal inspect. + {"GET", "/containers/mycontainer/json", 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) + } + }) + } +} + +func TestExtractContainerNameSkipsReservedSegments(t *testing.T) { + for _, reserved := range []string{"create", "json", "exec"} { + if got := extractContainerName("/containers/" + reserved); got != "" { + t.Errorf("extractContainerName(/containers/%s) = %q, want \"\"", reserved, got) + } + } + if got := extractContainerName("/containers/mycontainer"); got != "mycontainer" { + t.Errorf("extractContainerName(/containers/mycontainer) = %q, want \"mycontainer\"", got) + } + // Reserved words are only reserved in the name position. + if got := extractContainerName("/containers/mycontainer/json"); got != "mycontainer" { + t.Errorf("extractContainerName(/containers/mycontainer/json) = %q, want \"mycontainer\"", got) + } +} diff --git a/rs/src/proxy.rs b/rs/src/proxy.rs index 85ccedc..b304db2 100644 --- a/rs/src/proxy.rs +++ b/rs/src/proxy.rs @@ -439,6 +439,45 @@ mod tests { assert_eq!(result.action, Action::Allow); } + /// Cross-language parity guard for #24. /containers/ is ambiguous: + /// is usually a container name, but Docker also has reserved endpoints at + /// that position. Treating one as a container name routes the request down + /// the lifecycle path, where an unknown container is allowed through — Go + /// allowed DELETE /containers/json for exactly that reason. + #[test] + fn test_route_reserved_path_segments() { + let router = Router::new(make_manager(vec!["alpine"])); + let cases = [ + // Reserved: must not be mistaken for a container to remove. + ("DELETE", "/containers/json", Action::Deny), + ("DELETE", "/containers/create", Action::Deny), + // Listing stays allowed, via the GET/HEAD passthrough. + ("GET", "/containers/json", Action::Allow), + // A real container name is still routed as a container. + ("DELETE", "/containers/mycontainer", Action::Allow), + ("GET", "/containers/mycontainer", Action::Allow), + // Reserved words are only reserved in the name position. + ("GET", "/containers/mycontainer/json", Action::Allow), + ]; + for (method, path, want) in cases { + let got = router.route(method, path, None); + assert_eq!(got.action, want, "route({} {})", method, path); + } + } + + #[test] + fn test_extract_container_name_skips_reserved_segments() { + for reserved in ["create", "json", "exec"] { + let path = format!("/containers/{}", reserved); + assert_eq!(extract_container_name(&path), None, "{} should be reserved", path); + } + assert_eq!(extract_container_name("/containers/mycontainer"), Some("mycontainer")); + assert_eq!( + extract_container_name("/containers/mycontainer/json"), + Some("mycontainer") + ); + } + #[test] fn test_route_image_pull() { let router = Router::new(make_manager(vec!["alpine"])); diff --git a/ts/src/proxy.test.ts b/ts/src/proxy.test.ts index 3e23155..6f58eb2 100644 --- a/ts/src/proxy.test.ts +++ b/ts/src/proxy.test.ts @@ -175,4 +175,28 @@ describe("Router", () => { assert.equal(r.action, Action.Allow); assert.equal(r.service, "nginx-svc"); }); + + // Cross-language parity guard for #24. /containers/ is ambiguous: is + // usually a container name, but Docker also has reserved endpoints at that + // position. Treating one as a container name routes the request down the + // lifecycle path, where an unknown container is allowed through — Go allowed + // DELETE /containers/json for exactly that reason. + it("does not treat reserved path segments as container names", () => { + const cases: [string, string, Action][] = [ + // Reserved: must not be mistaken for a container to remove. + ["DELETE", "/containers/json", Action.Deny], + ["DELETE", "/containers/create", Action.Deny], + // Listing stays allowed, via the GET/HEAD passthrough. + ["GET", "/containers/json", Action.Allow], + // A real container name is still routed as a container. + ["DELETE", "/containers/mycontainer", Action.Allow], + ["GET", "/containers/mycontainer", Action.Allow], + // Reserved words are only reserved in the name position. + ["GET", "/containers/mycontainer/json", Action.Allow], + ]; + for (const [method, path, want] of cases) { + const r = router.route(method, path); + assert.equal(r.action, want, `route(${method} ${path})`); + } + }); }); From b4dc45735940c31d1869b1fbc945c92365c83ddc Mon Sep 17 00:00:00 2001 From: Adrian Bienkowski Date: Fri, 2 Oct 2026 18:11:59 -0400 Subject: [PATCH 2/2] docs: update test coverage counts for reserved-segment tests (#24) --- AGENTS.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e69d057..1b358e7 100644 --- a/AGENTS.md +++ b/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: 97 unit tests (main/listener: 23, policy: 10, middleware: 29, proxy: 31, audit: 4) -- Rust: 135 unit tests (main/listener: 23, policy: 15, middleware: 50, proxy: 37, handler: 4, audit: 4, transport: 2) -- TypeScript: 154 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 41, proxy: 26, policy: 10, handler: 6, shutdown: 5, transport: 5, audit: 4) -- Integration, per implementation: 27 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose) +- Go: 99 unit tests (main/listener: 23, policy: 10, middleware: 29, proxy: 33, audit: 4) +- Rust: 137 unit tests (main/listener: 23, policy: 15, middleware: 50, proxy: 39, handler: 4, audit: 4, transport: 2) +- TypeScript: 155 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 41, proxy: 27, policy: 10, handler: 6, shutdown: 5, transport: 5, audit: 4) +- Integration, per implementation: 30 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 ## 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` (27 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose +- Integration: `make test-integration` (30 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose ## Contribution Workflow