diff --git a/AGENTS.md b/AGENTS.md index 90b83e9..0e19440 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: 101 unit tests (main/listener: 23, policy: 10, middleware: 29, proxy: 35, audit: 4) -- Rust: 139 unit tests (main/listener: 23, policy: 15, middleware: 50, proxy: 41, handler: 4, audit: 4, transport: 2) -- TypeScript: 156 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 41, proxy: 28, policy: 10, handler: 6, shutdown: 5, transport: 5, audit: 4) -- Integration, per implementation: 32 tests via deploy/test.sh and 15 socket tests via deploy/test-sock.sh (docker-compose) +- Go: 102 unit tests (main/listener: 23, 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: 35 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`) ## 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` (32 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose +- Integration: `make test-integration` (35 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose ## Contribution Workflow diff --git a/README.md b/README.md index d3083f8..f6a2dfa 100644 --- a/README.md +++ b/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/) | 101 unit + 32 integration | stdlib net/http + yaml.v3 | -| Rust | [rs/](rs/) | 139 unit | tokio, hyper, serde, clap | -| TypeScript | [ts/](ts/) | 156 unit (1 skipped) | Node 22 ESM, built-in http | +| Go | [go/](go/) | 102 unit + 35 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 | ### Build All diff --git a/deploy/test.sh b/deploy/test.sh index 7f6d5cd..8ce1dd1 100755 --- a/deploy/test.sh +++ b/deploy/test.sh @@ -325,6 +325,29 @@ check "DELETE /containers/ -> 403 (empty name, not a container)" "403" "$S" S=$(post_empty "$PROXY/containers//start") check "POST /containers//start -> 403 (empty name, not a container)" "403" "$S" +# Dotted API-version prefix (#52). The Docker CLI versions every request +# (/v1.45/containers/...). The proxy must strip the prefix and route the rest +# like the unversioned path: a lifecycle call reaches the daemon (404, no such +# container — not a proxy 403), and a reserved segment is still denied. +# TypeScript only stripped undotted prefixes, so these fell to the default deny. +S=$(post_empty "$PROXY/v1.45/containers/no-such-container/start") +check "POST /v1.45/containers/*/start -> 404 (daemon answered, not proxy 403)" "404" "$S" + +S=$(delete_status "$PROXY/v1.45/containers/json") +check "DELETE /v1.45/containers/json -> 403 (reserved survives version strip)" "403" "$S" + +# #52: versioned create, the `docker run` path. Mirrors the unversioned +# allowed-image create above: the daemon answers 201, or 404 when the image is +# not present locally — anything but a proxy 403. +S=$(post_json '{"Image":"chainsafe/lodestar:beacon","Cmd":["--rcConfig","/data/config.yml"]}' "$PROXY/v1.45/containers/create") +if [ "$S" = "201" ] || [ "$S" = "404" ]; then + echo " PASS: POST /v1.45/containers/create with allowed image -> $S (not 403)" + PASS=$((PASS+1)) +else + echo " FAIL: POST /v1.45/containers/create with allowed image (expected 201|404, got $S)" + FAIL=$((FAIL+1)) +fi + # ─── Summary ────────────────────────────────────────── echo "" diff --git a/go/internal/proxy/router_test.go b/go/internal/proxy/router_test.go index 7b45541..fbff51a 100644 --- a/go/internal/proxy/router_test.go +++ b/go/internal/proxy/router_test.go @@ -401,6 +401,52 @@ allowed_image_prefixes: } } +// TestRouteVersionedPaths is the cross-language parity guard for #52. +// +// The Docker CLI prefixes every request with a dotted API version +// (/v1.43/containers/create). The router must strip the prefix and route the +// rest exactly like the unversioned path. TypeScript only stripped undotted +// prefixes (/v1/...), so dotted paths fell through to the default deny. +// The TS table also pins TS-only over-strip rows, because Go and Rust strip +// any /v…/ first segment (#55). +func TestRouteVersionedPaths(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 + body map[string]interface{} + want Action + }{ + // #52: dotted version, container lifecycle delete. + {"DELETE", "/v1.43/containers/foo", nil, ActionAllow}, + // #52: dotted version, container lifecycle start. + {"POST", "/v1.43/containers/beacon/start", nil, ActionAllow}, + // #52: dotted version, create with a policy-allowed image. + {"POST", "/v1.43/containers/create", map[string]interface{}{"Image": "chainsafe/lodestar:next"}, ActionCreateContainer}, + // #52: undotted control — stripped correctly everywhere already. + {"POST", "/v1/containers/beacon/start", nil, ActionAllow}, + // #52: the reserved segment survives the strip (#24 parity). + {"DELETE", "/v1.43/containers/json", nil, ActionDeny}, + } + for _, tt := range tests { + t.Run(tt.method+" "+tt.path, func(t *testing.T) { + got := r.Route(tt.method, tt.path, tt.body) + 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 TestExtractContainerNameSkipsEmptySegment(t *testing.T) { for _, path := range []string{"/containers/", "/containers//start"} { if got := extractContainerName(path); got != "" { diff --git a/rs/src/proxy.rs b/rs/src/proxy.rs index a357dd3..a8d37f4 100644 --- a/rs/src/proxy.rs +++ b/rs/src/proxy.rs @@ -507,6 +507,36 @@ mod tests { } } + /// Cross-language parity guard for #52. The Docker CLI prefixes every + /// request with a dotted API version (/v1.43/containers/create). The + /// router must strip the prefix and route the rest exactly like the + /// unversioned path. TypeScript only stripped undotted prefixes + /// (/v1/...), so dotted paths fell through to the default deny. + /// The TS table also pins TS-only over-strip rows, because Go and Rust + /// strip any /v…/ first segment (#55). + #[test] + fn test_route_versioned_paths() { + let router = Router::new(make_manager(vec!["alpine"])); + let create_body: HashMap = + serde_json::from_value(serde_json::json!({"Image": "alpine:latest"})).unwrap(); + let cases = [ + // #52: dotted version, container lifecycle delete. + ("DELETE", "/v1.43/containers/foo", None, Action::Allow), + // #52: dotted version, container lifecycle start. + ("POST", "/v1.43/containers/beacon/start", None, Action::Allow), + // #52: dotted version, create with a policy-allowed image. + ("POST", "/v1.43/containers/create", Some(&create_body), Action::CreateContainer), + // #52: undotted control — stripped correctly everywhere already. + ("POST", "/v1/containers/beacon/start", None, Action::Allow), + // #52: the reserved segment survives the strip (#24 parity). + ("DELETE", "/v1.43/containers/json", None, Action::Deny), + ]; + for (method, path, body, want) in cases { + let got = router.route(method, path, body); + assert_eq!(got.action, want, "route({} {})", method, path); + } + } + #[test] fn test_extract_container_name_skips_empty_segment() { for path in ["/containers/", "/containers//start"] { diff --git a/ts/src/proxy.test.ts b/ts/src/proxy.test.ts index 016ff40..5b3b680 100644 --- a/ts/src/proxy.test.ts +++ b/ts/src/proxy.test.ts @@ -224,4 +224,41 @@ describe("Router", () => { assert.equal(r.action, want, `route(${method} ${path})`); } }); + + // Guard for #52. The first five rows are the cross-language parity table: + // the Docker CLI prefixes every request with a dotted API version + // (/v1.43/containers/create), and the router must strip the prefix and route + // the rest exactly like the unversioned path. stripAPIVersion only stripped + // undotted prefixes (/v1/...), so dotted paths fell through to the default + // deny. The last three rows are TS-only: they pin TS's intended strip + // (/v or /v. only). Go and Rust currently strip any + // /v…/ first segment and so ALLOW both over-strip paths — tracked in + // #55; do not copy those rows there as parity. + it("routes dotted API-version paths like the unversioned ones", () => { + const cases: [string, string, Record | undefined, Action][] = [ + // #52: dotted version, container lifecycle delete. + ["DELETE", "/v1.43/containers/foo", undefined, Action.Allow], + // #52: dotted version, container lifecycle start. + ["POST", "/v1.43/containers/beacon/start", undefined, Action.Allow], + // #52: dotted version, create with a policy-allowed image. + ["POST", "/v1.43/containers/create", { Image: "nginx:latest" }, Action.CreateContainer], + // #52: undotted control — stripped correctly everywhere already. + ["POST", "/v1/containers/beacon/start", undefined, Action.Allow], + // #52: the reserved segment survives the strip (#24 parity). + ["DELETE", "/v1.43/containers/json", undefined, Action.Deny], + // #52 sanity row, cannot fail: GET /version lands on the GET passthrough + // whatever is stripped. Kept as documentation of the expected outcome. + ["GET", "/version", undefined, Action.Allow], + // #52 over-strip guard (TS-only, see header): stripping /volumes/ as a + // version prefix would make this DELETE /containers/foo -> Allow. + ["DELETE", "/volumes/containers/foo", undefined, Action.Deny], + // #52 over-strip guard (TS-only, see header): two dots is not an API + // version; stripping /v1.2.3/ would make this DELETE /containers/foo -> Allow. + ["DELETE", "/v1.2.3/containers/foo", undefined, Action.Deny], + ]; + for (const [method, path, body, want] of cases) { + const r = router.route(method, path, body); + assert.equal(r.action, want, `route(${method} ${path})`); + } + }); }); diff --git a/ts/src/proxy.ts b/ts/src/proxy.ts index 4132eb2..70ff61f 100644 --- a/ts/src/proxy.ts +++ b/ts/src/proxy.ts @@ -116,7 +116,8 @@ export class Router { } function stripAPIVersion(path: string): string { - const match = path.match(/^\/v\d+\//); + // Accepts /v and /v. (the Docker CLI sends /v1.43/) (#52). + const match = path.match(/^\/v\d+(\.\d+)?\//); return match ? path.slice(match[0].length - 1) : path; }