Skip to content

Use async ES|QL for execute-esql so frozen-tier queries don't time out - #64

Open
stephmilovic wants to merge 2 commits into
elastic:mainfrom
stephmilovic:async-esql-query
Open

Use async ES|QL for execute-esql so frozen-tier queries don't time out#64
stephmilovic wants to merge 2 commits into
elastic:mainfrom
stephmilovic:async-esql-query

Conversation

@stephmilovic

@stephmilovic stephmilovic commented Aug 18, 2026

Copy link
Copy Markdown

Summary

execute-esql was a thin POST /_query wrapper on a 30s HTTP timeout, so frozen-tier / searchable-snapshot queries died before they finished. It now submits POST /_query/async and polls GET /_query/async/{id}, same path Discover uses.

Fast queries still return on the first response. If the query is still running after 5 minutes, we cancel it and return a clear error.

To test

  • npx vitest run src/elastic/client/esqlClient.test.ts src/test/integration/server.integration.test.ts
  • Against a cluster with frozen data, run execute-esql (or Execute in the threat-hunt workbench) on a query that previously timed out. It should return rows instead of a 30s timeout.

MCP hosts can still cut off a single tool call around 60s. This unblocks queries that finish within that window, and it keeps the ES query alive across poll requests instead of dying on the client timeout.

PR developed with Cursor + Cursor Grok 4.6

Made with Cursor

…complete

The sync POST /_query path dies on the 30s HTTP timeout when searchable
snapshots have to thaw. Submit and poll /_query/async instead, matching
what Discover already does.

Co-authored-by: Cursor <cursoragent@cursor.com>
@patrykkopycinski

Copy link
Copy Markdown
Collaborator

Security Review: PR #64 — Async ES|QL for execute-esql

Overall risk: 🟢 LOW — 0 critical, 0 high, 2 medium, 2 low

The PR switches execute-esql from synchronous POST /_query (30s timeout) to async POST /_query/async + polling GET /_query/async/{id}, matching what Discover already does. The implementation is clean, well-tested, and introduces no new security vulnerabilities.


Scanning Results

Check Status Notes
XSS N/A No UI rendering changes
Query Injection ✅ Safe Query passed as structured JSON body field, not interpolated
Auth Bypass / RBAC ✅ No change Same pre-authenticated esClient injection
Path Traversal ✅ Mitigated encodeURIComponent(id) on poll path; test verifies abc+abc%2B
Command Injection N/A No shell execution
CSRF N/A MCP tool, not web endpoint
Input Validation ✅ Acceptable z.string() for query — by design for LLM-provided ES

Medium Findings

MEDIUM-001: keep_alive and ESQL_ASYNC_MAX_WAIT_MS are independently hardcoded — can drift

keep_alive: "5m" is a hardcoded string in the submit body, while ESQL_ASYNC_MAX_WAIT_MS = 300_000 is an independent constant. If someone increases ESQL_ASYNC_MAX_WAIT_MS without updating keep_alive, the client will keep polling a query that ES has already expired and cleaned up — every poll returns is_running: true with no results, burning the full timeout budget silently.

Recommendation: Derive keep_alive from ESQL_ASYNC_MAX_WAIT_MS:

const KEEP_ALIVE = `${Math.ceil(ESQL_ASYNC_MAX_WAIT_MS / 60_000)}m`;

MEDIUM-002: No keep_alive extension on poll — subtle wall-clock edge case

The submit sets keep_alive: "5m" and waits up to 5s. The poll loop then runs for up to 5 min. Total wall clock: up to 5 min 5 sec, but the query's ES-side keep_alive is only 5 min from submit time. In the worst case, ES could clean up the async query ~5s before the client gives up polling, causing final polls to 404 instead of returning results or hitting the timeout cleanly.

ES async ES|QL supports extending keep_alive on the poll GET request. The code doesn't pass it.

Recommendation: Add keep_alive to the poll request params:

params: {
  format: "json",
  wait_for_completion_timeout: ESQL_ASYNC_POLL_WAIT,
  keep_alive: KEEP_ALIVE,  // extend on each poll
},

Low Findings

LOW-001: Async query ID in error message

The timeout error includes the async query ID (Async query id: ${id}). Not PII or a secret — including it aids debugging. No action needed unless the error propagates to end users.

LOW-002: completedResult treats missing is_running as "complete"

If ES returns is_running: false but missing columns/values (e.g., an error-shaped response), completedResult returns undefined and the loop continues polling. Defensive but could mask an error response as "still running." Low impact since the poll will eventually time out and clean up.


What's Done Right

  • Resource cleanup: Async queries are deleted on all exit paths (timeout, poll error, success). deleteAsyncQuery swallows errors correctly.
  • Bounded polling: ESQL_ASYNC_MAX_POLLS = 30 iterations, each with 10s wait + 20s HTTP timeout. No infinite loop risk.
  • Test coverage: Immediate completion, polling, ID encoding, missing ID, timeout + cleanup, poll-error + cleanup, delete-failure resilience, transport errors. Integration test verifies async path hits correct ES base URL.

Verdict: No blockers. Safe to merge as-is. The medium findings are reliability improvements, not security vulnerabilities.

Derive keep_alive from the max wait, and extend it on each poll so ES
does not expire the query a few seconds before we give up.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants