diff --git a/AGENTS.md b/AGENTS.md index 811ce1b7..bb485405 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,12 +96,13 @@ docker run -p 8080:8080 --rm -e PROFILES=http \ ### MCP Tools (src/main/java/org/apache/solr/mcp/server/) -Four service classes expose MCP tools via `@McpTool` annotations: +Five service classes expose MCP tools via `@McpTool` annotations: - **SearchService** (`search/`) - Full-text search with filtering, faceting, sorting, pagination - **IndexingService** (`indexing/`) - Document indexing supporting JSON, CSV, XML, and markdown formats - **CollectionService** (`collection/`) - List collections, get stats, health checks - **SchemaService** (`schema/`) - Schema introspection and additive modification (add-fields, add-field-types) +- **UrlIndexingService** (`indexing/`) - `index-url`: fetches an allow-listed http(s) URL (GitHub raw content by default, `SOLR_INDEX_URL_ALLOWED_HOSTS`), capped at `SOLR_INDEX_URL_MAX_BYTES` (10 MB), and hands the body to the same parsers as the inline tools via `IndexingService.indexPayload` ### Document Creators (Strategy Pattern) @@ -401,6 +402,8 @@ Environment variables: - `SOLR_URL`: Solr URL (default: `http://localhost:8983/solr/`) - `PROFILES`: Transport mode (`stdio` or `http`) - `OAUTH2_ISSUER_URI`: OAuth2 issuer URL (HTTP mode only) +- `SOLR_INDEX_URL_ALLOWED_HOSTS`: hosts `index-url` may fetch (default `raw.githubusercontent.com,*.githubusercontent.com,github.com`; `*` = any) +- `SOLR_INDEX_URL_MAX_BYTES`, `SOLR_INDEX_URL_CONNECT_TIMEOUT`, `SOLR_INDEX_URL_READ_TIMEOUT`, `SOLR_INDEX_URL_TOTAL_TIMEOUT`, `SOLR_INDEX_URL_MAX_CONCURRENT_FETCHES`: `index-url` body cap, timeouts and concurrency limit (defaults `10MB`, `10s`, `30s`, `5m`, `4`) Dependencies managed in `gradle/libs.versions.toml`. diff --git a/README.md b/README.md index 0d0c1f7c..675b82df 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ Using a different client, or want STDIO/HTTP/Docker options? See the per-client |------|-------------| | `search` | Full-text search with filtering, faceting, sorting, and pagination | | `index-json-documents` | Index documents from a JSON string into a collection | +| `index-url` | Index a UTF-8 JSON, CSV, XML or Markdown document from an http(s) URL on the allow-list (both transports; default 10 MB limit) | | `index-csv-documents` | Index documents from a CSV string into a collection | | `index-xml-documents` | Index documents from an XML string into a collection | | `index-markdown-documents` | Index a markdown document into a collection, extracting front matter, title, headings, and body text | @@ -110,6 +111,25 @@ Using a different client, or want STDIO/HTTP/Docker options? See the per-client Every tool advertises MCP behavior hints (`readOnlyHint`, `destructiveHint`, `idempotentHint`) so clients can build sensible approval UX — `search` and the metadata tools are read-only, indexing is destructive but idempotent, schema modification is additive. +**Index from a URL:** call `index-url` with +`{"collection":"shows","url":"https://raw.githubusercontent.com/apache/solr-mcp/main/src/test/resources/shows.json"}` +in either transport. The server fetches the URL from its own network with no +credentials or custom headers, so `localhost` means the server, not your client. +Only allow-listed hosts are fetched: by default `raw.githubusercontent.com`, +`*.githubusercontent.com` and `github.com`. `SOLR_INDEX_URL_ALLOWED_HOSTS` takes a +comma-separated list of exact hosts, `*.suffix` patterns, or `*` for any host the +server can reach; link-local addresses and the known cloud-metadata addresses +(AWS, Alibaba Cloud, Azure) are always refused. The +body is limited to 10 MB (`SOLR_INDEX_URL_MAX_BYTES`); for larger datasets index +directly with Solr, for example `bin/solr post -c shows shows.json`, which needs no +model in the loop. The format comes from the URL path extension, then the +`Content-Type`; add `"format":"csv"` when neither identifies it. Non-2xx responses +and HTML pages are errors, and nothing is indexed unless the whole document parses. +`SOLR_INDEX_URL_CONNECT_TIMEOUT` (`10s`), `SOLR_INDEX_URL_READ_TIMEOUT` (`30s`, per +read) and `SOLR_INDEX_URL_TOTAL_TIMEOUT` (`5m`, the whole fetch including redirects) +bound one fetch, and `SOLR_INDEX_URL_MAX_CONCURRENT_FETCHES` (`4`) bounds how many run +at once; a call beyond that limit fails immediately with a retry message. + ### Resources | Resource URI | Description | diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index f230fe5c..d88eebe0 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -61,8 +61,8 @@ it speaks MCP (JSON-RPC) to an AI client over one of two transports — **STDIO* (streamable-HTTP; a network listener). On the other side it speaks SolrJ HTTP to **one** backend Solr instance whose location and credentials the operator fixes at startup via environment (`SOLR_URL`, optional `SOLR_USERNAME`/`SOLR_PASSWORD`). -It exposes eleven tools (search, three indexing formats, collection create/list/ -stats/health, schema get/add-fields/add-field-types), two resources +It exposes thirteen tools (search, four inline indexing formats, URL ingestion, +collection create/list/stats/health, schema get/add-fields/add-field-types), two resources (`solr://collections`, `solr://{collection}/schema`), and prompt/completion helpers. It translates natural-language requests — as structured by the calling LLM into tool arguments — into Solr API calls, and returns Solr results back to @@ -109,7 +109,7 @@ deferred to a later version (see §12). This is a deliberate scoping choice for | STDIO transport | stdin/stdout JSON-RPC | child process of the client only | **Yes** | | HTTP transport | servlet on `:8080/mcp` + OAuth2 filter chain | **network listener** | **Yes (highest network exposure)** | | Read tools | `search`, `list-collections`, `get-collection-stats`, `check-health`, `get-schema` | reads backend Solr | **Yes** | -| Write/index tools | `index-json/csv/xml-documents` | writes backend Solr index | **Yes** | +| Write/index tools | `index-json/csv/xml/markdown-documents`, `index-url` | writes backend Solr index; `index-url` also makes an outbound GET to an allow-listed host | **Yes** | | Admin/schema tools | `create-collection`, `add-fields`, `add-field-types` | mutates backend Solr collections/schema | **Yes (privileged)** | | Backend SolrJ client | `SolrConfig` → `HttpJdkSolrClient` | outbound HTTP to `SOLR_URL` | **Yes (auth passthrough)** | | Actuator endpoints (HTTP) | `/actuator/*` (sbom, metrics, prometheus, loggers, info) | network | **Yes** | @@ -213,13 +213,18 @@ reaching the backend Solr directly, bypassing this server, is out of model (§3) | `OAUTH2_ISSUER_URI` | empty (placeholder) | With HTTP security on and no issuer, the chain still returns 401/403 on every non-permitted endpoint (locked down, no token validator). A real issuer enables JWT signature/issuer/exp/**audience** validation. | Q-httpsec | | `MCP_CORS_ALLOWED_ORIGINS` | MCP Inspector localhost proxy | Explicit CORS allowlist; wildcard-with-credentials is rejected by construction (`setAllowedOrigins`, not patterns). | *(documented)* | | `SOLR_USERNAME` / `SOLR_PASSWORD` | unset | When both set, static HTTP Basic Auth to backend Solr on every request; when unset, unauthenticated backend calls. | Q-backendcreds | +| `SOLR_INDEX_URL_ALLOWED_HOSTS` | `raw.githubusercontent.com,*.githubusercontent.com,github.com` | Which hosts `index-url` may fetch; exact hosts, `*.suffix` patterns, or `*`, which widens the boundary to the server's whole network (link-local and cloud-metadata addresses stay refused). | *(documented)* | +| `SOLR_INDEX_URL_MAX_BYTES` | `10MB` | Caps one `index-url` fetch; the body is parsed in memory, so this bounds memory per call to a small multiple of the value (raw bytes, decoded string, parsed documents). Concurrent callers multiply it. | *(documented)* | +| `SOLR_INDEX_URL_READ_TIMEOUT` | `30s` | Bounds how long a remote endpoint can hold an `index-url` call open per read. The connect timeout (`10s`) is operational, not security-relevant. | *(documented)* | +| `SOLR_INDEX_URL_TOTAL_TIMEOUT` | `5m` | Deadline for one whole `index-url` fetch, redirects included, so a host that drips bytes cannot outlast the per-read timeout. | *(documented)* | +| `SOLR_INDEX_URL_MAX_CONCURRENT_FETCHES` | `4` | How many `index-url` calls may run at once; further calls fail immediately. Bounds total memory (each call holds a small multiple of `SOLR_INDEX_URL_MAX_BYTES`) and outbound connections. | *(documented)* | **How HTTP mode enforces auth** *(maintainer — Q-transport.)*: the transport is streamable HTTP running in **stateless** mode (`spring.ai.mcp.server.protocol=stateless`), so there is no sampling, progress or elicitation channel and no per-request context feature. `/mcp` is `permitAll()` at the filter-chain level; authentication is enforced instead by -`@PreAuthorize("isAuthenticated()")` on **every** MCP entry point — all 11 +`@PreAuthorize("isAuthenticated()")` on **every** MCP entry point — all 13 tools, both resources, every prompt and completion handler — following the spring-ai-community/mcp-security "secured tools" pattern. A finding that reads `permitAll()` on `/mcp` as an authentication bypass without checking the @@ -307,11 +312,15 @@ Two adversaries are in scope; several are explicitly not. high. *(documented — docs/security/stdio.md; `application-stdio.properties`.)* 5. **Backend credentials are startup config, not caller input.** `SOLR_URL` and the optional Basic-Auth credentials are read once from the environment and are - never taken from a tool argument, so the AI client cannot repoint the server - or inject a target URL. *Violation:* a tool argument alters the backend - target or credential. *Severity:* critical (SSRF/credential-redirect if - broken). *(documented — docs/security/stdio.md & http.md; `SolrConfig`, - `SolrConfigurationProperties`.)* + never taken from a tool argument, so the AI client cannot repoint the server's + **Solr backend** or its credentials. *Violation:* a tool argument alters the + Solr backend target or a credential. *Severity:* critical + (SSRF/credential-redirect if broken). `index-url` performs an outbound GET to + a caller-supplied `http(s)` URL whose host must be on an operator allow-list + (GitHub raw content by default); that is a §9-bounded property, not a backend + target, and it never carries credentials or caller-supplied headers. + *(documented — docs/security/stdio.md & http.md; `SolrConfig`, + `SolrConfigurationProperties`, `UrlFetcher`.)* 6. **XML indexing is XXE-hardened.** `XmlDocumentCreator` builds a `DocumentBuilderFactory` with secure processing on, DOCTYPE disallowed, external general/parameter entities off, XInclude off, entity-expansion off. @@ -327,6 +336,27 @@ Two adversaries are in scope; several are explicitly not. ## §9 Security properties the project does *not* provide +- **It does not verify what an allow-listed URL serves.** `index-url` fetches + any `http(s)` URL whose host matches `SOLR_INDEX_URL_ALLOWED_HOSTS` (default: + GitHub raw-content hosts; `*` allows any host the server can reach, including + loopback and RFC1918). Link-local addresses (`169.254.0.0/16`, `fe80::/10`) + and the known cloud-metadata literals (`fd00:ec2::254`, `100.100.100.200`, + `168.63.129.16`) are refused on every redirect hop regardless; other providers' + metadata endpoints are not enumerated. The fetch carries no credentials or + caller headers, refuses an https→http redirect, and reads at most + `SOLR_INDEX_URL_MAX_BYTES` (default 10 MB); a refused or over-cap response is + abandoned without reading its body (the JDK may drain a small remainder in the + background for keep-alive). One fetch is bounded by a per-read timeout and by + a total deadline (`SOLR_INDEX_URL_TOTAL_TIMEOUT`, default 5 minutes, redirects + included), and at most `SOLR_INDEX_URL_MAX_CONCURRENT_FETCHES` (default 4) + fetches run at once, further calls failing immediately; response headers are + not size-capped, which is accepted for allow-listed hosts. The address check + runs on the resolved addresses before + the connection is made, so a DNS answer that changes in between (DNS + rebinding) can bypass it; the JDK's positive DNS cache (30 s by default) means + both lookups usually see the same answer, with the default allow-list it + requires control of a GitHub host's DNS, and with `*` the operator has accepted + the network boundary. *(documented — `UrlTargetPolicy`, `UrlFetcher`.)* - **It does not defend against prompt injection / tool poisoning via Solr content.** Search results, schema, and stats returned by a tool flow **back into the model's context**. A document indexed into the backend Solr (by @@ -425,6 +455,10 @@ Two adversaries are in scope; several are explicitly not. - **Wiring `SOLR_URL` (or credentials) from user/tool input** instead of deployer environment — would convert the server into an SSRF/credential-relay primitive. Explicitly forbidden. *(documented.)* +- **Setting `SOLR_INDEX_URL_ALLOWED_HOSTS=*` on a network with reachable + internal services** that you would not expose to every authenticated MCP + caller — `index-url` lets such a caller fetch from them (§9). *(documented — + docs/security/http.md.)* - **Indexing untrusted documents into a Solr that the same MCP server reads back to the model** — creates a stored-prompt-injection loop. - **Sharing one MCP server (and its one backend credential) across mutually @@ -447,10 +481,17 @@ Two adversaries are in scope; several are explicitly not. - **"XXE in XML indexing."** The `DocumentBuilderFactory` is hardened (DOCTYPE disallowed, external entities off). `KNOWN-NON-FINDING`. *(documented — `XmlDocumentCreator`.)* -- **"`SOLR_URL` allows SSRF."** It is deployer-only startup config, never taken - from a tool argument; an SSRF report requires the operator to have violated the - documented contract. `OUT-OF-MODEL` (operator config) / `BY-DESIGN`. - *(documented.)* +- **"`SOLR_URL` allows SSRF."** The *Solr* target is deployer-only startup + config, never taken from a tool argument; an SSRF report requires the operator + to have violated the documented contract. `OUT-OF-MODEL` (operator config) / + `BY-DESIGN`. *(documented.)* +- **"`index-url` allows SSRF."** With the default allow-list the server fetches + only GitHub raw-content hosts: `KNOWN-NON-FINDING`. With `*` the operator has + chosen the boundary: `OUT-OF-MODEL: trusted-input`. A report is `VALID` only if + it shows a non-allow-listed host being fetched, a credential or caller header + being forwarded, a refused address being reached other than through DNS + rebinding (§9), or an https→http downgrade being followed. *(documented — §9, + §12.)* - **"Solr query injection via the `search` tool."** Expressing arbitrary Solr queries is the feature; the blast radius is the backend Solr's, governed by apache/solr's model. Route Solr-side query-parser exposure there. PRs #122 @@ -479,8 +520,12 @@ Two adversaries are in scope; several are explicitly not. - Adding a **destructive** tool (delete-collection, delete-by-query, schema field deletion, config API) — today the tool set is read/additive-only, which materially bounds the blast radius. -- Allowing any **backend-target or credential** value to originate from a tool - argument or per-request input (would open SSRF/credential-relay). +- Allowing a **credential** to originate from a tool argument or per-request + input (would open credential-relay). *The backend-target half was exercised + deliberately on 2026-09-15 by `index-url` + ([#208](https://github.com/apache/solr-mcp/issues/208)) behind an operator + allow-list; see §8.5 and §9.* +- Changing the **default** of `SOLR_INDEX_URL_ALLOWED_HOSTS` to `*`. - Adding **per-caller identity passthrough** or an authorization layer over Solr collections/actions (would add new §8 properties). - **Supporting more than one user per instance.** The one-instance-per-user @@ -499,9 +544,9 @@ Two adversaries are in scope; several are explicitly not. | Disposition | Meaning | Licensed by | | --- | --- | --- | -| `VALID` | A §8 property breaks via an in-scope adversary (auth bypass, wrong-audience token accepted, CORS wildcard+credentials, network listener in STDIO, tool-arg repoints backend, XXE in XML indexing, dishonest tool hint). | §8, §6, §7 | +| `VALID` | A §8 property breaks via an in-scope adversary (auth bypass, wrong-audience token accepted, CORS wildcard+credentials, network listener in STDIO, tool-arg repoints the Solr backend or forwards a credential, `index-url` fetches a non-allow-listed host, XXE in XML indexing, dishonest tool hint). | §8, §6, §7 | | `VALID-HARDENING` | No §8 break, but a §11 misuse is made too easy (e.g. admin tools exposed with no opt-out); fixed at maintainer discretion. Per-tool / read-only-subset proposals route to [#66](https://github.com/apache/solr-mcp/issues/66). | §11 | -| `OUT-OF-MODEL: trusted-input` | Requires control of deployer config (`SOLR_URL`, credentials, issuer, CORS list). | §5/§6/§10 | +| `OUT-OF-MODEL: trusted-input` | Requires control of deployer config (`SOLR_URL`, credentials, issuer, CORS list, `SOLR_INDEX_URL_ALLOWED_HOSTS=*`). | §5/§6/§10 | | `OUT-OF-MODEL: adversary-not-in-scope` | Requires owning the client's stdin (STDIO), a maliciously-connected client, or direct backend access. | §7 | | `OUT-OF-MODEL: non-default-build` | Only manifests with `HTTP_SECURITY_ENABLED=false` or an otherwise discouraged toggle. | §5a | | `OUT-OF-MODEL: unsupported-component` | Lands in the `docker compose`/sample dev stack. | §3 | diff --git a/docs/FAQ.md b/docs/FAQ.md index 064c7ba9..81b164b2 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -72,7 +72,7 @@ Anthropic puts it, *"tool descriptions occupy more context window space"*, and at scale agents *"need to process hundreds of thousands of tokens before reading a request."*[code-execution] -For this server (11 tools across search, indexing, schema, and +For this server (13 tools across search, indexing, schema, and collections), the upfront overhead is a few thousand tokens — real but bounded. diff --git a/docs/security/http.md b/docs/security/http.md index 043964a9..a64c8d4e 100644 --- a/docs/security/http.md +++ b/docs/security/http.md @@ -124,6 +124,13 @@ exists for browser-based tooling. - `http.security.enabled=false` on a network-reachable deployment. - Passing `SOLR_URL` from MCP tool input — it must come from deployer-controlled environment. +- Setting `SOLR_INDEX_URL_ALLOWED_HOSTS=*` on a deployment whose network has + internal services you would not expose to every authenticated MCP caller. + `index-url` fetches any allow-listed host the server can reach; with `*` that + includes loopback and RFC1918 services (link-local addresses and the known + AWS, Alibaba Cloud and Azure metadata addresses stay refused; other providers' + metadata endpoints are not enumerated). Keep the default GitHub allow-list or + add specific hosts. ## Primary sources diff --git a/docs/security/stdio.md b/docs/security/stdio.md index e557e79e..b1673edf 100644 --- a/docs/security/stdio.md +++ b/docs/security/stdio.md @@ -27,6 +27,13 @@ that launched the process. No code changes are required for STDIO security. MCP client. - **Treat `SOLR_URL` as deployer-controlled config**, not user-controlled input. It is read once at startup. Never wire it from an MCP tool argument. +- **`index-url` is the one tool that makes an outbound request to a + caller-supplied address.** Only hosts on `SOLR_INDEX_URL_ALLOWED_HOSTS` are + fetched (GitHub raw content by default); link-local addresses and the known + cloud-metadata addresses (AWS, Alibaba Cloud, Azure) are always refused; no + credentials or caller headers are sent; and + the body is capped by `SOLR_INDEX_URL_MAX_BYTES`. Under STDIO the reachable + network is the launching user's own, the same boundary the process already has. - **Scope the Solr instance.** STDIO mode delegates Solr-side authorization to Solr itself (Basic Auth, mTLS, network policy). Point at a Solr that the launching user is already authorized to use. diff --git a/docs/superpowers/specs/2026-09-15-url-ingestion-design.md b/docs/superpowers/specs/2026-09-15-url-ingestion-design.md new file mode 100644 index 00000000..0cc5bc99 --- /dev/null +++ b/docs/superpowers/specs/2026-09-15-url-ingestion-design.md @@ -0,0 +1,661 @@ +# Spec: `index-url` — index a document set from an http(s) URL + +**Date:** 2026-09-15 (rewritten the same day; supersedes the streaming design, see §9) +**Status:** ready to implement +**Tracking issue:** https://github.com/apache/solr-mcp/issues/208 +**Base:** upstream `main` @ `b4ffe18`. Branch `feat/index-url` on the author's fork. + +Written for an implementer with no access to the discussions behind it. §3 lists +every decision with its reason; §4 to §7 are the specification; §8 is the definition +of done; §9 records what was tried before and why it was dropped. Every default value, +message, exception type and boundary condition is stated exactly; where two readings +were possible, the chosen one is written out. + +--- + +## 1. Goal and non-goals + +**Goal.** Let an MCP client index a JSON, CSV, XML or Markdown document set into a +Solr collection by naming a URL, so the payload never passes through the model's +context. Identical behaviour in the STDIO and HTTP transports. Datasets up to a +configurable cap (default 10 MB) are handled by this tool; larger ones are routed to +Solr's own bulk tooling by the model's guidance (§6.1), not by this server. + +**Why this exists.** The server's differentiator is the onboarding loop: look at a +dataset, design the schema, add fields, index, verify counts, search, iterate, all in +one conversation. Every existing indexing tool takes the payload as a tool-call +argument, so the model must emit every byte. Measured on PR #197: 61 documents took +over two minutes, of which Solr and the server took under one second; the rest was the +model emitting ~9,500 tokens. The loop therefore breaks at the indexing step for +anything beyond a few dozen records, and in a chat-only client such as Claude Desktop +there is no way around it because the client cannot run a shell command. A URL +argument is ~20 tokens regardless of size, and the server fetches the bytes itself. + +**Why a cap and not streaming.** Once the bytes bypass the model, the bottleneck is +gone. A 10 MB body parsed in memory by the existing parsers is instant. Datasets +larger than a conversation would ever onboard belong to `bin/solr post` or Solr's +`/update` handler, which do bulk ingestion better than this server can, with no model +in the loop. A streaming implementation existed and worked (§9); it was dropped because +it duplicated Solr's own parsers, conflicted with open PR #205, and defended a case the +guidance now routes elsewhere. + +**Non-goals.** + +- Local-file ingestion (`index-file`). Removed from scope (§9). Under STDIO the model + gives the user the `bin/solr post` command; under HTTP an attached file goes through + the inline tools as today. +- Schema definition from a URL. Schema payloads are a few KB and the model should read + and reconcile them against `get-schema`. +- HTML pages. `text/html` is an error, not converted. +- Authenticated or header-customised fetches. +- Streaming or unlimited size. That is the phase-2 follow-up in §10, which keeps this + tool's contract and changes only its insides. +- Progress notifications. The HTTP transport is `stateless`; unchanged. + +--- + +## 2. What exists on `main` that this builds on (verified 2026-09-15 @ `b4ffe18`) + +| Component | Location | Role | +|---|---|---| +| `IndexingService.indexJsonDocuments` / `indexCsvDocuments` / `indexXmlDocuments` / `indexMarkdownDocuments` | `indexing/IndexingService.java` lines 219, 294, 393, 467 | The four inline tools. Each calls `indexingDocumentCreator.createSchemalessDocumentsFrom(String)`, then `indexDocuments(collection, docs)`, then returns `"Successfully indexed " + successCount + " of " + docs.size() + " documents into collection '" + collection + "'"`. The summary text is duplicated four times. | +| `IndexingService.indexDocuments(String collection, List)` | same file | Batches, adds, commits, returns the success count. | +| `IndexingDocumentCreator.createSchemalessDocumentsFrom{Json,Csv,Xml,Markdown}(String)` | `indexing/documentcreator/` | Fully materialised parsers. Throw the unchecked `DocumentProcessingException`. | +| `IndexingService.SCHEMA_FIRST_GUIDANCE` | **does not exist on `main`**; added by this PR as a package-private constant next to `indexPayload` | Trailing sentence for the `index-url` description: "Before indexing, use get-schema and add-fields (or design-schema) to define compatible fields. Use string with docValues for categories/facets, text_general for prose, and explicit numeric types and multiValued settings. Do not rely on schemaless type guessing; existing field types cannot be changed with these tools." The four inline tool descriptions are left unchanged. | +| `IndexingService.indexDataPrompt` | same file | The `index-data` MCP prompt; step 3 chooses the tool. | +| `spring.ai.mcp.server.instructions` | `application.properties` | Server instructions shown to every client. | +| `McpToolRegistrationTest` | `src/test/java/.../McpToolRegistrationTest.java` | Reflection-only checks: tool names unique, parameters annotated, every `@McpTool` method carries `@PreAuthorize`. Holds an explicit list of service classes. | +| `McpClientIntegrationTestBase` + `McpClientIntegrationTest` (HTTP) + `McpClientStdioIntegrationTest` | same directory | Full MCP round trips per transport against Testcontainers Solr. The base asserts the expected tool list and per-tool hints. | +| `SolrConfig` | `config/SolrConfig.java` line 104 | `@EnableConfigurationProperties(SolrConfigurationProperties.class)`; new property records are registered here. | +| `spring-web` 6.2.18 | production classpath | Provides `RestClient`. Apache HttpClient and Jetty client are **not** on the production classpath. | + +--- + +## 3. Decisions and why + +**D1. Hosts are allow-listed, restricted by default to GitHub's raw-content hosts.** +Default `solr.index-url.allowed-hosts` = +`raw.githubusercontent.com,*.githubusercontent.com,github.com`. `*` allows any host. +*Why:* the tutorial example (`raw.githubusercontent.com/apache/solr-mcp/main/src/test/resources/shows.json`) +works with no configuration; an HTTP deployment on a corporate network is safe without +the operator remembering a knob, which matches the project's posture of HTTP security +being on by default; an operator hosting data on S3 or an internal server adds one +entry. An earlier draft allowed any host by default and documented the SSRF exposure; +that made the server an open SSRF primitive in HTTP mode and would have drawn a +maintainer objection on the threat model. `github.com` is included because +`github.com///raw/...` URLs redirect to `raw.githubusercontent.com`. + +**D2. Link-local and the known cloud-metadata addresses are refused even when the allow-list is `*`.** +`169.254.0.0/16` and `fe80::/10` (everything `InetAddress.isLinkLocalAddress()` reports), +plus the literals `fd00:ec2::254` (AWS IPv6), `100.100.100.200` (Alibaba Cloud) and +`168.63.129.16` (Azure WireServer). Other providers' metadata endpoints are not +enumerated, and the documentation must say "the known cloud-metadata addresses", not +"cloud-metadata addresses", so the claim matches the code. +*Why:* the single worst outcome of `*` is a cloud instance handing its credentials to +a caller. Refusing these needs no configuration and blocks no legitimate document +host. Loopback and RFC1918 are **not** refused; they are governed by the allow-list +like any other host. + +**D3. The body is read into memory, capped at `max-bytes` (default 10 MB), and parsed by the existing creators.** +*Why:* it reuses code that is already tested, keeps the server's field-name +sanitising and nested-XML flattening, adds no parser code, and makes "nothing was +indexed" true for every fetch or parse failure, because indexing starts only after the +whole body has been parsed. 10 MB is roughly 15,000 shows-sized JSON records, far +beyond what a conversation onboards. The cap is a property so an operator can raise it. + +**D4. Datasets over the cap are routed to Solr's own tooling by guidance, not by this server.** +*Why:* `bin/solr post -c ` and `curl` to `/update` already do bulk +ingestion with no model in the loop and no new attack surface, and they run where the +file is, which also covers the Claude Desktop local-file case that no server-side tool +can reach (the client cannot send files to an MCP server; SEP-2631 is the open draft +that would change that). The model emits the exact command (§6.1). + +**D5. The HTTP client is Spring's `RestClient` over `SimpleClientHttpRequestFactory`.** +*Why:* it is already on the classpath, its `readTimeout` applies to every socket read +including the body (the JDK `java.net.http.HttpClient`'s timeout covers only the +response headers, which is what forced a hand-written watchdog in the earlier draft), +and it gives the outbound call Micrometer observations for free. `RestClient`'s +`exchange(...)` is used rather than `retrieve()` because `exchange` does not apply +status handlers, so non-2xx responses are handled by this code, not by an exception +from Spring. Apache HttpClient would also work but is not on the classpath and is not +worth a new dependency. + +**D6. Redirects are followed manually: up to five are followed; the sixth redirect response is an error; `https` to `http` is refused.** +*Why:* release-asset and `github.com/.../raw` URLs redirect. Manual following is +required so the allow-list and D2 run on every hop; `HttpURLConnection` follows +redirects itself by default, so the request factory turns that off (§4.3). + +**D7. No credentials and no caller-supplied headers, ever; a URL with embedded `user:pass@` is rejected.** +*Why:* `THREAT_MODEL.md` §12 names credential-from-tool-argument as a model-changing +condition. The server sends only `Accept` and `User-Agent`. + +**D8. Format resolution: explicit `format` > requested URL's path extension > final URL's path extension > `Content-Type` > error. `text/plain` and `text/html` never resolve.** +*Why:* `raw.githubusercontent.com` serves `.json` as `text/plain; charset=utf-8`, as +do most static hosts, so `Content-Type` alone would mis-route the canonical example. +`text/html` is refused so an ordinary web page cannot be indexed as CSV rows. The +final URL is consulted so a short link that redirects to `.../shows.json` resolves. + +**D9. Non-2xx is an error before any body is read.** +*Why:* a GitHub 404 body is the literal text `404: Not Found`; the CSV parser would +index it as one valid document. + +**D10. One tool with an optional `format`, not four per-format URL tools.** +*Why:* PR #197 kept per-format inline tools because each signature is optimised for its +format's token shape. `index-url` carries no payload in its arguments, so there is no +shape to optimise. + +**D11. Registered in both transports with no `@Profile` gate.** +*Why:* PR #194 was closed because a STDIO-only tool gave the two transports different +surfaces. This design has no transport-specific tool and reverses no prior decision. + +**D12. Defaults chosen by the spec author, to be confirmed in the PR:** the default +allow-list contents (D1) and the 10 MB cap (D3). Both are single constants. + +**D13. One fetch has a total deadline (`total-timeout`, default 5 minutes) and at most +`max-concurrent-fetches` (default 4) run at once; a call beyond the limit fails at once.** +*Why:* the read timeout is per read, so an allow-listed host that drips one byte at a +time never trips it and could hold a call open for hours; the deadline covers the whole +fetch including redirects, and 5 minutes still allows a 10 MB document at 35 KB/s. +Each call holds a small multiple of the cap in memory, so without a concurrency limit +authenticated callers could multiply that without bound; failing immediately (rather +than queueing) keeps tool calls predictable and lets the model retry. Response header +size is not capped (`HttpURLConnection` offers no hook) and is accepted for +allow-listed hosts. + +--- + +## 4. Server implementation + +### 4.1 Tool contract + +``` +index-url(collection: String, url: String, format: String?) -> String +``` + +| Parameter | `required` | Description text (verbatim for `@McpToolParam`) | +|---|---|---| +| `collection` | `true` | `Solr collection to index into` | +| `url` | `true` | `Absolute http or https URL of a UTF-8 JSON, CSV, XML or Markdown document, at most the configured size limit (default 10 MB). The host must be on the server's allow-list (GitHub raw content by default). Fetched from the MCP server's network, not the client's. No credentials or custom headers are sent.` | +| `format` | `false` | `Optional format: json, csv, xml, markdown or md; defaults to the URL path extension, then the Content-Type` | + +`@McpTool` attributes: `name = "index-url"`, +`annotations = @McpTool.McpAnnotations(idempotentHint = true, openWorldHint = true)`. +The method carries `@PreAuthorize("isAuthenticated()")`. + +Description (verbatim, followed by `IndexingService.SCHEMA_FIRST_GUIDANCE`): + +> Index a UTF-8 JSON, CSV, XML or Markdown document set from an http(s) URL without +> sending its contents through the model. Available in both STDIO and HTTP mode. The +> URL is fetched by the MCP server with no credentials or custom headers; its host must +> be on the server's allow-list (GitHub raw content by default) and the body must be +> within the configured size limit (default 10 MB). For larger datasets, index directly +> with Solr (bin/solr post or the /update handler) instead. Redirects are followed. +> Non-2xx responses and HTML pages are errors; nothing is indexed unless the whole +> document parses. Reuse the URL for another collection. + +Return value: the same summary string the inline tools return (§4.4). + +### 4.2 Classes + +All in `org.apache.solr.mcp.server.indexing` unless stated. The package is +`@NullMarked`; use `org.jspecify.annotations.Nullable` where a value may be null. + +| Class | Kind | Responsibility | +|---|---|---| +| `UrlIndexingService` | `public`, `@Service @Observed` | The `@McpTool` method. Public constructor `(IndexingService, UrlIndexingProperties)` **annotated `@Autowired`**, which builds `new UrlFetcher(properties)`; package-private constructor `(IndexingService, UrlFetcher)` for the unit test (the service itself needs nothing from the properties; the fetcher owns them). Spring refuses to choose between two constructors without the annotation; every Spring test fails to start if it is missing. | +| `UrlIndexingProperties` | `public record`, `@ConfigurationProperties(prefix = "solr.index-url")` | `List allowedHosts` (`@DefaultValue({"raw.githubusercontent.com", "*.githubusercontent.com", "github.com"})`), `Duration connectTimeout` (`@DefaultValue("10s")`), `Duration readTimeout` (`@DefaultValue("30s")`), `DataSize maxBytes` (`@DefaultValue("10MB")`), `Duration totalTimeout` (`@DefaultValue("5m")`), `int maxConcurrentFetches` (`@DefaultValue("4")`). The record's compact constructor rejects `maxBytes` outside `1 .. Integer.MAX_VALUE - 1` bytes with `IllegalArgumentException("solr.index-url.max-bytes must be between 1 byte and 2 GB")`, a non-positive `totalTimeout` with `"solr.index-url.total-timeout must be positive"`, and `maxConcurrentFetches < 1` with `"solr.index-url.max-concurrent-fetches must be at least 1"`, so a misconfiguration fails at startup; there is no "0 means unlimited". Registered by adding it to the `@EnableConfigurationProperties` annotation on `SolrConfig`. | +| `UrlTargetPolicy` | package-private final, static `check(URI uri, List allowedHosts, List resolved)` | Pure, no I/O. Throws only `IllegalArgumentException`. Order of checks and messages in §4.3 step 3. | +| `UrlFetcher` | package-private final | Owns the `RestClient`. Resolves the host, calls the policy, performs the request, follows redirects, enforces the size cap, returns `FetchedBody(URI finalUri, String mediaType, Charset charset, byte[] body)`. Throws `IllegalArgumentException` for caller-fixable problems and `IOException` for network problems (§4.3). | +| `IndexFormats` | package-private final, static `normalize(String keyword)` | Maps `json`, `csv`, `xml`, `md`, `markdown` (any case, trimmed) to `json`/`csv`/`xml`/`markdown`; anything else, including blank, throws `IllegalArgumentException("Cannot determine the file format. Supply format=json, csv, xml or markdown.")`. Used only by `UrlIndexingService`; the existing private switch in `IndexingService.resolveIndexTool` is left as is. | +| `IndexingService.indexPayload(String collection, String payload, String format)` | new package-private method | See §4.4. | + +No other production classes. In particular there is no streaming reader, no watchdog, +no byte-cap stream, and no scheduler. + +### 4.3 Fetch algorithm (`UrlIndexingService.indexUrl` then `UrlFetcher.fetch`) + +1. If `collection.isBlank()` throw `IllegalArgumentException("Provide a non-empty collection name.")`. + Do not null-check `collection` or `url`: `required = true` makes null impossible. +2. If `url.isBlank()`, or `URI.create(url.trim())` throws, throw + `IllegalArgumentException(INVALID_URL)` where + `INVALID_URL = "Provide an absolute http or https URL."`. +3. If `format` is non-null and non-blank, `explicit = IndexFormats.normalize(format)` + (throws before any network activity); otherwise `explicit = null`. +4. `fetcher.fetch(uri)`; an `IOException` from it becomes an `IllegalStateException` + per §4.5 (two rows: read timeout vs everything else). +5. Inside `UrlFetcher.fetch`, with `current = uri` and `redirects = 0`, loop: + 1. `UrlTargetPolicy.check(current, allowedHosts, List.of())` — syntactic checks + only, before any DNS lookup, in this order: scheme must be `http` or `https` + (case-insensitive), `getHost()` non-null and `getPort()` at most 65535 + (`java.net.URI` accepts any integer port and `HttpURLConnection` would later + throw its own raw message), else `INVALID_URL`; + `getUserInfo()` must be null, else + `"Remove the credentials from the URL; this server never sends credentials."`; + host must match the allow-list (§4.3.1), else the allow-list message (§4.5). + 2. `addresses = InetAddress.getAllByName(current.getHost())`; an + `UnknownHostException` propagates as an `IOException`. + 3. `UrlTargetPolicy.check(current, allowedHosts, addresses)` — repeats step 5.1 + and then, for **every** address, throws + `"This server does not fetch link-local or cloud-metadata addresses."` if + `isLinkLocalAddress()` is true or the address is one of the D2 literals. This + applies regardless of the allow-list, including `*`. (`getAllByName` returns an + `Inet4Address` for an IPv4-mapped literal such as `::ffff:169.254.169.254`, and + decodes a bare decimal host such as `2852039166` to `169.254.169.254`, so both + are caught here.) + 4. Send `GET current` through the `RestClient` with headers exactly + `Accept: application/json, text/csv, application/xml, text/xml, text/markdown, text/plain;q=0.5, */*;q=0.1` + and `User-Agent: solr-mcp`, and nothing else. Use + `restClient.get().uri(current).exchange((request, response) -> ...)` so that + non-2xx statuses reach this code instead of a Spring exception. Inside the + exchange function: + - Let `status = response.getStatusCode().value()`. + - If `status` is 301, 302, 303, 307 or 308 **and** a `Location` header is + present: record the target and return a redirect marker. Otherwise, if the + status is not 2xx, return a status marker. Otherwise read headers and body as + in steps 5.7 to 5.9 and return them. (Markers are a small sealed interface or + record; the body stream must be fully consumed or closed before returning.) + 5. On a redirect marker: increment `redirects`; if it is now **greater than 5**, + throw `"The URL redirected more than 5 times. Use the final URL directly."` + (five redirects are followed; the sixth redirect response fails). Compute + `target = current.resolve(location)`; if `current` is `https` and `target` is + `http` throw + `"The URL redirects from https to http, which is refused. Use the final https URL directly."`; + set `current = target` and continue the loop from 5.1. + 6. On a status marker for status *N*: throw + `"The URL returned HTTP N; nothing was indexed. Check that it is public and points at a raw document, not a web page."`. + 7. Media type = the `Content-Type` value up to the first `;`, trimmed, lower-cased; + `""` if the header is absent. Charset = the `charset=` parameter (quotes + stripped) via `Charset.forName`; UTF-8 if absent; an + `IllegalCharsetNameException` or `UnsupportedCharsetException` becomes + `"The URL declares an unsupported charset. Supply a UTF-8 document."`. + 8. If a `Content-Length` header is present, numeric, and greater than `maxBytes`, + throw the size error (§4.5) **without reading the body** (a non-numeric value is + treated as absent). **Abandoning a response:** on this path, on the over-cap path + in step 5.9, and on every redirect or non-2xx return, close the body stream + (`response.getBody().close()`, ignoring `IOException`) *before* returning or + throwing. Spring's own `close()` otherwise drains the unread body to keep the + connection reusable, which would download a refused 2 GB response in full; + closing the stream first makes the JDK drop the connection (or hand at most a + small remainder to its keep-alive cleaner) and turns Spring's drain into a + no-op on a closed stream. `UrlFetcherTest` proves it with a 2 MB body streamed + at 10 KB/s under a 5-second preemptive timeout. + 9. `bytes = body.readNBytes((int) maxBytes + 1)` (safe: the record validates + `maxBytes <= Integer.MAX_VALUE - 1`). If `bytes.length > maxBytes` throw the + size error. The count is of raw body bytes after transfer decoding and before + charset decoding. Close the response in a `finally` on every path out of the + exchange function. + 10. Return `FetchedBody(current, mediaType, charset, bytes)`. +6. Back in the service: `selected = explicit != null ? explicit : resolveFormat(uri, fetched.finalUri(), fetched.mediaType())` (§4.3.2). +7. `payload = new String(fetched.body(), fetched.charset())`. +8. `return indexingService.indexPayload(collection, payload, selected)`, mapping + its exceptions per §4.5. + +#### 4.3.1 Allow-list matching + +Host is `uri.getHost()` lower-cased with any trailing `.` removed. Each entry is +trimmed and lower-cased. An entry matches when: + +- it is exactly `*` (matches every host), or +- it starts with `*.` and the host ends with the entry's suffix including the leading + dot, and the host is longer than that suffix (so `*.githubusercontent.com` matches + `raw.githubusercontent.com` but not `githubusercontent.com`), or +- it equals the host exactly (this is the only way an IP literal or `localhost` + matches, other than `*`). + +An empty list matches nothing: every call fails with the allow-list message. That is +the behaviour when an operator sets `SOLR_INDEX_URL_ALLOWED_HOSTS=`; it is not +special-cased. + +#### 4.3.2 Format resolution + +Extension of a URI = the substring after the last `.` of the last `/`-separated +segment of `getPath()`, lower-cased; `null` if the path is null, has no `.` in its +last segment, or ends with `.`. Query string and fragment are excluded because +`getPath()` excludes them. + +For each of `requested`, then `finalUri`: if the extension is non-null and +`IndexFormats.normalize(extension)` succeeds, that is the format. Otherwise: + +| Media type | Format | +|---|---| +| `application/json` | `json` | +| `text/csv` | `csv` | +| `application/xml`, `text/xml` | `xml` | +| `text/markdown` | `markdown` | +| `text/html` | error `FORMAT_UNRESOLVED + " HTML pages are not supported."` | +| anything else, including `text/plain`, `application/octet-stream` and `""` | error `FORMAT_UNRESOLVED` | + +`FORMAT_UNRESOLVED = "Cannot determine the format from the URL path or Content-Type. Supply format=json, csv, xml or markdown."` + +### 4.4 `IndexingService.indexPayload` (refactor, no behaviour change) + +```java +String indexPayload(String collection, String payload, String format) throws SolrServerException, IOException +``` + +- `switch (format)`: `"json"` → `createSchemalessDocumentsFromJson`, `"csv"` → `...FromCsv`, + `"xml"` → `...FromXml`, `"markdown"` → `...FromMarkdown`; any other value throws + `IllegalArgumentException("Unsupported document format: " + format)` (unreachable + from `index-url`, which passes normalised values). +- Then `int successCount = indexDocuments(collection, docs)` and return + `"Successfully indexed " + successCount + " of " + docs.size() + " documents into collection '" + collection + "'"`. +- Each of the four inline tool methods becomes: validate its arguments exactly as it + does today, then `return indexPayload(collection, , "")` inside its + existing try/catch. Their error messages, exception types and logging do not change. + `IndexingServiceTest` must pass **unmodified**; that is the check that the refactor + is behaviour-preserving. + +### 4.5 Error mapping in `UrlIndexingService` + +`IllegalArgumentException` for caller-fixable problems, `IllegalStateException` for +environment problems; log the cause at `debug` for caller problems and `warn` for Solr +problems; never include the response body, resolved IP, or stack detail in a message. +Because parsing completes before indexing starts, every row above the Solr row can +truthfully say nothing was indexed. + +| Condition | Exception | Message (verbatim) | +|---|---|---| +| blank `collection` | IAE | `Provide a non-empty collection name.` | +| blank / unparsable / relative / non-http(s) `url`, null host, or port above 65535 | IAE | `Provide an absolute http or https URL.` | +| `url` has userinfo | IAE | `Remove the credentials from the URL; this server never sends credentials.` | +| host not on the allow-list | IAE | `The URL's host is not on this server's allow-list. Allowed by default: raw.githubusercontent.com, *.githubusercontent.com, github.com. The operator can change SOLR_INDEX_URL_ALLOWED_HOSTS (use * to allow any host). Nothing was indexed.` (the "Allowed by default" list is a constant, not the live configuration) | +| link-local / metadata address (D2) | IAE | `This server does not fetch link-local or cloud-metadata addresses.` | +| unknown explicit `format` | IAE | `Cannot determine the file format. Supply format=json, csv, xml or markdown.` | +| `https` → `http` redirect | IAE | `The URL redirects from https to http, which is refused. Use the final https URL directly.` | +| sixth redirect | IAE | `The URL redirected more than 5 times. Use the final URL directly.` | +| `Location` header that is not a parsable URI, or that fails the syntactic checks (non-http(s), no host, userinfo, port above 65535) on a redirect hop | IAE | `The URL redirected to an invalid location. Use the final URL directly.` (allow-list and refused-address failures on a hop keep their own messages) | +| non-2xx status *N* | IAE | `The URL returned HTTP N; nothing was indexed. Check that it is public and points at a raw document, not a web page.` | +| unsupported charset | IAE | `The URL declares an unsupported charset. Supply a UTF-8 document.` | +| body larger than `maxBytes` (by `Content-Length` or by reading) | IAE | `The document is larger than this server's limit of ; nothing was indexed. Index datasets this large directly with Solr (bin/solr post or the /update handler); the index-data prompt shows the command.` where `` is `maxBytes.toMegabytes() + " MB"` if `maxBytes.toBytes() % 1048576 == 0`, otherwise `maxBytes.toBytes() + " bytes"` (so the default renders as `10 MB`) | +| format unresolved | IAE | `FORMAT_UNRESOLVED`, with ` HTML pages are not supported.` appended for `text/html` | +| more than `maxConcurrentFetches` calls already running (checked after the pre-fetch validations, before any network activity) | ISE | `The server is already running 4 index-url calls, the configured maximum; try again in a moment. Nothing was indexed.` (the number is the configured limit; singular `call` when it is 1) | +| `ResourceAccessException` whose cause chain contains a `SocketTimeoutException` (per-read timeout, or the total deadline, which `UrlFetcher` raises as a `SocketTimeoutException` after checking `System.nanoTime()` at every hop and after every 8 KB chunk of body) | ISE | `The URL did not deliver the document within the read or total timeout; nothing was indexed. Try again or ask the operator to raise SOLR_INDEX_URL_READ_TIMEOUT or SOLR_INDEX_URL_TOTAL_TIMEOUT.` | +| any other `IOException` / `ResourceAccessException` (unknown host, connect refused, connect timeout) | ISE | `Cannot reach the URL from the MCP server. The URL is fetched from the server's network, not the client's, so localhost and private addresses refer to the server's side. Check the address and try again.` | +| `DocumentProcessingException` from `indexPayload` | IAE | `Cannot parse the URL content as . Check its syntax and format. Nothing was indexed.` | +| `SolrServerException`, `SolrException` or `IOException` from `indexPayload` | ISE | `Solr could not complete URL indexing. Check collection availability and field types with get-schema, then verify the indexed count before retrying; some documents may already be indexed.` | + +`RestClient` wraps I/O failures in `org.springframework.web.client.ResourceAccessException`. +`UrlFetcher.fetch` catches it and rethrows the cause if the cause is an `IOException`, +otherwise wraps it in a new `IOException(cause)`. Consequently `fetch` throws exactly +two exception types, `IllegalArgumentException` and `IOException`, and the service +applies the two ISE rows above by checking whether the `IOException` or anything in its +cause chain is a `java.net.SocketTimeoutException`. + +### 4.6 Configuration + +Add to `application.properties` (not to a profile file; the tool exists in both): + +```properties +# index-url: which hosts may be fetched (exact host, *.suffix, or * for any), the +# connect and per-read timeouts, and the maximum body size. Link-local and +# cloud-metadata addresses are refused regardless of the allow-list. +solr.index-url.allowed-hosts=raw.githubusercontent.com,*.githubusercontent.com,github.com +solr.index-url.connect-timeout=10s +solr.index-url.read-timeout=30s +solr.index-url.total-timeout=5m +solr.index-url.max-bytes=10MB +solr.index-url.max-concurrent-fetches=4 +``` + +Environment names via Boot's relaxed binding: `SOLR_INDEX_URL_ALLOWED_HOSTS` +(comma-separated), `SOLR_INDEX_URL_CONNECT_TIMEOUT`, `SOLR_INDEX_URL_READ_TIMEOUT`, +`SOLR_INDEX_URL_TOTAL_TIMEOUT`, `SOLR_INDEX_URL_MAX_BYTES`, +`SOLR_INDEX_URL_MAX_CONCURRENT_FETCHES`. Documentation uses the environment names. + +The body is read in 8 KB chunks (`readCapped`) with the deadline checked after each +chunk and at the top of every redirect hop; a missed deadline is raised as +`SocketTimeoutException` so the service maps it with the per-read timeout. The +concurrency limit is a `Semaphore` in `UrlIndexingService`, acquired with +`tryAcquire()` after the pre-fetch validations and released in a `finally`. + +Request factory: a subclass of `SimpleClientHttpRequestFactory` overriding +`prepareConnection(HttpURLConnection, String)` to call `super`, then +`connection.setInstanceFollowRedirects(false)`; `setConnectTimeout` and +`setReadTimeout` from the properties. The `RestClient` is built once in the +`UrlIndexingService` public constructor and passed to `UrlFetcher`. + +### 4.7 Native image + +`RestClient` over `HttpURLConnection` and the JDK `com.sun.net.httpserver.HttpServer` +used by tests are already exercised natively elsewhere in the Spring ecosystem, and a +GraalVM spike on 2026-09-15 confirmed `HttpServer` and https fetches work in this +project's native build without hints. A `@ConfigurationProperties` record needs no +hint under Spring AOT. Do not add to `SolrNativeHints` unless `nativeTest` proves it +necessary. + +--- + +## 5. Tests + +Naming: `*Test` = unit (Mockito allowed, and then `@DisabledInNativeImage`), +`*IntegrationTest` = Testcontainers Solr. Every new test class except the one Mockito +class runs natively. + +| Test | Kind | Asserts | +|---|---|---| +| `UrlTargetPolicyTest` | unit, no mocks | Addresses built with `InetAddress.getByName` on literals, so no DNS. Allow-list: exact match is case-insensitive and ignores a trailing dot; `*.githubusercontent.com` matches `raw.githubusercontent.com` and not `githubusercontent.com`; `*` matches `10.0.0.1` and `localhost`; an empty list rejects `raw.githubusercontent.com`; a non-listed host is rejected with the allow-list message. D2: `169.254.169.254`, `fe80::1`, `fd00:ec2::254`, `::ffff:169.254.169.254` rejected even with `*`; one link-local address among several rejects. Syntax: `ftp://`, `file://`, `data.json`, `/data.json`, `http:///data.json` and a port of 99999 rejected with `INVALID_URL`; `http://user:pw@host/` rejected with the credentials message. D2 literals `100.100.100.200` and `168.63.129.16` rejected with `*`. An exact `::1` entry matches the bracketed host `[::1]`. Allowed: `127.0.0.1`, `::1`, `10.0.0.1`, `93.184.216.34` with `*`. | +| `IndexFormatsTest` | unit, no mocks | `json,csv,xml,md,markdown` in any case with surrounding spaces normalise; `""`, `" "`, `yaml`, `txt`, `html`, `jsonl` throw with the exact message. | +| `UrlFetcherTest` | unit, no mocks | A JDK `HttpServer` on `127.0.0.1:0` with a cached-thread-pool executor (a sleeping handler must not block the dispatcher). Two fetchers: `open` (allow-list `*`) and `restricted` (allow-list `127.0.0.1` only); both connect 5s, read **1s**, cap **1 KB**. Cases with `open` unless stated: 200 with `application/json; charset=utf-8` returns media type `application/json`, UTF-8 and the bytes; `charset=iso-8859-1` honoured; no `Content-Type` gives `""` and UTF-8; `charset=no-such-charset` errors; 404 errors before any body is exposed; absolute and relative `Location` both resolve to the final URI; a chain of exactly 5 redirects succeeds and a chain of 6 fails with the redirect message; a `/loop` that redirects to itself fails with the redirect message; `redirectTarget("https://a/x", "http://a/y")` fails with the downgrade message and `redirectTarget("http://a/x", "https://a/y")` succeeds (static method, no server); a redirect to `http://169.254.169.254/` fails with the D2 message; with `restricted`, a redirect to `http://example.invalid/` fails with the allow-list message and no DNS lookup is attempted (the test cannot observe DNS directly; it asserts the message and that the run takes under one second); `http://169.254.169.254/` fails with the D2 message and the server records no request; `http://nonexistent.invalid/x` throws `UnknownHostException`; a handler that declares `Content-Length: 2048` fails with the size message and the server observes the body was never requested past the headers (assert via the message and that the handler's `getResponseBody().write` was never reached, using a flag); a chunked handler that sends 1,025 bytes fails with the size message; a handler that sends 10 bytes then sleeps 3 s makes `fetch` throw `java.net.SocketTimeoutException` (the unwrapped cause) within 5 s; a header-recording handler sees `User-agent: solr-mcp` and an `Accept` header and no `Authorization` or `Cookie`. Added in review: 301/303/307/308 are each followed (parameterised); a 302 with no `Location` is the HTTP-status error; `charset="iso-8859-1"` (quoted) is honoured; a redirect to `mailto:` or to a credentialed URL fails with `INVALID_REDIRECT`; a malformed `Location` fails with `INVALID_REDIRECT`; `contentLengthOf` returns -1 for `banana` and for a missing header; and the no-drain proof, `/slow-big`: `Content-Length` 2 MB streamed at 10 KB/s must fail with the size message inside a 5-second `assertTimeoutPreemptively`. Total deadline: a third fetcher with read timeout 5 s and total timeout 1 s against `/drip` (one byte every 200 ms) must throw `SocketTimeoutException` inside a 4-second `assertTimeoutPreemptively`. | +| `UrlIndexingServiceTest` | unit, Mockito, `@DisabledInNativeImage` | `UrlFetcher` and `IndexingService` mocked. One case per row of §4.5 that the service maps (not the fetcher-internal ones, which pass through unchanged and are asserted once); explicit format wins over `.json` extension; `.csv?token=x` resolves to `csv` over `text/plain`; `/data` with `text/xml` resolves to `xml`; the final URI's extension is used when the requested URI has none; `text/plain` with `.txt` errors and `indexPayload` is never called; `text/html` errors with the HTML suffix; unknown explicit format errors before `fetch` is called; blank collection and blank URL error before `fetch` is called; a blank explicit `format` is treated as absent; `text/csv` and `text/markdown` media types resolve; the Solr row is exercised with `SolrServerException`, `IOException` and `SolrException`; concurrency: with a limit of 1 and a fetcher stub blocked on a latch, a second call fails with `busyMessage(1)` while the first is in flight, the first completes after release, and a third call succeeds, proving the permit is returned. Note: the test URL constant must **not** end in `.json` when the case relies on the media type. | +| `IndexingServiceTest` | existing, **unmodified** | Passes after the `indexPayload` refactor. | +| `UrlIndexingIntegrationTest` | integration | `@SpringBootTest` + `@Import(TestcontainersConfiguration.class)`, with `solr.index-url.allowed-hosts=127.0.0.1` and `solr.index-url.max-bytes=64KB` via `@TestPropertySource` (this is the one end-to-end check that the allow-list binds from configuration). A JDK `HttpServer` on `127.0.0.1:0` serves: `/shows.json` as `text/plain; charset=utf-8` (61 records indexed, count 61 via `SolrQuery("*:*")`, summary contains "Indexed field names" and does not contain "Stranger Things"; the CSV and XML summaries also list field names and the Markdown summary does not, which pins the `indexPayload` ternary); generated `/shows.csv` (50 rows), `/shows.xml` (40 `` elements under ``), `/shows.md` (one document); `/export.csv?token=abc` (30 rows, CSV by extension); `/data` with `application/json` (61); `/moved` 302 to `/shows.json`. Failure cases, each asserting the exact message with `assertEquals` (no `startsWith`) and, after an explicit `solrClient.commit`, a collection count of 0: `/data.txt` as `text/plain`; `/page` as `text/html`; `/missing` 404; `/big.csv` (a generated 100 KB CSV with `Content-Length`, over the 64 KB test cap); and `http://example.invalid/x.json` (allow-list message; no DNS is attempted because the allow-list check precedes resolution). The metadata refusal is **not** tested here because the allow-list check would reject `169.254.169.254` first with the allow-list message; it is covered end to end by the client tests, which use `*`. Each test method creates its own `_default` collection named `url__`. | +| `McpClientIntegrationTestBase` | existing, edited | `listToolsReturnsExpectedTools` also asserts `index-url` is present; `toolsExposeBehaviorHints` asserts `assertHint(tools, "index-url", false, true, true)` and `openWorldHint == TRUE`. Gains `serveShowsJson()` returning a loopback `HttpServer` serving `shows.json` as `text/plain`, and `showsJsonUrl(server)`. Both client tests run with allow-list `*` so that the metadata refusal is reachable: HTTP adds `"solr.index-url.allowed-hosts=*"` to its `@SpringBootTest(properties = ...)`; STDIO adds `.addEnvVar("SOLR_INDEX_URL_ALLOWED_HOSTS", "*")` to its `ServerParameters`. | +| `McpClientIntegrationTest` (HTTP) | existing, edited | One round trip: `create-collection shows-url-copy`, `index-url` from the test server, summary contains `61 of 61` and not `Stranger Things`, `search` count 61. One call to `http://169.254.169.254/latest/meta-data/` is an MCP tool error (`isError() == TRUE`) whose text contains `link-local or cloud-metadata`. | +| `McpClientStdioIntegrationTest` | existing, edited | The same round trip and the same refused call. | +| `McpToolRegistrationTest` | existing, edited | Add `UrlIndexingService.class` to the unique-names list and to the `@PreAuthorize` list. It is reflection-only and cannot assert per-transport registration; the base class assertions do that. | + +--- + +## 6. Model-facing and user-facing text + +### 6.1 Model-facing + +**`IndexingService.indexDataPrompt`, step 3** — on `main` the step reads, verbatim: + +``` +3. Index the documents. + - Call `%s` with `collection=%s` and `%s=`. + - The tool batches internally and commits at the end. ... + - On error, read the message carefully: ... +``` + +Replace only the first bullet (`- Call `%s` with ...`) with the three bullets below, in +this order, leaving the other two bullets and every `%s` placeholder and its +`String.format` argument unchanged (the third new bullet reuses the same three +placeholders in the same order as the bullet it replaces): + +> - If the data is reachable at an http(s) URL and is within the server's size limit +> (10 MB unless the operator changed it), prefer `index-url` with `collection` and +> `url`; optionally override the detected `format`. The URL is fetched by the MCP +> server, so it must be reachable from the server's network and its host must be on +> the server's allow-list (GitHub raw content by default). +> - If the data is larger than that limit, or is a file on the user's machine that is +> too large to paste, do not push it through this conversation. Give the user this +> command to run where the file is, with their collection name and Solr URL filled +> in, then continue with step 4: +> `bin/solr post -c ` +> or `curl -X POST '//update?commit=true' -H 'Content-Type: application/json' --data-binary @` +> (use `Content-Type: text/csv` or `application/xml` for those formats). +> - Otherwise, for small pasted or attached data, call `%s` with `collection=%s` and +> `%s=`. Use one path only; do not also send inline data after a +> successful URL call. + +**`spring.ai.mcp.server.instructions`** in `application.properties` — on `main` the +value is three sentences: "This server provides tools …", "Discover before acting: … +before searching or indexing.", "Surface boundaries: …". Insert the following as a new +sentence between the second and the third (i.e. immediately before "Surface +boundaries:"), as its own continuation line ending in ` \`: + +> For data at an http(s) URL under the size limit, prefer index-url; for larger +> datasets tell the user to run bin/solr post or curl against Solr's /update handler; +> use the inline indexing tools only for small pasted data. + +### 6.2 User-facing + +- **`README.md` tool table**: add, directly after the `index-json-documents` row, + `| index-url | Index a UTF-8 JSON, CSV, XML or Markdown document from an http(s) URL on the allow-list (both transports; default 10 MB limit) |`. +- **`README.md`**, after the "Before indexing" paragraph, a paragraph headed + **Index from a URL:** giving the `shows.json` raw GitHub example call, stating that the + server fetches from its own network with no credentials, that only allow-listed hosts + are fetched with the default list spelled out, that `SOLR_INDEX_URL_ALLOWED_HOSTS` + accepts exact hosts, `*.suffix` patterns or `*`, that the default limit is 10 MB via + `SOLR_INDEX_URL_MAX_BYTES`, and that larger datasets go straight to Solr with + `bin/solr post`. Mention `SOLR_INDEX_URL_CONNECT_TIMEOUT` (10s) and + `SOLR_INDEX_URL_READ_TIMEOUT` (30s) in one sentence. +- **`docs/tutorial.md`**: in the ingestion section, replace the sentences that say the + server does not fetch URLs with: use `index-url` with the raw GitHub URL of + `shows.json`; the host is on the default allow-list; for a file on your own machine + either paste small data or run `bin/solr post -c shows shows.json` where the file is, + because the client cannot send files to the server and the server cannot see your + disk. +- **`docs/security/stdio.md`**, after the `SOLR_URL` bullet: one bullet stating + `index-url` is the only tool that makes an outbound request to a caller-supplied + address; hosts are allow-listed (GitHub raw content by default), link-local and + metadata addresses are always refused, no credentials or caller headers are sent, + and the body is capped. +- **`docs/security/http.md`**, in the "Forbidden" list: one bullet, "Setting + `SOLR_INDEX_URL_ALLOWED_HOSTS=*` on a deployment whose network has internal services + you would not expose to every authenticated MCP caller." + +--- + +## 7. `THREAT_MODEL.md` edits (all required) + +1. **§8 property 5** — change "so the AI client cannot repoint the server or inject a + target URL" to "so the AI client cannot repoint the server's **Solr backend** or its + credentials"; change the *Violation* clause to "a tool argument alters the Solr + backend target or a credential". Append: "`index-url` performs an outbound GET to a + caller-supplied `http(s)` URL whose host must be on an operator allow-list + (GitHub raw content by default); that is a §9-bounded property, not a backend + target, and it never carries credentials or caller-supplied headers." Add + `UrlFetcher` to the documented-in list. +2. **§9** — add as the first bullet: "**It does not verify what an allow-listed URL + serves.** `index-url` fetches any `http(s)` URL whose host matches + `SOLR_INDEX_URL_ALLOWED_HOSTS` (default: GitHub raw-content hosts; `*` allows any + host the server can reach, including loopback and RFC1918). Link-local and + cloud-metadata addresses are refused on every redirect hop regardless. The fetch + carries no credentials or caller headers, refuses an https→http redirect, and reads + at most `SOLR_INDEX_URL_MAX_BYTES` (default 10 MB). The address check runs on the + resolved addresses before the connection is made, so a DNS answer that changes in + between (DNS rebinding) can bypass it; with the default allow-list that requires + control of a GitHub host's DNS, and with `*` the operator has accepted the network + boundary. *(documented — `UrlTargetPolicy`, `UrlFetcher`.)*" +3. **§11** — add: "**Setting `SOLR_INDEX_URL_ALLOWED_HOSTS=*` on a network with + reachable internal services** that you would not expose to every authenticated MCP + caller. *(documented — docs/security/http.md.)*" +4. **§11a** — change the `SOLR_URL` non-finding to say "The *Solr* target is + deployer-only startup config". Add: "**\"`index-url` allows SSRF.\"** With the default + allow-list the server fetches only GitHub raw-content hosts: `KNOWN-NON-FINDING`. + With `*` the operator has chosen the boundary: `OUT-OF-MODEL: trusted-input`. A + report is `VALID` only if it shows a non-allow-listed host being fetched, a + credential or caller header being forwarded, a refused address being reached other + than through DNS rebinding (§9), or an https→http downgrade being followed." +5. **§12** — replace the "backend-target or credential from a tool argument" bullet + with: "Allowing a **credential** to originate from a tool argument or per-request + input (would open credential-relay). *The backend-target half was exercised + deliberately on 2026-09-15 by `index-url` + ([#208](https://github.com/apache/solr-mcp/issues/208)) behind an operator + allow-list; see §8.5 and §9.*" Add a new bullet: "Changing the **default** of + `SOLR_INDEX_URL_ALLOWED_HOSTS` to `*`." +6. **§13** — in the `VALID` row change "tool-arg repoints backend" to "tool-arg repoints + the Solr backend or forwards a credential, `index-url` fetches a non-allow-listed + host". In the `OUT-OF-MODEL: trusted-input` row add `SOLR_INDEX_URL_ALLOWED_HOSTS=*` + to the examples. +7. **§5a** — add rows: `SOLR_INDEX_URL_ALLOWED_HOSTS` (default + `raw.githubusercontent.com,*.githubusercontent.com,github.com`; "which hosts + `index-url` may fetch; `*` widens the boundary to the server's whole network"); + `SOLR_INDEX_URL_MAX_BYTES` (`10MB`; "caps one fetch; memory per call is a small + multiple"); `SOLR_INDEX_URL_READ_TIMEOUT` (`30s`; per read); + `SOLR_INDEX_URL_TOTAL_TIMEOUT` (`5m`; whole fetch, redirects included); + `SOLR_INDEX_URL_MAX_CONCURRENT_FETCHES` (`4`; further calls fail immediately). The + connect timeout is operational and is not listed. +8. **§5a "How HTTP mode enforces auth"** — "all 11 tools" becomes "all 13 tools". + `main` at `b4ffe18` has 12 `@McpTool` methods (`git grep -c "@McpTool("` summed + over `src/main/java`); the document's "11" was already stale. This PR adds one. +9. **§1** — "It exposes eleven tools (search, three indexing formats, …)" becomes + "It exposes thirteen tools (search, four inline indexing formats, URL ingestion, + collection create/list/stats/health, schema get/add-fields/add-field-types)"; the + §2 component table's "Write/index tools" row becomes + `index-json/csv/xml/markdown-documents`, `index-url` with "writes backend Solr + index; `index-url` also makes an outbound GET to an allow-listed host" in the + "Touches outside process?" column. + +--- + +## 8. Delivery and definition of done + +**One PR** from `feat/index-url` (fork) against `apache/solr-mcp` `main`, opened as a +**draft** first so the threat-model changes can be reviewed while the rest is +finished. The description cites #208 (design), #194 (why there is no `index-file`), +#197 (why one tool), and #205 (no conflict: this PR adds no parser code and touches +only the four inline tool methods to extract `indexPayload`). It states the two +D12 defaults as the points needing maintainer confirmation. + +**Implementation order:** `IndexFormats` + test → `UrlTargetPolicy` + test → +`UrlIndexingProperties` and `SolrConfig` registration → `UrlFetcher` + test → +`IndexingService.indexPayload` refactor (run `IndexingServiceTest` unmodified) → +`UrlIndexingService` + unit test → `McpToolRegistrationTest` lists → integration test → +client tests → §6 → §7 → §8 checks. Each step is test-first: write the test, run it +and see it fail for the expected reason, then implement. + +**Done means all of the following, run serially with JDK 25 on `JAVA_HOME`, with the +outputs pasted in the PR:** + +```bash +./gradlew spotlessApply +./gradlew build # rat, spotlessCheck, buildSrc tests, all tests +grep -L 'skipped="0"' build/test-results/test/*.xml +# must print exactly one file: TEST-...OtlpExportIntegrationTest.xml, which is +# @Disabled on main since 375a710 and is not this PR's concern +./gradlew nativeTest -Pnative # GraalVM JDK 25 on JAVA_HOME +grep -B1 ' "json"; + case "csv" -> "csv"; + case "xml" -> "xml"; + case "md", "markdown" -> "markdown"; + default -> throw new IllegalArgumentException(UNKNOWN_FORMAT); + }; + } +} diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java b/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java index 13504967..6cf79151 100644 --- a/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java +++ b/src/main/java/org/apache/solr/mcp/server/indexing/IndexingService.java @@ -219,10 +219,7 @@ public IndexingService(SolrClient solrClient, IndexingDocumentCreator indexingDo public String indexJsonDocuments(@McpToolParam(description = "Solr collection to index into") String collection, @McpToolParam(description = "JSON string containing documents to index") String json) throws IOException, SolrServerException { - List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromJson(json); - int successCount = indexDocuments(collection, schemalessDoc); - return "Successfully indexed " + successCount + " of " + schemalessDoc.size() + " documents into collection '" - + collection + "'" + describeIndexedFields(schemalessDoc); + return indexPayload(collection, json, "json"); } /** @@ -294,10 +291,7 @@ public String indexJsonDocuments(@McpToolParam(description = "Solr collection to public String indexCsvDocuments(@McpToolParam(description = "Solr collection to index into") String collection, @McpToolParam(description = "CSV string containing documents to index") String csv) throws IOException, SolrServerException { - List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromCsv(csv); - int successCount = indexDocuments(collection, schemalessDoc); - return "Successfully indexed " + successCount + " of " + schemalessDoc.size() + " documents into collection '" - + collection + "'" + describeIndexedFields(schemalessDoc); + return indexPayload(collection, csv, "csv"); } /** @@ -393,10 +387,7 @@ public String indexCsvDocuments(@McpToolParam(description = "Solr collection to public String indexXmlDocuments(@McpToolParam(description = "Solr collection to index into") String collection, @McpToolParam(description = "XML string containing documents to index") String xml) throws ParserConfigurationException, SAXException, IOException, SolrServerException { - List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromXml(xml); - int successCount = indexDocuments(collection, schemalessDoc); - return "Successfully indexed " + successCount + " of " + schemalessDoc.size() + " documents into collection '" - + collection + "'" + describeIndexedFields(schemalessDoc); + return indexPayload(collection, xml, "xml"); } /** @@ -468,10 +459,48 @@ public String indexMarkdownDocuments(@McpToolParam(description = "Solr collectio @McpToolParam( description = "Markdown string to index, optionally starting with YAML front matter") String markdown) throws IOException, SolrServerException { - List schemalessDoc = indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(markdown); + return indexPayload(collection, markdown, "markdown"); + } + + /** + * Trailing sentence for indexing tool descriptions: prepare the schema before + * indexing rather than relying on schemaless guesses. + */ + static final String SCHEMA_FIRST_GUIDANCE = "Before indexing, use get-schema and add-fields (or design-schema) " + + "to define compatible fields. Use string with docValues for categories/facets, text_general for prose, " + + "and explicit numeric types and multiValued settings. Do not rely on schemaless type guessing; " + + "existing field types cannot be changed with these tools."; + + /** + * The one indexing path shared by the four inline tools and {@code index-url}: + * parse the whole payload with the creator for its format, batch-index the + * documents, and summarise. Structured formats also list the indexed field + * names; Markdown, being one document, does not. + * + * @param collection + * target collection + * @param payload + * the whole document set as text + * @param format + * {@code json}, {@code csv}, {@code xml} or {@code markdown} + * @return the human-readable summary the tools return + * @throws IOException + * on Solr communication failure + * @throws SolrServerException + * if Solr rejects the update + */ + String indexPayload(String collection, String payload, String format) throws IOException, SolrServerException { + List schemalessDoc = switch (format) { + case "json" -> indexingDocumentCreator.createSchemalessDocumentsFromJson(payload); + case "csv" -> indexingDocumentCreator.createSchemalessDocumentsFromCsv(payload); + case "xml" -> indexingDocumentCreator.createSchemalessDocumentsFromXml(payload); + case "markdown" -> indexingDocumentCreator.createSchemalessDocumentsFromMarkdown(payload); + default -> throw new IllegalArgumentException("Unsupported document format: " + format); + }; int successCount = indexDocuments(collection, schemalessDoc); - return "Successfully indexed " + successCount + " of " + schemalessDoc.size() + " documents into collection '" - + collection + "'"; + String summary = "Successfully indexed " + successCount + " of " + schemalessDoc.size() + + " documents into collection '" + collection + "'"; + return format.equals("markdown") ? summary : summary + describeIndexedFields(schemalessDoc); } /** @@ -677,7 +706,21 @@ public String indexDataPrompt( %s 3. Index the documents. - - Call `%s` with `collection=%s` and `%s=`. + - If the data is reachable at an http(s) URL and is within the server's size limit + (10 MB unless the operator changed it), prefer `index-url` with `collection` and + `url`; optionally override the detected `format`. The URL is fetched by the MCP + server, so it must be reachable from the server's network and its host must be on + the server's allow-list (GitHub raw content by default). + - If the data is larger than that limit, or is a file on the user's machine that is + too large to paste, do not push it through this conversation. Give the user this + command to run where the file is, with their collection name and Solr URL filled + in, then continue with step 4: + `bin/solr post -c ` + or `curl -X POST '//update?commit=true' -H 'Content-Type: application/json' --data-binary @` + (use `Content-Type: text/csv` or `application/xml` for those formats). + - Otherwise, for small pasted or attached data, call `%s` with `collection=%s` and + `%s=`. Use one path only; do not also send inline data after a + successful URL call. - The tool batches internally and commits at the end. The return value is the count of successfully indexed documents. - On error, read the message carefully: an "unknown field" error means the schema is @@ -691,7 +734,8 @@ public String indexDataPrompt( Next step suggestion: once data is indexed, the `search-collection` prompt drives searching it. - """.formatted(indexTool.paramName(), collection, collection, sampleSection, indexTool.name(), - collection, indexTool.paramName(), collection); + """ + .formatted(indexTool.paramName(), collection, collection, sampleSection, indexTool.name(), collection, + indexTool.paramName(), collection); } } diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/UrlFetcher.java b/src/main/java/org/apache/solr/mcp/server/indexing/UrlFetcher.java new file mode 100644 index 00000000..04efeee0 --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/indexing/UrlFetcher.java @@ -0,0 +1,303 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.InetAddress; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.nio.charset.Charset; +import java.nio.charset.IllegalCharsetNameException; +import java.nio.charset.StandardCharsets; +import java.nio.charset.UnsupportedCharsetException; +import java.util.List; +import java.util.Locale; +import org.springframework.http.HttpHeaders; +import org.springframework.http.client.SimpleClientHttpRequestFactory; +import org.springframework.web.client.ResourceAccessException; +import org.springframework.web.client.RestClient; + +/** + * Performs the {@code index-url} GET with Spring's {@link RestClient} over + * {@code HttpURLConnection}: resolves and policy-checks the host on every hop, + * follows up to {@value #MAX_REDIRECTS} redirects without downgrading to plain + * http, sends only {@code Accept} and {@code User-Agent}, refuses bodies over + * the configured cap, and returns the whole 2xx body with its media type and + * charset. Caller-fixable problems surface as {@link IllegalArgumentException} + * with the messages the tool returns verbatim; network failures surface as + * {@link IOException}, which the read timeout turns into a + * {@link java.net.SocketTimeoutException} when a body stops arriving. + */ +final class UrlFetcher { + + static final int MAX_REDIRECTS = 5; + static final String ACCEPT = "application/json, text/csv, application/xml, text/xml, text/markdown, " + + "text/plain;q=0.5, */*;q=0.1"; + static final String USER_AGENT = "solr-mcp"; + static final String TOO_MANY_REDIRECTS = "The URL redirected more than " + MAX_REDIRECTS + + " times. Use the final URL directly."; + static final String DOWNGRADE = "The URL redirects from https to http, which is refused. " + + "Use the final https URL directly."; + static final String INVALID_REDIRECT = "The URL redirected to an invalid location. Use the final URL directly."; + static final String UNSUPPORTED_CHARSET = "The URL declares an unsupported charset. Supply a UTF-8 document."; + + private final RestClient restClient; + private final List allowedHosts; + private final int maxBytes; + private final long totalTimeoutNanos; + private final String tooLarge; + + /** + * A 2xx response, read in full. + * + * @param finalUri + * the URL that answered, after redirects + * @param mediaType + * lower-cased media type without parameters, or empty if the + * response carried no {@code Content-Type} + * @param charset + * the declared charset, or UTF-8 when none was declared + * @param body + * the raw body bytes, at most the configured cap + */ + record FetchedBody(URI finalUri, String mediaType, Charset charset, byte[] body) { + } + + /** Outcome of one request; only a 2xx carries a body. */ + private sealed interface Exchange { + } + + private record Redirect(String location) implements Exchange { + } + + private record Status(int code) implements Exchange { + } + + private record Body(String mediaType, Charset charset, byte[] bytes) implements Exchange { + } + + UrlFetcher(UrlIndexingProperties properties) { + var factory = new SimpleClientHttpRequestFactory() { + @Override + protected void prepareConnection(HttpURLConnection connection, String httpMethod) throws IOException { + super.prepareConnection(connection, httpMethod); + connection.setInstanceFollowRedirects(false); // the policy must see every hop + } + }; + factory.setConnectTimeout(properties.connectTimeout()); + factory.setReadTimeout(properties.readTimeout()); + this.restClient = RestClient.builder().requestFactory(factory).build(); + this.allowedHosts = properties.allowedHosts(); + this.maxBytes = (int) properties.maxBytes().toBytes(); // the record bounds it below Integer.MAX_VALUE + this.totalTimeoutNanos = properties.totalTimeout().toNanos(); + long bytes = properties.maxBytes().toBytes(); + String limit = bytes % 1048576 == 0 ? properties.maxBytes().toMegabytes() + " MB" : bytes + " bytes"; + this.tooLarge = "The document is larger than this server's limit of " + limit + "; nothing was indexed. " + + "Index datasets this large directly with Solr (bin/solr post or the /update handler); " + + "the index-data prompt shows the command."; + } + + /** + * Fetches a URL, following redirects. + * + * @param uri + * the caller-supplied URL + * @return the 2xx response, read in full + * @throws IllegalArgumentException + * for a refused URL or host, too many redirects, an https to http + * downgrade, a non-2xx status, an unsupported charset, or a body + * over the cap + * @throws IOException + * if the host does not resolve, the connection fails, or a read + * times out ({@link java.net.SocketTimeoutException}) + */ + FetchedBody fetch(URI uri) throws IOException { + URI current = uri; + int redirects = 0; + long deadline = System.nanoTime() + totalTimeoutNanos; + while (true) { + if (System.nanoTime() - deadline > 0) { + throw new SocketTimeoutException("total timeout exceeded before hop " + redirects); + } + try { + UrlTargetPolicy.check(current, allowedHosts, List.of()); // syntax and allow-list before any DNS + } catch (IllegalArgumentException e) { + boolean syntactic = UrlTargetPolicy.INVALID_URL.equals(e.getMessage()) + || UrlTargetPolicy.EMBEDDED_CREDENTIALS.equals(e.getMessage()); + if (redirects > 0 && syntactic) { + throw new IllegalArgumentException(INVALID_REDIRECT); // the caller never supplied this URL + } + throw e; + } + UrlTargetPolicy.check(current, allowedHosts, List.of(InetAddress.getAllByName(current.getHost()))); + switch (send(current, deadline)) { + case Redirect redirect -> { + if (++redirects > MAX_REDIRECTS) { + throw new IllegalArgumentException(TOO_MANY_REDIRECTS); + } + current = redirectTarget(current, redirect.location()); + } + case Status status -> throw new IllegalArgumentException("The URL returned HTTP " + status.code() + + "; nothing was indexed. Check that it is public and points at a raw document, not a web page."); + case Body body -> { + return new FetchedBody(current, body.mediaType(), body.charset(), body.bytes()); + } + } + } + } + + /** + * Resolves a {@code Location} header against the current URL, refusing an https + * to http downgrade. + */ + static URI redirectTarget(URI current, String location) { + URI target; + try { + target = current.resolve(location); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException(INVALID_REDIRECT); // never the JDK's parse message + } + if ("https".equalsIgnoreCase(current.getScheme()) && "http".equalsIgnoreCase(target.getScheme())) { + throw new IllegalArgumentException(DOWNGRADE); + } + return target; + } + + private Exchange send(URI uri, long deadline) throws IOException { + try { + return restClient.get().uri(uri).header("Accept", ACCEPT).header("User-Agent", USER_AGENT) + .exchange((request, response) -> { + int status = response.getStatusCode().value(); + if (isRedirect(status)) { + String location = response.getHeaders().getFirst("Location"); + if (location != null) { + abandon(response); + return new Redirect(location); + } + } + if (status / 100 != 2) { + abandon(response); + return new Status(status); + } + String contentType = response.getHeaders().getFirst("Content-Type"); + if (contentType == null) { + contentType = ""; + } + Charset charset; + try { + charset = charsetOf(contentType); + } catch (IllegalCharsetNameException | UnsupportedCharsetException e) { + throw new IllegalArgumentException(UNSUPPORTED_CHARSET); + } + if (contentLengthOf(response.getHeaders()) > maxBytes) { + abandon(response); // before reading a byte + throw new IllegalArgumentException(tooLarge); + } + byte[] bytes; + try { + bytes = readCapped(response.getBody(), maxBytes + 1, deadline); + } catch (IOException e) { + abandon(response); // a deadline or read failure must not turn into a drain + throw e; + } + if (bytes.length > maxBytes) { + abandon(response); + throw new IllegalArgumentException(tooLarge); + } + return new Body(mediaTypeOf(contentType), charset, bytes); + }); + } catch (ResourceAccessException e) { + if (e.getCause() instanceof IOException io) { + throw io; + } + throw new IOException(e.getMessage(), e); + } + } + + /** + * Closes the body stream before Spring's own {@code close()} runs. Spring + * drains an unread body to keep the connection reusable, which would download a + * refused or over-cap response in full; closing the stream first makes the JDK + * drop the connection (or hand at most a small remainder to its keep-alive + * cleaner) and turns Spring's drain into a no-op on a closed stream. + */ + private static void abandon(org.springframework.http.client.ClientHttpResponse response) { + try { + response.getBody().close(); + } catch (IOException ignored) { + // nothing to abandon + } + } + + /** + * Reads at most {@code limit} bytes, checking the total deadline after every + * chunk so a host that keeps sending slowly cannot outlast the per-read + * timeout. + */ + private static byte[] readCapped(InputStream in, int limit, long deadline) throws IOException { + var out = new ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + int n; + while (out.size() < limit && (n = in.read(buffer, 0, Math.min(buffer.length, limit - out.size()))) != -1) { + out.write(buffer, 0, n); + if (System.nanoTime() - deadline > 0) { + throw new SocketTimeoutException("total timeout exceeded while reading the body"); + } + } + return out.toByteArray(); + } + + /** {@code Content-Length} as a long, or -1 when absent or not a number. */ + static long contentLengthOf(HttpHeaders headers) { + String value = headers.getFirst(HttpHeaders.CONTENT_LENGTH); + if (value == null) { + return -1; + } + try { + return Long.parseLong(value.trim()); + } catch (NumberFormatException e) { + return -1; + } + } + + private static boolean isRedirect(int status) { + return status == 301 || status == 302 || status == 303 || status == 307 || status == 308; + } + + private static String mediaTypeOf(String contentType) { + int semicolon = contentType.indexOf(';'); + String type = semicolon < 0 ? contentType : contentType.substring(0, semicolon); + return type.trim().toLowerCase(Locale.ROOT); + } + + private static Charset charsetOf(String contentType) { + for (String parameter : contentType.split(";")) { + String trimmed = parameter.trim(); + if (trimmed.regionMatches(true, 0, "charset=", 0, 8)) { + String name = trimmed.substring(8).trim(); + if (name.length() >= 2 && name.startsWith("\"") && name.endsWith("\"")) { + name = name.substring(1, name.length() - 1); + } + return Charset.forName(name); + } + } + return StandardCharsets.UTF_8; + } +} diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/UrlIndexingProperties.java b/src/main/java/org/apache/solr/mcp/server/indexing/UrlIndexingProperties.java new file mode 100644 index 00000000..d2babacb --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/indexing/UrlIndexingProperties.java @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import java.time.Duration; +import java.util.List; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.bind.DefaultValue; +import org.springframework.util.unit.DataSize; + +/** + * Limits on {@code index-url} fetches. Hosts are allow-listed (GitHub raw + * content by default; {@code *} allows any host); link-local addresses and the + * known cloud-metadata addresses are refused regardless. The read timeout + * applies to every socket read, so it bounds a body that stops arriving; the + * total timeout bounds a whole fetch including redirects, so a host that drips + * bytes cannot hold a call open indefinitely; the size cap bounds memory per + * call, because the body is parsed in memory; and the concurrency limit bounds + * how many such calls run at once. + * + * @param allowedHosts + * exact hosts, {@code *.suffix} patterns, or {@code *} + * ({@code SOLR_INDEX_URL_ALLOWED_HOSTS}); an empty list allows + * nothing + * @param connectTimeout + * TCP/TLS connect timeout ({@code SOLR_INDEX_URL_CONNECT_TIMEOUT}) + * @param readTimeout + * longest wait for any single read, headers or body + * ({@code SOLR_INDEX_URL_READ_TIMEOUT}) + * @param maxBytes + * body size cap, between 1 byte and 2 GB + * ({@code SOLR_INDEX_URL_MAX_BYTES}) + * @param totalTimeout + * deadline for one whole fetch, redirects included + * ({@code SOLR_INDEX_URL_TOTAL_TIMEOUT}) + * @param maxConcurrentFetches + * how many {@code index-url} calls may run at once; further calls + * fail immediately ({@code SOLR_INDEX_URL_MAX_CONCURRENT_FETCHES}) + */ +@ConfigurationProperties(prefix = "solr.index-url") +public record UrlIndexingProperties(@DefaultValue( { + "raw.githubusercontent.com", "*.githubusercontent.com", "github.com"}) List allowedHosts, + @DefaultValue("10s") Duration connectTimeout, @DefaultValue("30s") Duration readTimeout, + @DefaultValue("10MB") DataSize maxBytes, @DefaultValue("5m") Duration totalTimeout, + @DefaultValue("4") int maxConcurrentFetches){ + + /** Fails at startup rather than on the first tool call. */ + public UrlIndexingProperties { + if (maxBytes.toBytes() < 1 || maxBytes.toBytes() > Integer.MAX_VALUE - 1) { + throw new IllegalArgumentException("solr.index-url.max-bytes must be between 1 byte and 2 GB"); + } + if (totalTimeout.isZero() || totalTimeout.isNegative()) { + throw new IllegalArgumentException("solr.index-url.total-timeout must be positive"); + } + if (maxConcurrentFetches < 1) { + throw new IllegalArgumentException("solr.index-url.max-concurrent-fetches must be at least 1"); + } + } +} diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/UrlIndexingService.java b/src/main/java/org/apache/solr/mcp/server/indexing/UrlIndexingService.java new file mode 100644 index 00000000..19dc5835 --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/indexing/UrlIndexingService.java @@ -0,0 +1,224 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import io.micrometer.observation.annotation.Observed; +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.util.Locale; +import java.util.concurrent.Semaphore; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.common.SolrException; +import org.apache.solr.mcp.server.indexing.documentcreator.DocumentProcessingException; +import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springaicommunity.mcp.annotation.McpTool; +import org.springaicommunity.mcp.annotation.McpToolParam; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.stereotype.Service; + +/** + * URL ingestion for both transports. The server fetches an http(s) URL whose + * host is on the operator's allow-list (GitHub raw content by default), never + * sends credentials or caller headers, caps the body, and hands the decoded + * payload to the same parse-then-index path the inline tools use. Because + * parsing completes before indexing starts, every fetch or parse failure leaves + * the collection untouched. + */ +@Service +@Observed +public class UrlIndexingService { + + private static final Logger logger = LoggerFactory.getLogger(UrlIndexingService.class); + + static final String UNREACHABLE = "Cannot reach the URL from the MCP server. The URL is fetched from the " + + "server's network, not the client's, so localhost and private addresses refer to the server's side. " + + "Check the address and try again."; + static final String READ_TIMEOUT = "The URL did not deliver the document within the read or total timeout; " + + "nothing was indexed. Try again or ask the operator to raise SOLR_INDEX_URL_READ_TIMEOUT or " + + "SOLR_INDEX_URL_TOTAL_TIMEOUT."; + static final String FORMAT_UNRESOLVED = "Cannot determine the format from the URL path or Content-Type. " + + "Supply format=json, csv, xml or markdown."; + static final String HTML_NOT_SUPPORTED = " HTML pages are not supported."; + static final String SOLR_FAILED = "Solr could not complete URL indexing. Check collection availability and " + + "field types with get-schema, then verify the indexed count before retrying; some documents may " + + "already be indexed."; + + private final IndexingService indexingService; + private final UrlFetcher fetcher; + private final Semaphore permits; + private final String busy; + + /** + * Creates the URL-ingestion tool with a fetcher built from the configured + * limits. + * + * @param indexingService + * the indexing pipeline + * @param properties + * allow-list, timeouts, size cap and concurrency limit + */ + @Autowired + public UrlIndexingService(IndexingService indexingService, UrlIndexingProperties properties) { + this(indexingService, new UrlFetcher(properties), properties.maxConcurrentFetches()); + } + + UrlIndexingService(IndexingService indexingService, UrlFetcher fetcher, int maxConcurrentFetches) { + this.indexingService = indexingService; + this.fetcher = fetcher; + this.permits = new Semaphore(maxConcurrentFetches); + this.busy = busyMessage(maxConcurrentFetches); + } + + /** The error returned when {@code max} fetches are already running. */ + static String busyMessage(int max) { + return "The server is already running " + max + " index-url call" + (max == 1 ? "" : "s") + + ", the configured maximum; try again in a moment. Nothing was indexed."; + } + + /** + * Indexes a document set from a URL without returning its contents. + * + * @param collection + * target collection with a prepared schema + * @param url + * absolute http(s) URL on the allow-list, reachable from the server + * @param format + * optional format override; otherwise inferred from the URL path + * extension, then the Content-Type + * @return indexed document counts and field names + */ + @PreAuthorize("isAuthenticated()") + @McpTool( + name = "index-url", + annotations = @McpTool.McpAnnotations(idempotentHint = true, openWorldHint = true), + description = "Index a UTF-8 JSON, CSV, XML or Markdown document set from an http(s) URL without " + + "sending its contents through the model. Available in both STDIO and HTTP mode. The URL is " + + "fetched by the MCP server with no credentials or custom headers; its host must be on the " + + "server's allow-list (GitHub raw content by default) and the body must be within the configured " + + "size limit (default 10 MB). For larger datasets, index directly with Solr (bin/solr post or the " + + "/update handler) instead. Redirects are followed. Non-2xx responses and HTML pages are errors; " + + "nothing is indexed unless the whole document parses. Reuse the URL for another collection. " + + IndexingService.SCHEMA_FIRST_GUIDANCE) + public String indexUrl( + @McpToolParam(description = "Solr collection to index into", required = true) String collection, + @McpToolParam( + description = "Absolute http or https URL of a UTF-8 JSON, CSV, XML or Markdown document, at most " + + "the configured size limit (default 10 MB). The host must be on the server's allow-list " + + "(GitHub raw content by default). Fetched from the MCP server's network, not the " + + "client's. No credentials or custom headers are sent.", + required = true) String url, + @McpToolParam( + description = "Optional format: json, csv, xml, markdown or md; defaults to the URL path " + + "extension, then the Content-Type", + required = false) @Nullable String format) { + if (collection.isBlank()) { + throw new IllegalArgumentException("Provide a non-empty collection name."); + } + URI uri = parse(url); + @Nullable String explicit = format == null || format.isBlank() ? null : IndexFormats.normalize(format); + if (!permits.tryAcquire()) { + throw new IllegalStateException(busy); + } + try { + return fetchAndIndex(collection, uri, explicit); + } finally { + permits.release(); + } + } + + private String fetchAndIndex(String collection, URI uri, @Nullable String explicit) { + UrlFetcher.FetchedBody fetched; + try { + fetched = fetcher.fetch(uri); + } catch (IOException e) { + logger.debug("Could not fetch URL for indexing", e); + throw new IllegalStateException(causedByTimeout(e) ? READ_TIMEOUT : UNREACHABLE); + } + String selected = explicit != null ? explicit : resolveFormat(uri, fetched.finalUri(), fetched.mediaType()); + String payload = new String(fetched.body(), fetched.charset()); + try { + return indexingService.indexPayload(collection, payload, selected); + } catch (DocumentProcessingException e) { + logger.debug("Could not parse URL content for indexing", e); + throw new IllegalArgumentException("Cannot parse the URL content as " + selected + + ". Check its syntax and format. Nothing was indexed."); + } catch (SolrServerException | SolrException | IOException e) { + logger.warn("URL indexing failed for collection {}", collection, e); + throw new IllegalStateException(SOLR_FAILED); + } + } + + private static URI parse(String url) { + if (url.isBlank()) { + throw new IllegalArgumentException(UrlTargetPolicy.INVALID_URL); + } + try { + return URI.create(url.trim()); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException(UrlTargetPolicy.INVALID_URL); + } + } + + private static boolean causedByTimeout(Throwable e) { + for (Throwable t = e; t != null; t = t.getCause() == t ? null : t.getCause()) { + if (t instanceof SocketTimeoutException) { + return true; + } + } + return false; + } + + /** + * Requested URL's path extension, then the final URL's after redirects, then + * the media type. {@code text/plain} carries no information and never resolves; + * {@code text/html} is refused explicitly. + */ + private static String resolveFormat(URI requested, URI finalUri, String mediaType) { + for (URI candidate : new URI[]{requested, finalUri}) { + @Nullable String extension = extensionOf(candidate); + if (extension != null) { + try { + return IndexFormats.normalize(extension); + } catch (IllegalArgumentException ignored) { + // not a supported extension; fall through to the next source + } + } + } + return switch (mediaType) { + case "application/json" -> "json"; + case "text/csv" -> "csv"; + case "application/xml", "text/xml" -> "xml"; + case "text/markdown" -> "markdown"; + case "text/html" -> throw new IllegalArgumentException(FORMAT_UNRESOLVED + HTML_NOT_SUPPORTED); + default -> throw new IllegalArgumentException(FORMAT_UNRESOLVED); + }; + } + + private static @Nullable String extensionOf(URI uri) { + String path = uri.getPath(); + if (path == null) { + return null; + } + String last = path.substring(path.lastIndexOf('/') + 1); + int dot = last.lastIndexOf('.'); + return dot < 0 || dot == last.length() - 1 ? null : last.substring(dot + 1).toLowerCase(Locale.ROOT); + } +} diff --git a/src/main/java/org/apache/solr/mcp/server/indexing/UrlTargetPolicy.java b/src/main/java/org/apache/solr/mcp/server/indexing/UrlTargetPolicy.java new file mode 100644 index 00000000..bb634a74 --- /dev/null +++ b/src/main/java/org/apache/solr/mcp/server/indexing/UrlTargetPolicy.java @@ -0,0 +1,131 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import java.net.InetAddress; +import java.net.URI; +import java.net.UnknownHostException; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +/** + * Pure checks that decide whether {@code index-url} may fetch a URL: absolute + * http(s) with a host and no embedded credentials, a host on the operator's + * allow-list, and no link-local or cloud-metadata address even when the + * allow-list is {@code *}. The caller resolves the host, so this class never + * performs I/O and the same checks run on every redirect hop. + */ +final class UrlTargetPolicy { + + static final String INVALID_URL = "Provide an absolute http or https URL."; + static final String EMBEDDED_CREDENTIALS = "Remove the credentials from the URL; this server never sends credentials."; + static final String HOST_NOT_ALLOWED = "The URL's host is not on this server's allow-list. Allowed by default: " + + "raw.githubusercontent.com, *.githubusercontent.com, github.com. The operator can change " + + "SOLR_INDEX_URL_ALLOWED_HOSTS (use * to allow any host). Nothing was indexed."; + static final String REFUSED_ADDRESS = "This server does not fetch link-local or cloud-metadata addresses."; + + /** + * The EC2 IPv6 instance-metadata address; not link-local, so listed explicitly. + */ + private static final Set METADATA_ADDRESSES = Set.of(literal("fd00:ec2::254"), // AWS IPv6 + literal("100.100.100.200"), // Alibaba Cloud + literal("168.63.129.16")); // Azure WireServer + + private UrlTargetPolicy() { + } + + /** + * Rejects URLs this server will not fetch. + * + * @param uri + * the URL as supplied, or the target of a redirect + * @param allowedHosts + * exact hosts, {@code *.suffix} patterns, or {@code *}; an empty + * list allows nothing + * @param resolved + * every address the host resolves to; empty when only the syntactic + * and allow-list checks are wanted + * @throws IllegalArgumentException + * with the message the tool returns verbatim + */ + static void check(URI uri, List allowedHosts, List resolved) { + String scheme = uri.getScheme(); + String host = uri.getHost(); + if (scheme == null || host == null || uri.getPort() > 65535) { + // java.net.URI accepts any integer port; HttpURLConnection would later throw + // a raw "port out of range" IllegalArgumentException that is not one of ours + throw new IllegalArgumentException(INVALID_URL); + } + String lowered = scheme.toLowerCase(Locale.ROOT); + if (!lowered.equals("http") && !lowered.equals("https")) { + throw new IllegalArgumentException(INVALID_URL); + } + if (uri.getUserInfo() != null) { + throw new IllegalArgumentException(EMBEDDED_CREDENTIALS); + } + if (!isAllowed(host, allowedHosts)) { + throw new IllegalArgumentException(HOST_NOT_ALLOWED); + } + for (InetAddress address : resolved) { + if (address.isLinkLocalAddress() || METADATA_ADDRESSES.contains(address)) { + throw new IllegalArgumentException(REFUSED_ADDRESS); + } + } + } + + /** + * Allow-list matching: {@code *} matches everything; {@code *.suffix} matches + * any host that ends in {@code .suffix} and is longer than it; any other entry + * must equal the host. Comparison is case-insensitive and ignores a trailing + * dot on the host. + */ + static boolean isAllowed(String host, List allowedHosts) { + String normalized = normalizeHost(host); + for (String entry : allowedHosts) { + String pattern = entry.trim().toLowerCase(Locale.ROOT); + if (pattern.equals("*")) { + return true; + } + if (pattern.startsWith("*.")) { + String suffix = pattern.substring(1); // ".example.com" + if (normalized.endsWith(suffix) && normalized.length() > suffix.length()) { + return true; + } + } else if (pattern.equals(normalized)) { + return true; + } + } + return false; + } + + private static String normalizeHost(String host) { + String lowered = host.toLowerCase(Locale.ROOT); + if (lowered.startsWith("[") && lowered.endsWith("]")) { + lowered = lowered.substring(1, lowered.length() - 1); // IPv6 literal + } + return lowered.endsWith(".") ? lowered.substring(0, lowered.length() - 1) : lowered; + } + + private static InetAddress literal(String address) { + try { + return InetAddress.getByName(address); + } catch (UnknownHostException e) { + throw new IllegalStateException("Not a literal address: " + address, e); + } + } +} diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 089d7d3f..ca549ce6 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -18,10 +18,22 @@ spring.application.name=solr-mcp spring.profiles.active=${PROFILES:stdio} spring.ai.mcp.server.instructions=This server provides tools to search, index, and manage Apache Solr collections. \ Discover before acting: use list-collections and get-schema to see what exists before searching or indexing. \ +For data at an http(s) URL under the size limit, prefer index-url; for larger datasets tell the user to run \ +bin/solr post or curl against Solr's /update handler; use the inline indexing tools only for small pasted data. \ Surface boundaries: there are no tools to delete documents or collections, drop or alter existing fields, \ or debug relevance scoring (debugQuery); vector/KNN search is not exposed; search facets are simple field \ facets only (no range, pivot, or JSON facets). Schema modification is additive only via add-fields and \ add-field-types; changing an existing field's type requires reindexing and is not supported by this server. +# index-url: which hosts may be fetched (exact host, *.suffix, or * for any), the +# connect, per-read and whole-fetch timeouts, the maximum body size, and how many +# fetches may run at once. Link-local addresses and the known cloud-metadata +# addresses (AWS, Alibaba Cloud, Azure) are refused regardless of the allow-list. +solr.index-url.allowed-hosts=raw.githubusercontent.com,*.githubusercontent.com,github.com +solr.index-url.connect-timeout=10s +solr.index-url.read-timeout=30s +solr.index-url.total-timeout=5m +solr.index-url.max-bytes=10MB +solr.index-url.max-concurrent-fetches=4 spring.ai.mcp.server.name=${spring.application.name} spring.ai.mcp.server.version=1.0.0 # Solr configuration diff --git a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTest.java b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTest.java index 71db0ccc..71f2539c 100644 --- a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTest.java @@ -16,10 +16,17 @@ */ package org.apache.solr.mcp.server; +import static org.junit.jupiter.api.Assertions.*; + +import com.fasterxml.jackson.core.type.TypeReference; import io.modelcontextprotocol.client.McpClient; import io.modelcontextprotocol.client.McpSyncClient; import io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport; +import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; +import java.util.Map; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.web.server.LocalServerPort; import org.springframework.context.annotation.Import; @@ -33,7 +40,8 @@ */ @SpringBootTest( webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, - properties = {"http.security.enabled=false", "spring.docker.compose.enabled=false"}) + properties = {"http.security.enabled=false", "spring.docker.compose.enabled=false", + "solr.index-url.allowed-hosts=*"}) @ActiveProfiles("http") @Import(TestcontainersConfiguration.class) @Tag("integration") @@ -49,4 +57,40 @@ protected McpSyncClient createClient() { return McpClient.sync(transport).build(); } + /** + * URL ingestion must work over HTTP exactly as it does over STDIO (#208): the + * base class asserts the tool and its hints; this is the round trip. + */ + @Test + @Order(42) + void indexesFromAUrlThroughHttpMcp() throws Exception { + var server = serveShowsJson(); + try { + String collection = "shows-url-copy"; + assertNotError(mcpClient.callTool(new CallToolRequest("create-collection", Map.of("name", collection)))); + var indexed = mcpClient.callTool( + new CallToolRequest("index-url", Map.of("collection", collection, "url", showsJsonUrl(server)))); + assertNotError(indexed); + assertTrue(extractText(indexed).contains("61 of 61"), extractText(indexed)); + assertFalse(extractText(indexed).contains("Stranger Things"), "payload leaked into the summary"); + var searched = mcpClient.callTool( + new CallToolRequest("search", Map.of("collection", collection, "query", "*:*", "rows", 0))); + assertNotError(searched); + Map response = OBJECT_MAPPER.readValue(extractText(searched), new TypeReference<>() { + }); + assertEquals(SHOWS_DOC_COUNT, getNumFound(response)); + } finally { + server.stop(0); + } + } + + @Test + @Order(43) + void aRefusedAddressIsAnMcpToolError() { + var result = mcpClient.callTool(new CallToolRequest("index-url", + Map.of("collection", SHOWS_COLLECTION, "url", "http://169.254.169.254/latest/meta-data/"))); + assertEquals(Boolean.TRUE, result.isError()); + assertTrue(extractText(result).contains("link-local or cloud-metadata"), extractText(result)); + } + } diff --git a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java index 3b9f2302..13a37a72 100644 --- a/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java +++ b/src/test/java/org/apache/solr/mcp/server/McpClientIntegrationTestBase.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import com.sun.net.httpserver.HttpServer; import io.modelcontextprotocol.client.McpSyncClient; import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; import io.modelcontextprotocol.spec.McpSchema.CallToolResult; @@ -33,8 +34,12 @@ import io.modelcontextprotocol.spec.McpSchema.ResourceReference; import io.modelcontextprotocol.spec.McpSchema.TextContent; import io.modelcontextprotocol.spec.McpSchema.Tool; +import java.io.IOException; import java.io.InputStream; +import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.List; import java.util.Map; import java.util.Objects; @@ -105,6 +110,7 @@ void listToolsReturnsExpectedTools() { assertTrue(toolNames.contains("create-collection"), "Should have create-collection tool"); assertTrue(toolNames.contains("index-json-documents"), "Should have index-json-documents tool"); assertTrue(toolNames.contains("index-markdown-documents"), "Should have index-markdown-documents tool"); + assertTrue(toolNames.contains("index-url"), "index-url must be registered in every transport"); assertTrue(toolNames.contains("search"), "Should have search tool"); assertTrue(toolNames.contains("list-collections"), "Should have list-collections tool"); assertTrue(toolNames.contains("check-health"), "Should have check-health tool"); @@ -137,6 +143,11 @@ void toolsExposeBehaviorHints() { assertHint(tools, "index-csv-documents", false, true, true); assertHint(tools, "index-xml-documents", false, true, true); assertHint(tools, "index-markdown-documents", false, true, true); + // index-url: same write semantics, and openWorld because it contacts a + // caller-chosen external system. + assertHint(tools, "index-url", false, true, true); + assertEquals(Boolean.TRUE, tools.get("index-url").annotations().openWorldHint(), + "index-url should declare openWorldHint"); } private static void assertReadOnly(Map tools, String name) { @@ -845,6 +856,30 @@ private static String loadClasspathResource(String resourcePath) throws Exceptio } } + /** + * Serves {@code src/test/resources/shows.json} at {@code /shows.json} on an + * ephemeral loopback port, as {@code text/plain} the way raw GitHub does, so + * {@code index-url} has to resolve the format from the extension. The caller + * stops the server. + */ + protected static HttpServer serveShowsJson() throws IOException { + byte[] body = Files.readAllBytes(Path.of("src/test/resources/shows.json")); + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/shows.json", exchange -> { + exchange.getResponseHeaders().add("Content-Type", "text/plain; charset=utf-8"); + exchange.sendResponseHeaders(200, body.length); + try (var out = exchange.getResponseBody()) { + out.write(body); + } + }); + server.start(); + return server; + } + + protected static String showsJsonUrl(HttpServer server) { + return "http://127.0.0.1:" + server.getAddress().getPort() + "/shows.json"; + } + protected static String extractText(CallToolResult result) { assertNotNull(result.content(), "Result content should not be null"); assertFalse(result.content().isEmpty(), "Result content should not be empty"); diff --git a/src/test/java/org/apache/solr/mcp/server/McpClientStdioIntegrationTest.java b/src/test/java/org/apache/solr/mcp/server/McpClientStdioIntegrationTest.java index c6120a72..490fcf8e 100644 --- a/src/test/java/org/apache/solr/mcp/server/McpClientStdioIntegrationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/McpClientStdioIntegrationTest.java @@ -16,13 +16,20 @@ */ package org.apache.solr.mcp.server; +import static org.junit.jupiter.api.Assertions.*; + +import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import io.modelcontextprotocol.client.McpClient; import io.modelcontextprotocol.client.McpSyncClient; import io.modelcontextprotocol.client.transport.ServerParameters; import io.modelcontextprotocol.client.transport.StdioClientTransport; import io.modelcontextprotocol.json.jackson.JacksonMcpJsonMapper; +import io.modelcontextprotocol.spec.McpSchema.CallToolRequest; +import java.util.Map; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; import org.testcontainers.containers.SolrContainer; import org.testcontainers.junit.jupiter.Container; import org.testcontainers.junit.jupiter.Testcontainers; @@ -47,10 +54,43 @@ protected McpSyncClient createClient() { String jarPath = "build/libs/" + BuildInfoReader.getJarFileName(); var params = ServerParameters.builder("java").args("-jar", jarPath).addEnvVar("SOLR_URL", solrUrl) - .addEnvVar("SPRING_DOCKER_COMPOSE_ENABLED", "false").build(); + .addEnvVar("SPRING_DOCKER_COMPOSE_ENABLED", "false").addEnvVar("SOLR_INDEX_URL_ALLOWED_HOSTS", "*") + .build(); var transport = new StdioClientTransport(params, new JacksonMcpJsonMapper(new ObjectMapper())); return McpClient.sync(transport).build(); } + @Test + @Order(42) + void indexesFromAUrlThroughStdioMcp() throws Exception { + var server = serveShowsJson(); + try { + String collection = "shows-url-copy"; + assertNotError(mcpClient.callTool(new CallToolRequest("create-collection", Map.of("name", collection)))); + var indexed = mcpClient.callTool( + new CallToolRequest("index-url", Map.of("collection", collection, "url", showsJsonUrl(server)))); + assertNotError(indexed); + assertTrue(extractText(indexed).contains("61 of 61"), extractText(indexed)); + assertFalse(extractText(indexed).contains("Stranger Things"), "payload leaked into the summary"); + var searched = mcpClient.callTool( + new CallToolRequest("search", Map.of("collection", collection, "query", "*:*", "rows", 0))); + assertNotError(searched); + Map response = OBJECT_MAPPER.readValue(extractText(searched), new TypeReference<>() { + }); + assertEquals(SHOWS_DOC_COUNT, getNumFound(response)); + } finally { + server.stop(0); + } + } + + @Test + @Order(43) + void aRefusedAddressIsAnMcpToolError() { + var result = mcpClient.callTool(new CallToolRequest("index-url", + Map.of("collection", SHOWS_COLLECTION, "url", "http://169.254.169.254/latest/meta-data/"))); + assertEquals(Boolean.TRUE, result.isError()); + assertTrue(extractText(result).contains("link-local or cloud-metadata"), extractText(result)); + } + } diff --git a/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java b/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java index 3813675f..c4a76776 100644 --- a/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java +++ b/src/test/java/org/apache/solr/mcp/server/McpToolRegistrationTest.java @@ -26,6 +26,7 @@ import java.util.stream.Stream; import org.apache.solr.mcp.server.collection.CollectionService; import org.apache.solr.mcp.server.indexing.IndexingService; +import org.apache.solr.mcp.server.indexing.UrlIndexingService; import org.apache.solr.mcp.server.schema.SchemaService; import org.apache.solr.mcp.server.search.SearchService; import org.apache.solr.mcp.server.util.PromptNames; @@ -153,6 +154,7 @@ void testAllMcpToolsHaveUniqueNames() { // IndexingService addToolNames(IndexingService.class, toolNames); + addToolNames(UrlIndexingService.class, toolNames); // CollectionService addToolNames(CollectionService.class, toolNames); @@ -237,7 +239,8 @@ void everyMcpEndpointIsPreAuthorized() { McpComplete.class); List violations = Stream - .of(CollectionService.class, SchemaService.class, SearchService.class, IndexingService.class) + .of(CollectionService.class, SchemaService.class, SearchService.class, IndexingService.class, + UrlIndexingService.class) .flatMap(c -> Arrays.stream(c.getDeclaredMethods())) .filter(m -> mcpAnnotations.stream().anyMatch(m::isAnnotationPresent)) .filter(m -> !m.isAnnotationPresent(PreAuthorize.class)) diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/IndexFormatsTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/IndexFormatsTest.java new file mode 100644 index 00000000..6ed0dbae --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/IndexFormatsTest.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +class IndexFormatsTest { + + @ParameterizedTest + @CsvSource({"json,json", "csv,csv", "xml,xml", "md,markdown", "markdown,markdown", "JSON,json", " Csv ,csv", + "MD,markdown"}) + void normalizesKeywordsCaseInsensitivelyAndTrimmed(String keyword, String expected) { + assertEquals(expected, IndexFormats.normalize(keyword)); + } + + @ParameterizedTest + @ValueSource(strings = {"", " ", "yaml", "txt", "html", "jsonl"}) + void rejectsUnknownOrBlankKeywords(String keyword) { + var e = assertThrows(IllegalArgumentException.class, () -> IndexFormats.normalize(keyword)); + assertEquals("Cannot determine the file format. Supply format=json, csv, xml or markdown.", e.getMessage()); + } +} diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/UrlFetcherTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/UrlFetcherTest.java new file mode 100644 index 00000000..f3b05a9f --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/UrlFetcherTest.java @@ -0,0 +1,399 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import static org.junit.jupiter.api.Assertions.*; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.net.UnknownHostException; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.http.HttpHeaders; +import org.springframework.util.unit.DataSize; + +/** + * Drives the fetcher against a JDK HTTP server on an ephemeral loopback port: + * no mocks, no Solr, runs natively. Read timeout 1 s and cap 1 KB keep the + * failure cases fast. + */ +class UrlFetcherTest { + + private static final Duration CONNECT = Duration.ofSeconds(5); + private static final Duration READ = Duration.ofSeconds(1); + private static final DataSize CAP = DataSize.ofKilobytes(1); + private static final Duration TOTAL = Duration.ofMinutes(5); + private static final String SIZE_MESSAGE = "The document is larger than this server's limit of 1024 bytes; " + + "nothing was indexed. Index datasets this large directly with Solr (bin/solr post or the /update " + + "handler); the index-data prompt shows the command."; + + private HttpServer server; + private ExecutorService handlers; + private String base; + private UrlFetcher open; + private UrlFetcher restricted; + private UrlFetcher dripping; + private final Map> lastRequestHeaders = new ConcurrentHashMap<>(); + private final AtomicInteger requests = new AtomicInteger(); + + @BeforeEach + void setUp() throws IOException { + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + handlers = Executors.newCachedThreadPool(); + server.setExecutor(handlers); + server.createContext("/shows.json", + ex -> respond(ex, 200, "application/json; charset=utf-8", "[{\"id\":\"1\"}]")); + server.createContext("/latin1.csv", ex -> respond(ex, 200, "text/csv; charset=iso-8859-1", "id\n1\n")); + server.createContext("/bad-charset", ex -> respond(ex, 200, "text/csv; charset=no-such-charset", "id\n")); + server.createContext("/untyped", ex -> respond(ex, 200, null, "id\n1\n")); + server.createContext("/missing", ex -> respond(ex, 404, "text/plain", "404: Not Found")); + server.createContext("/moved", ex -> redirect(ex, base + "/shows.json")); + server.createContext("/relative", ex -> redirect(ex, "/shows.json")); + server.createContext("/loop", ex -> redirect(ex, base + "/loop")); + server.createContext("/to-metadata", ex -> redirect(ex, "http://169.254.169.254/latest/meta-data/")); + server.createContext("/to-invalid", ex -> redirect(ex, "http://example.invalid/x.json")); + server.createContext("/bad-location", ex -> redirect(ex, "http://[not-an-address/x.json")); + server.createContext("/to-mailto", ex -> redirect(ex, "mailto:someone@example.invalid")); + server.createContext("/to-userinfo", ex -> redirect(ex, "http://u:p@" + base.substring(7) + "/shows.json")); + server.createContext("/no-location", ex -> { + record(ex); + ex.sendResponseHeaders(302, -1); + ex.close(); + }); + server.createContext("/quoted-charset", ex -> respond(ex, 200, "text/csv; charset=\"iso-8859-1\"", "id\n1\n")); + for (int status : new int[]{301, 303, 307, 308}) { + server.createContext("/r" + status, ex -> redirect(ex, status, base + "/shows.json")); + } + server.createContext("/drip", ex -> { + record(ex); + ex.getResponseHeaders().add("Content-Type", "text/csv"); + ex.sendResponseHeaders(200, 0); + try (OutputStream out = ex.getResponseBody()) { + for (int i = 0; i < 100; i++) { + out.write('x'); // one byte every 200 ms: under the cap, never idle, never finishing soon + out.flush(); + sleep(200); + } + } catch (IOException ignored) { + // the client gave up, as expected + } + }); + server.createContext("/slow-big", ex -> { + record(ex); + ex.getResponseHeaders().add("Content-Type", "text/csv"); + ex.sendResponseHeaders(200, 2 * 1024 * 1024); // far over the 1 KB cap + try (OutputStream out = ex.getResponseBody()) { + for (int i = 0; i < 2048; i++) { + out.write(new byte[1024]); // steady 10 KB/s: never idle, so a drain would run for minutes + out.flush(); + sleep(100); + } + } catch (IOException ignored) { + // the client dropped the connection, as expected + } + }); + for (int i = 1; i <= 6; i++) { + int hop = i; + server.createContext("/chain" + hop, + ex -> redirect(ex, base + (hop == 1 ? "/shows.json" : "/chain" + (hop - 1)))); + } + server.createContext("/declared-big", ex -> { + record(ex); + ex.getResponseHeaders().add("Content-Type", "text/csv"); + ex.sendResponseHeaders(200, 2048); + try (OutputStream out = ex.getResponseBody()) { + out.write(1); // the JDK server flushes the headers with the first body byte + out.flush(); + sleep(3000); // a client that read on would hit the 1 s read timeout instead + out.write(new byte[2047]); + } catch (IOException ignored) { + // the client went away, as expected + } + }); + server.createContext("/chunked-big", ex -> { + record(ex); + ex.getResponseHeaders().add("Content-Type", "text/csv"); + ex.sendResponseHeaders(200, 0); + try (OutputStream out = ex.getResponseBody()) { + out.write(new byte[1025]); + } catch (IOException ignored) { + // the client may abort mid-body + } + }); + server.createContext("/stall", ex -> { + record(ex); + ex.getResponseHeaders().add("Content-Type", "text/csv"); + ex.sendResponseHeaders(200, 0); + try (OutputStream out = ex.getResponseBody()) { + out.write("id,n\n1,1\n\n".getBytes(StandardCharsets.UTF_8)); + out.flush(); + sleep(3000); + } catch (IOException ignored) { + // the client aborted, as expected + } + }); + server.start(); + base = "http://127.0.0.1:" + server.getAddress().getPort(); + open = new UrlFetcher(new UrlIndexingProperties(List.of("*"), CONNECT, READ, CAP, TOTAL, 4)); + restricted = new UrlFetcher(new UrlIndexingProperties(List.of("127.0.0.1"), CONNECT, READ, CAP, TOTAL, 4)); + // a read timeout longer than the total deadline, so only the deadline can fire + dripping = new UrlFetcher( + new UrlIndexingProperties(List.of("*"), CONNECT, Duration.ofSeconds(5), CAP, Duration.ofSeconds(1), 4)); + } + + @AfterEach + void tearDown() { + server.stop(0); + handlers.shutdownNow(); + } + + @Test + void returnsBodyMediaTypeAndCharsetOfATwoHundredResponse() throws Exception { + var fetched = open.fetch(URI.create(base + "/shows.json")); + assertEquals("application/json", fetched.mediaType()); + assertEquals(StandardCharsets.UTF_8, fetched.charset()); + assertEquals("[{\"id\":\"1\"}]", new String(fetched.body(), StandardCharsets.UTF_8)); + assertEquals(URI.create(base + "/shows.json"), fetched.finalUri()); + } + + @Test + void honoursAQuotedCharset() throws Exception { + assertEquals(StandardCharsets.ISO_8859_1, open.fetch(URI.create(base + "/quoted-charset")).charset()); + } + + @ParameterizedTest + @ValueSource(ints = {301, 303, 307, 308}) + void followsEveryRedirectStatus(int status) throws Exception { + assertEquals(URI.create(base + "/shows.json"), open.fetch(URI.create(base + "/r" + status)).finalUri()); + } + + @Test + void aRedirectStatusWithoutLocationIsAStatusError() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/no-location"))); + assertTrue(e.getMessage().startsWith("The URL returned HTTP 302;"), e.getMessage()); + } + + @ParameterizedTest + @ValueSource(strings = {"/to-mailto", "/to-userinfo"}) + void aRedirectToANonHttpOrCredentialedLocationIsARedirectError(String path) { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + path))); + assertEquals(UrlFetcher.INVALID_REDIRECT, e.getMessage()); + } + + /** + * Spring's response close() drains the body to keep the connection alive; the + * fetcher must drop the connection instead, or a refused 2 MB body streamed at + * 10 KB/s would hold the call open for minutes. + */ + @Test + void anOverCapBodyIsNotDrainedAfterRefusal() { + assertTimeoutPreemptively(Duration.ofSeconds(5), () -> { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/slow-big"))); + assertEquals(SIZE_MESSAGE, e.getMessage()); + }); + } + + /** + * The read timeout is per read, so a host that drips one byte at a time never + * trips it; the total deadline must. + */ + @Test + void aDrippingBodyFailsAtTheTotalDeadline() { + assertTimeoutPreemptively(Duration.ofSeconds(4), + () -> assertThrows(SocketTimeoutException.class, () -> dripping.fetch(URI.create(base + "/drip")))); + } + + @Test + void aNonNumericContentLengthCountsAsUnknown() { + var headers = new HttpHeaders(); + headers.add("Content-Length", "banana"); + assertEquals(-1L, UrlFetcher.contentLengthOf(headers)); + headers.set("Content-Length", "42"); + assertEquals(42L, UrlFetcher.contentLengthOf(headers)); + assertEquals(-1L, UrlFetcher.contentLengthOf(new HttpHeaders())); + } + + @Test + void honoursADeclaredCharset() throws Exception { + assertEquals(StandardCharsets.ISO_8859_1, open.fetch(URI.create(base + "/latin1.csv")).charset()); + } + + @Test + void defaultsToUtf8AndEmptyMediaTypeWithoutContentType() throws Exception { + var fetched = open.fetch(URI.create(base + "/untyped")); + assertEquals("", fetched.mediaType()); + assertEquals(StandardCharsets.UTF_8, fetched.charset()); + } + + @Test + void rejectsAnUnsupportedCharset() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/bad-charset"))); + assertEquals(UrlFetcher.UNSUPPORTED_CHARSET, e.getMessage()); + } + + @Test + void nonTwoHundredIsAnErrorBeforeAnyBodyIsExposed() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/missing"))); + assertEquals("The URL returned HTTP 404; nothing was indexed. " + + "Check that it is public and points at a raw document, not a web page.", e.getMessage()); + } + + @Test + void followsAbsoluteAndRelativeRedirectsToTheFinalUrl() throws Exception { + assertEquals(URI.create(base + "/shows.json"), open.fetch(URI.create(base + "/moved")).finalUri()); + assertEquals(URI.create(base + "/shows.json"), open.fetch(URI.create(base + "/relative")).finalUri()); + } + + @Test + void followsFiveRedirectsButNotSix() throws Exception { + assertEquals(URI.create(base + "/shows.json"), open.fetch(URI.create(base + "/chain5")).finalUri()); + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/chain6"))); + assertEquals(UrlFetcher.TOO_MANY_REDIRECTS, e.getMessage()); + } + + @Test + void aRedirectLoopIsAnError() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/loop"))); + assertEquals(UrlFetcher.TOO_MANY_REDIRECTS, e.getMessage()); + } + + @Test + void refusesAnHttpsToHttpDowngradeOnRedirect() { + var e = assertThrows(IllegalArgumentException.class, + () -> UrlFetcher.redirectTarget(URI.create("https://a.invalid/x"), "http://a.invalid/y")); + assertEquals(UrlFetcher.DOWNGRADE, e.getMessage()); + assertEquals(URI.create("https://a.invalid/y"), + UrlFetcher.redirectTarget(URI.create("http://a.invalid/x"), "https://a.invalid/y")); + } + + @Test + void aRedirectToAMetadataAddressIsRefused() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/to-metadata"))); + assertEquals(UrlTargetPolicy.REFUSED_ADDRESS, e.getMessage()); + } + + /** + * The allow-list message proves the ordering: had DNS been consulted first, + * {@code example.invalid} would have surfaced as an UnknownHostException. + */ + @Test + void aRedirectToAHostOffTheAllowListIsRefusedBeforeDns() { + var e = assertThrows(IllegalArgumentException.class, () -> restricted.fetch(URI.create(base + "/to-invalid"))); + assertEquals(UrlTargetPolicy.HOST_NOT_ALLOWED, e.getMessage()); + } + + @Test + void aMalformedRedirectLocationIsAnError() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/bad-location"))); + assertEquals(UrlFetcher.INVALID_REDIRECT, e.getMessage()); + } + + @Test + void aMetadataAddressIsRefusedWithoutSendingARequest() { + int before = requests.get(); + var e = assertThrows(IllegalArgumentException.class, + () -> open.fetch(URI.create("http://169.254.169.254/latest/meta-data/"))); + assertEquals(UrlTargetPolicy.REFUSED_ADDRESS, e.getMessage()); + assertEquals(before, requests.get()); + } + + @Test + void anUnresolvableHostIsAnUnknownHostException() { + assertThrows(UnknownHostException.class, () -> open.fetch(URI.create("http://nonexistent.invalid/x.json"))); + } + + @Test + void aDeclaredContentLengthOverTheCapFailsBeforeTheBodyIsRead() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/declared-big"))); + assertEquals(SIZE_MESSAGE, e.getMessage()); + } + + @Test + void aChunkedBodyOverTheCapFails() { + var e = assertThrows(IllegalArgumentException.class, () -> open.fetch(URI.create(base + "/chunked-big"))); + assertEquals(SIZE_MESSAGE, e.getMessage()); + } + + @Test + void aStalledBodyThrowsASocketTimeout() { + long start = System.nanoTime(); + assertThrows(SocketTimeoutException.class, () -> open.fetch(URI.create(base + "/stall"))); + assertTrue(System.nanoTime() - start < Duration.ofSeconds(5).toNanos(), "read timeout did not fire"); + } + + @Test + void sendsOnlyAcceptAndUserAgentAndNeverCredentials() throws Exception { + open.fetch(URI.create(base + "/shows.json")); + assertEquals(List.of(UrlFetcher.USER_AGENT), lastRequestHeaders.get("User-agent")); + assertEquals(List.of(UrlFetcher.ACCEPT), lastRequestHeaders.get("Accept")); + assertNull(lastRequestHeaders.get("Authorization")); + assertNull(lastRequestHeaders.get("Cookie")); + } + + private void respond(HttpExchange exchange, int status, String contentType, String body) throws IOException { + record(exchange); + byte[] bytes = body.getBytes(StandardCharsets.ISO_8859_1); + if (contentType != null) { + exchange.getResponseHeaders().add("Content-Type", contentType); + } + exchange.sendResponseHeaders(status, bytes.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(bytes); + } + } + + private void redirect(HttpExchange exchange, String location) throws IOException { + redirect(exchange, 302, location); + } + + private void redirect(HttpExchange exchange, int status, String location) throws IOException { + record(exchange); + exchange.getResponseHeaders().add("Location", location); + exchange.sendResponseHeaders(status, -1); + exchange.close(); + } + + private void record(HttpExchange exchange) { + requests.incrementAndGet(); + lastRequestHeaders.clear(); + exchange.getRequestHeaders().forEach((k, v) -> lastRequestHeaders.put(k, List.copyOf(v))); + } + + private static void sleep(long millis) { + try { + Thread.sleep(millis); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } +} diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingIntegrationTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingIntegrationTest.java new file mode 100644 index 00000000..5a7eb916 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingIntegrationTest.java @@ -0,0 +1,216 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import static org.junit.jupiter.api.Assertions.*; + +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; +import java.io.IOException; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.SolrQuery; +import org.apache.solr.mcp.server.TestcontainersConfiguration; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.TestPropertySource; +import org.testcontainers.junit.jupiter.Testcontainers; + +/** + * Drives {@code index-url} end to end: a JDK HTTP server on an ephemeral + * loopback port serves the documents, the real service indexes them into a real + * Solr. The allow-list is bound from configuration to {@code 127.0.0.1} only, + * which is why the metadata-address refusal is exercised by the MCP client + * tests (allow-list {@code *}) rather than here. The cap is lowered to 64 KB so + * the over-cap case is small. + */ +@SpringBootTest +@Import(TestcontainersConfiguration.class) +@TestPropertySource(properties = {"solr.index-url.allowed-hosts=127.0.0.1", "solr.index-url.max-bytes=64KB"}) +@Tag("integration") +@Testcontainers(disabledWithoutDocker = true) +class UrlIndexingIntegrationTest { + + private static HttpServer server; + private static String base; + + @Autowired + private UrlIndexingService service; + + @Autowired + private SolrClient solrClient; + + @BeforeAll + static void startServer() throws IOException { + byte[] showsJson = Files.readAllBytes(Path.of("src/test/resources/shows.json")); + server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.createContext("/shows.json", ex -> respond(ex, 200, "text/plain; charset=utf-8", showsJson)); + server.createContext("/shows.csv", ex -> respond(ex, 200, "text/plain", csv("csv", 50))); + server.createContext("/shows.xml", ex -> respond(ex, 200, "text/plain", xml(40))); + server.createContext("/shows.md", + ex -> respond(ex, 200, "text/plain", "# One show\n\nA body.\n".getBytes(StandardCharsets.UTF_8))); + server.createContext("/export.csv", ex -> respond(ex, 200, "text/plain", csv("export", 30))); + server.createContext("/data", ex -> respond(ex, 200, "application/json", showsJson)); + server.createContext("/data.txt", ex -> respond(ex, 200, "text/plain", csv("txt", 5))); + server.createContext("/page", + ex -> respond(ex, 200, "text/html", "id,x".getBytes(StandardCharsets.UTF_8))); + server.createContext("/missing", + ex -> respond(ex, 404, "text/plain", "404: Not Found".getBytes(StandardCharsets.UTF_8))); + server.createContext("/moved", ex -> { + ex.getResponseHeaders().add("Location", base + "/shows.json"); + ex.sendResponseHeaders(302, -1); + ex.close(); + }); + server.createContext("/big.csv", ex -> respond(ex, 200, "text/csv", csv("big", 6_000))); // ~100 KB + server.start(); + base = "http://127.0.0.1:" + server.getAddress().getPort(); + } + + @AfterAll + static void stopServer() { + server.stop(0); + } + + @Test + void indexesJsonServedAsTextPlainByExtension() throws Exception { + String collection = newCollection("json"); + String summary = service.indexUrl(collection, base + "/shows.json", null); + assertTrue(summary.contains("61 of 61"), summary); + assertTrue(summary.contains("Indexed field names"), "structured formats list their fields: " + summary); + assertFalse(summary.contains("Stranger Things"), "payload leaked into the summary"); + assertEquals(61, count(collection)); + } + + @Test + void indexesCsvXmlAndMarkdown() throws Exception { + String csv = newCollection("csv"); + String csvSummary = service.indexUrl(csv, base + "/shows.csv", null); + assertTrue(csvSummary.contains("50 of 50"), csvSummary); + assertTrue(csvSummary.contains("Indexed field names"), csvSummary); + assertEquals(50, count(csv)); + + String xml = newCollection("xml"); + String xmlSummary = service.indexUrl(xml, base + "/shows.xml", null); + assertTrue(xmlSummary.contains("40 of 40"), xmlSummary); + assertTrue(xmlSummary.contains("Indexed field names"), xmlSummary); + assertEquals(40, count(xml)); + + String md = newCollection("md"); + String mdSummary = service.indexUrl(md, base + "/shows.md", null); + assertTrue(mdSummary.contains("1 of 1"), mdSummary); + assertFalse(mdSummary.contains("Indexed field names"), "Markdown is one document, no field list: " + mdSummary); + assertEquals(1, count(md)); + } + + @Test + void resolvesFormatFromExtensionBeforeQueryStringAndFromMediaTypeWithoutExtension() throws Exception { + String byExtension = newCollection("query"); + assertTrue(service.indexUrl(byExtension, base + "/export.csv?token=abc", null).contains("30 of 30")); + assertEquals(30, count(byExtension)); + + String byMediaType = newCollection("mediatype"); + assertTrue(service.indexUrl(byMediaType, base + "/data", null).contains("61 of 61")); + assertEquals(61, count(byMediaType)); + } + + @Test + void followsARedirect() throws Exception { + String collection = newCollection("redirect"); + assertTrue(service.indexUrl(collection, base + "/moved", null).contains("61 of 61")); + assertEquals(61, count(collection)); + } + + @Test + void refusedOrUnparsableSourcesIndexNothing() throws Exception { + String collection = newCollection("errors"); + + var plain = assertThrows(IllegalArgumentException.class, + () -> service.indexUrl(collection, base + "/data.txt", null)); + assertEquals(UrlIndexingService.FORMAT_UNRESOLVED, plain.getMessage()); + + var html = assertThrows(IllegalArgumentException.class, + () -> service.indexUrl(collection, base + "/page", null)); + assertEquals(UrlIndexingService.FORMAT_UNRESOLVED + UrlIndexingService.HTML_NOT_SUPPORTED, html.getMessage()); + + var missing = assertThrows(IllegalArgumentException.class, + () -> service.indexUrl(collection, base + "/missing", null)); + assertEquals( + "The URL returned HTTP 404; nothing was indexed. " + + "Check that it is public and points at a raw document, not a web page.", + missing.getMessage()); + + var big = assertThrows(IllegalArgumentException.class, + () -> service.indexUrl(collection, base + "/big.csv", null)); + // 64 KB is not a whole number of megabytes, so the limit renders in bytes + assertEquals("The document is larger than this server's limit of 65536 bytes; nothing was indexed. " + + "Index datasets this large directly with Solr (bin/solr post or the /update handler); " + + "the index-data prompt shows the command.", big.getMessage()); + + var offList = assertThrows(IllegalArgumentException.class, + () -> service.indexUrl(collection, "http://example.invalid/x.json", null)); + assertEquals(UrlTargetPolicy.HOST_NOT_ALLOWED, offList.getMessage()); + + solrClient.commit(collection); + assertEquals(0, count(collection)); + } + + private String newCollection(String suffix) throws Exception { + String name = "url_" + suffix + "_" + System.nanoTime(); + CollectionAdminRequest.createCollection(name, "_default", 1, 1).process(solrClient); + return name; + } + + private long count(String collection) throws Exception { + return solrClient.query(collection, new SolrQuery("*:*").setRows(0)).getResults().getNumFound(); + } + + private static void respond(HttpExchange exchange, int status, String contentType, byte[] body) throws IOException { + exchange.getResponseHeaders().add("Content-Type", contentType); + exchange.sendResponseHeaders(status, body.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(body); + } catch (IOException ignored) { + // the client stops reading an over-cap body early + } + } + + private static byte[] csv(String prefix, int rows) { + var sb = new StringBuilder("id,title\n"); + for (int i = 0; i < rows; i++) { + sb.append(prefix).append('-').append(i).append(",Title ").append(i).append('\n'); + } + return sb.toString().getBytes(StandardCharsets.UTF_8); + } + + private static byte[] xml(int rows) { + var sb = new StringBuilder(""); + for (int i = 0; i < rows; i++) { + sb.append("xml-").append(i).append("Title ").append(i).append(""); + } + return sb.append("").toString().getBytes(StandardCharsets.UTF_8); + } +} diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingPropertiesTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingPropertiesTest.java new file mode 100644 index 00000000..ac30ca5e --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingPropertiesTest.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import static org.junit.jupiter.api.Assertions.*; + +import java.time.Duration; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.util.unit.DataSize; + +class UrlIndexingPropertiesTest { + + private static final String RANGE = "solr.index-url.max-bytes must be between 1 byte and 2 GB"; + + @Test + void acceptsTheDefaultCap() { + var properties = properties(DataSize.ofMegabytes(10)); + assertEquals(10L * 1024 * 1024, properties.maxBytes().toBytes()); + } + + @Test + void acceptsTheBounds() { + assertDoesNotThrow(() -> properties(DataSize.ofBytes(1))); + assertDoesNotThrow(() -> properties(DataSize.ofBytes(Integer.MAX_VALUE - 1))); + } + + @Test + void rejectsZeroBecauseThereIsNoUnlimited() { + var e = assertThrows(IllegalArgumentException.class, () -> properties(DataSize.ofBytes(0))); + assertEquals(RANGE, e.getMessage()); + } + + @Test + void rejectsACapThatDoesNotFitAnInt() { + var e = assertThrows(IllegalArgumentException.class, () -> properties(DataSize.ofGigabytes(3))); + assertEquals(RANGE, e.getMessage()); + } + + @Test + void rejectsANonPositiveTotalTimeout() { + var e = assertThrows(IllegalArgumentException.class, () -> new UrlIndexingProperties(List.of("*"), + Duration.ofSeconds(10), Duration.ofSeconds(30), DataSize.ofMegabytes(10), Duration.ZERO, 4)); + assertEquals("solr.index-url.total-timeout must be positive", e.getMessage()); + } + + @Test + void rejectsFewerThanOneConcurrentFetch() { + var e = assertThrows(IllegalArgumentException.class, () -> new UrlIndexingProperties(List.of("*"), + Duration.ofSeconds(10), Duration.ofSeconds(30), DataSize.ofMegabytes(10), Duration.ofMinutes(5), 0)); + assertEquals("solr.index-url.max-concurrent-fetches must be at least 1", e.getMessage()); + } + + private static UrlIndexingProperties properties(DataSize maxBytes) { + return new UrlIndexingProperties(List.of("*"), Duration.ofSeconds(10), Duration.ofSeconds(30), maxBytes, + Duration.ofMinutes(5), 4); + } +} diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingServiceTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingServiceTest.java new file mode 100644 index 00000000..1b44a615 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/UrlIndexingServiceTest.java @@ -0,0 +1,264 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.net.URI; +import java.net.UnknownHostException; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.common.SolrException; +import org.apache.solr.mcp.server.indexing.documentcreator.DocumentProcessingException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledInNativeImage; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +/** + * Error mapping and format resolution of the {@code index-url} tool with the + * fetcher and the indexing path mocked; the real fetch is covered by + * {@link UrlFetcherTest} and the integration test. The URL constant ends in + * {@code .json}, so cases that rely on the media type use their own URLs. + */ +@ExtendWith(MockitoExtension.class) +@DisabledInNativeImage +class UrlIndexingServiceTest { + + private static final String URL = "https://raw.githubusercontent.com/apache/solr-mcp/main/shows.json"; + private static final String SUMMARY = "Successfully indexed 2 of 2 documents into collection 'shows'"; + + @Mock + IndexingService indexingService; + + @Mock + UrlFetcher fetcher; + + private UrlIndexingService service; + + @BeforeEach + void setUp() { + service = new UrlIndexingService(indexingService, fetcher, 4); + } + + @Test + void refusesACallBeyondTheConfiguredConcurrentFetches() throws Exception { + var entered = new CountDownLatch(1); + var release = new CountDownLatch(1); + when(fetcher.fetch(any())).thenAnswer(invocation -> { + entered.countDown(); + release.await(10, TimeUnit.SECONDS); + return fetched(URL, "application/json", "[]"); + }); + when(indexingService.indexPayload("shows", "[]", "json")).thenReturn(SUMMARY); + var single = new UrlIndexingService(indexingService, fetcher, 1); + var executor = Executors.newSingleThreadExecutor(); + try { + Future first = executor.submit(() -> single.indexUrl("shows", URL, null)); + assertTrue(entered.await(10, TimeUnit.SECONDS), "first fetch never started"); + + var e = assertThrows(IllegalStateException.class, () -> single.indexUrl("shows", URL, null)); + assertEquals(UrlIndexingService.busyMessage(1), e.getMessage()); + + release.countDown(); + assertEquals(SUMMARY, first.get(10, TimeUnit.SECONDS)); + // the permit is returned: a third call goes through + assertEquals(SUMMARY, single.indexUrl("shows", URL, null)); + } finally { + executor.shutdownNow(); + } + } + + @Test + void decodesTheBodyWithItsCharsetAndReturnsTheIndexingSummary() throws Exception { + when(fetcher.fetch(URI.create(URL))).thenReturn(fetched(URL, "text/plain", StandardCharsets.ISO_8859_1, + "[{\"id\":\"café\"}]".getBytes(StandardCharsets.ISO_8859_1))); + when(indexingService.indexPayload(eq("shows"), anyString(), eq("json"))).thenReturn(SUMMARY); + + assertEquals(SUMMARY, service.indexUrl("shows", URL, null)); + var payload = ArgumentCaptor.forClass(String.class); + verify(indexingService).indexPayload(eq("shows"), payload.capture(), eq("json")); + assertEquals("[{\"id\":\"café\"}]", payload.getValue()); + } + + @Test + void explicitFormatOverridesExtensionAndMediaType() throws Exception { + when(fetcher.fetch(any())).thenReturn(fetched(URL, "application/json", "id\n1\n")); + when(indexingService.indexPayload("shows", "id\n1\n", "csv")).thenReturn(SUMMARY); + + assertEquals(SUMMARY, service.indexUrl("shows", URL, " CSV ")); + } + + @Test + void extensionOverridesMediaTypeAndIgnoresTheQueryString() throws Exception { + String url = "https://example.invalid/export.csv?token=abc"; + when(fetcher.fetch(any())).thenReturn(fetched(url, "text/plain", "id\n1\n")); + when(indexingService.indexPayload("shows", "id\n1\n", "csv")).thenReturn(SUMMARY); + + assertEquals(SUMMARY, service.indexUrl("shows", url, null)); + } + + @Test + void mediaTypeResolvesTheFormatWhenNeitherUrlHasAnExtension() throws Exception { + String url = "https://example.invalid/data"; + when(fetcher.fetch(any())).thenReturn(fetched(url, "text/xml", "")); + when(indexingService.indexPayload("shows", "", "xml")).thenReturn(SUMMARY); + + assertEquals(SUMMARY, service.indexUrl("shows", url, null)); + } + + @Test + void csvAndMarkdownMediaTypesResolveTheirFormats() throws Exception { + when(fetcher.fetch(URI.create("https://example.invalid/a"))) + .thenReturn(fetched("https://example.invalid/a", "text/csv", "id\n1\n")); + when(fetcher.fetch(URI.create("https://example.invalid/b"))) + .thenReturn(fetched("https://example.invalid/b", "text/markdown", "# T\n")); + when(indexingService.indexPayload("shows", "id\n1\n", "csv")).thenReturn(SUMMARY); + when(indexingService.indexPayload("shows", "# T\n", "markdown")).thenReturn(SUMMARY); + + assertEquals(SUMMARY, service.indexUrl("shows", "https://example.invalid/a", null)); + assertEquals(SUMMARY, service.indexUrl("shows", "https://example.invalid/b", null)); + } + + @Test + void aBlankExplicitFormatIsTreatedAsAbsent() throws Exception { + when(fetcher.fetch(any())).thenReturn(fetched(URL, "text/plain", "[]")); + when(indexingService.indexPayload("shows", "[]", "json")).thenReturn(SUMMARY); + + assertEquals(SUMMARY, service.indexUrl("shows", URL, " ")); + } + + @Test + void theFinalUrlsExtensionIsUsedWhenTheRequestedUrlHasNone() throws Exception { + String requested = "https://example.invalid/latest"; + var body = fetched("https://example.invalid/releases/shows.json", "text/plain", "[]"); + when(fetcher.fetch(URI.create(requested))).thenReturn(body); + when(indexingService.indexPayload("shows", "[]", "json")).thenReturn(SUMMARY); + + assertEquals(SUMMARY, service.indexUrl("shows", requested, null)); + } + + @Test + void textPlainWithoutAKnownExtensionIsAFormatErrorAndIndexesNothing() throws Exception { + String url = "https://example.invalid/data.txt"; + when(fetcher.fetch(any())).thenReturn(fetched(url, "text/plain", "id\n1\n")); + + var e = assertThrows(IllegalArgumentException.class, () -> service.indexUrl("shows", url, null)); + assertEquals(UrlIndexingService.FORMAT_UNRESOLVED, e.getMessage()); + verifyNoInteractions(indexingService); + } + + @Test + void htmlIsAFormatErrorThatSaysSo() throws Exception { + String url = "https://example.invalid/page"; + when(fetcher.fetch(any())).thenReturn(fetched(url, "text/html", "")); + + var e = assertThrows(IllegalArgumentException.class, () -> service.indexUrl("shows", url, null)); + assertEquals(UrlIndexingService.FORMAT_UNRESOLVED + UrlIndexingService.HTML_NOT_SUPPORTED, e.getMessage()); + verifyNoInteractions(indexingService); + } + + @Test + void anUnknownExplicitFormatIsRejectedBeforeFetching() { + var e = assertThrows(IllegalArgumentException.class, () -> service.indexUrl("shows", URL, "yaml")); + assertEquals(IndexFormats.UNKNOWN_FORMAT, e.getMessage()); + verifyNoInteractions(fetcher, indexingService); + } + + @Test + void blankCollectionAndInvalidUrlsAreRejectedBeforeFetching() { + var c = assertThrows(IllegalArgumentException.class, () -> service.indexUrl(" ", URL, null)); + assertEquals("Provide a non-empty collection name.", c.getMessage()); + var u = assertThrows(IllegalArgumentException.class, () -> service.indexUrl("shows", " ", null)); + assertEquals(UrlTargetPolicy.INVALID_URL, u.getMessage()); + var p = assertThrows(IllegalArgumentException.class, () -> service.indexUrl("shows", "http://[bad", null)); + assertEquals(UrlTargetPolicy.INVALID_URL, p.getMessage()); + verifyNoInteractions(fetcher, indexingService); + } + + @Test + void fetcherArgumentErrorsPassThroughUnchanged() throws Exception { + when(fetcher.fetch(any())).thenThrow(new IllegalArgumentException(UrlTargetPolicy.HOST_NOT_ALLOWED)); + + var e = assertThrows(IllegalArgumentException.class, () -> service.indexUrl("shows", URL, null)); + assertEquals(UrlTargetPolicy.HOST_NOT_ALLOWED, e.getMessage()); + verifyNoInteractions(indexingService); + } + + @Test + void aReadTimeoutIsReportedAsSuchEvenWhenWrapped() throws Exception { + when(fetcher.fetch(any())).thenThrow(new SocketTimeoutException("Read timed out")) + .thenThrow(new IOException("wrapped", new SocketTimeoutException("Read timed out"))); + + for (int i = 0; i < 2; i++) { + var e = assertThrows(IllegalStateException.class, () -> service.indexUrl("shows", URL, null)); + assertEquals(UrlIndexingService.READ_TIMEOUT, e.getMessage()); + } + } + + @Test + void otherNetworkFailuresAreReportedFromTheServersPointOfView() throws Exception { + when(fetcher.fetch(any())).thenThrow(new UnknownHostException("example.invalid")); + + var e = assertThrows(IllegalStateException.class, () -> service.indexUrl("shows", URL, null)); + assertEquals(UrlIndexingService.UNREACHABLE, e.getMessage()); + } + + @Test + void parseFailuresNameTheFormatAndSayNothingWasIndexed() throws Exception { + when(fetcher.fetch(any())).thenReturn(fetched(URL, "application/json", "not json")); + when(indexingService.indexPayload("shows", "not json", "json")) + .thenThrow(new DocumentProcessingException("bad")); + + var e = assertThrows(IllegalArgumentException.class, () -> service.indexUrl("shows", URL, null)); + assertEquals("Cannot parse the URL content as json. Check its syntax and format. Nothing was indexed.", + e.getMessage()); + } + + @Test + void solrFailuresAreStateErrors() throws Exception { + when(fetcher.fetch(any())).thenReturn(fetched(URL, "application/json", "[]")); + when(indexingService.indexPayload("shows", "[]", "json")).thenThrow(new SolrServerException("down")) + .thenThrow(new IOException("connection reset")) + .thenThrow(new SolrException(SolrException.ErrorCode.SERVER_ERROR, "commit failed")); + + for (int i = 0; i < 3; i++) { + var e = assertThrows(IllegalStateException.class, () -> service.indexUrl("shows", URL, null)); + assertEquals(UrlIndexingService.SOLR_FAILED, e.getMessage()); + } + } + + private static UrlFetcher.FetchedBody fetched(String url, String mediaType, String body) { + return fetched(url, mediaType, StandardCharsets.UTF_8, body.getBytes(StandardCharsets.UTF_8)); + } + + private static UrlFetcher.FetchedBody fetched(String url, String mediaType, Charset charset, byte[] body) { + return new UrlFetcher.FetchedBody(URI.create(url), mediaType, charset, body); + } +} diff --git a/src/test/java/org/apache/solr/mcp/server/indexing/UrlTargetPolicyTest.java b/src/test/java/org/apache/solr/mcp/server/indexing/UrlTargetPolicyTest.java new file mode 100644 index 00000000..c387aec4 --- /dev/null +++ b/src/test/java/org/apache/solr/mcp/server/indexing/UrlTargetPolicyTest.java @@ -0,0 +1,143 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.mcp.server.indexing; + +import static org.junit.jupiter.api.Assertions.*; + +import java.net.InetAddress; +import java.net.URI; +import java.net.UnknownHostException; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Pure checks on the URL, the allow-list and the resolved addresses. Every + * address is built from a literal, so no DNS is involved. + */ +class UrlTargetPolicyTest { + + private static final List ANY = List.of("*"); + private static final List GITHUB = List.of("raw.githubusercontent.com", "*.githubusercontent.com", + "github.com"); + private static final List PUBLIC = addresses("93.184.216.34"); + + @Test + void exactHostMatchesCaseInsensitivelyAndIgnoresATrailingDot() { + assertDoesNotThrow( + () -> UrlTargetPolicy.check(uri("http://RAW.githubusercontent.com./x.json"), GITHUB, PUBLIC)); + } + + @Test + void wildcardMatchesSubdomainsButNotTheBareSuffix() { + assertDoesNotThrow( + () -> UrlTargetPolicy.check(uri("https://gist.githubusercontent.com/x.csv"), GITHUB, PUBLIC)); + var e = assertThrows(IllegalArgumentException.class, + () -> UrlTargetPolicy.check(uri("https://githubusercontent.com/x.csv"), GITHUB, PUBLIC)); + assertEquals(UrlTargetPolicy.HOST_NOT_ALLOWED, e.getMessage()); + } + + @Test + void starAllowsAnyHostIncludingLoopbackAndPrivateAddresses() { + assertDoesNotThrow(() -> UrlTargetPolicy.check(uri("http://10.0.0.1/x.json"), ANY, addresses("10.0.0.1"))); + assertDoesNotThrow(() -> UrlTargetPolicy.check(uri("http://localhost/x.json"), ANY, addresses("127.0.0.1"))); + assertDoesNotThrow(() -> UrlTargetPolicy.check(uri("http://[::1]/x.json"), ANY, addresses("::1"))); + } + + @Test + void anEmptyAllowListRejectsEveryHost() { + var e = assertThrows(IllegalArgumentException.class, + () -> UrlTargetPolicy.check(uri("https://raw.githubusercontent.com/x.json"), List.of(), PUBLIC)); + assertEquals(UrlTargetPolicy.HOST_NOT_ALLOWED, e.getMessage()); + } + + @Test + void aHostNotOnTheListIsRejectedBeforeAddressesAreConsidered() { + var e = assertThrows(IllegalArgumentException.class, + () -> UrlTargetPolicy.check(uri("https://example.invalid/x.json"), GITHUB, List.of())); + assertEquals(UrlTargetPolicy.HOST_NOT_ALLOWED, e.getMessage()); + } + + @ParameterizedTest + @ValueSource( + strings = {"169.254.169.254", "fe80::1", "fd00:ec2::254", "::ffff:169.254.169.254", "100.100.100.200", + "168.63.129.16"}) + void refusesLinkLocalAndCloudMetadataAddressesEvenWithStar(String literal) { + var e = assertThrows(IllegalArgumentException.class, + () -> UrlTargetPolicy.check(uri("http://example.invalid/x.json"), ANY, addresses(literal))); + assertEquals(UrlTargetPolicy.REFUSED_ADDRESS, e.getMessage()); + } + + @Test + void refusesWhenAnyOfSeveralAddressesIsLinkLocal() { + var e = assertThrows(IllegalArgumentException.class, () -> UrlTargetPolicy + .check(uri("http://example.invalid/x.json"), ANY, addresses("93.184.216.34", "169.254.169.254"))); + assertEquals(UrlTargetPolicy.REFUSED_ADDRESS, e.getMessage()); + } + + @ParameterizedTest + @ValueSource( + strings = {"ftp://example.invalid/data.json", "file:///etc/passwd", "data.json", "/data.json", + "http:///data.json", "http://user@/data.json"}) + void rejectsNonHttpSchemesRelativeUrlsAndMissingHosts(String url) { + var e = assertThrows(IllegalArgumentException.class, () -> UrlTargetPolicy.check(uri(url), ANY, PUBLIC)); + assertEquals(UrlTargetPolicy.INVALID_URL, e.getMessage()); + } + + @Test + void rejectsAPortOutOfRangeBeforeConnecting() { + var e = assertThrows(IllegalArgumentException.class, + () -> UrlTargetPolicy.check(uri("https://raw.githubusercontent.com:99999/x.json"), GITHUB, PUBLIC)); + assertEquals(UrlTargetPolicy.INVALID_URL, e.getMessage()); + } + + @Test + void anExactIpv6EntryMatchesTheBracketedHost() { + assertDoesNotThrow( + () -> UrlTargetPolicy.check(uri("http://[::1]:8000/x.json"), List.of("::1"), addresses("::1"))); + } + + @Test + void rejectsEmbeddedCredentialsBeforeTheAllowList() { + var e = assertThrows(IllegalArgumentException.class, + () -> UrlTargetPolicy.check(uri("http://user:pw@raw.githubusercontent.com/x.json"), GITHUB, PUBLIC)); + assertEquals(UrlTargetPolicy.EMBEDDED_CREDENTIALS, e.getMessage()); + } + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "::1", "10.0.0.1", "93.184.216.34"}) + void allowsOrdinaryAddressesWithStar(String literal) { + assertDoesNotThrow(() -> UrlTargetPolicy.check(uri("http://example.invalid/x.json"), ANY, addresses(literal))); + } + + private static URI uri(String url) { + return URI.create(url); + } + + private static List addresses(String... literals) { + try { + var list = new java.util.ArrayList(); + for (String literal : literals) { + list.add(InetAddress.getByName(literal)); // no DNS for a literal + } + return List.copyOf(list); + } catch (UnknownHostException e) { + throw new IllegalStateException(e); + } + } +}