fix(s3): valid direct serve pre-signed URLs for alternate hosts

config.example.yaml

@@ -133,12 +133,15 @@ storage:
   # How long presigned URLs remain valid (e.g. "5m", "1h"). Default: "15m".
   direct_serve_ttl: "15m"
 
-  # Public base URL to substitute into presigned URLs. Set this when the
-  # proxy reaches storage at an internal address (127.0.0.1, a Docker
-  # service name) but clients must use a public hostname. Only scheme and
-  # host are used; the signed path and query are preserved. For S3/MinIO
-  # the reverse proxy at this address must forward requests with the
-  # internal Host header or the SigV4 signature will not validate.
+  # Public base URL for presigned URLs. Set this when the proxy reaches
+  # storage at an internal address (127.0.0.1, a Docker service name, or
+  # VPC DNS) but clients must use a public hostname. Only scheme and host
+  # are used. For S3/MinIO the URL is signed for this host, so the server
+  # there must receive that Host header: a reverse proxy or CDN that sends
+  # the internal or bucket host to the origin fails signature validation.
+  # With virtual-hosted addressing (no use_path_style) the URL carries no
+  # bucket name, so this must be the bucket's own hostname. For GCS and
+  # Azure the host is replaced after signing.
   # direct_serve_base_url: "https://minio.example.com"
 
 # Database configuration

go.mod

