Skip to content

fix(security): align requireRole() API with requireOwnership() (#1570)#1571

Merged
OneStepAt4time merged 1 commit intodevelopfrom
fix/requirerole-api-1570
Apr 9, 2026
Merged

fix(security): align requireRole() API with requireOwnership() (#1570)#1571
OneStepAt4time merged 1 commit intodevelopfrom
fix/requirerole-api-1570

Conversation

@OneStepAt4time
Copy link
Copy Markdown
Owner

Summary

  • Refactored requireRole() to send the 403 response itself on denial (matching requireOwnership() contract) instead of returning boolean and relying on callers to chain reply.status(403).send(...)
  • Updated all 3 call sites to the consistent if (!requireRole(req, reply, ...)) return; pattern
  • Added JSDoc on both requireRole() and requireOwnership() documenting the guard contract

Why

The old API was asymmetric: requireOwnership() handled the response itself, but requireRole() forced every caller to manually send the 403. This created a silent auth bypass risk — a future caller could forget the reply chain and the request would proceed unauthorized.

Test plan

  • npx tsc --noEmit — passes
  • npm run build — passes
  • npm test — 2605 passed, 0 failed
  • Verify 403 response body for role-denied requests matches expected shape

Closes #1570

Generated by Hephaestus (Aegis dev agent)

requireRole() now sends the 403 response itself (like requireOwnership)
instead of returning boolean and forcing callers to chain reply.status().
This eliminates the silent auth bypass risk where a caller forgets to send
the response on denial.

Generated by Hephaestus (Aegis dev agent)
@OneStepAt4time
Copy link
Copy Markdown
Owner Author

🔧 PR #1571 ready for review: fix(server): requireRole() API asymmetry with requireOwnership() (NEW-DESIGN-01) (#1570). CI CLEAN. Please review.

Copy link
Copy Markdown
Contributor

@aegis-gh-agent aegis-gh-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved. requireRole() now takes reply param and sends 403 itself, consistent with requireOwnership(). Clean API alignment (#1570).

@OneStepAt4time OneStepAt4time merged commit 22fe9b7 into develop Apr 9, 2026
9 checks passed
@OneStepAt4time OneStepAt4time deleted the fix/requirerole-api-1570 branch April 9, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant