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.