@@ -5,8 +5,11 @@ go 1.27.3
 require (
 	github.com/BurntSushi/toml v1.6.0
 	github.com/Masterminds/semver/v3 v3.5.0
+	github.com/aws/aws-sdk-go-v2 v1.47.5
 	github.com/aws/aws-sdk-go-v2/config v1.33.12
 	github.com/aws/aws-sdk-go-v2/service/ecr v1.67.1
+	github.com/aws/aws-sdk-go-v2/service/s3 v1.111.0
+	github.com/aws/smithy-go v1.28.7
 	github.com/git-pkgs/archives v0.10.0
 	github.com/git-pkgs/artifacts v0.2.1
 	github.com/git-pkgs/cooldown v0.3.1
@@ -51,7 +54,6 @@ require (
 	github.com/PuerkitoBio/purell v1.1.1 // indirect
 	github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
 	github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect
-	github.com/aws/aws-sdk-go-v2 v1.47.5 // indirect
 	github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.20 // indirect
 	github.com/aws/aws-sdk-go-v2/credentials v1.20.12 // indirect
 	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.20.5 // indirect
@@ -63,12 +65,10 @@ require (
 	github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.11.2 // indirect
 	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.14.8 // indirect
 	github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.20.2 // indirect
-	github.com/aws/aws-sdk-go-v2/service/s3 v1.111.0 // indirect
 	github.com/aws/aws-sdk-go-v2/service/signin v1.12.0 // indirect
 	github.com/aws/aws-sdk-go-v2/service/sso v1.39.0 // indirect
 	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.44.0 // indirect
 	github.com/aws/aws-sdk-go-v2/service/sts v1.51.6 // indirect
-	github.com/aws/smithy-go v1.28.7 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenk/backoff v2.2.1+incompatible // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect

internal/config/config.go

@@ -519,10 +519,10 @@ type StorageConfig struct {
 	// Uses Go duration syntax (e.g. "5m", "1h"). Default: "15m".
 	DirectServeTTL string `json:"direct_serve_ttl" yaml:"direct_serve_ttl"`
 
-	// DirectServeBaseURL overrides the scheme and host of presigned URLs
-	// before returning them to clients. Useful when the proxy reaches
-	// storage at an internal address (e.g. 127.0.0.1 or a Docker hostname)
-	// but clients must use a public one.
+	// DirectServeBaseURL sets the scheme and host of presigned URLs. Useful
+	// when the proxy reaches storage at an internal address (e.g. 127.0.0.1
+	// or a Docker hostname) but clients must use a public one. S3 URLs are
+	// signed for this host; GCS and Azure URLs have it replaced after signing.
 	DirectServeBaseURL string `json:"direct_serve_base_url" yaml:"direct_serve_base_url"`
 
 	// CacheArtifacts stores fetched artifacts so later downloads are served

internal/handler/handler.go

@@ -242,11 +242,7 @@ type Proxy struct {
 	NPMFullMetadata bool
 	DirectServe     bool
 	DirectServeTTL  time.Duration
-	// DirectServeBaseURL, if set, replaces the scheme and host of presigned
-	// URLs so clients receive a public address even when the proxy reaches
-	// storage at an internal one.
-	DirectServeBaseURL string
-	HTTPClient         *http.Client
+	HTTPClient      *http.Client
 
 	// RateLimits is the gate under the upstream clients. Artifact fetches
 	// check it before calling Fetcher, so held hosts never reach its breaker.
@@ -576,7 +572,7 @@ func (p *Proxy) readStored(ctx context.Context, pkgPURL, versionPURL, filename s
 	if p.DirectServe {
 		signed, err := p.Storage.SignedURL(ctx, artifact.StoragePath, p.DirectServeTTL)
 		if err == nil {
-			result.RedirectURL = rewriteSignedURLHost(signed, p.DirectServeBaseURL)
+			result.RedirectURL = signed
 			p.recordCacheHit(artifact.Ecosystem, versionPURL, filename)
 			return result, nil
 		}
@@ -617,26 +613,6 @@ func (p *Proxy) readStored(ctx context.Context, pkgPURL, versionPURL, filename s
 	return result, nil
 }
 
-// rewriteSignedURLHost replaces the scheme and host of a signed URL with those
-// from baseURL, preserving the path and query (which carry the signature).
-// Returns signed unchanged if baseURL is empty or either URL fails to parse.
-func rewriteSignedURLHost(signed, baseURL string) string {
-	if baseURL == "" {
-		return signed
-	}
-	s, err := url.Parse(signed)
-	if err != nil {
-		return signed
-	}
-	b, err := url.Parse(baseURL)
-	if err != nil || b.Scheme == "" || b.Host == "" {
-		return signed
-	}
-	s.Scheme = b.Scheme
-	s.Host = b.Host
-	return s.String()
-}
-
 func (p *Proxy) recordCacheHit(ecosystem, versionPURL, filename string) {
 	_ = p.DB.RecordArtifactHit(versionPURL, filename)
 	metrics.RecordArtifactCacheHit(ecosystem)

internal/handler/handler_test.go

@@ -639,74 +639,6 @@ func TestGetOrFetchArtifact_DirectServe_Redirect(t *testing.T) {
 	}
 }
 
-func TestGetOrFetchArtifact_DirectServe_BaseURLRewrite(t *testing.T) {
-	proxy, db, store, _ := setupTestProxy(t)
-	seedPackage(t, db, store, "npm", "lodash", "4.17.21", "lodash-4.17.21.tgz", "cached content")
-
-	proxy.DirectServe = true
-	proxy.DirectServeBaseURL = "https://cdn.example.com"
-	store.signedURL = "http://127.0.0.1:9000/bucket/npm/lodash?X-Amz-Signature=abc&X-Amz-Expires=900"
-
-	result, err := proxy.GetOrFetchArtifact(context.Background(), "npm", "lodash", "4.17.21", "lodash-4.17.21.tgz")
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
-	}
-
-	want := "https://cdn.example.com/bucket/npm/lodash?X-Amz-Signature=abc&X-Amz-Expires=900"
-	if result.RedirectURL != want {
-		t.Errorf("RedirectURL = %q, want %q", result.RedirectURL, want)
-	}
-}
-
-func TestRewriteSignedURLHost(t *testing.T) {
-	tests := []struct {
-		name    string
-		signed  string
-		baseURL string
-		want    string
-	}{
-		{
-			"empty base url is no-op",
-			"http://127.0.0.1:9000/bucket/key?sig=abc",
-			"",
-			"http://127.0.0.1:9000/bucket/key?sig=abc",
-		},
-		{
-			"replaces scheme and host",
-			"http://127.0.0.1:9000/bucket/key?sig=abc",
-			"https://cdn.example.com",
-			"https://cdn.example.com/bucket/key?sig=abc",
-		},
-		{
-			"preserves path and query",
-			"http://minio:9000/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101",
-			"https://files.example.com",
-			"https://files.example.com/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101",
-		},
-		{
-			"ignores base url path",
-			"http://127.0.0.1:9000/bucket/key?sig=abc",
-			"https://cdn.example.com/ignored",
-			"https://cdn.example.com/bucket/key?sig=abc",
-		},
-		{
-			"invalid base url is no-op",
-			"http://127.0.0.1:9000/bucket/key?sig=abc",
-			"://bad",
-			"http://127.0.0.1:9000/bucket/key?sig=abc",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := rewriteSignedURLHost(tt.signed, tt.baseURL)
-			if got != tt.want {
-				t.Errorf("rewriteSignedURLHost(%q, %q) = %q, want %q", tt.signed, tt.baseURL, got, tt.want)
-			}
-		})
-	}
-}
-
 func TestGetOrFetchArtifact_DirectServe_FallbackOnUnsupported(t *testing.T) {
 	proxy, db, store, _ := setupTestProxy(t)
 	seedPackage(t, db, store, "npm", "lodash", "4.17.21", "lodash-4.17.21.tgz", "cached content")

internal/handler/profile.go

@@ -14,7 +14,7 @@ func (p *Proxy) WithCooldown(policy CooldownPolicy, shortest time.Duration) *Pro
 		GradleReadOnly: p.GradleReadOnly, HostedEnabled: p.HostedEnabled,
 		GradleMaxUploadSize: p.GradleMaxUploadSize, NPMFullMetadata: p.NPMFullMetadata,
 		DirectServe: p.DirectServe, DirectServeTTL: p.DirectServeTTL,
-		DirectServeBaseURL: p.DirectServeBaseURL, HTTPClient: p.HTTPClient,
+		HTTPClient: p.HTTPClient,
 		RateLimits: p.RateLimits, HostedNPMScopes: p.HostedNPMScopes,
 		HostedComposerVendors: p.HostedComposerVendors, StreamArtifacts: p.StreamArtifacts,
 		Scanners: p.Scanners, ScanSigningKey: p.ScanSigningKey, ScanFetchBaseURL: p.ScanFetchBaseURL,

internal/handler/profile_test.go

@@ -34,7 +34,6 @@ func TestWithCooldownCopiesProxyFields(t *testing.T) {
 	p.NPMFullMetadata = true
 	p.DirectServe = true
 	p.DirectServeTTL = time.Minute
-	p.DirectServeBaseURL = "https://storage.example.com"
 	p.RateLimits = registryclient.NewRateLimitGate(http.DefaultTransport)
 	p.HostedNPMScopes = []string{"@example"}
 	p.HostedComposerVendors = []string{"example"}

internal/server/server.go

@@ -189,7 +189,11 @@ func New(cfg *config.Config, logger *slog.Logger, buildInfo BuildInfo) (*Server,
 		// Fall back to file:// with Path
 		storageURL = "file://" + cfg.Storage.Path //nolint:staticcheck // backwards compat
 	}
-	store, err := storage.OpenBucket(context.Background(), storageURL)
+	var storageOpts []storage.Option
+	if cfg.Storage.DirectServe && cfg.Storage.DirectServeBaseURL != "" {
+		storageOpts = append(storageOpts, storage.WithSignEndpoint(cfg.Storage.DirectServeBaseURL))
+	}
+	store, err := storage.OpenBucket(context.Background(), storageURL, storageOpts...)
 	if err != nil {
 		_ = db.Close()
 		return nil, fmt.Errorf("initializing storage: %w", err)
@@ -312,7 +316,6 @@ func (s *Server) serve(listener net.Listener) error {
 	proxy.GradleMaxUploadSize = s.cfg.ParseGradleBuildCacheMaxUploadSize()
 	proxy.DirectServe = s.cfg.Storage.DirectServe
 	proxy.DirectServeTTL = s.cfg.ParseDirectServeTTL()
-	proxy.DirectServeBaseURL = s.cfg.Storage.DirectServeBaseURL
 	proxy.StreamArtifacts = !s.cfg.Storage.CacheArtifacts
 	if err := ConfigureDenylist(s.cfg, proxy); err != nil {
 		return err

internal/server/server_test.go

@@ -1740,6 +1740,63 @@ func TestNewServer_StorageConnectivityCheck(t *testing.T) {
 	_ = srv.db.Close()
 }
 
+func TestNewServer_DirectServeBaseURL(t *testing.T) {
+	tempDir := t.TempDir()
+	awsConfigFile := filepath.Join(tempDir, "aws-config")
+	awsCredsFile := filepath.Join(tempDir, "aws-credentials")
+	if err := os.WriteFile(awsConfigFile, nil, 0o600); err != nil {
+		t.Fatalf("writing empty AWS config file: %v", err)
+	}
+	if err := os.WriteFile(awsCredsFile, nil, 0o600); err != nil {
+		t.Fatalf("writing empty AWS credentials file: %v", err)
+	}
+	t.Setenv("AWS_PROFILE", "")
+	t.Setenv("AWS_DEFAULT_PROFILE", "")
+	t.Setenv("AWS_CONFIG_FILE", awsConfigFile)
+	t.Setenv("AWS_SHARED_CREDENTIALS_FILE", awsCredsFile)
+	t.Setenv("AWS_ACCESS_KEY_ID", "test")
+	t.Setenv("AWS_SECRET_ACCESS_KEY", "test")
+
+	s3Srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	}))
+	defer s3Srv.Close()
+
+	cfg := &config.Config{
+		Listen:  ":0",
+		BaseURL: "http://localhost:8080",
+		Storage: config.StorageConfig{
+			URL:                "s3://bucket?region=us-1&endpoint=" + s3Srv.URL + "&use_path_style=true",
+			DirectServe:        true,
+			DirectServeBaseURL: "https://cdn.example.com",
+		},
+		Database: config.DatabaseConfig{Path: filepath.Join(tempDir, "test.db")},
+	}
+
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+	srv, err := New(cfg, logger, BuildInfo{})
+	if err != nil {
+		t.Fatalf("New() failed: %v", err)
+	}
+	defer func() {
+		_ = srv.storage.Close()
+		_ = srv.db.Close()
+	}()
+
+	signed, err := srv.storage.SignedURL(context.Background(), "npm/lodash/4.17.21/lodash-4.17.21.tgz", time.Minute)
+	if err != nil {
+		t.Fatalf("SignedURL() failed: %v", err)
+	}
+
+	u, err := url.Parse(signed)
+	if err != nil {
+		t.Fatalf("parsing signed URL %q: %v", signed, err)
+	}
+	if u.Scheme != "https" || u.Host != "cdn.example.com" {
+		t.Errorf("signed URL = %q, want scheme https and host cdn.example.com", signed)
+	}
+}
+
 func TestNewServer_InvalidAccessLogFailsBeforeDatabaseInit(t *testing.T) {
 	tempDir := t.TempDir()
 	dbPath := filepath.Join(tempDir, "test.db")