CI is red on `main` and on every open PR (including the Dependabot PRs #1531, #1552–#1554) with: ``` panic: file requires newer Go version go1.27 (application built with go1.26) golangci-lint exit with code 2 ``` ## Cause `actions/setup-go` with `go-version: 'stable'` started resolving to **go1.27.0**. The pinned golangci-lint **v2.11.4** release binary is built with **go1.26**, and its `go/types` refuses to typecheck source files declaring a newer language version. Nothing in the failing PRs is at fault. ## Fix Bump to **golangci-lint v2.13.1**, whose release binaries are built with go1.27, so linting works under `stable` again. That bump also pulls in newer linter versions, which surface 57 findings this tree did not have before. Handled as follows: | Linter | Count | Resolution | |---|---|---| | `goconst` | 50 | 39 were repeated literals in `_test.go` files — goconst started counting those in v2.12.0 (golangci-lint#6480 exposed `ignore-tests`). Fixture data like `"1.0.0"` (89 occurrences) is not a missing constant, so `goconst.ignore-tests: true`. The other 11 were real duplication in production code (OpenAPI tags, the `bearer` security scheme name, `$ref`, `http`/`https`) and became named constants. | | `gosec` G710 | 1 | **True positive, fixed.** See below. | | `staticcheck` SA1019 | 6 | Go 1.26 deprecated `ecdsa.PublicKey.X/Y` and `PrivateKey.D`. Annotated with the same targeted `nolint` + rationale already used for `ScalarBaseMult` in `cmd/publisher/auth/common.go`. | ### The open redirect is real `TrailingSlashMiddleware` copied the entire `*url.URL` before rewriting the path. For an absolute-form request URI — legal for a Go HTTP server to parse, e.g. `GET http://evil.com/foo/ HTTP/1.1` — `r.URL.Scheme` and `r.URL.Host` are populated, and they were echoed straight back in the `Location` header. The redirect target is now built from the cleaned path and query alone. The existing `path.Clean` hardening for GHSA-v8vw-gw5j-w7m6 (protocol-relative `//evil.com/`) is unchanged. ### Also Switched `gomodguard` → `gomodguard_v2`; the former is deprecated as of v2.12.0 and was emitting a warning on every run. ## Verification - `golangci-lint v2.13.1` against this tree: **0 issues**, no deprecation warnings - `make validate`: all validations passed - `go build ./...` and tests for every touched package pass (DB-backed tests need Postgres; left to CI) ## Note on migrating the ecdsa key paths Moving those six sites to `PublicKey.Bytes` / `ParseUncompressedPublicKey` / `crypto/ecdh` touches key parsing and signature verification. It should be its own reviewed PR rather than riding along on a CI unblock. Happy to open one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
174 lines
5.8 KiB
Go
174 lines
5.8 KiB
Go
package api
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"log"
|
|
"net/http"
|
|
"net/url"
|
|
"path"
|
|
"strings"
|
|
"time"
|
|
|
|
"github.com/danielgtaylor/huma/v2"
|
|
"github.com/rs/cors"
|
|
|
|
v0 "github.com/modelcontextprotocol/registry/internal/api/handlers/v0"
|
|
"github.com/modelcontextprotocol/registry/internal/api/router"
|
|
"github.com/modelcontextprotocol/registry/internal/config"
|
|
"github.com/modelcontextprotocol/registry/internal/service"
|
|
"github.com/modelcontextprotocol/registry/internal/telemetry"
|
|
)
|
|
|
|
// NulByteValidationMiddleware rejects requests containing NUL bytes in URL path or query parameters.
|
|
// This prevents PostgreSQL encoding errors (SQLSTATE 22021) and returns a proper 400 Bad Request.
|
|
// Checks for both literal NUL bytes (\x00) and URL-encoded form (%00).
|
|
func NulByteValidationMiddleware(next http.Handler) http.Handler {
|
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
// Check URL path for literal NUL bytes or URL-encoded %00
|
|
// Path needs %00 check because handlers call url.PathUnescape() which would decode it
|
|
if containsNulByte(r.URL.Path) {
|
|
writeErrorResponse(w, http.StatusBadRequest, "Invalid request: URL path contains null bytes")
|
|
return
|
|
}
|
|
|
|
// Check raw query string for literal NUL bytes or URL-encoded %00
|
|
if containsNulByte(r.URL.RawQuery) {
|
|
writeErrorResponse(w, http.StatusBadRequest, "Invalid request: query parameters contain null bytes")
|
|
return
|
|
}
|
|
|
|
next.ServeHTTP(w, r)
|
|
})
|
|
}
|
|
|
|
// writeErrorResponse writes a JSON error response using huma's ErrorModel format
|
|
// for consistency with the rest of the API.
|
|
func writeErrorResponse(w http.ResponseWriter, status int, detail string) {
|
|
w.Header().Set("Content-Type", "application/json")
|
|
w.WriteHeader(status)
|
|
|
|
errModel := &huma.ErrorModel{
|
|
Title: http.StatusText(status),
|
|
Status: status,
|
|
Detail: detail,
|
|
}
|
|
_ = json.NewEncoder(w).Encode(errModel)
|
|
}
|
|
|
|
// containsNulByte checks if a string contains a NUL byte, either as a literal \x00
|
|
// or URL-encoded as %00.
|
|
func containsNulByte(s string) bool {
|
|
// Check for literal NUL byte
|
|
if strings.ContainsRune(s, '\x00') {
|
|
return true
|
|
}
|
|
// Check for URL-encoded NUL byte (%00)
|
|
// Using Contains directly since %00 has no case variation (both hex digits are 0)
|
|
return strings.Contains(s, "%00")
|
|
}
|
|
|
|
// TrailingSlashMiddleware redirects requests with trailing slashes to their canonical form
|
|
func TrailingSlashMiddleware(next http.Handler) http.Handler {
|
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
// Only redirect if the path is not "/" and ends with a "/"
|
|
if r.URL.Path != "/" && strings.HasSuffix(r.URL.Path, "/") {
|
|
// Build the target from the path and query alone, dropping any
|
|
// scheme/host the client may have supplied in an absolute-form
|
|
// request URI ("GET http://evil.com/foo/ HTTP/1.1"), which would
|
|
// otherwise be echoed back as an off-site redirect.
|
|
//
|
|
// path.Clean both removes the trailing slash and collapses any
|
|
// leading "//" to "/", which prevents an open-redirect via a
|
|
// protocol-relative path like "//evil.com/" (GHSA-v8vw-gw5j-w7m6).
|
|
newURL := url.URL{
|
|
Path: path.Clean(r.URL.Path),
|
|
RawQuery: r.URL.RawQuery,
|
|
}
|
|
|
|
// Use 308 Permanent Redirect to preserve the request method
|
|
http.Redirect(w, r, newURL.String(), http.StatusPermanentRedirect)
|
|
return
|
|
}
|
|
|
|
next.ServeHTTP(w, r)
|
|
})
|
|
}
|
|
|
|
// Server represents the HTTP server
|
|
type Server struct {
|
|
config *config.Config
|
|
registry service.RegistryService
|
|
humaAPI huma.API
|
|
server *http.Server
|
|
}
|
|
|
|
// NewServer creates a new HTTP server
|
|
func NewServer(cfg *config.Config, registryService service.RegistryService, metrics *telemetry.Metrics, versionInfo *v0.VersionBody) *Server {
|
|
// Create HTTP mux and Huma API
|
|
mux := http.NewServeMux()
|
|
|
|
api := router.NewHumaAPI(cfg, registryService, mux, metrics, versionInfo)
|
|
|
|
// Configure CORS with permissive settings for public API
|
|
corsHandler := cors.New(cors.Options{
|
|
AllowedOrigins: []string{"*"},
|
|
AllowedMethods: []string{
|
|
http.MethodGet,
|
|
http.MethodPost,
|
|
http.MethodPut,
|
|
http.MethodPatch,
|
|
http.MethodDelete,
|
|
http.MethodOptions,
|
|
},
|
|
AllowedHeaders: []string{"*"},
|
|
ExposedHeaders: []string{"Content-Type", "Content-Length"},
|
|
AllowCredentials: false, // Must be false when AllowedOrigins is "*"
|
|
MaxAge: 86400, // 24 hours
|
|
})
|
|
|
|
// Wrap the mux with middleware stack
|
|
// Order: NulByteValidation -> TrailingSlash -> CORS -> Mux
|
|
handler := NulByteValidationMiddleware(TrailingSlashMiddleware(corsHandler.Handler(mux)))
|
|
|
|
server := &Server{
|
|
config: cfg,
|
|
registry: registryService,
|
|
humaAPI: api,
|
|
server: &http.Server{
|
|
Addr: cfg.ServerAddress,
|
|
Handler: handler,
|
|
ReadHeaderTimeout: 10 * time.Second,
|
|
ReadTimeout: 30 * time.Second,
|
|
// WriteTimeout intentionally not set: the publish path runs
|
|
// outbound package validators sequentially (npm/pypi/nuget up to
|
|
// 10s each, OCI up to 30s), so any tight cap could cut off a
|
|
// legitimate multi-package publish mid-response — surfacing as a
|
|
// truncated read to the publisher even when the DB commit
|
|
// succeeded. Slow-response-read DoS is bounded upstream by
|
|
// NGINX ingress timeouts and the per-IP rate limit. Revisit once
|
|
// validators are parallelised or per-request package counts are
|
|
// bounded.
|
|
IdleTimeout: 120 * time.Second,
|
|
},
|
|
}
|
|
|
|
return server
|
|
}
|
|
|
|
// Handler returns the fully wrapped HTTP handler (middleware stack plus routes).
|
|
// Exposed so tests can exercise the middleware, e.g. CORS, without binding a port.
|
|
func (s *Server) Handler() http.Handler {
|
|
return s.server.Handler
|
|
}
|
|
|
|
// Start begins listening for incoming HTTP requests
|
|
func (s *Server) Start() error {
|
|
log.Printf("HTTP server starting on %s", s.config.ServerAddress)
|
|
return s.server.ListenAndServe()
|
|
}
|
|
|
|
// Shutdown gracefully shuts down the server
|
|
func (s *Server) Shutdown(ctx context.Context) error {
|
|
return s.server.Shutdown(ctx)
|
|
}
|