From 8a7f490eabb970bdfdd5e1d4ac34394aebe1fd97 Mon Sep 17 00:00:00 2001 From: Steven Miller Date: Tue, 27 Jan 2026 09:41:41 -0500 Subject: [PATCH 1/3] Add tests to reproduce image parsing issue --- cmd/api/main_test.go | 132 ++++++++++++++++++++++++++++++- lib/middleware/oapi_auth_test.go | 69 +++++++++++++++- 2 files changed, 199 insertions(+), 2 deletions(-) diff --git a/cmd/api/main_test.go b/cmd/api/main_test.go index 01b1614..c461bd3 100644 --- a/cmd/api/main_test.go +++ b/cmd/api/main_test.go @@ -4,15 +4,17 @@ import ( "bytes" "net/http" "net/http/httptest" + "net/url" "testing" "time" "github.com/getkin/kin-openapi/openapi3filter" "github.com/go-chi/chi/v5" "github.com/golang-jwt/jwt/v5" - nethttpmiddleware "github.com/oapi-codegen/nethttp-middleware" mw "github.com/kernel/hypeman/lib/middleware" "github.com/kernel/hypeman/lib/oapi" + nethttpmiddleware "github.com/oapi-codegen/nethttp-middleware" + "github.com/oapi-codegen/runtime" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -92,3 +94,131 @@ func TestMiddleware_ValidJWT(t *testing.T) { assert.Equal(t, http.StatusCreated, w.Code) } + +func TestOapiRuntimeBindStyledParameter_URLDecoding(t *testing.T) { + // Test if oapi-codegen's runtime.BindStyledParameterWithOptions URL-decodes path params + tests := []struct { + name string + input string + expected string + }{ + { + name: "simple string", + input: "alpine:latest", + expected: "alpine:latest", + }, + { + name: "URL-encoded slashes", + input: "docker.io%2Flibrary%2Falpine%3Alatest", + expected: "docker.io/library/alpine:latest", // Should be decoded + }, + { + name: "already decoded", + input: "docker.io/library/alpine:latest", + expected: "docker.io/library/alpine:latest", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var dest string + err := runtime.BindStyledParameterWithOptions( + "simple", "name", tt.input, &dest, + runtime.BindStyledParameterOptions{ + ParamLocation: runtime.ParamLocationPath, + Explode: false, + Required: true, + }, + ) + require.NoError(t, err) + assert.Equal(t, tt.expected, dest, "BindStyledParameterWithOptions should URL-decode the input") + }) + } +} + +func TestImageNameWithSlashes_URLEncoding(t *testing.T) { + // This test verifies how chi router handles image names with slashes. + // Image names like "docker.io/onkernel/chromium-headful:latest" contain slashes + // that need to be URL-encoded to work with the /images/{name} endpoint. + // + // FINDINGS: + // 1. Chi DOES route to the handler when slashes are URL-encoded (%2F) + // 2. Chi's URLParam returns the STILL-ENCODED value (e.g., "docker.io%2Flibrary%2Falpine%3Alatest") + // 3. The handler/middleware must URL-decode the parameter itself + // 4. The oapi-codegen runtime.BindStyledParameterWithOptions MAY handle decoding + // + // This is a server-side documentation issue - users need to know to URL-encode image names + // with slashes, and the server needs to ensure proper URL-decoding. + + r := chi.NewRouter() + + var capturedRaw string + var capturedDecoded string + r.Get("/images/{name}", func(w http.ResponseWriter, req *http.Request) { + capturedRaw = chi.URLParam(req, "name") + // URL-decode the parameter + decoded, _ := url.QueryUnescape(capturedRaw) + capturedDecoded = decoded + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"` + capturedDecoded + `"}`)) + }) + + token, err := generateValidJWT("user-123") + require.NoError(t, err) + + tests := []struct { + name string + path string + expectedStatus int + expectedRaw string + expectedDecoded string + }{ + { + name: "simple image name (no slashes)", + path: "/images/alpine:latest", + expectedStatus: http.StatusOK, + expectedRaw: "alpine:latest", + expectedDecoded: "alpine:latest", + }, + { + name: "URL-encoded slashes - routes correctly", + path: "/images/docker.io%2Flibrary%2Falpine%3Alatest", + expectedStatus: http.StatusOK, + expectedRaw: "docker.io%2Flibrary%2Falpine%3Alatest", // chi returns encoded + expectedDecoded: "docker.io/library/alpine:latest", // after QueryUnescape + }, + { + name: "unencoded slashes - route not matched", + path: "/images/docker.io/library/alpine:latest", + expectedStatus: http.StatusNotFound, // chi won't match this route + expectedRaw: "", + expectedDecoded: "", + }, + { + name: "nested image docker.io/onkernel/chromium-headful:latest", + path: "/images/docker.io%2Fonkernel%2Fchromium-headful%3Alatest", + expectedStatus: http.StatusOK, + expectedRaw: "docker.io%2Fonkernel%2Fchromium-headful%3Alatest", + expectedDecoded: "docker.io/onkernel/chromium-headful:latest", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + capturedRaw = "" + capturedDecoded = "" + + req := httptest.NewRequest(http.MethodGet, tt.path, nil) + req.Header.Set("Authorization", "Bearer "+token) + + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + assert.Equal(t, tt.expectedStatus, w.Code, "status code mismatch for path %s", tt.path) + if tt.expectedStatus == http.StatusOK { + assert.Equal(t, tt.expectedRaw, capturedRaw, "raw captured name mismatch") + assert.Equal(t, tt.expectedDecoded, capturedDecoded, "decoded name mismatch") + } + }) + } +} diff --git a/lib/middleware/oapi_auth_test.go b/lib/middleware/oapi_auth_test.go index dbb5a26..6fc1205 100644 --- a/lib/middleware/oapi_auth_test.go +++ b/lib/middleware/oapi_auth_test.go @@ -135,6 +135,74 @@ func TestJwtAuth_RejectsRegistryTokens(t *testing.T) { }) } +func TestExtractRepoFromPath(t *testing.T) { + tests := []struct { + name string + path string + expected string + }{ + // Simple repository names + { + name: "simple repo with manifests", + path: "/v2/test-alpine/manifests/latest", + expected: "test-alpine", + }, + { + name: "simple repo with blobs", + path: "/v2/test-alpine/blobs/sha256:abc123", + expected: "test-alpine", + }, + { + name: "simple repo with uploads", + path: "/v2/test-alpine/blobs/uploads/uuid-here", + expected: "test-alpine", + }, + + // Nested repository names (like builds/abc123) + { + name: "nested repo with manifests", + path: "/v2/builds/abc123/manifests/latest", + expected: "builds/abc123", + }, + { + name: "nested repo with blobs", + path: "/v2/builds/abc123/blobs/sha256:def456", + expected: "builds/abc123", + }, + { + name: "nested repo with uploads", + path: "/v2/builds/abc123/blobs/uploads/uuid-here", + expected: "builds/abc123", + }, + + // Base path (no repo) + { + name: "base path", + path: "/v2/", + expected: "", + }, + + // Edge cases + { + name: "repo named manifests-test", + path: "/v2/manifests-test/manifests/latest", + expected: "manifests-test", + }, + { + name: "repo named blobs-data", + path: "/v2/blobs-data/blobs/sha256:abc", + expected: "blobs-data", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractRepoFromPath(tt.path) + assert.Equal(t, tt.expected, result, "extractRepoFromPath(%q)", tt.path) + }) + } +} + func TestJwtAuth_RequiresAuthorization(t *testing.T) { nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) @@ -201,4 +269,3 @@ func TestJwtAuth_RequiresAuthorization(t *testing.T) { assert.Contains(t, rr.Body.String(), "invalid token") }) } - From 7b90c142da985df62ad6b962e39f0680dd702c03 Mon Sep 17 00:00:00 2001 From: Steven Miller Date: Tue, 27 Jan 2026 09:42:43 -0500 Subject: [PATCH 2/3] Fix registry path parsing --- lib/middleware/oapi_auth.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/middleware/oapi_auth.go b/lib/middleware/oapi_auth.go index 98c9c6c..d7cdea9 100644 --- a/lib/middleware/oapi_auth.go +++ b/lib/middleware/oapi_auth.go @@ -17,8 +17,10 @@ type contextKey string const userIDKey contextKey = "user_id" -// registryPathPattern matches /v2/{repository}/... paths -var registryPathPattern = regexp.MustCompile(`^/v2/([^/]+(?:/[^/]+)?)/`) +// registryPathPattern matches /v2/{repository}/{action}/... paths where action is manifests, blobs, or tags. +// The repository name is captured in group 1, and can contain slashes (e.g., "builds/abc123"). +// Uses non-greedy matching to capture the minimal repo name before the action keyword. +var registryPathPattern = regexp.MustCompile(`^/v2/(.+?)/(manifests|blobs|tags)/`) // RegistryTokenClaims contains the claims for a scoped registry access token. // This mirrors the type in lib/builds/registry_token.go to avoid circular imports. From 2fc0043261f1cdd108aebef93b4c5f88249d7ad1 Mon Sep 17 00:00:00 2001 From: Steven Miller Date: Tue, 27 Jan 2026 09:47:18 -0500 Subject: [PATCH 3/3] Use dependency for parsing repo name --- go.mod | 8 ++++---- go.sum | 2 -- lib/middleware/oapi_auth.go | 27 +++++++++++++++++++-------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index eba1441..7bc68b7 100644 --- a/go.mod +++ b/go.mod @@ -8,13 +8,16 @@ require ( github.com/cyphar/filepath-securejoin v0.6.1 github.com/digitalocean/go-qemu v0.0.0-20250212194115-ee9b0668d242 github.com/distribution/reference v0.6.0 + github.com/docker/distribution v2.8.3+incompatible github.com/getkin/kin-openapi v0.133.0 github.com/ghodss/yaml v1.0.0 github.com/go-chi/chi/v5 v5.2.3 github.com/golang-jwt/jwt/v5 v5.3.0 github.com/golang/protobuf v1.5.4 github.com/google/go-containerregistry v0.20.6 + github.com/google/uuid v1.6.0 github.com/google/wire v0.7.0 + github.com/gorilla/mux v1.8.1 github.com/gorilla/websocket v1.5.3 github.com/joho/godotenv v1.5.1 github.com/mdlayher/vsock v1.2.1 @@ -22,6 +25,7 @@ require ( github.com/nrednav/cuid2 v1.1.0 github.com/oapi-codegen/nethttp-middleware v1.1.2 github.com/oapi-codegen/runtime v1.1.2 + github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.1 github.com/opencontainers/runtime-spec v1.2.1 github.com/opencontainers/umoci v0.6.0 @@ -61,7 +65,6 @@ require ( github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/digitalocean/go-libvirt v0.0.0-20220804181439-8648fbde413e // indirect github.com/docker/cli v28.2.2+incompatible // indirect - github.com/docker/distribution v2.8.3+incompatible // indirect github.com/docker/docker v28.2.2+incompatible // indirect github.com/docker/docker-credential-helpers v0.9.3 // indirect github.com/docker/go-connections v0.5.0 // indirect @@ -74,8 +77,6 @@ require ( github.com/go-openapi/swag v0.23.0 // indirect github.com/go-test/deep v1.1.1 // indirect github.com/gogo/protobuf v1.3.2 // indirect - github.com/google/uuid v1.6.0 // indirect - github.com/gorilla/mux v1.8.1 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.2 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/klauspost/compress v1.18.0 // indirect @@ -90,7 +91,6 @@ require ( github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 // indirect github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 // indirect - github.com/opencontainers/go-digest v1.0.0 // indirect github.com/perimeterx/marshmallow v1.1.5 // indirect github.com/pierrec/lz4/v4 v4.1.22 // indirect github.com/pkg/errors v0.9.1 // indirect diff --git a/go.sum b/go.sum index 12ceb44..3edd372 100644 --- a/go.sum +++ b/go.sum @@ -96,8 +96,6 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-containerregistry v0.20.6 h1:cvWX87UxxLgaH76b4hIvya6Dzz9qHB31qAwjAohdSTU= github.com/google/go-containerregistry v0.20.6/go.mod h1:T0x8MuoAoKX/873bkeSfLD2FAkwCDf9/HZgsFJ02E2Y= -github.com/google/subcommands v1.2.0 h1:vWQspBTo2nEqTUFita5/KeEWlUL8kQObDFbub/EN9oE= -github.com/google/subcommands v1.2.0/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3yTrtFlrHVk= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= diff --git a/lib/middleware/oapi_auth.go b/lib/middleware/oapi_auth.go index d7cdea9..cf5f830 100644 --- a/lib/middleware/oapi_auth.go +++ b/lib/middleware/oapi_auth.go @@ -5,11 +5,12 @@ import ( "encoding/base64" "fmt" "net/http" - "regexp" "strings" + v2 "github.com/docker/distribution/registry/api/v2" "github.com/getkin/kin-openapi/openapi3filter" "github.com/golang-jwt/jwt/v5" + "github.com/gorilla/mux" "github.com/kernel/hypeman/lib/logger" ) @@ -17,10 +18,9 @@ type contextKey string const userIDKey contextKey = "user_id" -// registryPathPattern matches /v2/{repository}/{action}/... paths where action is manifests, blobs, or tags. -// The repository name is captured in group 1, and can contain slashes (e.g., "builds/abc123"). -// Uses non-greedy matching to capture the minimal repo name before the action keyword. -var registryPathPattern = regexp.MustCompile(`^/v2/(.+?)/(manifests|blobs|tags)/`) +// registryRouter is the OCI Distribution API router from docker/distribution. +// It properly parses repository names (which can contain slashes) from /v2/ paths. +var registryRouter = v2.Router() // RegistryTokenClaims contains the claims for a scoped registry access token. // This mirrors the type in lib/builds/registry_token.go to avoid circular imports. @@ -203,10 +203,21 @@ func isInternalVMRequest(r *http.Request) bool { // extractRepoFromPath extracts the repository name from a registry path. // e.g., "/v2/builds/abc123/manifests/latest" -> "builds/abc123" +// extractRepoFromPath extracts the repository name from a registry path. +// Uses the docker/distribution router which properly handles repository names +// that can contain slashes (e.g., "builds/abc123" from "/v2/builds/abc123/manifests/latest"). func extractRepoFromPath(path string) string { - matches := registryPathPattern.FindStringSubmatch(path) - if len(matches) >= 2 { - return matches[1] + // Create a minimal request for route matching + req, err := http.NewRequest(http.MethodGet, path, nil) + if err != nil { + return "" + } + + var match mux.RouteMatch + if registryRouter.Match(req, &match) { + if name, ok := match.Vars["name"]; ok { + return name + } } return "" }