service_oss: ACL applied only on bucket create — pre-provisioned OSS buckets render 403 (same shape as #77)

[tongjichao]

Summary

modules/file/service_oss.go::UploadFile only calls client.CreateBucket(bucketName, oss.ACL(oss.ACLPublicRead)) when the bucket does not yet exist (bucket == nil). For deployments where the OSS bucket is pre-provisioned by an ops tool (Terraform, console, ossutil) with a non-public ACL, the server never reapplies ACLPublicRead, so anonymous browser GETs against the asset URL return 403.

This is the OSS-backend counterpart to #77 (MinIO equivalent), found while reviewing the fix for #77.

Root cause

// modules/file/service_oss.go:47-60
bucket, err := client.Bucket(bucketName)
if err != nil {
    return nil, err
}
if bucket == nil {
    err = client.CreateBucket(bucketName, oss.ACL(oss.ACLPublicRead))
    // ...
}
// pre-existing bucket: never SetBucketACL — bug

The function's implicit contract is "after this returns, the bucket exists AND is publicly readable." That contract is violated whenever the bucket pre-exists.

Suggested fix shape (mirroring #77 fix)

1. Extract an ensureBucket(client, name) helper that:
   • Checks existence,
   • CreateBucket(..., ACLPublicRead) if absent,
   • Unconditionally client.SetBucketACL(name, oss.ACLPublicRead) so a pre-provisioned bucket is self-healed on first upload.
2. Memoize per process to avoid an extra round-trip per upload (the bug: group bucket missing anonymous read policy — group avatars return 403 #77 PR uses sync.Map[string]struct{}).
3. Also call from PresignedPutURL if/when that path is added (parity with MinIO, which bootstraps from both write entry points).

Impact

All OSS deployments where the bucket was created out-of-band without ACLPublicRead — image / file / avatar requests return 403, end-user impact is "broken images."

Out of scope (or in scope for a follow-up?)

• Same class of bug may exist on the other backends: service_cos.go / service_qiniu.go / service_seaweedfs.go do not auto-create buckets at all (operator must pre-provision), so the policy assumption lives outside the process and there's nothing to fix in code. Documenting the operator requirement in the README would still help.

[github-actions]

Thanks for opening this issue! 🐙

We've received it and will triage shortly. We'll follow up here with any updates.

— Octo Team 🐙