fix(elasticsearch): resolve Cloud ID to the real ES host and stop the timeout param collision

[waleedlatif1]

Summary

Three verified defects in the Elasticsearch integration, plus the consolidation of 13 duplicated copies of the connection helpers into one apps/sim/tools/elasticsearch/utils.ts.

---

1. Cloud ID resolved to a host that does not exist (all 13 tools)

An Elastic Cloud ID is <deployment label>:<base64 of "parentDomain$esUuid$kibanaUuid">. The reachable Elasticsearch endpoint is https://<esUuid>.<parentDomain>. Every tool carried its own copy of buildBaseUrl which did:

const parts = params.cloudId.split(':')
const decoded = Buffer.from(parts[1], 'base64').toString('utf-8')
const [esHost] = decoded.split('$')
return `https://${parts[0]}.${esHost}`   // parts[0] is the LABEL

parts[0] is the human-readable deployment label, not the Elasticsearch UUID, so every cloud request went to a name that resolves to nothing. Reproduction:

$ node -e "...decode a synthetic cloud id..."
builds my-dep.us-east-1.aws.found.io | correct esuuid.us-east-1.aws.found.io

The replacement (parseCloudId in utils.ts) follows the reference implementation in Beats' libbeat/cloudid/cloudid.go decodeCloudID():

• split the Cloud ID at its last colon, so a colon in the deployment label does not corrupt the payload;
• require at least three $-separated components;
• right-partition each component at its last colon for a per-service port (extractPortFromName), inheriting the parent domain's port and defaulting to 443;
• reject any component containing #, @, ?, or / — Beats' strings.IndexAny(component, "#@?/") reject set. This is the security-relevant part: an @ in the UUID component turns everything before it into URL userinfo, so https://<uuid>@evil.example.com would have sent the Authorization: ApiKey ... header to an attacker-controlled origin.

One addition beyond the Beats algorithm: the extracted port must be all digits. Beats validates the name but not the port, so uuid:80@evil.example.com survives its reject-set check (the @ ends up in the port half) and still yields an attacker-controlled authority. That is rejected here.

buildAuthHeaders was byte-identical in all 13 files and is now shared too. Net: −570/+49 lines.

2. elasticsearch_get_index declared a phantom index output — partly rejected

The declared output was index, but GET /<index> returns an object keyed by index name ({"logs-2024": {aliases, mappings, settings}}) — there is no index key at any level, so the entire payload was unreferenceable from downstream blocks. Fixed by returning { indices: <the keyed map> } and declaring indices.

Rejected sub-claim: the report also said the tool "silently keeps only one index when the request used a wildcard". That is not what the code does — transformResponse returned output: data verbatim, so every matched index was present; it was just unreachable because no declared output named it. A regression test now asserts a two-index wildcard response keeps both keys.

3. elasticsearch_cluster_health declared a param literally named timeout

apps/sim/tools/request-transport.ts reads params.timeout as the outbound HTTP deadline in milliseconds (Math.min(Number(rawTimeout), getMaxExecutionTimeout())). Measured against the old code with timeout: '30':

prepared.timeout = 30            // 30 ms client abort
prepared.url     = .../_cluster/health?timeout=30

So a cluster-health wait was also arming a client-side abort. The tool param is renamed clusterTimeout and mapped in tools.config.params (not tools.config.tool, which runs at serialization before variable resolution).

Two details this required:

• The subBlock id timeout is unchanged, so saved workflow state is not orphaned (check-block-registry.ts origin/staging passes the subblock-ID stability check).
• The generic handler merges as { ...inputs, ...transformedParams }, so the raw timeout input would still have reached the transport. tools.config.params therefore explicitly clears it ({ timeout: undefined }) alongside emitting clusterTimeout.

While there, the same mapper had a unit bug: it appended s to anything not already ending in s, so a user entering 1m got 1ms — a 1-millisecond server-side wait. It now only appends s to a bare integer.

4. Phantom outputs across the other 12 tools — rejected, none found

Every declared output on the other twelve tools was checked against the documented Elasticsearch response bodies (_search, _count, _bulk, _doc index/get/update/delete, PUT/DELETE /<index>, _cluster/health, _cluster/stats, _cat/indices?format=json). All declared fields are real response fields. The only phantom in the integration was get_index.index, covered above. No changes made.

---

Tests

