fix(elasticsearch): resolve Cloud IDs to the real host and correct the integration's output, timeout, and redirect defects - #7276
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThe PR centralizes Elasticsearch endpoint and authentication handling while correcting Cloud ID resolution, redirect credentials, timeout mapping, and output contracts.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/tools/elasticsearch/utils.ts | Centralizes Cloud ID endpoint resolution and authentication-header construction with hostname and port validation. |
| apps/sim/blocks/blocks/elasticsearch.ts | Preserves the saved timeout sub-block ID while mapping it away from the transport-reserved parameter and adds system-index configuration. |
| apps/sim/tools/elasticsearch/get_index.ts | Declares an aggregate indices output while retaining raw index-name keys for compatibility. |
| apps/sim/tools/elasticsearch/list_indices.ts | Makes response handling tolerant of malformed rows and adds an opt-in for system indices. |
| apps/sim/tools/elasticsearch/cluster_health.ts | Separates the Elasticsearch server-side wait from the transport timeout and applies redirect credential protection. |
| apps/sim/tools/elasticsearch/responses.test.ts | Verifies response contracts, optional outputs, and redirect policies across the integration. |
Reviews (4): Last reviewed commit: "fix(elasticsearch): reject a surviving c..." | Re-trigger Greptile
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
@cubic review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
… and redirect credentials
waleedlatif1
changed the title
|
@cubic review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
|
@cubic review |
@waleedlatif1 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 22 files
Confidence score: 3/5
- In
apps/sim/tools/elasticsearch/utils.ts, a cloud deployment withoutcloudIdcan fall through to a stale self-hosted host and send operations to the wrong cluster; branch explicitly on deployment type and handle the missing cloud ID before selecting a host.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/tools/elasticsearch/utils.ts">
<violation number="1" location="apps/sim/tools/elasticsearch/utils.ts:88">
P1: When `deploymentType` is `cloud` but `cloudId` is missing, this condition falls through to the self-hosted host. A stale host can therefore send a cloud-configured operation to the wrong cluster; branch on deployment type first and reject a missing Cloud ID.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| if (params.deploymentType === 'cloud' && params.cloudId) { | ||
| return parseCloudId(params.cloudId) | ||
| } |
There was a problem hiding this comment.
P1: When deploymentType is cloud but cloudId is missing, this condition falls through to the self-hosted host. A stale host can therefore send a cloud-configured operation to the wrong cluster; branch on deployment type first and reject a missing Cloud ID.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/tools/elasticsearch/utils.ts, line 88:
<comment>When `deploymentType` is `cloud` but `cloudId` is missing, this condition falls through to the self-hosted host. A stale host can therefore send a cloud-configured operation to the wrong cluster; branch on deployment type first and reject a missing Cloud ID.</comment>
<file context>
@@ -0,0 +1,125 @@
+ * Elastic Cloud ID or a self-hosted host URL.
+ */
+export function buildBaseUrl(params: ElasticsearchBaseParams): string {
+ if (params.deploymentType === 'cloud' && params.cloudId) {
+ return parseCloudId(params.cloudId)
+ }
</file context>
| if (params.deploymentType === 'cloud' && params.cloudId) { | |
| return parseCloudId(params.cloudId) | |
| } | |
| if (params.deploymentType === 'cloud') { | |
| if (!params.cloudId) { | |
| throw new Error('Cloud ID is required for cloud deployments') | |
| } | |
| return parseCloudId(params.cloudId) | |
| } |
1 participant
Summary
Fixes the Elastic Cloud endpoint bug, plus every defect a full
/validate-integrationpass turned up in this integration.1. Cloud ID resolved to a host that does not exist (all 13 tools)
A Cloud ID is
<deployment label>:<base64 of "parentDomain$esUuid$kibanaUuid">and the reachable endpoint ishttps://<esUuid>.<parentDomain>. Every tool builthttps://<label>.<parentDomain>— the human-readable label, not the Elasticsearch UUID — so Elastic Cloud was unusable in every operation.Replaced with
parseCloudIdin a newapps/sim/tools/elasticsearch/utils.ts, following Beats'libbeat/cloudid/cloudid.godecodeCloudID(): split at the last colon so a colon in the label cannot corrupt the payload; require ≥3$-separated components; right-partition each component at its last colon for a per-service port, inheriting the parent domain's port and defaulting to 443.Rejects the component characters Beats rejects (
#@?/) plus\. This is the security-relevant part — an@in the UUID component turns everything before it into URL userinfo, sohttps://<uuid>@evil.example.comwould sendAuthorization: ApiKey …to an attacker-controlled origin. Two checks beyond Beats. First, both ports must be all digits — Beats validates neither, anduuid:80@evil.example.comsurvives its reject set by landing the@in the port half. Second,:is rejected in a component name:extractPortFromNamehas already split at the last colon, so a colon surviving in the name half means the component carried two, andfound.io:9243:5would otherwise assemblehttps://<uuid>.found.io:9243:5and fail as a bareTypeError: Invalid URLinside the transport. The port check does not cover that case — it only fires when the trailing half is non-numeric — so the two checks carry separate cases.buildBaseUrlandbuildAuthHeaderswere duplicated across all 13 tool files, byte-identical apart from_bulk's NDJSON media type. Now shared, with_bulkpassing its content type as an argument.2.
elasticsearch_get_indexdeclared a phantom outputDeclared
index, butGET /{index}returns a map keyed by index name —{"logs-2024": {aliases, mappings, settings}}. There is noindexkey at any level, so the whole payload was unreferenceable from downstream blocks.Now returns
{ indices: data, ...data }and declaresindices. The raw per-index keys are spread alongside so references saved beforeindicesexisted keep resolving; an index legitimately namedindicesis spread last and wins. The error branch returns{ indices: {} }so both branches satisfy the declared shape.3.
elasticsearch_cluster_healthdeclared a param literally namedtimeouttools/request-transport.tsreadsparams.timeoutas the outbound HTTP deadline in milliseconds. On the block path this was inert — the mapper always suffixeds, andNumber('30s')isNaN, which the transport discards. On the agent tool-calling path it was live: a model emittingtimeout: 30as a number skips the mapper'stypeof === 'string'guard, and the generic handler merges raw inputs over the transform, so it arrived as a 30 ms client abort.Renamed to
clusterTimeout, mapped intools.config.params(nottools.config.tool, which runs at serialization before variable resolution). The mapper also clearstimeoutexplicitly, because the handler merges{ ...inputs, ...transformedParams }and the raw input would otherwise still reach the transport. The subBlock id staystimeout, so saved workflow state is not orphaned —check-block-registry.tssubblock-ID stability passes.Same mapper had a unit bug: it appended
sto anything not already ending ins, so1mbecame1ms— a 1-millisecond server-side wait. It now only appendssto a bare integer.4. Credentials survived a cross-origin redirect (all 13 tools)
Every tool sends
Authorization, but nothing stripped it on a redirect off the configured origin.prepareToolRequestonly populatesredirectPolicyfrom the tool, and the stripping branch inlib/core/security/input-validation.server.tsis gated on that policy existing — so a redirect carried the API key or Basic credentials to the redirect target.All 13 now declare
{ mode: 'legacy', sendCredentialsOnCrossOriginRedirect: false }.stripAuthOnRedirectis deliberately not used: it dropsAuthorizationon every hop including same-origin, which would 401 a reverse proxy in front of Elasticsearch issuing a legitimate same-origin redirect.mode: 'legacy'preserves method and body replay — under'standard', a 301/302 would rewrite POST to GET and break_search,_countand_bulk.5.
list_indicesdropped indices silently and could throwitem.index.startsWith('.')threw outright on a_catrow with noindexcolumn, and every system index was filtered out with no way to opt in. Now guarded, tolerant of a non-array body, and opt-in via an advancedincludeSystemIndicesdropdown. Default behavior is unchanged.6. Nullable outputs missing
optional: trueget_document._versionand._source(both absent on the 404 branch;_sourcealso whenever_source_excludesstrips it),delete_document._version,create_index.shards_acknowledgedand.index,cluster_stats.status.7. Block outputs did not cover every tool
list_indices.messageandcount._shardswere returned but undeclared, so they were missing from the reference picker.Backwards compatibility
Zero subBlock ids removed or renamed, zero
requiredflips (one addition, a new optional param), zerovisibilitychanges.check-block-registry.ts origin/stagingpasses the subblock-ID stability check.Behavior only moves in one direction: a valid Cloud ID that produced an unreachable host now produces the reachable one; a malformed one threw before and throws now.
get_indexkeeps its raw keys.list_indiceskeeps its default filter. ThetimeoutsubBlock keeps its id and its saved values.Type of Change
Testing
86 tests across
utils.test.ts,cluster_health.test.ts,responses.test.tsand the existingsearch.test.ts.Covers label-vs-UUID host resolution, colon-in-label, per-service and inherited ports, the
#@?/\reject set, both port-smuggle variants,<3components, self-hosted trailing-slash and missing-host, a sweep asserting all 13 tools resolve the same cloud host,_bulkmedia type,prepareToolRequestleaving no client deadline while emittingtimeout=30s, the1m/500ms/2hunit cases,get_indexdeclared-vs-actual output parity on both branches,list_indicesfiltering/opt-in/malformed rows, the redirect policy on all 13 tools, and theoptionalflags.Every fix was verified red-first by reverting it and watching its tests fail: get_index 2, list_indices 3, redirect policy 10, optional flags 4, timeout mapper 6.
bun run lint,bun run type-check(no Elasticsearch diagnostics),bun run check:audits(39 audits), andcheck-block-registry.ts origin/stagingall pass.tool-metadata:generateandgenerate-docsartifacts regenerated and committed.Checklist