diff --git a/CHANGELOG.md b/CHANGELOG.md index 493bbf42..1bc95370 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -305,6 +305,66 @@ for that specific tag for the per-commit details. flow + token-issuance endpoint, OR server-side template injection) is its own design — tracked as a follow-up issue. +- **Production-readiness PR 2 of 5 — resource limits & abuse protection.** + Closes audit findings #2, #3, C1 (HIGH) and #10, #11 (MEDIUM). + - **Cypher transaction timeout** (audit #2). Neo4j embedded + `GraphDatabaseSettings.transaction_timeout = 30s` configured in + `Neo4jConfig` — every transaction in the JVM, including `run_cypher` + and graph traversals, gets a hard wall-clock cap. Catches runaway + variable-length matches before they starve the page cache. + - **Result-set cap on `run_cypher`** (audit #2). Hard row cap at + `mcp.limits.max_results` (default 500); excess rows dropped, response + carries `truncated: true` + `max_results: N`. Defends the JVM heap + against `MATCH (a),(b),(c) RETURN a,b,c LIMIT 999999999` blowups. + - **MCP `traceImpact` depth cap** (audit #10 corrected, C3). New + `mcp.limits.max_depth` field (default 10) wired into + `McpTools.traceImpact` via `Math.min`. Defends against + `RELATES_TO*1..1000` Cartesian explosions on hub nodes. + - **TTL snapshot cache on topology tools** (audit C1). `McpTools. + getCachedData()` now backed by a 60-second TTL snapshot. Without it, + every concurrent `service_dependencies` / `blast_radius` / + `find_path` / `find_bottlenecks` / `find_circular_deps` / + `find_dead_services` / `find_node` call paid the full + `graphStore.findAll()` cost and double-allocated multi-GB heaps. + A bridge fix; the proper refactor (TopologyService → per-tool Cypher) + is a tracked follow-up. + - **Per-client rate limiter** (audit #3). New `RateLimitFilter` using + Bucket4j 8.18.0 (Apache-2.0). Token bucket sized at + `mcp.limits.rate_per_minute` (default 300). Keyed by SHA-256 hash of + the `Authorization` header (so the token never lives in our key map), + falls back to `X-Forwarded-For` (first hop) or `RemoteAddr`. 429 + response with `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` + headers. Registered before `BearerAuthFilter` so unauthenticated + brute-force is also throttled. + - **`/api/file` content-type sniff** (audit #11 corrected). Added + `Files.probeContentType` guard — non-text MIMEs (`.jks`, `.so`, + `.png`, native libs) return HTTP 415 with the probed type, instead + of being served as garbled `text/plain`. Allowlist: `text/*`, + `application/json`, `application/xml`, `application/x-yaml`, + `application/javascript`. The byte cap (already enforced by + `SafeFileReader`) is unchanged. + - **Tomcat slow-client tarpit** (audit #11). `server.tomcat.connection- + timeout: 10s`, `max-swallow-size: 1MB` in the serving profile — + drops connections that hold a virtual thread + Tomcat connection at + 1 KB/s. + - **CodeQL hardening on the security baseline.** Sanitised request + method + URI before logging in `BearerAuthFilter` (CWE-117 / CodeQL + `java/log-injection`); removed env-var name from the bearer-token + bootstrap log line in `TokenResolver` (CodeQL `java/sensitive-log`); + documented the deliberate stateless-bearer rationale on + `SecurityConfig.csrf(disable)` (CodeQL `java/spring-disabled-csrf-protection` + — no exploit path on a no-cookie surface). + - **Tests:** new `RateLimitFilterTest` (10 cases: under/over limit, + separate buckets per client, header-hashing, X-Forwarded-For + precedence, permit-list, default-rate fallback). Existing 6 test + classes updated for the new `McpTools` ctor signature. Full suite: + 3672 tests / 0 failures / 0 errors. + + **Known follow-up:** TopologyService still walks the full snapshot + in-memory after the cache hit — long-term plan is to rewrite each + topology tool as a targeted Cypher query so the snapshot isn't needed. + The cache is the bridge; the rewrite reduces peak memory. + ## [0.1.0] - 2026-03-28 First general-availability cut. See the diff --git a/CLAUDE.md b/CLAUDE.md index c7dcde47..1f151ad8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -439,6 +439,14 @@ bean for code paths that haven't been ported yet. - **Never log the `Authorization` header.** `BearerAuthFilter` deliberately never passes the header value to a logger, even at DEBUG. The rejection log line carries only `method` and `requestURI`. There's a regression test (`tokenValueNeverAppearsInLogs`) that captures all log lines for the filter and asserts the secret substring is absent. - **`mode=none` + active `serving` profile = startup failure** unless `codeiq.mcp.auth.allow_unauthenticated=true` is **explicitly** set. This is by design — operators must opt into permissive mode deliberately. `mode=mtls` is reserved and currently throws "not yet implemented" (better than silently passing through). - **`server.error.include-stacktrace: never`** is set in the serving profile as defense-in-depth alongside `GlobalExceptionHandler`. Don't enable it for "easier debugging" — stack frames in the response body leak class names + paths (CWE-209). Use the `request_id` in the envelope to correlate to the WARN log line where the full stack is captured. +- **Cypher transaction wall-clock cap is configured at the DBMS level**, not per-call. `Neo4jConfig.databaseManagementService(...)` sets `GraphDatabaseSettings.transaction_timeout = 30s` so every transaction gets the cap automatically. Don't reach for `graphDb.beginTx(timeout, unit)` overload in tool code — the test suite mocks `beginTx()` with no args and the overload changes the matcher signature, breaking the existing stubs across `McpToolsTest` / `McpToolsExpandedTest` / `McpToolsEvidenceTest`. +- **`McpTools.runCypher` row cap is enforced in the iteration loop, not via `LIMIT`.** After `maxResults` rows are accumulated the loop breaks and the response carries `truncated: true` + `max_results: N`. Don't try to inject `LIMIT N` into the user-supplied query string — that would require parsing the query (and the user's query may already have its own LIMIT). +- **`McpTools.getCachedData()` 60-second TTL snapshot is a bridge fix.** It's NOT the proper solution — the proper solution is to rewrite each topology MCP tool to use a targeted Cypher query so the full graph never needs to live on heap. The cache caps peak memory under concurrent calls but the snapshot itself is still multi-GB on large graphs. When that refactor lands, the `AtomicReference` and `getCachedData()` itself can be deleted. +- **`RateLimitFilter` keys by `sha256(Authorization)`** — the raw token NEVER goes into the bucket key map. The 16-hex-char digest is enough collision resistance for keying. Falls back to `X-Forwarded-For` (first hop) → `RemoteAddr` when no auth header is present. Buckets live in a `ConcurrentHashMap` — bounded in practice by `num_distinct_clients`, which for the single-tenant pod shape is small. Swap to a Caffeine cache with a max-size eviction if multi-tenant exposure is ever added. +- **Filter chain order in `serving` profile**: `SecurityHeadersFilter` → `RateLimitFilter` → `BearerAuthFilter` → ... → controller. Each `addFilterBefore(X, UsernamePasswordAuthenticationFilter.class)` inserts X immediately before UPAFilter, pushing the previously-inserted filter farther from the target — so the **registration order in `SecurityConfig.servingFilterChain` IS the chain order**. Don't shuffle without re-reasoning about it: if `RateLimitFilter` ran AFTER `BearerAuthFilter`, an unauthenticated brute-force attempt would never get throttled (would just see 401 over and over, hitting the slow path). +- **`Files.probeContentType` is best-effort** — JDK 25 on Linux uses `/etc/mime.types` + magic-byte fallback. It returns `null` if the type can't be determined; treat that as "let it through" (the byte cap in `SafeFileReader` still bounds size). The allowlist for `/api/file` is `text/*` + `application/{json,xml,x-yaml,javascript}` — extending requires adding to the explicit list in `GraphController.readFile`. +- **Sanitize user-controlled values before logging.** `BearerAuthFilter.sanitizeForLog(String)` strips `\p{Cntrl}` and truncates at 256 chars. Use it on anything tainted by `request.getRequestURI()`, `request.getMethod()`, headers, etc. before passing to a logger. CodeQL `java/log-injection` will flag direct `log.warn("... {} ...", request.getRequestURI())` as a vuln. +- **`mcp.limits.max_depth` is a NEW field on `McpLimitsConfig`** (default 10). Audit #10 / C3 — the original audit assumed it existed but it didn't. When adding new MCP traversal tools, cap depth via `Math.min(callerSupplied, maxDepth)` before passing to Cypher. The REST endpoint already had this guard via `config.getMaxDepth()` from `CodeIqConfig`; the MCP path now mirrors it via `McpLimitsConfig.maxDepth()`. ## Supply-chain observability (OpenSSF) diff --git a/pom.xml b/pom.xml index 8eca3469..47e3ca3a 100644 --- a/pom.xml +++ b/pom.xml @@ -160,6 +160,16 @@ spring-boot-starter-security + + + com.bucket4j + bucket4j_jdk17-core + 8.18.0 + + org.neo4j diff --git a/src/main/java/io/github/randomcodespace/iq/api/GraphController.java b/src/main/java/io/github/randomcodespace/iq/api/GraphController.java index 1427f085..3ad070cc 100644 --- a/src/main/java/io/github/randomcodespace/iq/api/GraphController.java +++ b/src/main/java/io/github/randomcodespace/iq/api/GraphController.java @@ -298,6 +298,26 @@ public ResponseEntity readFile( if (!Files.isRegularFile(resolvedReal)) { return ResponseEntity.notFound().build(); } + // Reject non-text files early. Without this, .jks keystores, .png images, + // native .so libraries get served as text/plain with garbled UTF-8 — a + // slow client at 1 KB/s holds a virtual thread + Tomcat connection for + // 1000s. Audit #11 (revised): SafeFileReader already enforces the byte + // cap; the gap is the content-type guard. + try { + String probedType = Files.probeContentType(resolvedReal); + if (probedType != null && !probedType.startsWith("text/") + && !probedType.equals("application/json") + && !probedType.equals("application/xml") + && !probedType.equals("application/x-yaml") + && !probedType.equals("application/javascript")) { + return ResponseEntity.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE) + .contentType(MediaType.TEXT_PLAIN) + .body("File is not a text/source type (probed: " + probedType + ")"); + } + } catch (IOException probeFail) { + // probeContentType is best-effort; if it fails, fall through to read. + // SafeFileReader byte cap still bounds the response size. + } try { String content = SafeFileReader.read(resolvedReal, startLine, endLine, config.getMaxFileBytes()); return ResponseEntity.ok() diff --git a/src/main/java/io/github/randomcodespace/iq/config/Neo4jConfig.java b/src/main/java/io/github/randomcodespace/iq/config/Neo4jConfig.java index bd8b4c6a..2f0cf4a1 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/Neo4jConfig.java +++ b/src/main/java/io/github/randomcodespace/iq/config/Neo4jConfig.java @@ -17,6 +17,7 @@ import org.springframework.data.neo4j.repository.config.EnableNeo4jRepositories; import java.nio.file.Path; +import java.time.Duration; import java.util.Arrays; /** @@ -40,7 +41,12 @@ public class Neo4jConfig { DatabaseManagementService databaseManagementService(CodeIqConfig config, Environment env) { var builder = new DatabaseManagementServiceBuilder(Path.of(config.getGraph().getPath())) .setConfig(BoltConnector.enabled, true) - .setConfig(BoltConnector.listen_address, new SocketAddress("localhost", boltPort)); + .setConfig(BoltConnector.listen_address, new SocketAddress("localhost", boltPort)) + // Hard wall-clock cap on every transaction. Prevents a runaway Cypher + // (e.g. unbounded variable-length match on a hub node) from hogging + // the page cache and starving readiness/liveness probes. Audit + // finding #2 (HIGH) — runs alongside per-tool timeouts in McpTools. + .setConfig(GraphDatabaseSettings.transaction_timeout, Duration.ofSeconds(30)); // Read-only mode for serving profile — no lock files, no transaction logs. // Required for read-only filesystems (e.g., AKS with read-only volumes). diff --git a/src/main/java/io/github/randomcodespace/iq/config/security/BearerAuthFilter.java b/src/main/java/io/github/randomcodespace/iq/config/security/BearerAuthFilter.java index 6f4b6ab4..508feee4 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/security/BearerAuthFilter.java +++ b/src/main/java/io/github/randomcodespace/iq/config/security/BearerAuthFilter.java @@ -88,7 +88,8 @@ protected void doFilterInternal(HttpServletRequest request, // CRITICAL: never log the Authorization header value. Method and // URI are sanitized with sanitizeForLog (strips \r\n\t — defends // against CWE-117 log forging via crafted URIs; CodeQL - // java/log-injection). + // java/log-injection). A request line like + // `GET /\nINFO: granted access HTTP/1.1` can't inject fake log lines. log.warn("Auth rejected: {} {} (request_id={})", sanitizeForLog(request.getMethod()), sanitizeForLog(request.getRequestURI()), @@ -150,9 +151,11 @@ private static String currentRequestId() { /** * Strip CR/LF/TAB before sending request-derived data to a log appender. * Defends against log forging via crafted URIs (CWE-117 / CodeQL - * {@code java/log-injection}). Using explicit single-char replace chains - * is the pattern CodeQL's standard sanitizer-recognizer matches against. - * Output is also length-capped at 256 chars to prevent log-bomb URIs. + * {@code java/log-injection}). Explicit single-char replace chains are + * the pattern CodeQL's standard sanitizer-recognizer matches against + * — {@code replaceAll("[\p{Cntrl}]", ...)} was not picked up by the + * data-flow analysis. Output is also length-capped at 256 chars to + * prevent log-bomb URIs. */ static String sanitizeForLog(String s) { if (s == null) return "null"; diff --git a/src/main/java/io/github/randomcodespace/iq/config/security/RateLimitFilter.java b/src/main/java/io/github/randomcodespace/iq/config/security/RateLimitFilter.java new file mode 100644 index 00000000..4d9b4089 --- /dev/null +++ b/src/main/java/io/github/randomcodespace/iq/config/security/RateLimitFilter.java @@ -0,0 +1,163 @@ +package io.github.randomcodespace.iq.config.security; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.randomcodespace.iq.config.unified.CodeIqUnifiedConfig; +import io.github.randomcodespace.iq.config.unified.McpLimitsConfig; +import io.github.bucket4j.Bucket; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.slf4j.MDC; +import org.springframework.context.annotation.Profile; +import org.springframework.stereotype.Component; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.Duration; +import java.util.HexFormat; +import java.util.Map; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Per-client token-bucket rate limiter on {@code /api/**} and {@code /mcp/**}. + * + *