New: tools/elasticsearch/utils.test.ts (28 tests) and tools/elasticsearch/cluster_health.test.ts (7 tests). 47 tests pass in tools/elasticsearch.

Coverage includes: label-vs-UUID host resolution, colon-in-label, per-service and inherited ports, the #@?/ reject set, the non-numeric-port smuggle, <3 components, empty ES component, self-hosted trailing-slash and missing-host, a parameterised sweep asserting all 13 tools resolve the same cloud host, prepareToolRequest leaving no HTTP deadline while still emitting timeout=30s on the wire, 1m not becoming 1ms, get_index declared outputs matching the transform's actual keys, and wildcard multi-index preservation.

Each test was verified to fail against the pre-fix code: reverting parseCloudId to the old algorithm turned 25 of 28 utils tests red, and reverting the get_index/cluster_health/block changes turned all 6 behavioural tests red. Both were then restored and re-run green.

Gates

bun run lint, bun run check:audits (39 audits, all green), bun run apps/sim/scripts/check-block-registry.ts origin/staging, and bun run type-check (no Elasticsearch diagnostics) all pass. tool-metadata:generate and generate-docs artifacts are regenerated and committed.

5. Two same-named interfaces in types.ts were declaration-merging

Follow-up from a validation pass over the branch. Commit 3 added an ElasticsearchIndexInfo interface for the GET /{index} state shape (aliases/mappings/settings), but that name was already taken further up the same file by the _cat/indices row shape (index, health, status, docsCount, storeSize, primaryShards, replicaShards). Two interface declarations with the same name in one module scope do not shadow — TypeScript declaration-merges them. The single resulting interface required all seven cat columns and carried the three optional index-state fields, so it described neither endpoint.

It compiles today only because transformResponse returns any from response.json(), so nothing in the integration ever assigns against the type. The defect is latent rather than live, but it is load-bearing in both directions, confirmed with a tsc probe:

• a valid GET /{index} entry is rejected by ElasticsearchIndexInfoResponse['output'] — TS2740: Type '{ mappings; settings; aliases }' is missing the following properties from type 'ElasticsearchIndexInfo': index, health, status, docsCount, and 3 more.
• a list_indices row carrying a nonexistent mappings key is accepted.

The new interface is renamed ElasticsearchIndexState — the name Elastic's own generated specification gives this object (indices._types.IndexState), so it is the endpoint's real name rather than one invented to dodge the clash. The _cat/indices row keeps ElasticsearchIndexInfo, matching the ElasticsearchListIndicesResponse that consumes it.

Type-only: not exported, no runtime behavior, no generated artifact moves.

---

Note for future readers: the cloud branch's missing fallback is a security property

buildBaseUrl deliberately throws when deploymentType === 'cloud' and cloudId is empty, instead of falling back to host. That reads like defensive tidiness and is an obvious candidate for "simplification". It is not — it is load-bearing, for two independent reasons:

1. Stale saved state. Switching the deployment dropdown leaves the previous host in saved workflow state. A fallback would send the cloud credential to a stale, unrelated origin.
2. deploymentType is model-settable. It is declared required: true with no explicit visibility, and apps/sim/tools/params.ts defaults a required param to user-or-llm. On the agent tool-calling path an LLM can therefore set deploymentType: 'cloud' while cloudId — which is user-only — stays empty. host, apiKey, username, and password are all user-only too, so the only reachable configuration is "cloud selected, no cloud ID, but a self-hosted host and credentials still present". Throwing is what stops that from silently shipping the user's Elasticsearch credentials to whatever origin host happens to hold.

Please do not replace the throw with params.cloudId ?? params.host.

Verification method for the output audit (section 4)

The per-tool output audit was re-run against elastic/elasticsearch-specification output/schema/schema.json — the generated OpenAPI spec that backs the published docs pages — rather than against the doc pages themselves, which truncate their response-field tables. Every declared output on all 13 tools resolves to a named property in that spec: cluster.stats.StatsResponseBase (confirming status, nodes.count.{total,data,master}, nodes.versions), cat.indices.IndicesRecord (confirming the seven cat columns are all string-typed in JSON format, which is why the parseInt is correct), indices.create.Response (index/acknowledged/shards_acknowledged all required), _types.WriteResponseBase, _global.count.Response, _global.bulk.Response, and indices._types.IndexState. No declared field rests on inference.

6. Credentials survived a cross-origin redirect (all 13 tools)

