s3: keep dedicated object-lock actions pinned during action resolution (#11475)

* s3: keep dedicated object-lock actions pinned during action resolution

A coarse action that already names a dedicated operation (governance
bypass, retention, legal hold, bucket object-lock config) now resolves to
itself before request shape is consulted. Previously a synthetic
DELETE ?versionId authorization request re-resolved to
s3:DeleteObjectVersion, so the bypass check was satisfied by the
delete-version grant alone; with the pin it evaluates
s3:BypassGovernanceRetention as intended.

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

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

* s3: cover pinned object-lock actions against competing query params

Locks in the resolution for every dedicated action in the pin set, incl.
the retention and legal-hold shapes carrying versionId.

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

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

---------

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

weed/s3api/s3_action_resolver.go

@@ -33,6 +33,22 @@ func ResolveS3Action(r *http.Request, baseAction string, bucket string, object s
 		return baseAction
 	}
 
+	// Dedicated object-lock actions already name the operation — a query
+	// parameter must not re-map them. In particular the governance-bypass
+	// check authorizes against a synthetic DELETE ?versionId request, whose
+	// shape would otherwise resolve to s3:DeleteObjectVersion and satisfy
+	// the bypass check with the delete-version permission alone.
+	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

@@ -212,6 +212,47 @@ func TestResolveS3Action_Quota(t *testing.T) {
 	}
 }
 
+// Dedicated object-lock actions already name the operation being authorized;
+// competing query parameters must not re-map them. Notably the synthetic
+// DELETE ?versionId request behind the governance-bypass check must stay
+// s3:BypassGovernanceRetention rather than resolving to s3:DeleteObjectVersion.
+func TestResolveS3ActionDedicatedObjectLockActions(t *testing.T) {
+	tests := []struct {
+		name       string
+		method     string
+		object     string
+		query      string
+		baseAction string
+		want       string
+	}{
+		{"bypass on versioned delete shape", http.MethodDelete, "key", "versionId=abc123",
+			s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, s3_constants.S3_ACTION_BYPASS_GOVERNANCE},
+		{"bypass on batch delete shape", http.MethodPost, "", "delete",
+			s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, s3_constants.S3_ACTION_BYPASS_GOVERNANCE},
+		{"get retention with versionId", http.MethodGet, "key", "retention&versionId=abc123",
+			s3_constants.ACTION_GET_OBJECT_RETENTION, s3_constants.S3_ACTION_GET_OBJECT_RETENTION},
+		{"put retention with versionId", http.MethodPut, "key", "retention&versionId=abc123",
+			s3_constants.ACTION_PUT_OBJECT_RETENTION, s3_constants.S3_ACTION_PUT_OBJECT_RETENTION},
+		{"get legal hold with versionId", http.MethodGet, "key", "legal-hold&versionId=abc123",
+			s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD, s3_constants.S3_ACTION_GET_OBJECT_LEGAL_HOLD},
+		{"put legal hold with versionId", http.MethodPut, "key", "legal-hold&versionId=abc123",
+			s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD, s3_constants.S3_ACTION_PUT_OBJECT_LEGAL_HOLD},
+		{"get object-lock config", http.MethodGet, "", "object-lock&versioning",
+			s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG, s3_constants.S3_ACTION_GET_BUCKET_OBJECT_LOCK},
+		{"put object-lock config", http.MethodPut, "", "object-lock",
+			s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG, s3_constants.S3_ACTION_PUT_BUCKET_OBJECT_LOCK},
+	}
+
+	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)
+			}
+		})
+	}
+}
+
 // A base action naming another service carries no S3 request shape, so a query
 // parameter on the request must not redirect it to an S3 action.
 func TestResolveS3ActionKeepsNonS3Service(t *testing.T) {

weed/s3api/s3api_object_lock_actions_authz_test.go

@@ -55,6 +55,39 @@ func TestGovernanceBypassUsesDedicatedAuthorization(t *testing.T) {
 	}
 }
 
+// The bypass check authorizes against a synthetic request shaped like the
+// real one; DELETE ?versionId used to re-resolve to s3:DeleteObjectVersion,
+// so the delete-version grant silently satisfied the bypass check.
+func TestGovernanceBypassDoesNotInheritDeleteObjectVersion(t *testing.T) {
+	iam := &IdentityAccessManagement{}
+	require.NoError(t, iam.PutPolicy("DeleteVersions",
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:DeleteObject","s3:DeleteObjectVersion"],"Resource":"arn:aws:s3:::worm/*"}]}`))
+	require.NoError(t, iam.PutPolicy("BypassGovernance",
+		`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:DeleteObject","s3:DeleteObjectVersion","s3:BypassGovernanceRetention"],"Resource":"arn:aws:s3:::worm/*"}]}`))
+	s3a := &S3ApiServer{iam: iam}
+
+	req := httptest.NewRequest(http.MethodDelete, "http://localhost:8333/worm/doc.txt?versionId=abc123", nil)
+	req = mux.SetURLVars(req, map[string]string{"bucket": "worm", "object": "doc.txt"})
+
+	req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), &Identity{
+		Name:        "deleter",
+		PolicyNames: []string{"DeleteVersions"},
+		Account:     &Account{Id: "test-account"},
+	}))
+	if s3a.checkGovernanceBypassPermission(req, "worm", "/doc.txt") {
+		t.Fatal("s3:DeleteObjectVersion alone satisfied the governance bypass check")
+	}
+
+	req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), &Identity{
+		Name:        "breaker",
+		PolicyNames: []string{"BypassGovernance"},
+		Account:     &Account{Id: "test-account"},
+	}))
+	if !s3a.checkGovernanceBypassPermission(req, "worm", "/doc.txt") {
+		t.Fatal("s3:BypassGovernanceRetention did not satisfy the governance bypass check")
+	}
+}
+
 func TestGovernanceBypassUsesBodyObjectKey(t *testing.T) {
 	iam := newTestIAM()
 	iam.identities[0].Actions = []Action{