fix(webapp): stop API auth failures leaking the controller's raw error string#3848
fix(webapp): stop API auth failures leaking the controller's raw error string#3848d-cs wants to merge 1 commit into
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
WalkthroughThis PR centralizes auth-failure sanitization and applies it to API bearer and PAT auth paths. A new module defines AuthFailureStatus (401|403), publicAuthError(status) mapping, and sanitizeAuthFailure(failure). apiBuilder.server.ts imports and uses sanitizeAuthFailure to log raw errors server-side and return only status-derived error messages to clients. Tests (unit and E2E) verify sanitized messages, and a server-changes note documents the update. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
…r string
Auth controllers (the OSS RBAC fallback and the cloud RBAC plugin) return an
`error` string on failure that the apiBuilder forwarded verbatim into the
response body. A controller can conflate an infrastructure failure with an
auth rejection — when the database is unreachable the plugin's key lookup
throws a Prisma error ("Can't reach database server at <prod RDS hostname>")
and returns it as the auth error. The SDK then surfaced that string in the
customer's run view via TriggerApiError, leaking internal infra detail.
This evaded the two prior leak fixes because both were scoped to exceptions
on 5xx responses; this leak is a returned value on a 4xx through the auth
channel.
Sanitize at the single auth chokepoint: derive the client message purely
from the status (401 -> "Invalid credentials", 403 -> "Forbidden") and log
the controller's raw string server-side. Applied to both the bearer bridge
and the PAT path. Status codes, body shape, and machine-readable fields are
unchanged; only the human-readable message text changes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
21c2ba2 to
2bd1bf6
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/plugins
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
Summary
API auth failures returned the auth controller's raw
errorstring verbatim in the response body. That string isn't guaranteed to be client-safe — a controller can surface an internal infrastructure error (e.g. a database connection failure) as the auth error, which then reaches the API client and is echoed by the SDK into the run view. This removes that channel: the client message is now derived purely from the HTTP status, and the controller's original string is logged server-side only.Design
The fix sits at the single auth chokepoint every authenticated API request funnels through — the apiBuilder bearer bridge and the PAT path. Both now route failures through
sanitizeAuthFailure, which maps the status to a fixed client-safe message (401 → "Invalid credentials",403 → "Forbidden") and drops the controller's prose. Status codes, response body shape, and machine-readable fields are unchanged; only the human-readable message text changes. Granular auth hints (e.g. preview-branch header guidance) are flattened as a deliberate trade-off — full detail stays in server logs.This is provenance-based rather than content-based: it holds regardless of what any current or future auth controller returns, so the same class of leak can't reappear through a different message string. It closes a gap left by the earlier 5xx/exception-focused passes in #3536 and #3664, which couldn't reach a value returned on a 4xx through the auth channel.
Test plan
api-auth,auth-api,auth-cross-cutting)