Skip to content

Do not leak implementation details in internal error messages - #7738

Open
Sovietaced wants to merge 2 commits into
flyteorg:mainfrom
Sovietaced:issue-7736
Open

Do not leak implementation details in internal error messages#7738
Sovietaced wants to merge 2 commits into
flyteorg:mainfrom
Sovietaced:issue-7736

Conversation

@Sovietaced

@Sovietaced Sovietaced commented Jul 31, 2026

Copy link
Copy Markdown
Member

Tracking issue

Closes #7736

Why are the changes needed?

The Flyte backend currently has some behavior that I don't think are ideal.

  1. Internal errors are largely passed through directly to the client which is a security issue.
  2. In many places errors are logged to stdout and then duplicated in the returned error which is an anti pattern
  3. Non connect RPC errors are currently returned as UNKNOWN and directly pass through error details to the user. Ultimately these are internal server issues.

What changes were proposed in this pull request?

Add an error interceptor that..

  1. Logs non-INTERNAL connect RPC errors at WARN level so every error is visible in logs for debugging
  2. Converts non connect RPC errors into INTERNAL and omits error messages to avoid leaking implementation details
  3. Logs INTERNAL errors at ERROR level since this represents a server side issue

Also updates existing RPCs that return internal to return a non connect RPC error with additional context that will get caught by the error interceptor. Note: I could potentially just keep the existing INTERNAL calls and just drop the error details from being passed through in the interceptor logic instead to be more explicit for RPC handler authors.

Also removes toConnectError helper that was used in one area that basically does what the error interceptor does but at each RPC call site.

How was this patch tested?

We use this error interceptor internally.

Labels

Please add one or more of the following labels to categorize your PR:

  • added: For new features.
  • changed: For changes in existing functionality.
  • deprecated: For soon-to-be-removed features.
  • removed: For features being removed.
  • fixed: For any bug fixed.
  • security: In case of vulnerabilities

This is important to improve the readability of release notes.

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Stack

If you do use git town to manage PR Stacks, the stack relevant to this PR
will show below. Otherwise, you can ignore this section.

Docs link

@Sovietaced Sovietaced added the security Issues related to Security improvements label Jul 31, 2026
@Sovietaced
Sovietaced marked this pull request as ready for review July 31, 2026 18:13
@Sovietaced Sovietaced changed the title Do not leak implementation details in error messages Do not leak implementation details in internal error messages Jul 31, 2026
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
Signed-off-by: Jason Parraga <sovietaced@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flyte2 security Issues related to Security improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't leak implementation details in internal errors sent to clients

1 participant