fix(rbac): gate Add Purchases button and createPlannedPurchases on update:purchases

frontend/src/__tests__/history-retry-button.test.ts

@@ -82,7 +82,19 @@ import { ADMINISTRATORS_GROUP_ID, PURCHASER_GROUP_ID } from '../permissions';
 // existing admins on first deploy of issue #923). retry-any:purchases is
 // carved out of admin:* and requires Purchaser group membership.
 const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMINISTRATORS_GROUP_ID, PURCHASER_GROUP_ID] };
-const REG_USER = { id: 'user-uuid', email: 'user@example.com', groups: [] };
+// REG_USER carries retry-own:purchases in effectivePermissions so canAccess
+// returns the correct value. The old shape (groups: [], no effectivePermissions)
+// silently allowed the creator check to bypass the permission check -- exactly
+// the bug canRetryFailedRow now fixes (issue #1418).
+const REG_USER = {
+  id: 'user-uuid',
+  email: 'user@example.com',
+  groups: [],
+  effectivePermissions: [
+    { action: 'retry-own', resource: 'purchases' },
+    { action: 'cancel-own', resource: 'purchases' },
+  ],
+};
 // Admin WITHOUT Purchaser membership. retry-any:purchases is carved
 // out of admin:* by issue #923, so this user MUST NOT see Retry on
 // rows they did not create. Regression guard for CR #924 F5 -- if a
@@ -473,3 +485,56 @@ describe('History inline Retry button (issue #47)', () => {
     expect(ids).toEqual(['fail-mine']);
   });
 });
+
+// Issue #1418 regression: user without retry-own:purchases sees no Retry button
+// even on rows they created (closes the same pattern fixed for approve-own by
+// PR #1422 / issue #1407).
+describe('History retry-own permission gate (issue #1418)', () => {
+  beforeEach(() => {
+    setupDOM();
+    jest.clearAllMocks();
+  });
+
+  test('user without retry-own:purchases sees no Retry button on own failed row', async () => {
+    // A Plan Authors-style user: holds plan verbs only, no purchase retry perm.
+    const planAuthorUser = {
+      id: 'plan-author-uuid',
+      email: 'plan-author@example.com',
+      groups: [],
+      effectivePermissions: [
+        { action: 'create', resource: 'plans' },
+        { action: 'update', resource: 'plans' },
+        { action: 'delete', resource: 'plans' },
+      ],
+    };
+    (getCurrentUser as jest.Mock).mockReturnValue(planAuthorUser);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [
+        makeRow({ purchase_id: 'own-fail', created_by_user_id: planAuthorUser.id }),
+      ],
+    });
+
+    await loadHistory();
+
+    // Fail-before: without the retry-own check the button appeared; now it must not.
+    expect(document.querySelectorAll('.history-retry-btn')).toHaveLength(0);
+  });
+
+  test('user with retry-own:purchases sees Retry on own failed rows', async () => {
+    (getCurrentUser as jest.Mock).mockReturnValue(REG_USER);
+    (api.getHistory as jest.Mock).mockResolvedValue({
+      summary: {},
+      purchases: [
+        makeRow({ purchase_id: 'own-fail', created_by_user_id: REG_USER.id }),
+        makeRow({ purchase_id: 'other-fail', created_by_user_id: OTHER_UUID }),
+      ],
+    });
+
+    await loadHistory();
+
+    const buttons = document.querySelectorAll<HTMLButtonElement>('.history-retry-btn');
+    const ids = Array.from(buttons).map((b) => b.dataset['retryId']);
+    expect(ids).toEqual(['own-fail']);
+  });
+});

frontend/src/__tests__/plans-permissions.test.ts

@@ -226,3 +226,64 @@ describe('Plans page permission gating (issue #365)', () => {
     });
   });
 });
