adityamparikh commented on issue #213: URL: https://github.com/apache/solr-mcp/issues/213#issuecomment-6318952618
**Spec amendment: unchecked Solr errors leak the Solr URL too** The spec above passes every `RuntimeException` through the aspect unchanged. That leaves the main way a Solr URL reaches clients wide open. SolrJ's `RemoteSolrException` is unchecked, and its message is `Error from server at <url>: <reason>`. Spring AI 2.0.1 sends that message, plus the root cause's message, as the tool error. The URL carries credentials verbatim if they were written into `solr.url`, because `SolrConfigurationProperties` only checks the scheme and host. For example, `get-schema` on a missing collection returns: ``` Error from server at http://localhost:<port>/solr/no_such_collection_xyz/schema: Expected mime type in [application/json, text/plain] but got text/html. … ``` Any tool that lets a `RemoteSolrException` escape does the same: schema, alias, create-collection, and indexing outside the 400 retry. `check-health` also returns the raw `e.getMessage()` in its `SolrHealthStatus`, which is a return value and never passes through the aspect. Amended proposal, now in #273: - **`SolrException` rule in `McpToolExceptionAspect`:** - If the message has no URL and there is no cause, rethrow the same instance. This covers the safe errors `SearchService` builds itself. - Otherwise, log at WARN and throw a cause-less `SolrException` with the same code. Its message has SolrJ's `Error from server at <url>: ` prefix stripped and any other URL replaced with `[redacted]`. - Solr's reason (`undefined field foo`) is kept, because it is what lets the model fix its next call. - **`SolrErrorText.redact`** (`util/`): does the stripping and replacement. `CollectionService.checkHealth` uses it for its error message too. - **Tests:** - `McpToolExceptionAspectTest`: a `RemoteSolrException` becomes Solr's reason with no URL, the same code and no cause; a safe `SolrException` comes back as the same instance. - `SolrErrorTextTest`: the redaction cases. - `McpClientIntegrationTestBase` (HTTP and stdio): `get-schema` on an unknown collection returns an error with no `Error from server` and no `://`. This test fails on the previous #273 head with the text above. - `CollectionServiceTest`: `check-health` redacts a URL with credentials. **New acceptance criterion:** no tool error, and no `check-health` status, contains a Solr URL. Separately, rejecting a username or password inside `solr.url` at startup closes the credentials case at its source, in a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
