Skip to content

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

Merged
cristim merged 1 commit into
mainfrom
fix/1418-action-button-rbac-audit
Jul 16, 2026
Merged

cristim merged 1 commit into
mainfrom
fix/1418-action-button-rbac-audit

Conversation

@cristim

@cristim cristim commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

  • Plans page: The "Add Purchases" button was gated on update:plans alone, allowing Plan Authors (who hold plan verbs but not purchase verbs) to see and trigger purchase scheduling. Now requires both update:plans AND update:purchases (canAddPurchases parameter added to renderPlanCard).
  • Backend: createPlannedPurchases used requirePermission("create","plans"), which Plan Authors pass. Changed to requirePermission("update","purchases") so only purchase-write roles can schedule purchases (real security boundary).
  • History retry button: canRetryFailedRow was missing an explicit retry-own:purchases check before the creator-match branch. A user with no retry permission at all could see Retry on rows they created. Added the same guard used by canCancelPendingRow and canApprovePendingRow.

Closes #1418

Gaps fixed

Gap Where Fix
"Add Purchases" visible to Plan Authors frontend/src/plans.ts canAddPurchases = canManagePlan && canAccess('update','purchases')
createPlannedPurchases accepted create:plans internal/api/handler_plans.go gate changed to update:purchases
Retry button visible to users with no retry permission frontend/src/history.ts added if (!canAccess('retry-own','purchases')) return false before creator match

Test plan

  • TestCreatePlannedPurchases_PermissionGate (3 sub-tests): pass (404 proves gate cleared), 403 for no-permission, admin bypass -- all pass
  • Plans page Add Purchases permission gate: Plan Authors see Edit Plan but not Add Purchases (fail-before/pass-after confirmed)
  • Standard User (update:plans + update:purchases) sees both Edit Plan and Add Purchases
  • History retry-own permission gate: user without retry-own:purchases sees no Retry on own failed row
  • user with retry-own:purchases sees Retry on own failed rows
  • Full frontend test suite: 78 suites / 2572 tests passed (exit 0)
  • npm run build: compiled (exit 0, pre-existing size warning only)
  • go build ./..., go vet ./..., go test ./internal/api/... (1728 tests): exit 0
  • gocyclo -over 10 on touched files: exit 0 (no findings)
  • golangci-lint run ./internal/api/...: exit 0, no issues

…int on update:purchases

Two RBAC gaps allowed Plan Authors (who hold plan verbs only) to either
see action buttons or hit endpoints they should not reach:

1. frontend/src/plans.ts -- "Add Purchases" button was gated on
   canManagePlan (update:plans) alone.  Plan Authors hold update:plans
   but not update:purchases.  Added a new canAddPurchases parameter that
   requires BOTH update:plans AND update:purchases.

2. internal/api/handler_plans.go -- createPlannedPurchases used
   requirePermission("create","plans"), which Plan Authors pass.
   Changed to requirePermission("update","purchases") so only roles
   with the purchase-write verb (Standard Users, Purchasers) can
   schedule purchases.

3. frontend/src/history.ts -- canRetryFailedRow was missing an explicit
   retry-own:purchases check before the creator-match branch.  A user
   holding no retry permission at all could see the Retry button on rows
   they created.  Added the same guard pattern used by canCancelPendingRow.

New backend test: TestCreatePlannedPurchases_PermissionGate (3 sub-tests
covering pass / 403 / admin bypass).  New frontend tests in
plans-permissions.test.ts and history-retry-button.test.ts verify the
button is hidden for Plan Authors and shown for Standard Users.

Closes #1418
@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-sprint Within the current sprint impact/many Affects most users effort/m Days type/bug Defect labels Jul 16, 2026
@cristim

cristim commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@cristim
cristim merged commit cc0fa9e into main Jul 16, 2026
20 checks passed
@cristim

cristim commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

Merged to main after independent adversarial security review (SHIP; backend enforcement + verb-change-per-role-definitions verified, fail-before/pass-after) and all CI green (closes #1418). Gates action buttons on the correct per-resource permissions: Add-Purchases + createPlannedPurchases on update:purchases (Plan Authors correctly excluded), Retry on explicit retry-own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/m Days impact/many Affects most users priority/p2 Backlog-worthy severity/medium Moderate harm triaged Item has been triaged type/bug Defect urgency/this-sprint Within the current sprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(rbac): action buttons (write/delete/approve) not correctly gated by per-resource permissions for all groups (regression after #660/PR #891)

1 participant