fix: route image pulls on the query string and normalise image references in all three (#83, #84)

AGENTS.md

@@ -39,17 +39,17 @@ deploy/ — Docker Compose + integration tests
 - Zero external deps where possible (Go: yaml.v3, Rust: tokio/hyper/serde/clap, TS: yaml)
 
 ## Test Coverage
-- Go: 114 unit tests (main/listener: 24, policy: 10, middleware: 26, proxy: 50, audit: 4)
-- Rust: 146 unit tests (main/listener: 23, policy: 15, middleware: 42, proxy: 49, handler: 11, audit: 4, transport: 2)
-- TypeScript: 165 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 33, proxy: 36, policy: 10, handler: 15, shutdown: 5, transport: 5, audit: 4)
-- Integration, per implementation: 72 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`, `router_pre55`, `router_readonly`, whose `readonly*` tests run with `--match`, `router_pre76`, `router_pre78` and `router_pre70`), and the `spec/docker_socket_policy.qnt` `run` tests (instances `network_endpoints`, `network_endpoints_pre87`)
+- Go: 118 unit tests (main/listener: 24, policy: 11, middleware: 27, proxy: 52, audit: 4)
+- Rust: 145 unit tests (main/listener: 23, policy: 13, middleware: 43, proxy: 47, handler: 13, audit: 4, transport: 2)
+- TypeScript: 169 unit tests, 1 skipped (flags: 44, listen: 13 incl. 1 skipped concurrency test (#46), middleware: 34, proxy: 35, policy: 12, handler: 17, shutdown: 5, transport: 5, audit: 4)
+- Integration, per implementation: 77 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`, `router_pre55`, `router_readonly`, whose `readonly*` tests run with `--match`, `router_pre76`, `router_pre78` and `router_pre70`; and, for the image pull, `pull`, `pull_pre83`, `pull_body_ignored` and `pull_pre84`), and the `spec/docker_socket_policy.qnt` `run` tests (instances `network_endpoints`, `network_endpoints_pre87`)
 
 ## 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` (72 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose
+- Integration: `make test-integration` (77 test cases) and `make test-integration-sock` (15 socket cases) via Docker Compose
 
 ## Contribution Workflow
 

Makefile

@@ -108,6 +108,10 @@ test-spec:
 	$(QUINT) test $(ROUTER_SPEC) --main=router_pre70
 	$(QUINT) test $(SPEC) --main=network_endpoints
 	$(QUINT) test $(SPEC) --main=network_endpoints_pre87
+	$(QUINT) test $(ROUTER_SPEC) --main=pull
+	$(QUINT) test $(ROUTER_SPEC) --main=pull_pre83
+	$(QUINT) test $(ROUTER_SPEC) --main=pull_body_ignored
+	$(QUINT) test $(ROUTER_SPEC) --main=pull_pre84
 
 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/) | 114 unit + 72 integration + 15 socket | stdlib net/http + yaml.v3 |
-| Rust | [rs/](rs/) | 146 unit + 72 integration + 15 socket | tokio, hyper, serde, clap |
-| TypeScript | [ts/](ts/) | 165 unit (1 skipped) + 72 integration + 15 socket | Node 22 ESM, built-in http |
+| Go | [go/](go/) | 118 unit + 77 integration + 15 socket | stdlib net/http + yaml.v3 |
+| Rust | [rs/](rs/) | 145 unit + 77 integration + 15 socket | tokio, hyper, serde, clap |
+| TypeScript | [ts/](ts/) | 169 unit (1 skipped) + 77 integration + 15 socket | Node 22 ESM, built-in http |
 
 ### Build All
 
@@ -193,7 +193,7 @@ docker pull attacker/malware:latest  # denied: image not in allowlist
 | Middleware | Type | What it checks |
 |------------|------|----------------|
 | ContainerConfigMutator | Mutator | Forces `network_mode`, `user`, `security_options`, `restart_policy` from policy, and makes `NetworkingConfig.EndpointsConfig` match a forced `network_mode` |