Every tool sends Authorization, but nothing was stripping it on a redirect off the configured origin. Verified against the transport rather than assumed: prepareToolRequest only populates redirectPolicy from tool.request.redirectPolicy, which Elasticsearch never declared, and the cross-origin stripping branch in lib/core/security/input-validation.server.ts is gated on that policy existing:

if (redirectHeaders && redirectPolicy && isCrossOrigin && (...)) { /* strip credentials */ }
if (redirectHeaders && options.stripAuthOnRedirect) { /* strip authorization */ }

With neither declared, a redirect carried the API key or Basic credentials to the redirect target. Opt-in is deliberate rather than an oversight — tools/index.test.ts asserts the redirect fields stay unset for tools that do not opt in — so the correct fix is to opt in, not to change the framework default.

All 13 tools now declare:

redirectPolicy: () => ({ mode: 'legacy', sendCredentialsOnCrossOriginRedirect: false })

This strips authorization, proxy-authorization, cookie and host via CROSS_ORIGIN_CREDENTIAL_HEADERS, and prepareToolRequest folds collectProvenanceSensitiveHeaders into the strip set as well — but only when the hop actually crosses origin.

The first attempt used stripAuthOnRedirect: true instead, which cubic correctly caught as the wrong primitive: it drops Authorization on every hop, including same-origin, so a reverse proxy in front of Elasticsearch performing a legitimate same-origin redirect would have 401'd. That flag is right for tools redirecting to signed storage URLs (obsidian, mintlify, s3, dataverse), where the credential must never follow at all; Elasticsearch's requirement is narrower.

mode: 'legacy' is deliberate — it preserves the existing method and body replay semantics, so the only behavior change is the cross-origin strip. Under 'standard', resolveRedirectHop applies Fetch method rules and a 301/302 would rewrite POST to GET, breaking _search, _count and _bulk. tools/github/utils.server.ts picks 'legacy' for the same reason.

On scope: this is pre-existing, not introduced by this PR. The tools always sent Authorization; the consolidation only moved where the header is built. It is fixed here rather than deferred to a follow-up because three things hold together, and the exception is not meant to generalize:

1. It is credential exfiltration, not a correctness or UX defect. A leaked Elasticsearch API key is unrecoverable in a way a wrong output field never is.
2. It is the same threat class in the same function this branch already hardens. This PR exists because buildBaseUrl could ship credentials to a stale or attacker-influenced host. A cross-origin redirect leaking the same credential from the same request is that bug's twin — closing one door while leaving the other open would invite exactly that question in review.
3. It is 13 one-line opt-ins with established precedent. obsidian, mintlify, and s3 — the same user-supplied-host profile — already set this flag for the same reason. No new mechanism, no design decision, nothing for a reviewer to weigh.

Had any one of those failed — a larger diff, a different function, or a novel approach — it would have been split into its own PR. Other pre-existing findings from this audit were deferred on exactly that basis; they are listed below.

Deferred to follow-ups (pre-existing, filed separately)

• Nullable tool outputs missing optional: true: get_document._version/_source (absent on the 404 branch, and _source also whenever _source_excludes strips it), delete_document._version, and cluster_stats.status (optional in StatsResponseBase).
• Block outputs not covering every tool: list_indices.message and count._shards are returned but undeclared, so they do not appear in the reference picker. Runtime resolution walks the real object, so hand-typed paths still resolve.

Both are declaration-only improvements that would force a tool-metadata:generate on a merge-ready branch, and neither is a security or correctness defect — which is precisely why they are deferred and the redirect fix is not.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Aug 29, 2026 5:27am

[greptile-apps]

Greptile Summary

The PR centralizes Elasticsearch connection helpers, corrects Elastic Cloud endpoint resolution, separates the cluster wait timeout from the HTTP deadline, and hardens redirect handling while preserving existing get_index references.

• Resolves Cloud IDs into validated Elasticsearch service endpoints.
• Preserves legacy top-level index keys while adding a discoverable indices aggregate.
• Renames the cluster-health tool parameter to avoid transport timeout collisions.
• Prevents credentials from crossing origins during redirects.
• Updates types, generated metadata, documentation, and regression tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; both previously reported output-compatibility issues are addressed by preserving raw top-level index keys and allowing a real indices index to retain its legacy meaning.

Important Files Changed

