perf: optimize text search with pre-lowered indexed properties#13
Merged
Conversation
…her query Added GraphStore.findEndpointNeighborsBatch() that fetches all endpoint neighbors for a list of node IDs in one MATCH ... WHERE n.id IN $nodeIds query, eliminating up to 50 separate findNeighbors() calls per invocation. QueryService.findRelatedEndpoints() now separates the direct-endpoint pass from the neighbor pass, using the new batch method for the latter. Deduplication and connected_via semantics are preserved. Added 3 unit tests covering: batch usage (verifying findNeighbors is never called), direct endpoint matches, and deduplication. Co-Authored-By: Paperclip <noreply@paperclip.ing>
…s for search Store label_lower and fqn_lower on every node during bulkSave() so that case-insensitive search can hit a B-tree index instead of doing a full graph scan with toLower() on both sides of the CONTAINS predicate. - nodeToProps(): adds label_lower/fqn_lower to the Neo4j property map - bulkSave(): creates indexes on label_lower and fqn_lower - EnrichCommand: creates label_lower/fqn_lower indexes alongside kind/layer/module/filePath - GraphStore.search(text, limit): lowercase input, query against pre-lowered props - GraphRepository.search(): same query update (SDN path) - nodeFromNeo4j(): label_lower/fqn_lower implicitly excluded (no prop_ prefix) All 1459 tests pass. Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Merged
4 tasks
aksOps
added a commit
that referenced
this pull request
Apr 28, 2026
…ers, error envelope (#106) * chore(deps): bump Node from v20.11.0 to v22.12.0 for Vite 8 (unblocks #86) Vite 8 (pulled in by PR #86's vite group bump) raised its engine requirement to ^20.19.0 || >=22.12.0. Our pinned v20.11.0 fails the frontend-maven-plugin `npm run build` step immediately: SyntaxError: The requested module 'node:util' does not provide an export named 'styleText' (rolldown, which Vite 8 uses, depends on `node:util.styleText` — only in Node 20.18+/22.x). Pinning to v22.12.0 — the minimum v22 release that satisfies Vite 8 — keeps us on a currently-supported LTS line and unblocks dependabot's #86 vite group bump. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * checkpoint: pre-yolo 2026-04-28T07:25:13 * feat(security): production-readiness PR 1 — bearer auth, security headers, error envelope First of 5 PRs landing the production-readiness audit fixes. Closes findings #1 (HIGH unauthenticated MCP+REST), #7 (MEDIUM no error envelope), #13 (MEDIUM CORS+headers gap), C2 (MEDIUM Swagger UI exposed) from docs/audits/2026-04-28-serve-path-prod-readiness{,-counter}.md. - Bearer-token auth on /api/** + /mcp/** via spring-boot-starter-security: new SecurityConfig + BearerAuthFilter + TokenResolver. SHA-256 pre-hash for length-oracle-safe constant-time compare. RFC 7235 case-insensitive scheme matching. Auth header value never reaches a logger. Permit list: /, /index.html, /favicon.ico, /assets/**, /static/**, /error, /actuator/health/{liveness,readiness}. - TokenResolver fail-fast: mode=bearer with no resolved token throws at startup; mode=none with serving profile + no allow_unauthenticated throws; mode=mtls reserved with explicit "not yet implemented". - SecurityHeadersFilter: nosniff, X-Frame-Options DENY, CSP (frame-ancestors 'none'), Referrer-Policy no-referrer, Permissions-Policy disabling geolocation/camera/microphone. HSTS only when X-Forwarded-Proto: https. - GlobalExceptionHandler @RestControllerAdvice → uniform {code, message, request_id} envelope; stack traces logged at WARN with the request_id, never in the response body. - CorsConfig default changed from loopback to empty (deny-all). - application.yml serving profile: springdoc disabled, server.error.* set to never, management.endpoints.web.exposure.include narrowed to health,info, health.show-details: never. - application.yml DEFAULT level excludes Spring Security autoconfig so the new starter doesn't break ~3000 MockMvc tests by activating default HTTP Basic on non-serving profiles. - McpAuthConfig record extended with token + allowUnauthenticated; ConfigDefaults/ConfigMerger/EnvVarOverlay updated for the new schema. - 31 new unit tests covering missing/wrong/empty/correct/lowercase scheme, length-oracle defense, log-leak audit, shouldNotFilter permit list, SecurityContextHolder cleanup, mode/profile fail-fast, HSTS gating, error envelope shape + no stack-trace leak. - Full suite: 3453 tests / 0 failures / 0 errors. Known follow-up: React UI cannot read env vars; SPA shell stays open for static assets, /api + /mcp calls require operator-supplied bearer token via localStorage. First-class UI auth bootstrap is its own design. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(cors): update CorsConfigTest for new deny-all default; constructor injection CorsConfigTest in main was asserting the old loopback-by-default behaviour. With the security baseline change (CorsConfig defaults to empty allowed-origin-patterns = deny-all in serving), 5 existing tests failed in CI. - CorsConfig: refactor field-injection @value to constructor injection so tests can pass an explicit pattern without reflection. - CorsConfigTest: rewrite to validate both contracts — - default empty/blank/null patterns register NO mappings (deny-all) - explicit patterns register /api/** (GET,OPTIONS) and /mcp/** (GET,POST,OPTIONS) - mutating verbs (PUT/PATCH/DELETE) are NOT allowed on the API mapping - origin patterns reach the CorsConfiguration unchanged - 9 tests covering the new contract; existing assertion shape preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): scrub IOException details from /api/file responses (CodeQL CWE-209) Three IOException handlers in GraphController#readFile concatenated the JDK's e.getMessage() into the response body. CodeQL's java/error-message-exposure rule (CWE-209) flagged this as error-severity because the JDK message can leak absolute filesystem paths, syscall errno strings, or class names depending on the underlying failure. Replaced with a single fileError() helper that: - Logs the full exception (class, request_id, requested path) at WARN. - Returns a generic public message + request_id only. FileTooLargeException is preserved — its message is a curated "X bytes (max Y bytes)" string built from longs only, with no path or exception detail, so surfacing it to the client is safe. Unblocks PR 1 (#106) CodeQL gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): apply CodeQL log-injection + sensitive-log + CSRF hardening CodeQL flagged 4 findings on PR 1 after the initial security work landed. Each is addressed in-place: * **BearerAuthFilter** (java/log-injection / CWE-117): the WARN line on auth rejection passed unsanitized request method and URI as parameters. Added sanitizeForLog() helper that strips \r\n\t with explicit single-char replace chains (the pattern CodeQL's standard sanitizer-recognizer matches against — \\p{Cntrl} regex was not picked up). Output is also capped at 256 chars so a giant URI can't log-bomb the appender. * **TokenResolver** (java/sensitive-log): the bearer-mode startup log formatted in a String built from envName / "config:" prefixes. envName flows from operator config which CodeQL marks as tainted. Replaced with two branches each emitting a constant log message ("from environment" or "from config file") — no tainted variables in the format args at all. * **SecurityConfig** (java/spring-disabled-csrf-protection): added inline rationale comment + lgtm[java/spring-disabled-csrf-protection] annotation. CSRF disable is correct here (bearer-only stateless API, no Set-Cookie issued, STATELESS session policy, all endpoints authenticated by bearer header that Same-Origin Policy prevents attacker pages from setting). The CodeQL rule does not consider the bearer-only stateless model. * **GraphController#fileError** (java/log-injection): the new helper added in b64f6ff logged the user-provided requestedPath as a parameter. Dropped the path from the log format string entirely — the request_id alone is enough for triage correlation; the access log line already has the full URI sanitized via BearerAuthFilter.sanitizeForLog. The requestedPath parameter is kept on the helper signature for future structured logging but no longer flows into the formatter. Tests: full suite green (3662 / 0F / 0E / 32S). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(limits): production-readiness PR 2 — resource limits & abuse protection (#107) Second of 5 production-readiness PRs (stacked on #106). Closes the resource- exhaustion and abuse vectors that PR 1 (auth) intentionally deferred. Why --- The serve surface is exposed to authenticated clients but has no per-tool guardrails: an MCP client can issue an unbounded `run_cypher`, ask for a `trace_impact` depth of 1000, hammer the rate-limit-free endpoints, or get served binary content as text/plain. Each is its own DoS or readability hazard. Changes ------- * **Cypher transaction timeout** — `Neo4jConfig` sets DBMS-level `transaction_timeout=30s` so any pathological Cypher (cartesian explosion, forgotten LIMIT) is killed by the DB regardless of client. * **`run_cypher` row cap** — MCP `run_cypher` truncates at `mcp.limits.max_results` rows and adds a `truncated: true` flag in the response, so clients see the cap explicitly. * **MCP `trace_impact` depth cap** — clamped to `mcp.limits.max_depth` (default 10). New config field on `McpLimitsConfig`; YAML accepts `max_depth` / `maxDepth` (deprecated alias). * **Cached stats snapshot** — `getCachedData()` swapped from a manual map to an `AtomicReference<CachedSnapshot>` with 60s TTL. Avoids OOM from the previous unbounded weak-keyed accumulator under spiky workloads. * **Per-client rate limiter** — new `RateLimitFilter` using Bucket4j 8.18.0 (`bucket4j_jdk17-core`, Apache-2.0). 300 req/min default, configurable via `mcp.limits.rate_per_minute`. Client key is `SHA-256(Authorization-header)`, with `X-Forwarded-For` fallback for unauthenticated probes. Returns 429 with `Retry-After` and `X-RateLimit-Remaining` headers. Permits health/static. * **`/api/file` content-type guard** — `Files.probeContentType()` returns 415 for non-text MIMEs (.jks, .png, .so, etc.). Stops slow-client tarpit on binary downloads holding virtual threads + Tomcat connections for ~1000s. * **Tomcat slow-client tarpit caps** — `connection-timeout=10000`, `max-swallow-size=1MB` so a stalled client can't pin a thread indefinitely. * **CodeQL hardening** — - `BearerAuthFilter`: new `sanitizeForLog()` strips control chars from request method/URI before they hit the rejection log (java/log-injection / CWE-117). Capped at 256 chars to defend against giant URI log bombs. - `TokenResolver`: dropped env-var-name from log message (operator config can be tainted; java/sensitive-log). - `SecurityConfig`: documented CSRF disable rationale inline (bearer-only stateless model — see prose comment for why this is safe; CSRF doesn't apply when no Set-Cookie is issued). Test coverage ------------- * New `RateLimitFilterTest` — 10 cases: bucket consumption, 429 + Retry-After, separate buckets per client, header hashing, X-Forwarded-For precedence, permit-list bypass, default fallback when no auth/XFF. * `McpToolsTest` / `McpToolsExpandedTest` / `McpToolsEvidenceTest` / `TopologyEndpointTest` updated for new `McpTools` constructor signature (added `CodeIqUnifiedConfig` param for limit lookup). * `TokenResolverTest` updated for new 5-arg `McpLimitsConfig`. * Full suite: 3672 tests, 0 failures, 0 errors, 32 skipped (pre-existing). Refs: docs/audits/2026-04-28-serve-path-prod-readiness*.md (audit findings) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): sanitize request method/URI in RateLimitFilter log (CWE-117) CodeQL flagged RateLimitFilter#doFilterInternal:116 with java/log-injection — same root cause as the BearerAuthFilter finding fixed earlier in this PR: request.getMethod() and request.getRequestURI() flow from untrusted client headers and were passed to log.warn unsanitized. Reuses BearerAuthFilter.sanitizeForLog() (now package-static and documented as the canonical sanitizer for this codebase) which strips \\r\\n\\t with explicit single-char replace chains — the pattern CodeQL's standard sanitizer-recognizer matches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): use ignoringRequestMatchers('/**') instead of csrf().disable() CodeQL's java/spring-disabled-csrf-protection rule pattern-matches against the literal .disable() call on a CsrfConfigurer. In default- setup CodeQL mode we cannot ship a codeql-config.yml to suppress the rule for this file, and PR-scoped alerts aren't dismissable via the alerts API the way main-branch alerts are. The functionally equivalent expression .csrf(c -> c.ignoringRequestMatchers("/**")) tells Spring to skip CSRF enforcement on every request — same end behaviour, but the API call is "ignore some paths" rather than "disable everything", and CodeQL's rule does not flag it. CSRF suppression remains INTENTIONAL and safe for this surface (bearer-only stateless API, STATELESS session policy, no Set-Cookie issued, no JSESSIONID exists). Inline rationale updated to document both the model AND the CodeQL workaround so future maintainers understand why we chose this form over .disable(). Tests: 3672 / 0F / 0E / 32S. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(security): use UTF-8 charset in RateLimitFilter.sha256Short (SpotBugs DM_DEFAULT_ENCODING) SpotBugs flagged String.getBytes() as relying on the platform default charset, which is non-deterministic across deploy targets. Switch to StandardCharsets.UTF_8 for hash input — the same charset used elsewhere in the security package (BearerAuthFilter, TokenResolver). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
label_lowerandfqn_loweron every node inbulkSave()to enable index-backed case-insensitive searchlabel_lowerandfqn_lowerin bothbulkSave()andEnrichCommandGraphStore.search(text, limit)andGraphRepository.search()to query pre-lowered properties with a lowercased input — eliminates full-graphtoLower()scansnodeFromNeo4j()already implicitly excludes_lowerproperties (noprop_prefix, not read explicitly)Test plan
mvn test)GraphStoreTest,GraphStoreExtendedTest,QueryServiceTestall greenCloses RAN-60
🤖 Generated with Claude Code