Security: Raw backend error messages are displayed to users#536
Security: Raw backend error messages are displayed to users#536tomaioo wants to merge 1 commit intoDialmasterOrg:mainfrom
Conversation
The UI surfaces `error.response?.data?.error` directly in snackbar messages. If backend errors include internal details (stack traces, SQL/infra messages, sensitive identifiers), those details may be exposed to end users. Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
There was a problem hiding this comment.
Thanks for the PR. A few things before this can go in:
The base branch should be dev, not main. All contributor PRs target dev and get promoted from there. Can you retarget?
The frontend tests are failing because usePasswordChange.test.ts still asserts the old behavior (the "Current password is incorrect" case and the 401/403/500 cases). Those need to be updated alongside the code change.
On the fix itself, I'm not sure if it's the right approach. Swallowing every backend message means a user who mistypes their current password just sees "Failed to update password" with no hint what went wrong. Your own PR description points at the better path: map error codes to safe predefined messages. Keep useful feedback like "Current password is incorrect" or "New password must be at least 8 characters," and only fall back to generic when the code is unknown. A blanket generic is a real UX regression here.
Also: the same error.response?.data?.error pattern shows up in a handful of other places (AuthSplash.tsx, VideosPage.tsx, ManualDownload.tsx). If the concern is real, it applies to all of them; if the backend is already returning sanitized messages, this PR isn't needed. Right now the scope is inconsistent.
And really... the root cause is on the backend: error responses shouldn't echo stack traces or infra detail in the first place. Hiding it on the client is just covering something up without really fixing the problem.
Can you retarget to dev, fix the tests, and rework the client side as a code-to-message map? I'll take another look once that's done.
Summary
Security: Raw backend error messages are displayed to users
Problem
Severity:
Low| File:client/src/components/Configuration/hooks/usePasswordChange.ts:L74The UI surfaces
error.response?.data?.errordirectly in snackbar messages. If backend errors include internal details (stack traces, SQL/infra messages, sensitive identifiers), those details may be exposed to end users.Solution
Display generic user-facing errors on the client and log detailed diagnostics only on the server. Consider mapping backend error codes to safe, predefined UI messages.
Changes
client/src/components/Configuration/hooks/usePasswordChange.ts(modified)