Filename	Overview
apps/sim/tools/elasticsearch/utils.ts	Centralizes authentication and URL construction with validated Cloud ID parsing.
apps/sim/tools/elasticsearch/get_index.ts	Adds the declared aggregate output while preserving legacy top-level index references and the prior collision fix.
apps/sim/tools/elasticsearch/cluster_health.ts	Separates Elasticsearch’s server-side wait timeout from the transport’s HTTP deadline.
apps/sim/blocks/blocks/elasticsearch.ts	Maps saved timeout state to the renamed runtime parameter without forwarding the transport-reserved key.
apps/sim/tools/elasticsearch/types.ts	Aligns response and parameter types with the corrected runtime contracts.

_{Reviews (7): Last reviewed commit: "fix(elasticsearch): strip credentials on..." | Re-trigger Greptile}

[cubic-dev-ai]

5 issues found across 21 files

Confidence score: 1/5

• apps/sim/tools/elasticsearch/create_index.ts can turn a decoded Cloud ID containing \ into an attacker-controlled URL origin, risking authenticated requests being sent to the wrong destination — validate or reject the decoded component before constructing the URL.
• apps/sim/tools/elasticsearch/bulk.ts now applies JSON handling to raw NDJSON, so Elasticsearch receives malformed bulk payloads and bulk operations fail — preserve application/x-ndjson or bypass JSON stringification.
• apps/sim/tools/elasticsearch/utils.ts may send cloud credentials to a supplied self-hosted host when cloudId is absent, creating a credential-routing risk — branch on deploymentType first and require a non-empty cloud ID for cloud deployments.
• apps/sim/tools/elasticsearch/get_index.ts can break existing {{getIndex.products.mappings}} references, while apps/sim/blocks/blocks/elasticsearch.ts risks changing reserved timeout state semantics; preserve top-level index paths and migrate the legacy timeout input separately.

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/create_index.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/create_index.ts:5">
P1: When a decoded Cloud ID component contains `\`, this import makes create-index use `parseCloudId`, which accepts it. `new URL` normalizes the resulting URL to an attacker-controlled origin, so the authenticated request can send `Authorization` there. Reject backslashes in the shared parser before returning the URL.</violation>
</file>

<file name="apps/sim/tools/elasticsearch/utils.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/utils.ts:75">
P1: When a cloud invocation omits `cloudId` but supplies `host`, `buildBaseUrl` silently uses the self-hosted host and sends the configured cloud credentials there. Branch on `deploymentType` first and require a non-empty Cloud ID for cloud deployments.</violation>
</file>

<file name="apps/sim/blocks/blocks/elasticsearch.ts">

<violation number="1" location="apps/sim/blocks/blocks/elasticsearch.ts:570">
P2: The block still publishes the server-side cluster-health wait under reserved name `timeout`. Rename the public input to `clusterTimeout` and migrate legacy `timeout` state separately so future execution paths cannot reintroduce the HTTP deadline collision.

(Based on your team's feedback about reserving `timeout` for transport deadlines.)</violation>
</file>

<file name="apps/sim/tools/elasticsearch/bulk.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/bulk.ts:5">
P1: Bulk requests now use the shared JSON content type, so `request-transport` JSON-stringifies the raw NDJSON body and Elasticsearch receives a malformed bulk payload. Preserve `application/x-ndjson` for this tool, or make the shared header helper support the bulk content type.</violation>
</file>

<file name="apps/sim/tools/elasticsearch/get_index.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/get_index.ts:94">
P1: Preserve or migrate the existing top-level index references before nesting the response under `indices`; otherwise saved paths such as `{{getIndex.products.mappings}}` stop resolving and downstream steps fail.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic</sub>

[waleedlatif1]

@greptile

Pushed 61cf417 addressing all six threads. Summary of what changed and what I pushed back on:

Fixed (4 P1s):

• bulk.ts — restored Content-Type: application/x-ndjson. The consolidation had flattened bulk onto the shared JSON type; _bulk 406s on application/json. Note the flagged mechanism was off: the body was never JSON-stringified (formatToolRequest passes string bodies through on both branches) — the header was the break.
• utils.ts buildBaseUrl — now branches on deploymentType alone and requires a Cloud ID for cloud deployments, so a stale hidden host can no longer receive the cloud credential.
• utils.ts reject set — added \, which the WHATWG URL parser treats as a path separator and which therefore truncates the authority (https://uuid\evil.example.com resolves to host uuid). Beats does not need this because Gos net/urlgives` no special meaning; our JS port does.
• get_index.ts — output is now { ...data, indices: data }. Reference resolution walks the runtime object, not the declared outputs, so the old top-level index keys did resolve and now keep resolving, while indices provides the declared/discoverable output that removes the original phantom index key.