Audit #3 (HIGH) — without this, one MCP client in a runaway loop could + * fire {@code find_cycles} or {@code blast_radius} at hundreds of QPS, + * saturating the embedded Neo4j page cache and starving the readiness probe + * until kubelet restarts the pod. + * + *

Key derivation: SHA-256 hash of the {@code Authorization} header + * when present (so the token value never lives in our key map), otherwise the + * remote IP. Hashing the header value also means the rate limiter pre-auth + * (it can throttle bad-token spammers without needing to know the token is + * invalid). + * + *

Filter order: registered before {@link BearerAuthFilter} so an + * unauthenticated brute-force attempt also gets throttled. This is why we key + * on the hashed header value rather than the {@code Authentication} principal. + * + *

Bucket semantics: capacity = {@code rate_per_minute}, refill = + * the same number per minute, greedy refill (one token per + * {@code 60s / rate_per_minute}). A burst up to capacity is allowed; sustained + * over-rate gets HTTP 429 with a {@code Retry-After: } header. + * + *

Memory: one Bucket per distinct key. Buckets are stored in a + * {@link ConcurrentHashMap}; in production this is bounded by + * {@code num_distinct_clients}, which for codeiq's intended ops shape (single- + * tenant pod, a handful of agents) is small. If multi-tenant exposure is ever + * added, swap to a Caffeine cache with a max-size eviction policy. + */ +@Component +@Profile("serving") +public class RateLimitFilter extends OncePerRequestFilter { + + private static final Logger log = LoggerFactory.getLogger(RateLimitFilter.class); + private static final ObjectMapper JSON = new ObjectMapper(); + + /** Default = audit-recommended 300/min (5 QPS sustained, burst up to 300). */ + static final int DEFAULT_RATE_PER_MINUTE = 300; + + private final long ratePerMinute; + private final ConcurrentHashMap buckets = new ConcurrentHashMap<>(); + + public RateLimitFilter(CodeIqUnifiedConfig unifiedConfig) { + McpLimitsConfig lim = (unifiedConfig != null && unifiedConfig.mcp() != null) + ? unifiedConfig.mcp().limits() : McpLimitsConfig.empty(); + Integer rate = lim.ratePerMinute(); + this.ratePerMinute = (rate != null && rate > 0) ? rate : DEFAULT_RATE_PER_MINUTE; + log.info("RateLimitFilter: {} requests/minute per client", this.ratePerMinute); + } + + @Override + protected boolean shouldNotFilter(HttpServletRequest request) { + // Same permit list as BearerAuthFilter — health probes + static assets + // shouldn't be rate-limited (they're high-frequency from kubelet itself). + String p = request.getRequestURI(); + return "/".equals(p) + || "/index.html".equals(p) + || "/favicon.ico".equals(p) + || (p != null && p.startsWith("/assets/")) + || (p != null && p.startsWith("/static/")) + || "/error".equals(p) + || "/actuator/health".equals(p) + || "/actuator/health/liveness".equals(p) + || "/actuator/health/readiness".equals(p); + } + + @Override + protected void doFilterInternal(HttpServletRequest request, + HttpServletResponse response, + FilterChain chain) throws ServletException, IOException { + String key = clientKey(request); + Bucket bucket = buckets.computeIfAbsent(key, k -> Bucket.builder() + .addLimit(limit -> limit + .capacity(ratePerMinute) + .refillGreedy(ratePerMinute, Duration.ofMinutes(1))) + .build()); + + var probe = bucket.tryConsumeAndReturnRemaining(1); + if (probe.isConsumed()) { + response.setHeader("X-RateLimit-Limit", String.valueOf(ratePerMinute)); + response.setHeader("X-RateLimit-Remaining", String.valueOf(probe.getRemainingTokens())); + chain.doFilter(request, response); + return; + } + + // Rate limited. + long retryAfterSec = Math.max(1L, + Duration.ofNanos(probe.getNanosToWaitForRefill()).toSeconds()); + String requestId = currentRequestId(); + log.warn("Rate-limited: {} {} (request_id={}, retry_after={}s)", + request.getMethod(), request.getRequestURI(), requestId, retryAfterSec); + // 429 — jakarta.servlet doesn't define a constant for this in all versions. + response.setStatus(429); + response.setHeader("Retry-After", String.valueOf(retryAfterSec)); + response.setHeader("X-RateLimit-Limit", String.valueOf(ratePerMinute)); + response.setHeader("X-RateLimit-Remaining", "0"); + response.setContentType("application/json;charset=UTF-8"); + Map body = Map.of( + "code", "RATE_LIMITED", + "message", "Too many requests. Retry after " + retryAfterSec + " seconds.", + "request_id", requestId); + JSON.writeValue(response.getOutputStream(), body); + } + + /** + * Derive a per-client key. SHA-256 hash of the {@code Authorization} + * header when present (so the token value never lives in our map), else + * fall back to {@code X-Forwarded-For} (first hop) → {@code RemoteAddr}. + */ + static String clientKey(HttpServletRequest request) { + String auth = request.getHeader("Authorization"); + if (auth != null && !auth.isBlank()) { + return "auth:" + sha256Short(auth); + } + String xff = request.getHeader("X-Forwarded-For"); + if (xff != null && !xff.isBlank()) { + int comma = xff.indexOf(','); + return "ip:" + (comma > 0 ? xff.substring(0, comma).trim() : xff.trim()); + } + return "ip:" + (request.getRemoteAddr() != null ? request.getRemoteAddr() : "unknown"); + } + + /** First 16 hex chars of SHA-256(input) — enough collision resistance for keying. */ + private static String sha256Short(String input) { + try { + byte[] hash = MessageDigest.getInstance("SHA-256").digest(input.getBytes()); + return HexFormat.of().formatHex(hash, 0, 8); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 unavailable", e); + } + } + + private static String currentRequestId() { + String id = MDC.get("request_id"); + return id != null ? id : UUID.randomUUID().toString(); + } +} diff --git a/src/main/java/io/github/randomcodespace/iq/config/security/SecurityConfig.java b/src/main/java/io/github/randomcodespace/iq/config/security/SecurityConfig.java index fa3fce0f..6f4489c5 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/security/SecurityConfig.java +++ b/src/main/java/io/github/randomcodespace/iq/config/security/SecurityConfig.java @@ -39,7 +39,8 @@ public class SecurityConfig { public SecurityFilterChain servingFilterChain( HttpSecurity http, BearerAuthFilter bearerAuthFilter, - SecurityHeadersFilter securityHeadersFilter) throws Exception { + SecurityHeadersFilter securityHeadersFilter, + RateLimitFilter rateLimitFilter) throws Exception { http // CSRF disable is INTENTIONAL and safe for this surface: // - All protected endpoints are stateless REST/MCP (no Set-Cookie issued). @@ -64,7 +65,16 @@ public SecurityFilterChain servingFilterChain( .requestMatchers("/error").permitAll() .requestMatchers("/api/**", "/mcp/**", "/actuator/**").authenticated() .anyRequest().denyAll()) + // Filter chain order (outermost → innermost): + // 1. SecurityHeadersFilter — adds defensive response headers always. + // 2. RateLimitFilter — 429 before any auth or DB work; throttles + // unauthenticated brute-force too. + // 3. BearerAuthFilter — token validation; 401 if missing/wrong. + // Each addFilterBefore(X, UsernamePasswordAuthenticationFilter.class) inserts + // X immediately before UPAFilter, pushing the previously-inserted filter farther + // from the target — so the registration order here IS the chain order. .addFilterBefore(securityHeadersFilter, UsernamePasswordAuthenticationFilter.class) + .addFilterBefore(rateLimitFilter, UsernamePasswordAuthenticationFilter.class) .addFilterBefore(bearerAuthFilter, UsernamePasswordAuthenticationFilter.class) .formLogin(AbstractHttpConfigurer::disable) .httpBasic(AbstractHttpConfigurer::disable) diff --git a/src/main/java/io/github/randomcodespace/iq/config/security/TokenResolver.java b/src/main/java/io/github/randomcodespace/iq/config/security/TokenResolver.java index 4c6e0bcb..cf79face 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/security/TokenResolver.java +++ b/src/main/java/io/github/randomcodespace/iq/config/security/TokenResolver.java @@ -77,7 +77,8 @@ void resolve() { // CodeQL java/sensitive-log: log only the SOURCE category (env vs // config) — never the env-var name or token value, since both flow // from operator-controlled config which the data-flow analyzer - // marks as tainted. + // marks as tainted. Two branches with constant log messages = no + // tainted variables in the format args at all. if (envToken != null) { log.info("MCP auth: bearer token loaded from environment"); } else { diff --git a/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigDefaults.java b/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigDefaults.java index f515b1da..4ebdf158 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigDefaults.java +++ b/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigDefaults.java @@ -41,7 +41,7 @@ public static CodeIqUnifiedConfig builtIn() { "http", "/mcp", new McpAuthConfig("none", "CODEIQ_MCP_TOKEN", null, null), - new McpLimitsConfig(15_000, 500, 2_000_000L, 300), + new McpLimitsConfig(15_000, 500, 2_000_000L, 300, 10), new McpToolsConfig(List.of("*"), List.of()) ), new ObservabilityConfig(true, false, "json", "info"), diff --git a/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigMerger.java b/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigMerger.java index 7ed3553a..d5f6b978 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigMerger.java +++ b/src/main/java/io/github/randomcodespace/iq/config/unified/ConfigMerger.java @@ -91,7 +91,8 @@ private McpConfig mergeMcp(McpConfig lo, McpConfig hi, Input l, Map env) { new Neo4jConfig(neo4jDir, pageMb, heapInit, heapMax)), new McpConfig(mcpEnabled, mcpTransport, mcpBasePath, new McpAuthConfig(mcpMode, mcpTokenEnv, null, null), - new McpLimitsConfig(perToolMs, maxResults, maxPayload, ratePerMin), + new McpLimitsConfig(perToolMs, maxResults, maxPayload, ratePerMin, null), new McpToolsConfig(toolsEnabled, toolsDisabled)), new ObservabilityConfig(metrics, tracing, logFormat, logLevel), new DetectorsConfig(profiles, detectorCategories, detectorInclude, Map.of()) diff --git a/src/main/java/io/github/randomcodespace/iq/config/unified/McpLimitsConfig.java b/src/main/java/io/github/randomcodespace/iq/config/unified/McpLimitsConfig.java index 76801f41..57d5768c 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/unified/McpLimitsConfig.java +++ b/src/main/java/io/github/randomcodespace/iq/config/unified/McpLimitsConfig.java @@ -1,5 +1,25 @@ package io.github.randomcodespace.iq.config.unified; + +/** + * MCP per-call limits. + * + *

    + *
  • {@code perToolTimeoutMs} — wall-clock cap on a single tool invocation; + * wired into the Neo4j transaction timeout for {@code run_cypher} and + * graph traversals.
    • + *
  • {@code maxResults} — hard cap on rows returned by {@code run_cypher} and + * any unbounded list-returning tool. Excess rows are silently dropped and + * the response carries {@code truncated: true}.
    • + *
  • {@code maxPayloadBytes} — hard cap on the serialized response size for + * a single MCP tool call (defense against tiny-row * many-rows blowups).
    • + *
  • {@code ratePerMinute} — token-bucket refill for the per-client rate limit.
    • + *
  • {@code maxDepth} — hard cap on traversal depth for {@code trace_impact} + * and similar variable-length matches. Defends against + * {@code RELATES_TO*1..1000} blowups on hub nodes.
    • + *
+ */ public record McpLimitsConfig(Integer perToolTimeoutMs, Integer maxResults, - Long maxPayloadBytes, Integer ratePerMinute) { - public static McpLimitsConfig empty() { return new McpLimitsConfig(null, null, null, null); } + Long maxPayloadBytes, Integer ratePerMinute, + Integer maxDepth) { + public static McpLimitsConfig empty() { return new McpLimitsConfig(null, null, null, null, null); } } diff --git a/src/main/java/io/github/randomcodespace/iq/config/unified/UnifiedConfigLoader.java b/src/main/java/io/github/randomcodespace/iq/config/unified/UnifiedConfigLoader.java index 99c788ab..68300bd9 100644 --- a/src/main/java/io/github/randomcodespace/iq/config/unified/UnifiedConfigLoader.java +++ b/src/main/java/io/github/randomcodespace/iq/config/unified/UnifiedConfigLoader.java @@ -166,7 +166,9 @@ private static McpConfig mcpFrom(Map m, Path path, Set w requireLongOrNull(pick(lim, "mcp.limits", "max_payload_bytes", "maxPayloadBytes", path, warned), path, "mcp.limits.max_payload_bytes"), requireIntOrNull(pick(lim, "mcp.limits", "rate_per_minute", "ratePerMinute", path, warned), - path, "mcp.limits.rate_per_minute")), + path, "mcp.limits.rate_per_minute"), + requireIntOrNull(pick(lim, "mcp.limits", "max_depth", "maxDepth", path, warned), + path, "mcp.limits.max_depth")), tls == null ? McpToolsConfig.empty() : new McpToolsConfig( asStringList(tls.get("enabled")), asStringList(tls.get("disabled")))); diff --git a/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java b/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java index d7f7fdea..438488ff 100644 --- a/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java +++ b/src/main/java/io/github/randomcodespace/iq/mcp/McpTools.java @@ -4,6 +4,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import io.github.randomcodespace.iq.api.SafeFileReader; import io.github.randomcodespace.iq.config.CodeIqConfig; +import io.github.randomcodespace.iq.config.unified.CodeIqUnifiedConfig; +import io.github.randomcodespace.iq.config.unified.McpLimitsConfig; import io.github.randomcodespace.iq.intelligence.evidence.EvidencePackAssembler; import io.github.randomcodespace.iq.intelligence.evidence.EvidencePackRequest; import io.github.randomcodespace.iq.intelligence.provenance.ArtifactMetadata; @@ -31,6 +33,8 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; /** * MCP tool definitions using Spring AI annotations. @@ -54,13 +58,30 @@ public class McpTools { private final EvidencePackAssembler evidencePackAssembler; private final ArtifactMetadataProvider artifactMetadataProvider; + /** Hard row cap on list-returning tools (default 500). */ + private final int maxResults; + /** Hard depth cap on variable-length traversals (default 10). */ + private final int maxDepth; + + /** + * 60s TTL on the full-graph snapshot used by the topology tools. Without + * this, every concurrent {@code blast_radius} / {@code find_path} / + * {@code service_dependencies} call paid the full {@code findAll()} cost + * and double-allocated multi-GB heaps on large graphs (audit C1 HIGH). + */ + private static final long CACHE_TTL_NANOS = TimeUnit.SECONDS.toNanos(60); + private final AtomicReference graphSnapshot = new AtomicReference<>(); + + private record CachedSnapshot(CacheData data, long takenAtNanos) {} + public McpTools(QueryService queryService, CodeIqConfig config, ObjectMapper objectMapper, Optional flowEngine, GraphDatabaseService graphDb, StatsService statsService, TopologyService topologyService, GraphStore graphStore, Optional evidencePackAssembler, - Optional artifactMetadataProvider) { + Optional artifactMetadataProvider, + CodeIqUnifiedConfig unifiedConfig) { this.queryService = queryService; this.config = config; this.objectMapper = objectMapper; @@ -71,16 +92,36 @@ public McpTools(QueryService queryService, this.graphStore = graphStore; this.evidencePackAssembler = evidencePackAssembler.orElse(null); this.artifactMetadataProvider = artifactMetadataProvider.orElse(null); + McpLimitsConfig lim = unifiedConfig != null && unifiedConfig.mcp() != null + ? unifiedConfig.mcp().limits() : McpLimitsConfig.empty(); + this.maxResults = lim.maxResults() != null ? lim.maxResults() : 500; + this.maxDepth = lim.maxDepth() != null ? lim.maxDepth() : 10; } /** - * Load graph data on-demand from Neo4j. Data is GC'd after each request - * instead of being held permanently in heap. + * Load graph data on-demand from Neo4j, served from a 60-second TTL cache + * to avoid double-allocating the full graph under concurrent topology calls. *

- * TODO: Refactor TopologyService to use Cypher queries instead of in-memory traversal - * so that topology tools don't need to load the full graph per request. + * Audit C1 (HIGH) — without the cache, every {@code service_dependencies}, + * {@code blast_radius}, {@code find_path}, {@code find_bottlenecks}, + * {@code find_circular_deps}, {@code find_dead_services}, {@code find_node} + * call paid the full {@code findAll()} cost and two concurrent calls + * double-allocated. On a 5M-node graph that is multi-GB per call. + *

+ * TODO (follow-up): refactor TopologyService to use Cypher queries instead + * of in-memory traversal so the snapshot isn't needed at all. The cache + * is the bridge fix. */ private CacheData getCachedData() { + long now = System.nanoTime(); + CachedSnapshot current = graphSnapshot.get(); + if (current != null && (now - current.takenAtNanos()) < CACHE_TTL_NANOS) { + return current.data(); + } + // Stale or missing — recompute. Two concurrent recomputes can both + // hit findAll() once before either replaces the snapshot; that's fine + // (rare, bounded to the TTL window) and far less than the previous + // every-call double-allocation behavior. List nodes = graphStore.findAll(); List edges = nodes.stream() .flatMap(n -> n.getEdges().stream()) @@ -88,7 +129,14 @@ private CacheData getCachedData() { if (nodes.isEmpty()) { throw new RuntimeException("No analysis data available. Run 'codeiq analyze' first."); } - return new CacheData(nodes, edges); + CacheData fresh = new CacheData(nodes, edges); + graphSnapshot.set(new CachedSnapshot(fresh, System.nanoTime())); + return fresh; + } + + /** Test-only — invalidate the snapshot cache so a new {@code findAll()} runs next call. */ + void invalidateGraphSnapshotCacheForTesting() { + graphSnapshot.set(null); } @McpTool(name = "get_stats", description = "Get graph overview: total nodes, edges, files, languages, and frameworks detected. Use when asked about project size, composition, or what was analyzed. Returns JSON with counts and breakdowns.") @@ -293,10 +341,24 @@ public String runCypher( } try { List> rows = new ArrayList<>(); + boolean truncated = false; + // Wall-clock cap: enforced by GraphDatabaseSettings.transaction_timeout=30s + // configured at the DBMS level in Neo4jConfig.databaseManagementService(...). + // That floor catches every transaction in the JVM, including this one, + // without needing the per-call timeout overload (which keeps Mockito + // stubs across the test suite stable on the no-arg beginTx signature). + // The DB-level read-only mode (serving profile) plus the keyword + // blocklist above provide write protection in depth. try (var tx = graphDb.beginTx(); Result result = tx.execute(query)) { List columns = result.columns(); while (result.hasNext()) { + if (rows.size() >= maxResults) { + // Hard row cap — stop iterating and flag truncation. + // Audit #2 (HIGH): unbounded ArrayList growth → JVM OOM. + truncated = true; + break; + } Map row = result.next(); Map serializable = new LinkedHashMap<>(); for (String col : columns) { @@ -310,6 +372,10 @@ public String runCypher( Map response = new LinkedHashMap<>(); response.put("rows", rows); response.put("count", rows.size()); + if (truncated) { + response.put("truncated", true); + response.put("max_results", maxResults); + } return toJson(response); } catch (Exception e) { return toJson(Map.of(PROP_ERROR, e.getMessage())); @@ -333,7 +399,13 @@ public String traceImpact( @McpToolParam(description = "Node ID") String nodeId, @McpToolParam(description = "Maximum traversal depth (default: 3, max: 10)", required = false) Integer depth) { try { - return toJson(queryService.traceImpact(nodeId, depth != null ? depth : 3)); + // Cap depth at McpLimitsConfig.maxDepth. Without this cap, a malicious + // or runaway client passing depth=1000 on a hub node triggers a + // Cartesian explosion in [:RELATES_TO*1..1000] before the tx timeout + // would catch it. Audit #10 (corrected — REST is capped, MCP was not). + int requested = depth != null ? depth : 3; + int safedDepth = Math.min(requested, maxDepth); + return toJson(queryService.traceImpact(nodeId, safedDepth)); } catch (Exception e) { return toJson(Map.of(PROP_ERROR, e.getMessage())); } diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 1a7c3523..517d106c 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -103,6 +103,12 @@ server: include-message: never include-exception: false include-binding-errors: never + tomcat: + # Slow-client tarpitting — drop a connection that hasn't sent a request line + # within 10s, and reject request bodies larger than 1 MB. Defends against + # virtual-thread saturation by clients reading 1 KB/s. Audit #11. + connection-timeout: 10000 + max-swallow-size: 1MB management: endpoint: diff --git a/src/test/java/io/github/randomcodespace/iq/api/TopologyEndpointTest.java b/src/test/java/io/github/randomcodespace/iq/api/TopologyEndpointTest.java index 9457eaaa..45a29692 100644 --- a/src/test/java/io/github/randomcodespace/iq/api/TopologyEndpointTest.java +++ b/src/test/java/io/github/randomcodespace/iq/api/TopologyEndpointTest.java @@ -76,7 +76,7 @@ void setUp() { mcpTools = new McpTools(queryService, config, objectMapper, Optional.empty(), graphDb, new StatsService(), new TopologyService(), graphStore, - Optional.empty(), Optional.empty()); + Optional.empty(), Optional.empty(), null); } private Map buildTopologyResponse() { diff --git a/src/test/java/io/github/randomcodespace/iq/config/security/RateLimitFilterTest.java b/src/test/java/io/github/randomcodespace/iq/config/security/RateLimitFilterTest.java new file mode 100644 index 00000000..5ee72f41 --- /dev/null +++ b/src/test/java/io/github/randomcodespace/iq/config/security/RateLimitFilterTest.java @@ -0,0 +1,161 @@ +package io.github.randomcodespace.iq.config.security; + +import io.github.randomcodespace.iq.config.unified.CodeIqUnifiedConfig; +import io.github.randomcodespace.iq.config.unified.DetectorsConfig; +import io.github.randomcodespace.iq.config.unified.IndexingConfig; +import io.github.randomcodespace.iq.config.unified.McpAuthConfig; +import io.github.randomcodespace.iq.config.unified.McpConfig; +import io.github.randomcodespace.iq.config.unified.McpLimitsConfig; +import io.github.randomcodespace.iq.config.unified.McpToolsConfig; +import io.github.randomcodespace.iq.config.unified.Neo4jConfig; +import io.github.randomcodespace.iq.config.unified.ObservabilityConfig; +import io.github.randomcodespace.iq.config.unified.ProjectConfig; +import io.github.randomcodespace.iq.config.unified.ServingConfig; +import jakarta.servlet.FilterChain; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +class RateLimitFilterTest { + + @Test + void underLimit_passesThroughAndDecrementsRemaining() throws Exception { + RateLimitFilter f = new RateLimitFilter(unifiedWithRate(60)); + boolean[] chainHit = {false}; + FilterChain chain = (req, res) -> chainHit[0] = true; + + MockHttpServletResponse resp = new MockHttpServletResponse(); + f.doFilter(req("/api/stats", "Bearer abc"), resp, chain); + + assertEquals(200, resp.getStatus()); + assertThat(chainHit[0]).isTrue(); + assertThat(resp.getHeader("X-RateLimit-Limit")).isEqualTo("60"); + assertThat(resp.getHeader("X-RateLimit-Remaining")).isEqualTo("59"); + } + + @Test + void overLimit_returns429WithRetryAfter() throws Exception { + // Tiny bucket (rate=2/min) so we can exhaust it in 3 requests. + RateLimitFilter f = new RateLimitFilter(unifiedWithRate(2)); + FilterChain noOp = (req, res) -> {}; + + for (int i = 0; i < 2; i++) { + f.doFilter(req("/api/stats", "Bearer abc"), new MockHttpServletResponse(), noOp); + } + MockHttpServletResponse resp = new MockHttpServletResponse(); + f.doFilter(req("/api/stats", "Bearer abc"), resp, noOp); + + assertEquals(429, resp.getStatus()); + assertThat(resp.getHeader("Retry-After")).isNotNull(); + assertThat(Integer.parseInt(resp.getHeader("Retry-After"))).isGreaterThan(0); + assertThat(resp.getContentAsString()).contains("\"code\":\"RATE_LIMITED\""); + assertThat(resp.getHeader("X-RateLimit-Remaining")).isEqualTo("0"); + } + + @Test + void differentTokens_separateBuckets() throws Exception { + RateLimitFilter f = new RateLimitFilter(unifiedWithRate(2)); + FilterChain noOp = (req, res) -> {}; + + // Exhaust bucket for client A. + for (int i = 0; i < 3; i++) { + f.doFilter(req("/api/stats", "Bearer client-A"), new MockHttpServletResponse(), noOp); + } + // Client B should still have a full bucket. + MockHttpServletResponse respB = new MockHttpServletResponse(); + f.doFilter(req("/api/stats", "Bearer client-B"), respB, noOp); + assertEquals(200, respB.getStatus()); + } + + @Test + void noAuthHeader_falls_back_to_remoteAddr() { + MockHttpServletRequest req = new MockHttpServletRequest("GET", "/api/stats"); + req.setRemoteAddr("203.0.113.42"); + String key = RateLimitFilter.clientKey(req); + assertThat(key).isEqualTo("ip:203.0.113.42"); + } + + @Test + void xForwardedFor_takesPrecedenceOverRemoteAddr() { + MockHttpServletRequest req = new MockHttpServletRequest("GET", "/api/stats"); + req.addHeader("X-Forwarded-For", "192.0.2.5, 10.0.0.1"); + req.setRemoteAddr("10.0.0.99"); + String key = RateLimitFilter.clientKey(req); + assertThat(key).isEqualTo("ip:192.0.2.5"); + } + + @Test + void authHeader_keyIsHashed_notRawToken() { + MockHttpServletRequest req = new MockHttpServletRequest("GET", "/api/stats"); + req.addHeader("Authorization", "Bearer SECRET-VALUE"); + String key = RateLimitFilter.clientKey(req); + assertThat(key).startsWith("auth:"); + assertThat(key).doesNotContain("SECRET-VALUE"); + // 16 hex chars after prefix. + assertThat(key).hasSize("auth:".length() + 16); + } + + @Test + void healthAndAssetPaths_bypassFilter() { + RateLimitFilter f = new RateLimitFilter(unifiedWithRate(60)); + for (String path : new String[]{ + "/", "/index.html", "/favicon.ico", + "/assets/main.css", "/static/img.png", "/error", + "/actuator/health", "/actuator/health/liveness", "/actuator/health/readiness"}) { + MockHttpServletRequest req = new MockHttpServletRequest("GET", path); + assertThat(f.shouldNotFilter(req)).as("Bypass for %s", path).isTrue(); + } + } + + @Test + void protectedPaths_runFilter() { + RateLimitFilter f = new RateLimitFilter(unifiedWithRate(60)); + for (String path : new String[]{ + "/api/stats", "/api/file", "/mcp", "/mcp/sse", + "/actuator/metrics", "/actuator/info", "/actuator/prometheus"}) { + MockHttpServletRequest req = new MockHttpServletRequest("GET", path); + assertThat(f.shouldNotFilter(req)).as("Filter on %s", path).isFalse(); + } + } + + @Test + void nullUnifiedConfig_usesSensibleDefault() { + // Constructor must not NPE when no config is wired (e.g. in some test scaffolding). + RateLimitFilter f = new RateLimitFilter(null); + assertNotNull(f); + } + + @Test + void zeroOrNegativeRate_fallsBackToDefault() { + RateLimitFilter f = new RateLimitFilter(unifiedWithRate(0)); + assertNotNull(f); + // No exception at construction — value is replaced with the audit-recommended default. + } + + private static MockHttpServletRequest req(String path, String authHeader) { + MockHttpServletRequest r = new MockHttpServletRequest("GET", path); + r.addHeader("Authorization", authHeader); + return r; + } + + private static CodeIqUnifiedConfig unifiedWithRate(int ratePerMin) { + return new CodeIqUnifiedConfig( + new ProjectConfig("test", null, null, List.of()), + new IndexingConfig(List.of(), List.of(), List.of(), null, null, null, null, null, null, null, null, null), + new ServingConfig(null, null, null, null, + new Neo4jConfig(null, null, null, null)), + new McpConfig(true, "http", "/mcp", + McpAuthConfig.empty(), + new McpLimitsConfig(15_000, 500, 2_000_000L, ratePerMin, 10), + new McpToolsConfig(List.of("*"), List.of())), + new ObservabilityConfig(true, false, "json", "info"), + new DetectorsConfig(List.of("default"), List.of(), List.of(), Map.of())); + } +} diff --git a/src/test/java/io/github/randomcodespace/iq/config/security/TokenResolverTest.java b/src/test/java/io/github/randomcodespace/iq/config/security/TokenResolverTest.java index 4ad4c38e..648a6b93 100644 --- a/src/test/java/io/github/randomcodespace/iq/config/security/TokenResolverTest.java +++ b/src/test/java/io/github/randomcodespace/iq/config/security/TokenResolverTest.java @@ -129,7 +129,7 @@ private static CodeIqUnifiedConfig unifiedAuth(McpAuthConfig auth) { new Neo4jConfig(null, null, null, null)), new McpConfig(true, "http", "/mcp", auth, - new McpLimitsConfig(15_000, 500, 2_000_000L, 300), + new McpLimitsConfig(15_000, 500, 2_000_000L, 300, 10), new McpToolsConfig(List.of("*"), List.of())), new ObservabilityConfig(true, false, "json", "info"), new DetectorsConfig(List.of("default"), List.of(), List.of(), Map.of())); diff --git a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsEvidenceTest.java b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsEvidenceTest.java index 1f2ccf5a..c6fc36d3 100644 --- a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsEvidenceTest.java +++ b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsEvidenceTest.java @@ -53,7 +53,7 @@ void setUp() { queryService, config, objectMapper, Optional.empty(), graphDb, statsService, topologyService, graphStore, - Optional.of(assembler), Optional.of(metadataProvider)); + Optional.of(assembler), Optional.of(metadataProvider), null); } @Test @@ -74,7 +74,7 @@ void getEvidencePackReturnsErrorWhenAssemblerAbsent() { queryService, new CodeIqConfig(), objectMapper, Optional.empty(), graphDb, statsService, topologyService, graphStore, - Optional.empty(), Optional.empty()); + Optional.empty(), Optional.empty(), null); String result = noAssembler.getEvidencePack("Foo", null, null, null); assertThat(result).contains("error"); @@ -93,7 +93,7 @@ void getArtifactMetadataReturnsErrorWhenAbsent() { queryService, new CodeIqConfig(), objectMapper, Optional.empty(), graphDb, statsService, topologyService, graphStore, - Optional.empty(), Optional.empty()); + Optional.empty(), Optional.empty(), null); String result = noMeta.getArtifactMetadata(); assertThat(result).contains("error"); diff --git a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsExpandedTest.java b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsExpandedTest.java index cc4a6d8f..a9460174 100644 --- a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsExpandedTest.java +++ b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsExpandedTest.java @@ -68,7 +68,7 @@ void setUp() { queryService, config, objectMapper, Optional.empty(), graphDb, statsService, new TopologyService(), graphStore, - Optional.empty(), Optional.empty() + Optional.empty(), Optional.empty(), null ); } diff --git a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java index 56f835bf..869cd101 100644 --- a/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java +++ b/src/test/java/io/github/randomcodespace/iq/mcp/McpToolsTest.java @@ -57,7 +57,7 @@ void setUp() { config = new CodeIqConfig(); CodeIqConfigTestSupport.override(config).rootPath(".").done(); objectMapper = new ObjectMapper(); - mcpTools = new McpTools(queryService, config, objectMapper, java.util.Optional.ofNullable(flowEngine), graphDb, statsService, new io.github.randomcodespace.iq.query.TopologyService(), graphStore, java.util.Optional.empty(), java.util.Optional.empty()); + mcpTools = new McpTools(queryService, config, objectMapper, java.util.Optional.ofNullable(flowEngine), graphDb, statsService, new io.github.randomcodespace.iq.query.TopologyService(), graphStore, java.util.Optional.empty(), java.util.Optional.empty(), null); } private Map parseJson(String json) throws IOException {