feat(swap-service): lock down service behind shared API key#43
Conversation
Add a shared ApiKeyGuard in @shapeshift/shared-utils and register it globally on swap-service via APP_GUARD. Every Nest route now requires an x-api-key header matching SERVICE_API_KEY (constant-time compare, fails closed, throws at boot if unset). /health is a raw Express route and stays open. Internal callers carry the key automatically via the shared service clients. user-service's SwapServiceClient (referral-fees) now authenticates to swap-service, so user-service also needs SERVICE_API_KEY set. user-service and notifications-service are not guarded yet (deferred); their guard wiring was removed from this change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 19 minutes and 23 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds shared API-key enforcement, requires ChangesShared API-key enforcement
Sequence Diagram(s)sequenceDiagram
participant "UserServiceClient"
participant "swap-service"
participant "ApiKeyGuard"
"UserServiceClient"->>"swap-service": axios request with x-api-key header
"swap-service"->>"ApiKeyGuard": canActivate(context)
"ApiKeyGuard"->>"ApiKeyGuard": compare SERVICE_API_KEY to x-api-key
"ApiKeyGuard"-->>"swap-service": allow request or throw UnauthorizedException
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/swap-service/src/app.module.ts`:
- Around line 12-14: The global ApiKeyGuard is being applied unconditionally
through APP_GUARD in AppModule, which will break existing routes before the
caller rollout lands. Update the AppModule provider setup to gate the APP_GUARD
registration behind a rollout flag, using the existing ApiKeyGuard and AppModule
symbols to keep the guard off until the dependent caller changes are deployed
together. If you choose not to gate it in code, ensure the guard is only enabled
in the same deployment unit as the caller updates.
In `@packages/shared-utils/src/service-clients.ts`:
- Line 18: The shared API key is being forwarded from the client factories in
service-clients.ts to user-service and notifications-service even though those
services are not using auth yet. Update the client setup so the API_KEY_HEADER
is only attached in SwapServiceClient, or make keyed auth explicitly opt-in per
client in the relevant factory methods, and keep the
user-service/notifications-service clients free of SERVICE_API_KEY.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77dbcf95-0664-4e4d-a124-ab69c14feee1
📒 Files selected for processing (7)
apps/swap-service/.env.exampleapps/swap-service/src/app.module.tsapps/swap-service/src/env.tsapps/user-service/.env.examplepackages/shared-utils/src/api-key.guard.tspackages/shared-utils/src/index.tspackages/shared-utils/src/service-clients.ts
Per review: UserServiceClient and NotificationsServiceClient were sending SERVICE_API_KEY to user-service and notifications-service, which aren't guarded in this change. That needlessly widens the secret's blast radius. Keep the x-api-key header on SwapServiceClient only — the sole client that calls the API-key-guarded swap-service. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ients Import ordering, single-line guard clause, and alphabetized export — no functional change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Description
Locks swap-service behind a shared service API key so it can only be called by trusted internal callers (public-api, user-service), closing the current state where every swap-service route — including reads of any user's swap history and affiliate fee data — is world-open.
ApiKeyGuardto@shapeshift/shared-utilsand registers it globally on swap-service viaAPP_GUARD. Every Nest route now requires anx-api-keyheader matchingSERVICE_API_KEY(constant-time compare, fails closed, throws at boot if unset).GET /healthis a raw Express route and stays open by design.UserServiceClient/SwapServiceClient/NotificationsServiceClient) now send the key, so existing service-to-service calls keep working. In particular, user-service'sSwapServiceClient(referral-fees) authenticates to the now-locked swap-service — so user-service also needsSERVICE_API_KEYset.SERVICE_API_KEYadded to swap-service's env schema and documented in the.env.examplefiles.Scope: swap-service only. user-service and notifications-service are intentionally not guarded here (deferred).
Companion PR (web): public-api becomes the keyed gateway and the browser stops calling swap-service directly — required for this to deploy without breaking the app. Must ship together.
Deploy requirements
SERVICE_API_KEYset on swap-service and user-service (same value).SWAP_SERVICE_API_KEYmust match (companion PR).Not addressed (deferred)
This locks the network, not attribution. The client-forged
/swapsattribution hole is only half-closed (the direct browser write path dies); the durable fix is the quote-linked{quoteId, txid}registration migration.Testing
SERVICE_API_KEYset, requests without a matchingx-api-keyget401; requests with it succeed.GET /healthworks without a key.SERVICE_API_KEYis unset.referral-fees(which calls swap-service) still works with the key configured.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation