fix: scope tagging policy checks and pin the table exhaustive (#979)

spinifex/gateway/tagging.go

@@ -3,7 +3,6 @@ package gateway
 import (
 	"context"
 	"errors"
-	"io"
 	"log/slog"
 	"net/http"
 	"strings"
@@ -45,12 +44,12 @@ func (gw *GatewayConfig) Tagging_Request(w http.ResponseWriter, r *http.Request)
 		return errors.New(awserrors.ErrorInvalidAction)
 	}
 
-	if err := gw.checkPolicy(r, "tagging", action); err != nil {
+	resources, err := gateway_tagging.ResourceARNs(action)
+	if err != nil {
 		return err
 	}
-
-	if gw.NATSConn == nil {
-		return errors.New(awserrors.ErrorServerInternal)
+	if err := gw.checkPolicyResources(r, "tagging", action, resources); err != nil {
+		return err
 	}
 
 	accountID, _ := r.Context().Value(ctxAccountID).(string)
@@ -59,10 +58,16 @@ func (gw *GatewayConfig) Tagging_Request(w http.ResponseWriter, r *http.Request)
 		return errors.New(awserrors.ErrorServerInternal)
 	}
 
-	body, err := io.ReadAll(r.Body)
+	// Read ahead of the NATS guard so an oversized body is rejected on its own
+	// terms rather than behind a connection that happens to be down.
+	body, err := readBoundedBody(r)
 	if err != nil {
 		slog.Error("Tagging_Request: failed to read body", "err", err)
-		return errors.New(awserrors.ErrorInvalidParameterValue)
+		return err
+	}
+
+	if gw.NATSConn == nil {
+		return errors.New(awserrors.ErrorServerInternal)
 	}
 
 	output, err := handler(r.Context(), gw, accountID, body)

spinifex/gateway/tagging/authz.go

@@ -0,0 +1,67 @@
+package gateway_tagging
+
+import (
+	"errors"
+	"log/slog"
+	"maps"
+	"slices"
+
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+)
+
+// The resource a policy check evaluates against when AWS documents no resource
+// type for the action, or when the identifier cannot be resolved at gate time.
+const anyResource = "*"
+
+// Where an action's resource is named. Only one member today; the type exists
+// so a scoped action reads the same way here as it does in the other services.
+type resourceSource uint8
+
+const (
+	sourceAny resourceSource = iota
+)
+
+// Every action Tagging_Request serves, with the resource AWS evaluates it
+// against. Exhaustive by contract: a completeness test compares this table with
+// the dispatch table in both directions, so an action cannot be added with a
+// silent account-wide grant.
+var taggingScopes = map[string]resourceSource{
+	// Account-level. The Resource Groups Tagging API defines no resource types,
+	// so AWS evaluates every tag: action against "*". GetResources aggregates
+	// the account's EC2 and ELBv2 resources but is not scoped to them.
+	"GetResources": sourceAny,
+}
+
+// HasScope reports whether action has an explicit tagging scope-table entry.
+// taggingActions lives in package gateway, so the completeness test calls this
+// rather than reaching into the table.
+func HasScope(action string) bool {
+	_, ok := taggingScopes[action]
+	return ok
+}
+
+// ScopedActions returns every action represented in the tagging scope table.
+func ScopedActions() []string {
+	return slices.Sorted(maps.Keys(taggingScopes))
+}
+
+// ResourceARNs resolves the resources a tagging request authorizes against.
+// It takes no region, account or body because every action AWS defines for this
+// service is account-level; adding a scoped one means widening this signature,
+// which is a compile error at the dispatcher rather than a silent "*".
+func ResourceARNs(action string) ([]string, error) {
+	source, ok := taggingScopes[action]
+	if !ok {
+		slog.Error("tagging authz: action is served but absent from the scope table", "action", action)
+		return nil, errors.New(awserrors.ErrorInvalidAction)
+	}
+
+	switch source {
+	case sourceAny:
+		return []string{anyResource}, nil
+
+	default:
+		slog.Error("tagging authz: unhandled resource source, failing closed", "action", action, "source", source)
+		return nil, errors.New(awserrors.ErrorInternalError)
+	}
+}

spinifex/gateway/tagging/authz_test.go

@@ -0,0 +1,35 @@
+package gateway_tagging_test
+
+import (
+	"testing"
+
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	gateway_tagging "github.com/mulgadc/spinifex/spinifex/gateway/tagging"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The Resource Groups Tagging API defines no resource types, so every action it
+// serves is evaluated against "*". Scoping GetResources to the resources it
+// happens to return would deny calls AWS permits.
+func TestResourceARNsAreAccountLevel(t *testing.T) {
+	for _, action := range gateway_tagging.ScopedActions() {
+		resources, err := gateway_tagging.ResourceARNs(action)
+		require.NoError(t, err, "action %q", action)
+		assert.Equal(t, []string{"*"}, resources, "action %q", action)
+	}
+}
+
+// An action absent from the dispatch table cannot reach the resolver, but if one
+// ever did it fails closed rather than authorizing account-wide.
+func TestResourceARNsUnknownAction(t *testing.T) {
+	_, err := gateway_tagging.ResourceARNs("BogusAction")
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorInvalidAction, err.Error())
+}
+
+func TestHasScope(t *testing.T) {
+	assert.True(t, gateway_tagging.HasScope("GetResources"))
+	assert.False(t, gateway_tagging.HasScope("TagResources"))
+	assert.Equal(t, []string{"GetResources"}, gateway_tagging.ScopedActions())
+}

spinifex/gateway/tagging_authz_completeness_test.go

@@ -0,0 +1,27 @@
+//test:in-package — taggingActions is unexported here, and the completeness test
+// exists to compare it against the scope table.
+
+package gateway
+
+import (
+	"testing"
+
+	gateway_tagging "github.com/mulgadc/spinifex/spinifex/gateway/tagging"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestTaggingScopeTableIsExhaustive is what stops the next tagging action being
+// added with a silent account-wide grant. It asserts both directions, so a scope
+// left behind by a deleted or renamed action fails too.
+func TestTaggingScopeTableIsExhaustive(t *testing.T) {
+	for action := range taggingActions {
+		assert.True(t, gateway_tagging.HasScope(action),
+			"tagging action %q has no resource scope entry: add one to taggingScopes in gateway/tagging/authz.go", action)
+	}
+
+	for _, action := range gateway_tagging.ScopedActions() {
+		_, ok := taggingActions[action]
+		assert.True(t, ok,
+			"taggingScopes has an entry for %q, which the dispatch table does not serve: remove it from gateway/tagging/authz.go", action)
+	}
+}

spinifex/gateway/tagging_authz_test.go

@@ -0,0 +1,83 @@
+//test:in-package — drives Tagging_Request through the gateway's unexported test
+// helpers (setupTaggingRequest, policyMockIAMService).
+
+package gateway
+
+import (
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/mulgadc/bluebottle/pkg/sigv4"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	gateway_tagging "github.com/mulgadc/spinifex/spinifex/gateway/tagging"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// dispatchTagging drives the gateway with no NATS connection. A permitted
+// request therefore fails at the NATS-availability guard rather than on
+// authorization, which is what proves the policy check ran ahead of the handler.
+func dispatchTagging(t *testing.T, gw *GatewayConfig, action, body string) error {
+	t.Helper()
+	return gw.Tagging_Request(httptest.NewRecorder(), setupTaggingRequest("ResourceGroupsTaggingAPI_20170126."+action, body))
+}
+
+// Every tagging action is account-level, so a grant scoped to a resource ARN
+// does not reach one. This is the whole enforcement surface the service admits:
+// AWS defines no resource types for the Resource Groups Tagging API, so there is
+// no ARN a scoped Deny could name.
+func TestTaggingRequest_ActionsAreAccountLevel(t *testing.T) {
+	scoped := scopedPolicyGateway(
+		statement("Allow", "tagging:GetResources", "arn:aws:ec2:"+authzRegion+":"+authzAccountID+":instance/i-dev"),
+	)
+	assertDenied(t, dispatchTagging(t, scoped, "GetResources", `{}`))
+
+	wide := scopedPolicyGateway(statement("Allow", "tagging:GetResources", "*"))
+	assertPermitted(t, dispatchTagging(t, wide, "GetResources", `{}`))
+}
+
+// A Deny scoped to a resource ARN must not fire: AWS evaluates every tag:
+// action against "*", so denying here would reject a call AWS permits. This is
+// the D2 guard — it fails the moment the dispatcher or the resolver passes any
+// resource beyond "*".
+func TestTaggingRequest_ScopedDenyDoesNotFire(t *testing.T) {
+	gw := scopedPolicyGateway(
+		statement("Allow", "tagging:*", "*"),
+		statement("Deny", "tagging:GetResources", "arn:aws:ec2:"+authzRegion+":"+authzAccountID+":instance/i-prod"),
+	)
+
+	assertPermitted(t, dispatchTagging(t, gw, "GetResources", `{}`))
+}
+
+// The property the per-service rollout rests on: every policy that works today
+// carries Resource "*", and resolving the scope table cannot turn one into a
+// denial.
+func TestTaggingRequest_WildcardPolicyStillPermitsEveryAction(t *testing.T) {
+	gw := scopedPolicyGateway(statement("Allow", "tagging:*", "*"))
+
+	for _, action := range gateway_tagging.ScopedActions() {
+		t.Run(action, func(t *testing.T) {
+			assertPermitted(t, dispatchTagging(t, gw, action, `{}`))
+		})
+	}
+}
+
+// The action half of the evaluator is unchanged: a grant on one action does not
+// carry to another the dispatcher serves.
+func TestTaggingRequest_UnrelatedActionGrantDenies(t *testing.T) {
+	gw := scopedPolicyGateway(statement("Allow", "ec2:DescribeTags", "*"))
+
+	assertDenied(t, dispatchTagging(t, gw, "GetResources", `{}`))
+}
+
+// A body past the signed path's cap cannot be used to exhaust memory: it is
+// rejected before the handler reads it.
+func TestTaggingRequest_OversizedBodyIsRejected(t *testing.T) {
+	gw := scopedPolicyGateway(statement("Allow", "tagging:*", "*"))
+	body := `{"PaginationToken":"` + strings.Repeat("a", sigv4.MaxPayloadLen) + `"}`
+
+	err := dispatchTagging(t, gw, "GetResources", body)
+	require.Error(t, err)
+	assert.Equal(t, awserrors.ErrorRequestEntityTooLarge, err.Error())
+}