s3: pin object-lock actions so request shape cannot re-derive them

ResolveS3Action derives the authorized action from the request's
method+query, overriding the caller's base action. A governance-bypass
check on DELETE ?versionId=... therefore evaluated s3:DeleteObjectVersion
instead of s3:BypassGovernanceRetention, letting an identity that holds
version-delete but not bypass delete a GOVERNANCE-locked version
(GHSA-j4hf-8r83-7fjj). The same substitution let s3:GetObjectVersion
satisfy GetObjectRetention/GetObjectLegalHold on ?versionId requests,
and silently dropped bucket-policy grants of the bypass action on
versioned deletes.

Object Lock actions name a concrete permission checked on top of another
operation's shape; resolve them to themselves, like iam:/sts: actions.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

weed/s3api/s3_action_resolver.go

@@ -33,6 +33,20 @@ func ResolveS3Action(r *http.Request, baseAction string, bucket string, object s
 		return baseAction
 	}
 
+	// Object Lock actions are checked on top of another operation's request
+	// shape (e.g. BypassGovernanceRetention on a versioned DELETE), so the
+	// shape must not re-derive them.
+	switch baseAction {
+	case s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION,
+		s3_constants.ACTION_GET_OBJECT_RETENTION,
+		s3_constants.ACTION_PUT_OBJECT_RETENTION,
+		s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD,
+		s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD,
+		s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG,
+		s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG:
+		return mapBaseActionToS3Format(baseAction)
+	}
+
 	if r == nil || r.URL == nil {
 		// No HTTP context available: fall back to coarse-grained mapping
 		// This ensures consistent behavior and avoids returning empty strings

weed/s3api/s3_action_resolver_test.go

@@ -114,6 +114,67 @@ func TestResolveS3Action_AttributesBeforeVersionId(t *testing.T) {
 	}
 }
 
+func TestResolveS3Action_ObjectLockActionsIgnoreRequestShape(t *testing.T) {
+	tests := []struct {
+		name       string
+		query      string
+		method     string
+		baseAction string
+		object     string
+		want       string
+	}{
+		{
+			name:       "bypass on versioned delete",
+			query:      "versionId=abc123",
+			method:     http.MethodDelete,
+			baseAction: s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION,
+			object:     "key",
+			want:       s3_constants.S3_ACTION_BYPASS_GOVERNANCE,
+		},
+		{
+			name:       "bypass on batch delete",
+			query:      "delete",
+			method:     http.MethodPost,
+			baseAction: s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION,
+			object:     "key",
+			want:       s3_constants.S3_ACTION_BYPASS_GOVERNANCE,
+		},
+		{
+			name:       "get retention with versionId",
+			query:      "retention&versionId=abc123",
+			method:     http.MethodGet,
+			baseAction: s3_constants.ACTION_GET_OBJECT_RETENTION,
+			object:     "key",
+			want:       s3_constants.S3_ACTION_GET_OBJECT_RETENTION,
+		},
+		{
+			name:       "get legal hold with versionId",
+			query:      "legal-hold&versionId=abc123",
+			method:     http.MethodGet,
+			baseAction: s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD,
+			object:     "key",
+			want:       s3_constants.S3_ACTION_GET_OBJECT_LEGAL_HOLD,
+		},
+		{
+			name:       "put retention with versionId",
+			query:      "retention&versionId=abc123",
+			method:     http.MethodPut,
+			baseAction: s3_constants.ACTION_PUT_OBJECT_RETENTION,
+			object:     "key",
+			want:       s3_constants.S3_ACTION_PUT_OBJECT_RETENTION,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r, _ := http.NewRequest(tt.method, "http://localhost/bucket/"+tt.object+"?"+tt.query, nil)
+			if got := ResolveS3Action(r, tt.baseAction, "bucket", tt.object); got != tt.want {
+				t.Errorf("ResolveS3Action() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
 // Bucket subresources registered with ACTION_ADMIN must resolve to their own
 // S3 actions so a policy granting one of them does not need s3:*, and so no
 // broader grant sweeps them in.

weed/s3api/s3api_object_lock_actions_authz_test.go

@@ -2,6 +2,7 @@ package s3api
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -55,6 +56,38 @@ func TestGovernanceBypassUsesDedicatedAuthorization(t *testing.T) {
 	}
 }
 
+func TestGovernanceBypassChecksBypassActionOnVersionedDelete(t *testing.T) {
+	arn := "arn:aws:s3:::test-bucket/test-object"
+	for _, tc := range []struct {
+		name         string
+		policyAction string
+		allowed      bool
+	}{
+		{"bypass permission", "s3:BypassGovernanceRetention", true},
+		{"delete version only", "s3:DeleteObjectVersion", false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			iam := newTestIAM()
+			iam.identities[0].Actions = nil
+			iam.identities[0].Account = &Account{Id: "test-account"}
+			iam.policyEngine = NewBucketPolicyEngine()
+			policy := fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"%s","Resource":"%s"}]}`,
+				tc.policyAction, arn)
+			require.NoError(t, iam.policyEngine.engine.SetBucketPolicy("test-bucket", policy))
+			s3a := &S3ApiServer{iam: iam}
+
+			req := httptest.NewRequest(http.MethodDelete, "http://localhost:8333/test-bucket/test-object?versionId=v1", nil)
+			req = mux.SetURLVars(req, map[string]string{"bucket": "test-bucket", "object": "test-object"})
+			require.NoError(t, signRawHTTPRequest(context.Background(), req,
+				"AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", "us-east-1"))
+
+			if got := s3a.checkGovernanceBypassPermission(req, "test-bucket", "/test-object"); got != tc.allowed {
+				t.Errorf("checkGovernanceBypassPermission() = %v, want %v", got, tc.allowed)
+			}
+		})
+	}
+}
+
 func TestGovernanceBypassUsesBodyObjectKey(t *testing.T) {
 	iam := newTestIAM()
 	iam.identities[0].Actions = []Action{