+
+// Issue #1406 / #1418: "Add Purchases" button on a plan card must be gated on
+// update:purchases in addition to update:plans. Plan Authors hold the plan verbs
+// but not the purchase verb -- they must NOT see the Add Purchases button.
+describe('Plans page Add Purchases permission gate (issue #1406 / #1418)', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    setupDom();
+    (api.getPlans as jest.Mock).mockResolvedValue({ plans: [samplePlan] });
+    (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });
+  });
+
+  test('Plan Authors (update:plans only, no update:purchases) see Edit Plan but not Add Purchases', async () => {
+    // Simulate a Plan Authors group: plan management verbs, no purchase verbs.
+    (state.getCurrentUser as jest.Mock).mockReturnValue({
+      id: 'plan-author-id',
+      email: 'plan-author@example.com',
+      groups: [],
+      effectivePermissions: [
+        { action: 'create', resource: 'plans' },
+        { action: 'update', resource: 'plans' },
+        { action: 'delete', resource: 'plans' },
+        { action: 'view', resource: 'plans' },
+        { action: 'view', resource: 'purchases' },
+      ],
+    });
+
+    await loadPlans();
+    const html = (document.getElementById('plans-list') as HTMLElement).innerHTML;
+
+    // Fail-before: Add Purchases was shown; pass-after: it must be absent.
+    expect(html).not.toContain('data-action="add-purchases"');
+    // Edit Plan is still shown (update:plans is sufficient for it).
+    expect(html).toContain('data-action="edit-plan"');
+    // Delete Plan is still shown (delete:plans is sufficient).
+    expect(html).toContain('data-action="delete-plan"');
+  });
+
+  test('Standard User (update:plans + update:purchases) sees both Edit Plan and Add Purchases', async () => {
+    (state.getCurrentUser as jest.Mock).mockReturnValue({
+      id: 'std-user-id',
+      email: 'std-user@example.com',
+      groups: [],
+      effectivePermissions: [
+        { action: 'create', resource: 'plans' },
+        { action: 'update', resource: 'plans' },
+        { action: 'delete', resource: 'plans' },
+        { action: 'update', resource: 'purchases' },
+        { action: 'view', resource: 'plans' },
+        { action: 'view', resource: 'purchases' },
+      ],
+    });
+
+    await loadPlans();
+    const html = (document.getElementById('plans-list') as HTMLElement).innerHTML;
+
+    expect(html).toContain('data-action="add-purchases"');
+    expect(html).toContain('data-action="edit-plan"');
+    expect(html).toContain('data-action="delete-plan"');
+  });
+});

frontend/src/history.ts

@@ -535,6 +535,14 @@ function canRetryFailedRow(p: HistoryPurchase): boolean {
   // verb directly so a non-seeded role with the same grant still
   // retries rows the backend would also let through.
   if (canAccess('retry-any', 'purchases')) return true;
+  // Mirror canCancelPendingRow / canApprovePendingRow (issue #1418): require
+  // the retry-own permission explicitly before checking creator match.
+  // Without this gate a role that holds NO retry permission at all (e.g. a
+  // Plan Authors group with only plan verbs) would still see the Retry button
+  // on rows they created, because the creator check alone was a sufficient
+  // condition. The backend authorizeSessionRetry is the real security boundary;
+  // this closes the UX gap.
+  if (!canAccess('retry-own', 'purchases')) return false;
   if (!p.created_by_user_id) return false;
   return p.created_by_user_id === user.id;
 }

frontend/src/plans.ts

@@ -942,7 +942,7 @@ async function loadPlanAccountNames(planId: string, cardEl: Element): Promise<vo
 // actions that require an account (Add Purchases, Edit) are suppressed
 // — only History and Delete remain — and a read-only "Unassigned" badge
 // is shown so operators can identify and re-scope the plan.
-function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan: boolean): string {
+function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan: boolean, canAddPurchases: boolean): string {
   const info = extractPlanInfo(plan);
   const status = getStatusBadge(plan.enabled, plan.auto_purchase);
   const rampSchedule = plan.ramp_schedule || { type: 'immediate', current_step: 0, total_steps: 1 };
@@ -1006,7 +1006,7 @@ function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan
           ` : ''}
         </div>
         <div class="plan-actions">
-          ${canManagePlan && !isUnassigned ? `<button data-action="add-purchases" data-id="${escapeHtmlAttr(plan.id)}" data-name="${escapeHtmlAttr(plan.name)}" class="primary">Add Purchases</button>` : ''}
+          ${canAddPurchases && !isUnassigned ? `<button data-action="add-purchases" data-id="${escapeHtmlAttr(plan.id)}" data-name="${escapeHtmlAttr(plan.name)}" class="primary">Add Purchases</button>` : ''}
           ${canManagePlan && !isUnassigned ? `<button data-action="edit-plan" data-id="${escapeHtmlAttr(plan.id)}">Edit</button>` : ''}
           <button data-action="view-history" data-id="${escapeHtmlAttr(plan.id)}" class="secondary">History</button>
           ${canDeletePlan ? `<button data-action="delete-plan" data-id="${escapeHtmlAttr(plan.id)}" class="danger">Delete</button>` : ''}
@@ -1030,6 +1030,11 @@ function renderPlans(plans: LocalPlan[]): void {
   // 600 times. Action buttons hidden for sessions that lack the verb.
   const canManagePlan = canAccess('update', 'plans');
   const canDeletePlan = canAccess('delete', 'plans');
+  // Issue #1406 / #1418: "Add Purchases" requires BOTH plan-management AND
+  // purchase-write permission. Plan Authors hold update:plans but not
+  // update:purchases; Standard Users hold both. Gate the button on the
+  // conjunction so Plan Authors cannot schedule purchases.
+  const canAddPurchases = canManagePlan && canAccess('update', 'purchases');
 
   // Issue #973: split plans into assigned (have plan_accounts rows) and
   // unassigned (legacy plans with zero plan_accounts rows, flagged by the
@@ -1039,13 +1044,13 @@ function renderPlans(plans: LocalPlan[]): void {
   const unassignedPlans = plans.filter(p => (p as unknown as BackendPlan).unassigned);
 
   const assignedHtml = assignedPlans.map(rawPlan =>
-    renderPlanCard(rawPlan as unknown as BackendPlan, canManagePlan, canDeletePlan)
+    renderPlanCard(rawPlan as unknown as BackendPlan, canManagePlan, canDeletePlan, canAddPurchases)
   ).join('');
 
   let unassignedHtml = '';
   if (unassignedPlans.length > 0) {
     const cards = unassignedPlans.map(rawPlan =>
-      renderPlanCard(rawPlan as unknown as BackendPlan, canManagePlan, canDeletePlan)
+      renderPlanCard(rawPlan as unknown as BackendPlan, canManagePlan, canDeletePlan, canAddPurchases)
     ).join('');
     unassignedHtml = `
       <div class="plans-section-header unassigned-plans-header">

internal/api/handler_plans.go

@@ -269,7 +269,12 @@ func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.La
 		return nil, err
 	}
 