Pushed back (1 P2): renaming the timeout subBlock id would orphan saved workflow state and fails the check-block-registry.ts subblock-ID stability gate, with no rename migration available. The collision it targets is already severed at the transport layer and pinned by a prepareToolRequest test. Detail on the thread.

53 tests pass in tools/elasticsearch. Each of the 5 new tests was verified red against the pre-fix code. lint, check:audits (39/39), check-block-registry.ts origin/staging, and type-check are green; metadata and docs artifacts regenerated.

[waleedlatif1]

@greptile

Pushed 90b2dbc closing the indices collision — the one finding from the last pass. Spread order reversed to { indices: data, ...data }, so every top-level index key now retains its exact pre-PR runtime meaning including an index literally named indices. New test covers it and was verified red against the previous order.

54 tests pass in tools/elasticsearch; lint, check:audits (39/39), check-block-registry.ts origin/staging, and type-check all green.

[waleedlatif1]

@greptile review

[waleedlatif1]

@cubic review

[cubic-dev-ai]

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

2 issues found across 21 files

Confidence score: 2/5

• apps/sim/tools/elasticsearch/utils.ts buildBaseUrl can route an unrecognized deploymentType to the stale self-hosted host while sending cloud credentials, creating a concrete credential-disclosure risk; reject unsupported values before selecting the host or shared auth headers.
• apps/sim/tools/elasticsearch/get_index.ts can let a matched index named indices overwrite the aggregate output.indices map, producing incorrect matched-index output; spread raw keys first and assign indices afterward.

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:82">
P1: When `deploymentType` is an unrecognized runtime value, `buildBaseUrl` falls through to the stale self-hosted `host` and the shared auth headers send the cloud credential there. Reject values other than `cloud` and `self_hosted` instead of treating every non-cloud value as self-hosted.</violation>
</file>

<file name="apps/sim/tools/elasticsearch/get_index.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/get_index.ts:106">
P2: When a matched index is named `indices`, `...data` overwrites the declared aggregate, so `output.indices` becomes that index's state instead of the map of matched indices. Spread raw keys first and assign `indices: data` last, or add a separate compatibility alias, so the declared output contract always holds.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[waleedlatif1]

@cubic review

[waleedlatif1]

@greptile review

[cubic-dev-ai]

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

3 issues found across 21 files

Confidence score: 2/5

• apps/sim/tools/elasticsearch/utils.ts now sends credentials on requests without stripping them across cross-origin redirects, so an Elasticsearch redirect could disclose the API key or Basic credentials; add an explicit credential-stripping or redirect policy before merging.
• apps/sim/tools/elasticsearch/utils.ts treats an empty deploymentType as self-hosted while cloud credentials may still be supplied, which can route requests to the stale host and fail or misdirect access; reject every non-nullish value other than self_hosted.
• apps/sim/tools/elasticsearch/types.ts cannot accurately represent an index named indices, leaving getIndexTool output inconsistent with the declared type; allow output.indices to be either a single index state or the aggregate map.

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/types.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/types.ts:197">
P2: When an Elasticsearch index is named `indices`, `getIndexTool` returns that index's state at `output.indices`, but this type requires an aggregate map. Type `indices` as `ElasticsearchIndexState | Record<string, ElasticsearchIndexState>` so consumers do not rely on an invalid shape.</violation>
</file>

<file name="apps/sim/tools/elasticsearch/utils.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/utils.ts:99">
P1: When `deploymentType` is the empty string, this guard routes the request to the stale self-hosted host while the caller can still supply cloud credentials. Reject every non-nullish value other than `self_hosted` so only the documented legacy nullish fallback can select `host`.</violation>

<violation number="2" location="apps/sim/tools/elasticsearch/utils.ts:129">
P1: Every Elasticsearch request now sends an `Authorization` header, but cross-origin redirects have no credential-stripping policy. A redirect from an Elasticsearch endpoint can therefore disclose the API key or Basic credentials; configure these requests to strip credentials and `host` on cross-origin redirects, or make that the transport default.

