Skip to content

Security: Raw backend error messages are displayed to users#536

Open
tomaioo wants to merge 1 commit intoDialmasterOrg:mainfrom
tomaioo:fix/security/raw-backend-error-messages-are-displayed
Open

Security: Raw backend error messages are displayed to users#536
tomaioo wants to merge 1 commit intoDialmasterOrg:mainfrom
tomaioo:fix/security/raw-backend-error-messages-are-displayed

Conversation

@tomaioo
Copy link
Copy Markdown

@tomaioo tomaioo commented Apr 15, 2026

Summary

Security: Raw backend error messages are displayed to users

Problem

Severity: Low | File: client/src/components/Configuration/hooks/usePasswordChange.ts:L74

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.

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)

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>
@dialmaster dialmaster self-requested a review April 15, 2026 21:57
Copy link
Copy Markdown
Collaborator

@dialmaster dialmaster left a comment

Choose a reason for hiding this comment

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

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.

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.

2 participants