adityamparikh commented on PR #160:
URL: https://github.com/apache/solr-mcp/pull/160#issuecomment-4995864931
Thanks @potiuk and the ASF Security team — this is a remarkably accurate
read of the
codebase from public artefacts. I've verified every §14 question against the
source;
answers below, one per question. Overall: **I support including solr-mcp in
this scan
cycle** and merging this PR once the answers are folded in and the one
inconsistency
noted at the end is fixed. (Final call on merge/enrollment belongs to the
PMC, of
course — flagging on the dev list.)
## Wave 1 — scope and trust posture
**Q-clienttrust — confirmed.** The client's identity is operator-authorized
(the
operator chose to connect it), but every tool argument it emits is treated as
untrusted data — the server validates nothing about the natural language
upstream of
it. Model-side defenses (approval UX, content filtering) belong to the
client and
operator.
**Q-transport / Q-httpsec — confirmed.** Both dispositions are right:
`HTTP_SECURITY_ENABLED=false` on a network deployment is
`OUT-OF-MODEL: non-default-build`, and unauthenticated STDIO is
`KNOWN-NON-FINDING`. Two precision notes you may want to fold in:
- The HTTP transport is streamable HTTP running in **stateless** mode
(`spring.ai.mcp.server.protocol=stateless`), which also means no
sampling/progress/elicitation and no per-request context features.
- `/mcp` is `permitAll()` at the filter-chain level; authentication is
enforced by
`@PreAuthorize("isAuthenticated()")` on every MCP entry point — all 11
tools, both
resources, all prompts and completion handlers carry it (the
spring-ai-community/mcp-security "secured tools" pattern).
**Q-adminexposure — confirmed as the intended v0 posture, and yes, a
`VALID-HARDENING` target.** There is no per-tool or per-collection gate
today, and
that's deliberate for v0 — but fine-grained per-tool authorization is
already on the
roadmap as #66 ("Have specific security roles"). Reports proposing a
read-only tool
subset or per-tool roles should be triaged `VALID-HARDENING` and routed to
that
issue.
## Wave 2 — data-flow and injection
**Q-promptinj — confirmed: disclaimed, `BY-DESIGN`.** The server returns Solr
content verbatim to the model and cannot meaningfully sanitise it; the
defense
belongs to the client (approval UX — which the server supports by
advertising honest
`readOnlyHint`/`destructiveHint` annotations) and to the operator (don't
back the
server with a Solr writable by untrusted parties, per §10/§11). Reports of
injected
instructions in returned Solr content should be closed `BY-DESIGN`.
**Q-queryinj — confirmed.** The server performs **no escaping** of `q`/`fq`
— full
Solr query-parser expressiveness is the feature, and its blast radius is
governed by
the backend Solr's posture (route such reports to apache/solr's model). This
is a
documented maintainer position: PRs #122 and #127, which added server-side
query
escaping/validation, were closed with the rationale that such hardening
belongs in
Solr/SolrJ rather than this bridge.
**Q-collection — confirmed: no host escape.** The base URL is fixed at
startup from
`SOLR_URL` and the `collection` argument is used only as a path segment
against it.
The server does not validate the collection string beyond that; what a given
path
reaches *within* the configured Solr host is governed by that Solr's own
authorization — the backend's boundary (apache/solr's model), not this
server's,
consistent with the #122/#127 position above.
## Wave 3 — backend, resources, telemetry
**Q-backendcreds — confirmed.** A single static credential (or none), read
once from
`SOLR_USERNAME`/`SOLR_PASSWORD` at startup and applied only when both are
set; no
per-caller credential and no identity passthrough. (If #66 lands,
authorization
becomes finer-grained *server-side*; the backend Solr credential would remain
shared.)
**Q-resource — confirmed.** No server-side cap on `rows`, facet cardinality,
or
indexing-body size; bounds are the backend Solr's. Also a documented
decision — PR
#127 (pagination/validation bounds) was closed for the same "Solr is
responsible for
security" reason.
**Q-sideeffects / Q-otel — confirmed.** The side-effect inventory is
complete:
outbound SolrJ connection (always), servlet listener (HTTP mode only), OTLP
export
(when configured). Securing OTLP and TLS is operator infrastructure. No child
processes, no file reads from tool input. (`docker compose` autostart exists
only in
http-profile `bootRun` local dev.)
## Wave 4 — meta / ownership
**Q-disclosure — `[email protected]`,** the Solr PMC's channel, same
as
apache/solr. One fix needed in this PR: `SECURITY.md` currently says
`[email protected]` while `THREAT_MODEL.md` §1 says
`[email protected]` —
please align `SECURITY.md` to the Solr channel. (PMC to confirm routing.)
**Q-ownership — confirmed.** `THREAT_MODEL.md` as the canonical
scanner-facing
model, `docs/security/*.md` remaining the operator-facing source of truth,
with the
cross-references as proposed in §15. That matches how the existing docs are
written.
---
**Summary of requested changes before merge:** align the disclosure address
in
`SECURITY.md` to `[email protected]`. Everything else can be folded
in from
the answers above, flipping the `*(inferred)*` tags to `*(maintainer)*`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
