nevzheng opened a new issue, #13107: URL: https://github.com/apache/gravitino/issues/13107
### What would you like to be improved? Error responses put raw exception text in the `message` field, and there is no policy for which error text is safe to send to clients. When an exception message carries server-private details such as hostnames, file paths, connection strings, or internal class names, that text reaches the caller, including unauthenticated callers on the authentication path. #12728 / #13057 made the `stack` field configurable (`gravitino.server.webserver.includeErrorStackTrace`), but that setting does not cover `message`, which #12728 treats as client-facing. This came up in review of #13057: https://github.com/apache/gravitino/pull/13057#discussion_r3976730053 This needs further thought before changing anything. `message` is also what clients show to users, so blanket redaction would hide useful errors (for example, "Catalog X not found"). The desired outcome is an agreed policy for which error text may reach clients, applied consistently across the main server, Iceberg REST, and Lance REST. Initial findings from an AI-assisted review of #13057, limited to the error-response paths of the main server, Iceberg REST and Lance REST. This is a starting inventory, not a full audit: - **REST operations:** `ExceptionHandlers.BaseExceptionHandler` builds 500 messages as `"Failed to operate … reason [" + getErrorMsg(e) + "]"`, where `getErrorMsg` returns the exception message. - **Authentication:** `AuthenticationFilter.sendAuthErrorResponse` returns `exception.getMessage()` for unexpected 500s. `LanceAuthenticationFilter` already returns a generic `"Authentication failed"` instead. - **Uncaught `java.lang.Error`:** `ErrorExceptionMapper` returns `"Server error while processing request: " + error` (added in #12976, which asserts this message in tests). - **Iceberg REST:** `IcebergRESTUtils.errorResponse` sets `message` from `ex.getMessage()`. ### How should we improve? Open questions for triage: - Should raw text be allowed only for errors whose messages Gravitino writes itself (typically 4xx), while unexpected 5xx return a generic message and keep the detail in server logs? #13057 now logs unexpected authentication failures server-side. - Should this be its own setting, or tied to `includeErrorStackTrace`? - How do clients that currently display `message` stay useful? Related: #12728, #13057, and discussion #11982 (separating public and internal error details). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
