diff --git a/main.go b/main.go index f423de9..7b4d487 100644 --- a/main.go +++ b/main.go @@ -184,7 +184,12 @@ func configFromEnv() Config { replicationMaxAttempts := mustParseInt("MAGPIE_REPLICATION_MAX_ATTEMPTS", defaultReplicationMaxAttempts) maxRestoreBytes := mustParseBytes("MAGPIE_MAX_RESTORE_BYTES", defaultMaxRestoreBytes) allowedBuckets := parseSet(os.Getenv("MAGPIE_ALLOWED_BUCKETS")) + // MAGPIE_PUBLIC_BUCKETS is a legacy whole-bucket public-read list (e.g. app-uploads). + // MAGPIE_PUBLIC_PREFIXES is preferred and takes bucket/prefix values. publicPrefixes := parsePublicPrefixes(os.Getenv("MAGPIE_PUBLIC_PREFIXES")) + for bucket := range parseSet(os.Getenv("MAGPIE_PUBLIC_BUCKETS")) { + publicPrefixes[bucket] = true + } rateLimitPerMinute := mustParseInt("MAGPIE_RATE_LIMIT_PER_MINUTE", 600) trustProxyHeaders := mustParseBool("MAGPIE_TRUST_PROXY_HEADERS", false) allowUnsignedLocal := mustParseBool("MAGPIE_ALLOW_UNSIGNED_PAYLOADS", false) @@ -297,16 +302,21 @@ func parsePublicPrefixes(raw string) map[string]bool { if value == "none" || value == "false" || value == "0" { return map[string]bool{} } - if strings.TrimSpace(raw) != "" { - prefixes := parseSet(raw) - for prefix := range prefixes { - if err := ValidateKey(strings.TrimSuffix(prefix, "/")); err != nil || !strings.Contains(strings.Trim(prefix, "/"), "/") { - log.Fatal("MAGPIE_PUBLIC_PREFIXES entries must be bucket/prefix values") - } - } - return prefixes + if strings.TrimSpace(raw) == "" { + return map[string]bool{} } - return map[string]bool{} + prefixes := parseSet(raw) + for prefix := range prefixes { + cleaned := strings.Trim(prefix, "/") + if cleaned == "" { + log.Fatal("MAGPIE_PUBLIC_PREFIXES entries must not be empty") + } + // Allow bare bucket names (whole-bucket public read) or bucket/prefix paths. + if err := ValidateKey(cleaned); err != nil { + log.Fatal("MAGPIE_PUBLIC_PREFIXES entries must be valid bucket or bucket/prefix values") + } + } + return prefixes } func printUsage() { diff --git a/server.go b/server.go index 896b808..d48f966 100644 --- a/server.go +++ b/server.go @@ -334,8 +334,15 @@ func (s *Server) bucketAllowed(bucket string) bool { func (s *Server) publicReadAllowed(key string) bool { for prefix := range s.cfg.PublicPrefixes { - prefix = strings.TrimSuffix(prefix, "/") + prefix = strings.Trim(prefix, "/") + if prefix == "" { + continue + } + // Bare bucket name: allow any key under that bucket. if !strings.Contains(prefix, "/") { + if key == prefix || strings.HasPrefix(key, prefix+"/") { + return true + } continue } if key == prefix || strings.HasPrefix(key, prefix+"/") { diff --git a/server_test.go b/server_test.go index ee685cb..3f1ea26 100644 --- a/server_test.go +++ b/server_test.go @@ -813,7 +813,7 @@ func TestS3PublicPrefixReadCanBeEnabled(t *testing.T) { cfg := s3TestConfig() cfg.AllowedBuckets = map[string]bool{"app-uploads": true} - cfg.PublicPrefixes = map[string]bool{"app-uploads/avatars": true, "app-uploads": true} + cfg.PublicPrefixes = map[string]bool{"app-uploads/avatars": true} srv := httptest.NewServer(NewServer(store, cfg)) defer srv.Close() @@ -833,6 +833,31 @@ func TestS3PublicPrefixReadCanBeEnabled(t *testing.T) { _ = readResponseBody(t, privateResp, http.StatusForbidden) } +func TestS3PublicBucketReadAllowsWholeBucket(t *testing.T) { + store, err := NewFileStore(t.TempDir()) + if err != nil { + t.Fatal(err) + } + if _, err := store.Put("app-uploads/static_sites/x/favicon.ico", strings.NewReader("ico"), "image/x-icon"); err != nil { + t.Fatal(err) + } + + cfg := s3TestConfig() + cfg.AllowedBuckets = map[string]bool{"app-uploads": true} + cfg.PublicPrefixes = map[string]bool{"app-uploads": true} + srv := httptest.NewServer(NewServer(store, cfg)) + defer srv.Close() + + resp, err := http.Get(srv.URL + "/app-uploads/static_sites/x/favicon.ico") + if err != nil { + t.Fatal(err) + } + body := readResponseBody(t, resp, http.StatusOK) + if string(body) != "ico" { + t.Fatalf("public bucket GET body = %q", string(body)) + } +} + func TestS3PublicReadHardensActiveContent(t *testing.T) { store, err := NewFileStore(t.TempDir()) if err != nil {