sec(api): refuse createPlan target_accounts outside the caller's allowed_accounts

internal/api/handler_plans.go

@@ -126,7 +126,8 @@ func mapCreatePlanStorageError(err error, notFoundMsg, genericMsg, logFmt string
 
 func (h *Handler) createPlan(ctx context.Context, httpReq *events.LambdaFunctionURLRequest) (any, error) {
 	// Require create:plans permission
-	if _, err := h.requirePermission(ctx, httpReq, "create", "plans"); err != nil {
+	session, err := h.requirePermission(ctx, httpReq, "create", "plans")
+	if err != nil {
 		return nil, err
 	}
 
@@ -145,6 +146,12 @@ func (h *Handler) createPlan(ctx context.Context, httpReq *events.LambdaFunction
 		return nil, err
 	}
 
+	// Refuse before the plan row exists: a scoped caller must not create a
+	// plan that buys for an account outside their allowed_accounts.
+	if err := h.requireAccountsAccess(ctx, session, req.TargetAccounts); err != nil {
+		return nil, err
+	}
+
 	plan := req.toPurchasePlan()
 
 	// Validate the plan

internal/api/plan_accounts_scope_test.go

@@ -2,10 +2,12 @@ package api
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -277,3 +279,81 @@ func TestRouterDispatch_ListPlanAccounts_ScopedCallerFiltered(t *testing.T) {
 	require.Len(t, got, 1)
 	assert.Equal(t, scopedInAccount, got[0].ID)
 }
+
+// ── createPlan: target_accounts ─────────────────────────────────────────────
+
+// seedCreatePlanStore wires the store so POST /api/plans would SUCCEED if the
+// scope guard were removed, and reports whether the plan row was written.
+func seedCreatePlanStore(store *MockConfigStore) (*planAccountsWrite, *bool) {
+	write := seedPlanAccountsStore(store, nil)
+	created := false
+	store.On("CreatePurchasePlan", mock.Anything, mock.AnythingOfType("*config.PurchasePlan")).
+		Run(func(mock.Arguments) { created = true }).Return(nil).Maybe()
+	return write, &created
+}
+
+func createPlanBody(accounts ...string) string {
+	return `{"name":"P","enabled":true,"auto_purchase":true,"provider":"aws","service":"ec2","target_accounts":["` +
+		strings.Join(accounts, `","`) + `"]}`
+}
+
+// A scoped caller must not create a plan (auto-purchase or not) against an
+// account outside their allowed_accounts. The refusal lands before the plan
+// row is written, so there is no orphan to roll back.
+func TestCreatePlan_ScopedCallerCannotTargetOutOfScopeAccount(t *testing.T) {
+	ctx := context.Background()
+	h, store := scopedHandler(t, scopedInAccount)
+	t.Cleanup(func() { store.AssertExpectations(t) })
+	write, created := seedCreatePlanStore(store)
+
+	_, err := h.createPlan(ctx, scopedRequest(createPlanBody(scopedOutAccount)))
+
+	require.Error(t, err)
+	assert.True(t, IsNotFoundError(err), "expected the enumeration-safe not-found refusal, got %v", err)
+	assert.False(t, *created, "CreatePurchasePlan must not run for an out-of-scope target account")
+	assert.False(t, write.called, "SetPlanAccounts must not run for an out-of-scope target account")
+}
+
+func TestCreatePlan_ScopedCallerMixedTargetsRefusedWhole(t *testing.T) {
+	ctx := context.Background()
+	h, store := scopedHandler(t, scopedInAccount)
+	t.Cleanup(func() { store.AssertExpectations(t) })
+	write, created := seedCreatePlanStore(store)
+
+	_, err := h.createPlan(ctx, scopedRequest(createPlanBody(scopedInAccount, scopeThirdAccount)))
+
+	require.Error(t, err)
+	assert.True(t, IsNotFoundError(err), "expected not-found, got %v", err)
+	assert.False(t, *created)
+	assert.False(t, write.called)
+}
+
+// Control: the same scoped caller may still create a plan for an account it holds.
+func TestCreatePlan_ScopedCallerCanTargetInScopeAccount(t *testing.T) {
+	ctx := context.Background()
+	h, store := scopedHandler(t, scopedInAccount)
+	t.Cleanup(func() { store.AssertExpectations(t) })
+	write, created := seedCreatePlanStore(store)
+
+	_, err := h.createPlan(ctx, scopedRequest(createPlanBody(scopedInAccount)))
+
+	require.NoError(t, err)
+	assert.True(t, *created)
+	require.True(t, write.called)
+	assert.Equal(t, []string{scopedInAccount}, write.ids)
+}
+
+// Control: an unrestricted caller may target any account.
+func TestCreatePlan_UnrestrictedCallerUnaffected(t *testing.T) {
+	ctx := context.Background()
+	h, store := scopedHandler(t)
+	t.Cleanup(func() { store.AssertExpectations(t) })
+	write, created := seedCreatePlanStore(store)
+
+	_, err := h.createPlan(ctx, scopedRequest(createPlanBody(scopedOutAccount)))
+
+	require.NoError(t, err)
+	assert.True(t, *created)
+	require.True(t, write.called)
+	assert.Equal(t, []string{scopedOutAccount}, write.ids)
+}

internal/api/scoping.go

@@ -86,7 +86,7 @@ func (h *Handler) requirePlanAccess(ctx context.Context, session *Session, planI
 // decides which accounts that plan buys commitments for, so either half
 // redirects purchasing.
 //
-// The scope is resolved once and unrestricted callers short-circuit BEFORE
+// Unrestricted callers short-circuit in both helpers BEFORE
 // any store lookup, mirroring requireExecutionAccess: an admin/API-key
 // session must not pay for (or need fixtures for) reads it cannot be refused
 // by. Empty and "*" allow-lists are unrestricted at exactly this seam
@@ -99,16 +99,23 @@ func (h *Handler) requirePlanAccess(ctx context.Context, session *Session, planI
 //
 // requirePermission must fire first; pass it the session that returned.
 func (h *Handler) requirePlanAccountsAccess(ctx context.Context, session *Session, planID string, accountIDs []string) error {
+	if err := h.requirePlanAccess(ctx, session, planID); err != nil {
+		return err
+	}
+	return h.requireAccountsAccess(ctx, session, accountIDs)
+}
+
+// requireAccountsAccess is requireAccountAccess over a batch: every account
+// must be in the session's scope or the whole batch is refused with
+// errNotFound. Unrestricted callers return before any store lookup.
+func (h *Handler) requireAccountsAccess(ctx context.Context, session *Session, accountIDs []string) error {
 	allowed, err := h.getAccountScope(ctx, session)
 	if err != nil {
 		return fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
 	if allowed.AllowsAll() {
 		return nil
 	}
-	if err := h.requirePlanAccess(ctx, session, planID); err != nil {
-		return err
-	}
 	for _, aid := range accountIDs {
 		if _, err := h.requireAccountAccess(ctx, session, aid); err != nil {
 			return err