-| RegistryGate | Gate | Validates image ref against `allowed_image_prefixes` |
+| RegistryGate | Gate | Validates the normalised image ref against `allowed_image_prefixes`, then `image_tag_pattern` and `image_digest_allowed` |
 | MountSourceGate | Gate | Validates volume binds against whitelist |
 | EnvFileGate | Gate | Strips `Env` field from create body; env must come from locked `env_file` |
 | CmdGate | Gate | Validates each CLI flag in `Cmd` array against allowlist + denylist |
@@ -220,7 +220,7 @@ The chain runs only on container create. `--readonly` is not a gate: the router
 | Any | `/exec/*` | **DENIED** |
 | Any | `/containers/{name}/attach…` (an `attach` segment anywhere under `/containers`) | **DENIED** |
 | POST | `/containers/{name}/rename\|update` | **DENIED** |
-| POST | `/images/create` | Validated by registry gate |
+| POST | `/images/create` | Pull: `fromImage` and `tag` from the query string get the image checks (see below); a request body or `fromSrc` (import) **DENIED** |
 | POST | `/auth` | **DENIED** |
 | POST | `/build` | **DENIED** |
 | POST | `/commit` | **DENIED** |
@@ -235,6 +235,10 @@ Attach is matched the same way: an `attach` segment under `/containers` is denie
 
 `/_ping`, `/version`, `/info` and `/events` are ordinary reads: `GET` and `HEAD` pass, and every other method is denied with `endpoint <METHOD> <path> is not allowed`, `OPTIONS` (a CORS preflight) included ([#78](https://github.com/ChainSafe/docker-socket-policy/issues/78)). `CONNECT` is refused with 403 `CONNECT is not allowed` before anything else, whatever its target or headers; the TypeScript proxy answers it on the raw socket, since Node never hands `CONNECT` to the request handler.
 
+An image pull is routed on its query string, as the Docker Engine API defines it and the Docker CLI sends it: `POST /images/create?fromImage=docker.io%2Flibrary%2Falpine&tag=3.19` and no body ([#83](https://github.com/ChainSafe/docker-socket-policy/issues/83)). A pull with any body is denied (`image pull must not have a request body`), whatever its `Content-Type` or framing: the daemon reads a form body's `fromImage` over the query's, so a body could name an image the router never checked. The image gets the same checks as `Image` on create: its normalised repository must match a policy's prefix, then the tag must match that policy's `image_tag_pattern` and a digest needs `image_digest_allowed` (the CLI sends a digest in `tag`). The proxy denies a pull with no `fromImage` (`fromImage query parameter is required for image pull`), an import (`image import (fromSrc) is not allowed`: it would store any tarball under an allowed name), a tag in both `fromImage` and `tag`, a repeated `fromImage`, `tag` or `fromSrc`, a query the daemon would reject, and a pull with neither tag nor digest, which makes the daemon pull every tag of the repository (`image pull without a tag or digest pulls every tag and is not allowed`; `docker pull alpine` sends `tag=latest`, `docker pull --all-tags` is denied).
+
+Image references are normalised before policy matching, in all three implementations and for both the request and the policy's `allowed_image_prefixes` (at load time) ([#84](https://github.com/ChainSafe/docker-socket-policy/issues/84)). The reference is split the way Docker does: the digest follows `@`, the tag follows the last `:` after the last `/` (so `localhost:5000/team/app:1.2` has tag `1.2`), and the first component is a registry host when it contains `.` or `:` or is `localhost`. Docker Hub's `docker.io/`, `index.docker.io/` and `registry-1.docker.io/` are dropped, and so is `library/` before a single-component name, so `alpine`, `library/alpine` and `docker.io/library/alpine` are the same image, as are `chainsafe/lodestar` and `docker.io/chainsafe/lodestar`. The registry host is compared lower-cased; upper case in the repository path is invalid, as it is for Docker (`invalid image reference "…"`). The prefix match stays on whole path components: `alpine` does not match `alpine-evil`, and `alpine` (Docker Hub) does not match `localhost:5000/alpine`. A prefix that is not a valid repository name, or that carries a tag or digest, fails the policy load.
+
 ## Configuration
 
 ### CLI Flags

deploy/config/pull-allowed.yaml

@@ -0,0 +1,6 @@
+# Integration-only policy: lets deploy/test.sh pull a real image through the
+# proxy (#83). busybox is a few MB, so the pull is quick.
+service_name: pull-allowed
+allowed_image_prefixes:
+  - busybox
+image_tag_pattern: '^latest$'

deploy/test.sh

@@ -304,30 +304,61 @@ fi
 echo ""
 echo "--- Docker API passthrough ---"
 
-# Image pull: allowed image passes proxy, daemon fails (image doesn't exist)
-S=$(post_json '{"fromImage":"chainsafe/lodestar:beacon"}' "$PROXY/images/create")
-if [ "$S" = "200" ]; then
-  echo "  PASS: image pull allowed -> $S"
-  PASS=$((PASS+1))
-elif [ "$S" = "500" ]; then
-  echo "  PASS: image pull allowed (500 = daemon can't fetch, not proxy) -> $S"
-  PASS=$((PASS+1))
-elif [ "$S" = "404" ]; then
-  echo "  PASS: image pull allowed (404 = daemon can't fetch, not proxy) -> $S"
+# #83: image pulls are routed on the query string, as the Docker CLI sends
+# them: fromImage (fully qualified, URL-encoded) and tag, with an empty body.
+# config/pull-allowed.yaml allows busybox:latest, a few MB, so the allowed pull
+# runs to completion against the daemon: 200 and a stream with no error in it.
+# The image is sent as docker.io/library/busybox, so the check also covers
+# normalisation of the policy's bare busybox prefix (#84). Before
+# #83 every such pull was denied with "empty request body".
+# pull_query QUERY [BODY [CONTENT_TYPE [HEADER]]]: POST /images/create?QUERY,
+# empty body unless given, JSON unless CONTENT_TYPE is given.
+pull_query() {
+  : >"$BODY"
+  out=$(curl -s -o "$BODY" -w '%{http_code}' --max-time 120 --connect-timeout 2 --unix-socket "$PROXY_SOCK" \
+    -X POST -H "Content-Type: ${3:-application/json}" ${4:+-H "$4"} -d "${2:-}" \
+    "$PROXY/v1.45/images/create?$1" 2>/dev/null || true)
+  echo "${out:-000}"
+}
+
+S=$(pull_query 'fromImage=docker.io%2Flibrary%2Fbusybox&tag=latest')
+if [ "$S" = "200" ] && grep -q '"status"' "$BODY" && ! grep -q '"error' "$BODY"; then
+  echo "  PASS: pull allowed image the CLI way -> 200 (pulled)"
   PASS=$((PASS+1))
 else
-  echo "  FAIL: image pull (expected 200|404|500, got $S)"
+  echo "  FAIL: pull allowed image the CLI way (expected 200 and a clean pull, got $S: $(head -c 200 "$BODY"))"
   FAIL=$((FAIL+1))
 fi
 
-# Unlisted image pull -> proxy denies
-S=$(post_json '{"fromImage":"alpine:latest"}' "$PROXY/images/create")
-check_denied "pull unlisted image -> 403" "$S" "$BODY" 'image alpine:latest not allowed by any policy'
+S=$(pull_query 'fromImage=docker.io%2Flibrary%2Falpine&tag=latest')
+check_denied "pull unlisted image the CLI way -> 403" "$S" "$BODY" 'image docker.io/library/alpine not allowed by any policy'
+
+S=$(pull_query 'fromImage=alpine&tag=latest')
+check_denied "pull alpine (unlisted, short name) -> 403" "$S" "$BODY" 'image alpine not allowed by any policy'
 
-# Real container lifecycle with busybox (image that exists)
-# Step 1: Pull busybox through proxy (should be denied — not in policy)
-S=$(post_json '{"fromImage":"busybox:latest"}' "$PROXY/images/create")
-check_denied "pull busybox (unlisted) -> 403" "$S" "$BODY" 'image busybox:latest not allowed by any policy'
+# #83: pull-allowed.yaml's tag pattern applies to pulls as it does to creates.
+S=$(pull_query 'fromImage=busybox&tag=musl')
+check_denied "pull allowed image with a tag outside the pattern -> 403" "$S" "$BODY" 'image tag "musl" does not match pattern'
+
+# #83: a pull has no body. Before #83 an allowed image in a JSON body
+# forwarded a pull of the query's image (alpine here, pulled for real). The
+# daemon reads a form body over the query, so routing on the query and
+# forwarding the body would let a form body name any image: with busybox
+# (allowed) in the query and alpine in the form body, the daemon pulls
+# alpine. Every body is denied, whatever its Content-Type or framing.
+body_msg='image pull must not have a request body'
+S=$(pull_query 'fromImage=alpine&tag=latest' '{"fromImage":"chainsafe/lodestar"}')
+check_denied "pull with allowed image in JSON body, unlisted in query -> 403" "$S" "$BODY" "$body_msg"
+
+S=$(pull_query 'fromImage=busybox&tag=latest' 'fromImage=alpine&tag=3.19' 'application/x-www-form-urlencoded')
+check_denied "pull with allowed image in query, unlisted in form body -> 403" "$S" "$BODY" "$body_msg"
+
+S=$(pull_query 'fromImage=busybox&tag=latest' 'fromImage=alpine&tag=3.19' 'application/x-www-form-urlencoded' 'Transfer-Encoding: chunked')
+check_denied "pull with a chunked form body -> 403" "$S" "$BODY" "$body_msg"
+
+# #83: an import (fromSrc) would store any tarball under an allowed name.
+S=$(pull_query 'fromSrc=http%3A%2F%2F127.0.0.1%3A9%2Fx.tar&repo=chainsafe%2Flodestar&tag=imported')
+check_denied "image import (fromSrc) -> 403" "$S" "$BODY" 'image import (fromSrc) is not allowed'
 
 # ─── Endpoint completeness ─────────────────────────────
 
@@ -542,15 +573,10 @@ check_denied "GET /containers/./json -> 403 (dot segment)" "$S" "$BODY" 'dot pat
 echo ""
 echo "--- Forced network_mode (#87) ---"
 
-# pull IMAGE TAG: pull through the proxy. The router reads the body (and the
-# query string, #83); the daemon reads the query string. A pull streams until
-# it completes, so it gets a longer timeout.
+# pull IMAGE TAG: pull through the proxy the way the Docker CLI does,
+# fromImage and tag in the query string and no body (#83); see pull_query.
 pull() {
-  : >"$BODY"
-  out=$(curl -s -o "$BODY" -w '%{http_code}' --max-time 120 --connect-timeout 2 --unix-socket "$PROXY_SOCK" \
-    -X POST -H "Content-Type: application/json" -d "{\"fromImage\":\"$1:$2\"}" \
-    "$PROXY/images/create?fromImage=$1&tag=$2" 2>/dev/null || true)
-  echo "${out:-000}"
+  pull_query "fromImage=$1&tag=$2"
 }
 
 # start_forced NAME IMAGE MODE: create NAME as the CLI does, start it, and

go/internal/middleware/chain_test.go

@@ -507,3 +507,42 @@ func TestChain_NetworkEndpoints(t *testing.T) {
 		t.Fatalf("%d mismatches:\n%s", len(mismatches), strings.Join(mismatches, "\n"))
 	}
 }
+
+// TestRegistryGate_ParsesReferences pins the registry gate's reference parser
+// (#84): a registry port is not a tag, Docker Hub spellings match the
+// normalised prefix, and the tag pattern sees only the tag. The policy's
+// prefixes are normalised, as the manager stores them. Same rows in Rust and
+// TypeScript.
+func TestRegistryGate_ParsesReferences(t *testing.T) {
+	gate := &RegistryGate{}
+	r := testRequest("POST", "/containers/create")
+	p := testPolicy(map[string]interface{}{
+		"allowed_image_prefixes": []string{"chainsafe/lodestar", "localhost:5000/team", "registry.example.com:443/a"},
+	})
+	p.ImageTagPattern = `^(v[0-9]+|[0-9.]+)$`
+
+	tests := []struct {
+		image   string
+		wantErr string
+	}{
+		{"localhost:5000/team/app:1.2", ""},
+		{"localhost:5000/team/app", ""},
+		{"docker.io/chainsafe/lodestar:v1", ""},
+		{"registry.example.com:443/a", ""},
+		{"Registry.Example.com:443/a:2", ""},
+		{"localhost:5000/team/app:latest", `image tag "latest" does not match pattern "^(v[0-9]+|[0-9.]+)$"`},
+		{"localhost:5000/other:1", `image "localhost:5000/other" is not in allowed prefixes`},
+		{"chainsafe/Lodestar:v1", `invalid image reference "chainsafe/Lodestar:v1"`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.image, func(t *testing.T) {
+			err := gate.Check(r, p, map[string]interface{}{"Image": tt.image})
+			switch {
+			case tt.wantErr == "" && err != nil:
+				t.Errorf("Check(%s) = %v, want nil", tt.image, err)
+			case tt.wantErr != "" && (err == nil || !strings.Contains(err.Error(), tt.wantErr)):
+				t.Errorf("Check(%s) = %v, want error containing %q", tt.image, err, tt.wantErr)
+			}
+		})
+	}
+}

go/internal/middleware/registry.go

@@ -3,8 +3,6 @@ package middleware
 import (
 	"fmt"
 	"net/http"
-	"regexp"
-	"strings"
 
 	"github.com/ChainSafe/docker-socket-policy/go/internal/policy"
 )
@@ -17,53 +15,15 @@ func (g *RegistryGate) Check(r *http.Request, p *policy.Policy, body map[string]
 		return nil
 	}
 
-	imageName, tagOrDigest := splitImageRef(image)
-
-	if !isAllowedPrefix(imageName, p.AllowedImagePrefixes) {
-		return fmt.Errorf("image %q is not in allowed prefixes: %v", imageName, p.AllowedImagePrefixes)
-	}
-
-	if tagOrDigest != "" {
-		if len(tagOrDigest) > 128 {
-			return fmt.Errorf("image tag/digest exceeds 128 characters")
-		}
-
-		if strings.HasPrefix(tagOrDigest, "sha256:") {
-			if !p.ImageDigestAllowed {
-				return fmt.Errorf("image digests not allowed for service %s", p.ServiceName)
-			}
-			matched, _ := regexp.MatchString(`^sha256:[a-f0-9]{64}$`, tagOrDigest)
-			if !matched {
-				return fmt.Errorf("invalid digest format: %s", tagOrDigest)
-			}
-		} else if p.ImageTagPattern != "" {
-			matched, err := regexp.MatchString(p.ImageTagPattern, tagOrDigest)
-			if err != nil {
-				return fmt.Errorf("invalid tag pattern regex: %w", err)
-			}
-			if !matched {
-				return fmt.Errorf("image tag %q does not match pattern %q", tagOrDigest, p.ImageTagPattern)
-			}
-		}
+	// The same parser as the router, so a registry port is not a tag (#84).
+	ref, err := policy.ParseImageRef(image)
+	if err != nil {
+		return err
 	}
 
-	return nil
-}
-
-func splitImageRef(ref string) (name, tagOrDigest string) {
-	for i, ch := range ref {
-		if ch == ':' || ch == '@' {
-			return ref[:i], ref[i+1:]
-		}
+	if !policy.MatchesImagePrefix(ref.Name, p.AllowedImagePrefixes) {
+		return fmt.Errorf("image %q is not in allowed prefixes: %v", ref.Name, p.AllowedImagePrefixes)
 	}
-	return ref, ""
-}
 
-func isAllowedPrefix(imageName string, prefixes []string) bool {
-	for _, prefix := range prefixes {
-		if imageName == prefix || strings.HasPrefix(imageName, prefix+"/") {
-			return true
-		}
-	}
-	return false
+	return policy.CheckImageTag(p, ref)
 }