-	session, err := h.requirePermission(ctx, httpReq, "create", "plans")
+	// Require update:purchases — creating scheduled purchase rows is a
+	// purchase-write operation. Standard Users (DefaultUserPermissions)
+	// hold this verb; Plan Authors (create/update/delete:plans only)
+	// do not, closing issue #1406 / #1418. The prior gate ("create","plans")
+	// incorrectly admitted Plan Authors because they hold create:plans.
+	session, err := h.requirePermission(ctx, httpReq, "update", "purchases")
 	if err != nil {
 		return nil, err
 	}

internal/api/router_660_permission_flips_test.go

@@ -185,6 +185,60 @@ func TestUpdatePlan_PermissionGate(t *testing.T) {
 	})
 }
 
+// TestCreatePlannedPurchases_PermissionGate covers
+// POST /api/plans/{id}/purchases (issue #1406 / #1418).
+//
+// The handler was previously gated on create:plans, which incorrectly admitted
+// Plan Authors (create/update/delete:plans only, no purchase verbs). The fix
+// changes the gate to update:purchases: Standard Users hold it; Plan Authors do
+// not. Regression test: user without update:purchases is rejected with 403.
+func TestCreatePlannedPurchases_PermissionGate(t *testing.T) {
+	ctx := context.Background()
+	const userID = "44444444-4444-4444-4444-444444444444"
+	const planID = "dddddddd-dddd-dddd-dddd-dddddddddddd"
+	// Minimal valid body: count in [1,52] and a parseable YYYY-MM-DD date.
+	const body = `{"count":1,"start_date":"2026-07-01"}`
+
+	t.Run("user with update:purchases can add planned purchases", func(t *testing.T) {
+		mockAuth := authForUserWith(ctx, t, userID, "update", "purchases", true)
+		mockAuth.On("GetAllowedAccountsAPI", ctx, userID).Return([]string{}, nil)
+		mockStore := new(MockConfigStore)
+		// After the permission + plan-access gates pass, the handler fetches the
+		// plan. Returning nil, nil yields a 404 ClientError — a domain error that
+		// proves the permission gate was cleared without wiring up the full write path.
+		mockStore.GetPurchasePlanFn = func(_ context.Context, _ string) (*config.PurchasePlan, error) {
+			return nil, nil
+		}
+
+		h := &Handler{auth: mockAuth, config: mockStore}
+		_, err := h.createPlannedPurchases(ctx, reqWithBearerAndBody("user-token", body), planID)
+		assertNotForbidden(t, err)
+	})
+
+	t.Run("user without update:purchases (Plan Authors) is rejected with 403", func(t *testing.T) {
+		// This is the regression case for issue #1406: a Plan Authors user holds
+		// create/update/delete:plans but not update:purchases. Before the fix the
+		// gate was create:plans (which Plan Authors pass); after the fix it is
+		// update:purchases (which Plan Authors do not hold).
+		mockAuth := authForUserWith(ctx, t, userID, "update", "purchases", false)
+		h := &Handler{auth: mockAuth, config: new(MockConfigStore)}
+		_, err := h.createPlannedPurchases(ctx, reqWithBearerAndBody("user-token", body), planID)
+		assert403(t, err)
+	})
+
+	t.Run("admin bypasses permission check", func(t *testing.T) {
+		mockAuth := authForAdmin(ctx, t)
+		mockStore := new(MockConfigStore)
+		mockStore.GetPurchasePlanFn = func(_ context.Context, _ string) (*config.PurchasePlan, error) {
+			return nil, nil
+		}
+
+		h := &Handler{auth: mockAuth, config: mockStore}
+		_, err := h.createPlannedPurchases(ctx, reqWithBearerAndBody("admin-token", body), planID)
+		assertNotForbidden(t, err)
+	})
+}
+
 // ---- Planned Purchases ----------------------------------------------------
 
 // TestPausePlannedPurchase_PermissionGate covers POST /api/purchases/planned/{id}/pause.