(Based on your team's feedback about stripping credentials on cross-origin redirects by default.)</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[waleedlatif1]

@cubic review

[waleedlatif1]

@greptile review

[cubic-dev-ai]

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

3 issues found across 21 files

Confidence score: 3/5

• apps/sim/tools/elasticsearch/get_document.ts and apps/sim/tools/elasticsearch/count.ts strip Authorization on same-origin redirects, which can turn valid Elasticsearch requests into 401s and break document retrieval or count operations — preserve credentials for trusted same-origin redirects while still stripping them for untrusted targets.
• apps/sim/tools/elasticsearch/types.ts keeps ElasticsearchIndexState unavailable to consumers even though ElasticsearchIndexInfoResponse references it, blocking downstream TypeScript adoption of the documented response shape — export the interface.

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/types.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/types.ts:189">
P3: Consumers cannot import the new `ElasticsearchIndexState` type referenced by `ElasticsearchIndexInfoResponse`. Export the interface so downstream TypeScript code can adopt the documented response shape.</violation>
</file>

<file name="apps/sim/tools/elasticsearch/get_document.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/get_document.ts:107">
P2: When Elasticsearch returns a same-origin 3xx, `stripAuthOnRedirect` removes `Authorization` before replaying the request, so a valid authenticated request can become a 401. Configure the redirect policy to strip credentials only on cross-origin hops.

(Based on your team's feedback about stripping credentials only on cross-origin redirects.)</violation>
</file>

<file name="apps/sim/tools/elasticsearch/count.ts">

<violation number="1" location="apps/sim/tools/elasticsearch/count.ts:82">
P2: When Elasticsearch or its reverse proxy returns a same-origin redirect, this flag removes the Authorization header before replaying the request, so the canonical endpoint can return 401 and the count operation fails. Use a redirect policy that disables credentials only for cross-origin redirects while preserving the legacy method behavior.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P3: Consumers cannot import the new ElasticsearchIndexState type referenced by ElasticsearchIndexInfoResponse. Export the interface so downstream TypeScript code can adopt the documented response shape.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/tools/elasticsearch/types.ts, line 189:

<comment>Consumers cannot import the new `ElasticsearchIndexState` type referenced by `ElasticsearchIndexInfoResponse`. Export the interface so downstream TypeScript code can adopt the documented response shape.</comment>

<file context>
@@ -185,15 +185,24 @@ export interface ElasticsearchIndexResponse extends ToolResponse {
 }
 
+/** One entry of a `GET /{index}` response, keyed by index name. */
+interface ElasticsearchIndexState {
+  aliases?: Record<string, unknown>
+  mappings?: Record<string, unknown>
</file context>

Suggested change

	interface ElasticsearchIndexState {
	export interface ElasticsearchIndexState {

[waleedlatif1]

Leaving this one as-is — it would break the file's own convention rather than follow it.

types.ts deliberately keeps response-detail interfaces module-private and exports only the ToolResponse wrappers. ElasticsearchIndexState is not an exception; it is the rule. The clearest counter-example is its immediate neighbour:

interface ElasticsearchIndexInfo { index: string; health: string; /* … */ }   // not exported

export interface ElasticsearchListIndicesResponse extends ToolResponse {
  output: { message: string; indices: ElasticsearchIndexInfo[] }             // referenced from an exported type
}

That is the same shape as the finding — a private interface referenced by an exported response type — and it predates this PR. The same is true of ElasticsearchIndexExistsResponse, ElasticsearchMappingResponse, ElasticsearchRefreshResponse and ElasticsearchIndexStatsResponse. Exporting only the one interface this PR happens to touch would leave the file half-converted and make the next reader wonder why that one is special.

On the practical side, there is no consumer to unblock. getIndexTool is typed ToolConfig<ElasticsearchGetIndexParams, ElasticsearchIndexInfoResponse>, the executor handles tool outputs generically, and nothing in the repo imports these detail interfaces — I checked (grep -rn 'ElasticsearchIndexInfo' tools blocks lib app returns only types.ts and get_index.ts's import of the response type). TypeScript also still lets a consumer reach the shape structurally via ElasticsearchIndexInfoResponse['output']['indices'] without the name being exported.

If we do want these public, that is a worthwhile consistency pass across all five private interfaces in the file — but as its own change, not smuggled into a security fix where a reviewer is looking at redirect behavior. Happy to file it if you'd like.

[waleedlatif1]

@cubic review

[waleedlatif1]

@greptile review

[cubic-dev-ai]

@cubic review

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

1 issue found across 21 files

Confidence score: 4/5

• In apps/sim/tools/elasticsearch/utils.ts, extractPortFromName can accept a Cloud ID with multiple colons and produce a malformed hostname, potentially resolving to an unintended host/port; reject components containing extra colons before parsing.

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:57">
P2: When a Cloud ID component contains more than one colon, `extractPortFromName` leaves a colon in the hostname and this check does not reject it, allowing a malformed Cloud ID to resolve to an unintended host/port. Reject any colon remaining in the extracted component name before constructing the URL.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When a Cloud ID component contains more than one colon, extractPortFromName leaves a colon in the hostname and this check does not reject it, allowing a malformed Cloud ID to resolve to an unintended host/port. Reject any colon remaining in the extracted component name before constructing the URL.

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 57:

<comment>When a Cloud ID component contains more than one colon, `extractPortFromName` leaves a colon in the hostname and this check does not reject it, allowing a malformed Cloud ID to resolve to an unintended host/port. Reject any colon remaining in the extracted component name before constructing the URL.</comment>

<file context>
@@ -0,0 +1,142 @@
+  }
+
+  for (const component of [parentDomain.name, elasticsearch.name]) {
+    if (CLOUD_ID_REJECTED_CHARACTERS.test(component)) {
+      throw new Error('Invalid Cloud ID format')
+    }
</file context>

Suggested change

	if (CLOUD_ID_REJECTED_CHARACTERS.test(component)) {
	if (component.includes(':') || CLOUD_ID_REJECTED_CHARACTERS.test(component)) {

[waleedlatif1]

Declining this one — I ran it rather than reasoning about it, and no input reaches an unintended host. Every extra-colon case either resolves to exactly what the Cloud ID encodes, or fails closed.

Driving parseCloudId directly:

single colon in parent (normal port)   -> https://uuid.found.io:9243   | host=uuid.found.io port=9243
EXTRA colon in parent                  -> https://uuid.found.io:9243   | host=uuid.found.io port=9243
EXTRA colon in es component            -> https://uuid:80.found.io     | URL-INVALID
colon-smuggled second host in es       -> https://uuid.found.io        | host=uuid.found.io port=(443)
attempt userinfo via colon+at          -> THROWS: Invalid Cloud ID format

Three things follow:

1. A leftover colon cannot redirect credentials. Reaching a different origin from the authority requires @ (userinfo), and @ is already in the reject set — the last case throws. A : can only set a port on the same host or corrupt the authority.
2. A corrupted authority fails closed, it does not silently connect. https://uuid:80.found.io is not a parseable URL: everything after the colon must be digits, and 80.found.io is not, so new URL rejects it and assertExternalRequestUrl in prepareToolRequest throws before any request goes out. There is no "resolves to an unintended host" path here.
3. The one case that does produce a valid URL is the correct answer. For a parent component of found.io:9243:443, https://uuid.found.io:9243 is the host and port that Cloud ID actually encodes. This is also exactly what the reference implementation does — Beats' extractPortFromName in libbeat/cloudid/cloudid.go uses the same strings.LastIndex right-partition and likewise leaves an inner colon in the name. Rejecting it would make us stricter than the algorithm we are implementing, for no security gain.

Worth noting the threat model too: cloudId is user-only, so the user pastes their own. Even granting a hostile value, a colon grants nothing beyond what the parent-domain component already permits by design — a Cloud ID names its own host.

The one real (cosmetic) difference is the error a user sees: case 2 surfaces a URL parse error from the transport rather than Invalid Cloud ID format from the parser. If you want that tidied I am happy to file it, but it is a message-quality change, not the host/port vulnerability described, so I would rather not land it in a security fix at 5/5 on the other reviewer. Leaving this open for you to weigh in rather than resolving my own disagreement.

[waleedlatif1]

Closing for now — not because of a defect. This batch grew to 17 PRs across ~700 changed call sites, and we would rather revisit it as smaller, independently testable pieces than merge this much at once.

Nothing here is lost: the branch fix/elasticsearch-cloud-id is preserved and this PR can be reopened. Review state, the reasoning on every thread, and the red-first verification all stay attached.