fix: honor MAGPIE_PUBLIC_BUCKETS for unsigned media reads
All checks were successful
Deploy / Deploy over SSH (push) Successful in 1m25s
CI / Test and build (push) Successful in 1m38s

Production still set MAGPIE_PUBLIC_BUCKETS=app-uploads, but only
MAGPIE_PUBLIC_PREFIXES was read, so media.elektrine.com returned
SignatureDoesNotMatch 403. Accept bare bucket public reads and the
legacy PUBLIC_BUCKETS env.
This commit is contained in:
Maxfield Luke 2026-07-29 05:50:22 -04:00
parent aa43ce2562
commit 6a717adca2
3 changed files with 53 additions and 11 deletions

20
main.go
View file

@ -184,7 +184,12 @@ func configFromEnv() Config {
replicationMaxAttempts := mustParseInt("MAGPIE_REPLICATION_MAX_ATTEMPTS", defaultReplicationMaxAttempts) replicationMaxAttempts := mustParseInt("MAGPIE_REPLICATION_MAX_ATTEMPTS", defaultReplicationMaxAttempts)
maxRestoreBytes := mustParseBytes("MAGPIE_MAX_RESTORE_BYTES", defaultMaxRestoreBytes) maxRestoreBytes := mustParseBytes("MAGPIE_MAX_RESTORE_BYTES", defaultMaxRestoreBytes)
allowedBuckets := parseSet(os.Getenv("MAGPIE_ALLOWED_BUCKETS")) 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")) 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) rateLimitPerMinute := mustParseInt("MAGPIE_RATE_LIMIT_PER_MINUTE", 600)
trustProxyHeaders := mustParseBool("MAGPIE_TRUST_PROXY_HEADERS", false) trustProxyHeaders := mustParseBool("MAGPIE_TRUST_PROXY_HEADERS", false)
allowUnsignedLocal := mustParseBool("MAGPIE_ALLOW_UNSIGNED_PAYLOADS", false) allowUnsignedLocal := mustParseBool("MAGPIE_ALLOW_UNSIGNED_PAYLOADS", false)
@ -297,17 +302,22 @@ func parsePublicPrefixes(raw string) map[string]bool {
if value == "none" || value == "false" || value == "0" { if value == "none" || value == "false" || value == "0" {
return map[string]bool{} return map[string]bool{}
} }
if strings.TrimSpace(raw) != "" { if strings.TrimSpace(raw) == "" {
return map[string]bool{}
}
prefixes := parseSet(raw) prefixes := parseSet(raw)
for prefix := range prefixes { for prefix := range prefixes {
if err := ValidateKey(strings.TrimSuffix(prefix, "/")); err != nil || !strings.Contains(strings.Trim(prefix, "/"), "/") { cleaned := strings.Trim(prefix, "/")
log.Fatal("MAGPIE_PUBLIC_PREFIXES entries must be bucket/prefix values") 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 return prefixes
} }
return map[string]bool{}
}
func printUsage() { func printUsage() {
fmt.Println("usage: magpie [serve|reindex|stats|scrub|repair|version|backup|restore|smoke]") fmt.Println("usage: magpie [serve|reindex|stats|scrub|repair|version|backup|restore|smoke]")

View file

@ -334,8 +334,15 @@ func (s *Server) bucketAllowed(bucket string) bool {
func (s *Server) publicReadAllowed(key string) bool { func (s *Server) publicReadAllowed(key string) bool {
for prefix := range s.cfg.PublicPrefixes { 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 !strings.Contains(prefix, "/") {
if key == prefix || strings.HasPrefix(key, prefix+"/") {
return true
}
continue continue
} }
if key == prefix || strings.HasPrefix(key, prefix+"/") { if key == prefix || strings.HasPrefix(key, prefix+"/") {

View file

@ -813,7 +813,7 @@ func TestS3PublicPrefixReadCanBeEnabled(t *testing.T) {
cfg := s3TestConfig() cfg := s3TestConfig()
cfg.AllowedBuckets = map[string]bool{"app-uploads": true} 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)) srv := httptest.NewServer(NewServer(store, cfg))
defer srv.Close() defer srv.Close()
@ -833,6 +833,31 @@ func TestS3PublicPrefixReadCanBeEnabled(t *testing.T) {
_ = readResponseBody(t, privateResp, http.StatusForbidden) _ = 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) { func TestS3PublicReadHardensActiveContent(t *testing.T) {
store, err := NewFileStore(t.TempDir()) store, err := NewFileStore(t.TempDir())
if err != nil { if err != nil {