Skip to content

[Fleet] Validate SSL certificate paths for whitespace#266365

Merged
juliaElastic merged 21 commits intoelastic:mainfrom
juliaElastic:fix-ssl-path-spaces-validation
May 1, 2026
Merged

[Fleet] Validate SSL certificate paths for whitespace#266365
juliaElastic merged 21 commits intoelastic:mainfrom
juliaElastic:fix-ssl-path-spaces-validation

Conversation

@juliaElastic
Copy link
Copy Markdown
Contributor

@juliaElastic juliaElastic commented Apr 29, 2026

Summary

Fixes #265694

TLS certificate fields in Fleet settings forms (Agent binary source, Output, Fleet Server Hosts, Fleet Proxy) accept file paths with spaces without validation. These paths are propagated into the generated agent policy, causing Elastic Agent to fail to resolve the certificate file and become unhealthy.

Changes

Shared validator — common/services/ssl_validators.ts

  • validateSslCertPath(value): rejects whitespace in file paths; exempt when value is PEM content (leading -----BEGIN). Works for Linux, Windows (C:\, C:/), and UNC (\\server\share) paths without platform-specific handling.

Client-side adapters — ssl_form_validators.ts

  • validateSslPathInput — adapter for useInput (returns string[] | undefined)
  • validateSslPathsCombo — adapter for useComboInput (returns Array<{message, index}> | undefined)

Four form hooks wired

  • use_download_source_flyout_form.tsx — 3 inputs
  • use_output_form.tsx + output_form_validators.tsx — 2 combo inputs + extended existing validateSSLCertificate/validateSSLKey
  • use_fleet_server_host_form.tsx — 9 inputs across 3 SSL groups (server, ES, agent)
  • use_fleet_proxy_form.tsx — 3 inputs

Four server-side handlers hardened

  • download_source/handler.ts, fleet_server_hosts/handler.ts, fleet_proxies/handler.ts, output/handler.ts — each gains a throwIfSslPathInvalid helper calling the shared common function, returning 400 Bad Request for invalid paths.

How to verify

  1. Navigate to Fleet → Settings → Agent binary sources → Add / Edit source
  2. In the TLS Certificate section, enter a path with spaces (e.g. /path/to my cert.pem) in any certificate field
  3. Confirm the form shows an error and cannot be saved
  4. Confirm a valid path (e.g. /path/to/cert.pem) or inline PEM content passes validation

🤖 Generated with Claude Code

image

Paths containing spaces in TLS certificate fields (certificate
authorities, client certificate, key) are silently accepted and
propagated into the generated agent policy. Elastic Agent cannot
resolve such paths, causing Fleet Server and agents to become
unhealthy.

Add validateSslCertPath() in common/services/ssl_validators.ts so
the check is shared between the UI layer and the server. PEM content
(identified by a leading "-----BEGIN" header) is exempt — only values
treated as file paths are rejected. Covers all path formats: Linux
absolute/relative, Windows C:\ and C:/, and UNC \\server\share.

Client side: two adapter functions bridge the common validator to the
useInput (string[] return) and useComboInput ({message,index}[] return)
signatures, wired into all four affected form hooks — agent binary
source, output, Fleet Server hosts (three SSL groups), and Fleet proxy.

Server side: each of the four route handlers (download_source,
fleet_server_hosts, fleet_proxies, output) gains a throwIfSslPathInvalid
helper that throws Boom.badRequest when an invalid path is submitted,
providing defence-in-depth independent of client-side validation.

Fixes: elastic#265694

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@juliaElastic juliaElastic added release_note:fix backport:all-open Backport to all branches that could still receive a release backport:version Backport to applied version labels v9.4.0 v9.3.4 and removed backport:all-open Backport to all branches that could still receive a release labels Apr 29, 2026
…n state

- Fix `useInput.onChange` to re-run the validator when errors already exist,
  so correcting one error and introducing another updates the field immediately
- Add missing `sslCertificateAuthoritiesInput.validate()` calls in the download
  source and output form `validate()` callbacks (combo inputs were silently
  skipped on submit)
- Add `validateSslPathInput` for non-Logstash SSL cert/key in the output form
  (Elasticsearch/Remote ES outputs were not validated at all)
- Include SSL input `isInvalid` state in `isDisabled` across all four form
  hooks so the Save button stays disabled after validation errors are shown
- Add unit tests verifying that submitting with a space in any SSL cert/key/CA
  path blocks submission, sets field errors, and disables the Save button
- Fix pre-existing test data in `use_fleet_server_host_form.test.tsx` that
  used paths with spaces (`'cert authorities'`) which now correctly fail

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@juliaElastic juliaElastic marked this pull request as ready for review April 29, 2026 12:17
@juliaElastic juliaElastic requested a review from a team as a code owner April 29, 2026 12:17
@botelastic botelastic Bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Apr 29, 2026
@infra-vault-gh-plugin-prod
Copy link
Copy Markdown

Pinging @elastic/fleet (Team:Fleet)

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Apr 29, 2026

Approvability

Verdict: Needs human review

This PR introduces new SSL path validation that rejects whitespace in certificate paths across multiple Fleet forms and APIs - a runtime behavior change affecting previously valid configurations. Additionally, a shared input hook is modified with broader blast radius than the SSL fields alone, as flagged in an unresolved review comment. The author does not own any of the changed files, which are owned by @elastic/fleet.

You can customize Macroscope's approvability policy. Learn more.

juliaElastic and others added 3 commits April 29, 2026 14:57
Kafka SSL inputs (cert/key/CA) are only relevant when the output type is
Kafka; the shared SSL inputs (cert/key/CA) are only relevant for non-Kafka
types. Checking them unconditionally meant that stale validation errors on
hidden fields could keep the Save button disabled after the user switched
output types.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The server explicitly sets compression_level = null when compression is
not Gzip (output.ts lines 988, 1055), but KafkaSchema used
schema.maybe(schema.number()) which rejects null. The OutputResponseSchema
validates GET/PUT responses against this schema, causing a 500.

Fix: use schema.maybe(schema.oneOf([schema.literal(null), schema.number()]))
to accept number | null | undefined, matching the stored value. Also update
the KafkaOutput TypeScript type to reflect that null is a valid value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@elastic-vault-github-plugin-prod elastic-vault-github-plugin-prod Bot requested a review from a team as a code owner April 29, 2026 13:48
@Supplementing Supplementing self-requested a review April 29, 2026 19:18
Comment thread x-pack/platform/plugins/shared/fleet/server/routes/download_source/handler.ts Outdated
Comment thread x-pack/platform/plugins/shared/fleet/common/services/ssl_validators.ts Outdated
setValue(newValue);
if (errors && validate && validate(newValue) === undefined) {
setErrors(undefined);
if (errors && validate) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just flagging that this impacts every input across all of fleet that uses a validator, not just the SSL path fields. It will re-run validation on every keystroke when errors exists. Probably a non-issue, but a bigger blast radius so worth flagging just in case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for flagging.
The old behavior was actually a bug: showing a stale error that doesn't reflect the current field value is confusing.

On the blast radius concern: The behavior only activates when errors is already non-null — i.e., after the first failed validation trigger (form submit or explicit validate() call). Pristine fields are completely unaffected. This is the standard "validate-on-submit, re-validate-on-change" pattern, and it's exactly the right UX for error correction.

On performance: Fleet validators are synchronous pure functions doing simple regex/string checks. Re-running them on keystrokes is negligible — no async calls, no side effects.

juliaElastic and others added 6 commits April 30, 2026 09:26
…/sections/settings/components/edit_output_flyout/use_output_form.tsx

Co-authored-by: Mason Herron <46727170+Supplementing@users.noreply.github.com>
- Fix error message: "cannot contain spaces" → "cannot contain whitespace"
  (`\s` matches tabs and newlines, not just spaces)
- Move `ssl_form_validators.ts` from `edit_output_flyout/` to shared
  `settings/components/` since it's used by four different flyout forms
- Extract `throwIfSslPathInvalid` server helper into
  `server/routes/utils/ssl_utils.ts` to remove the four duplicate
  definitions across the route handlers

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… content, not paths

Two bugs introduced by the SSL path validation PR:

1. `output?.type === 'logstash'` was false when creating a new output
   (output is undefined), causing sslCertificateInput/sslKeyInput to use
   path validation instead of content validation. Fixed to use
   `typeInput.value === 'logstash'` which reflects the current form state.

2. `validateSSLCertificate`/`validateSSLKey` incorrectly called
   `validateSslPathInput`. For Logstash these fields hold inline PEM
   content, not file paths, so whitespace validation must not apply.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…flict test

The ssl.key/secrets.ssl.key conflict test used 'cert authorities' (with a
space) and 'path/to/cert' as test data. Our new path validation fires first
and returns 'SSL certificate path cannot contain whitespace' instead of the
expected 'Cannot specify both ssl.key and secrets.ssl.key' error.

Replace with valid paths that have no whitespace so the conflict check is
reached.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s test data

Logstash ssl.certificate and ssl.key accept both file paths and inline
PEM content. Restores validateSslPathInput calls in validateSSLCertificate
and validateSSLKey (removed incorrectly in the previous commit).

validateSslCertPath already exempts PEM content (values starting with
'-----BEGIN'), so inline certificates are unaffected. Only file paths
with whitespace are rejected.

Updates the two Cypress tests to use '-----BEGIN CERTIFICATE-----' /
'-----BEGIN PRIVATE KEY-----' as placeholder values so they pass path
validation and correctly represent the inline-PEM use case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
juliaElastic and others added 2 commits April 30, 2026 11:23
…nders

- Remove multi-paragraph JSDoc from validateSslCertPath (one-line comment
  kept for the non-obvious PEM exemption)
- Add comment in throwIfSslPathInvalid explaining why object values are
  skipped (SOSecret can be a { id } secret reference)
- Guard setErrors in useInput/useSecretInput onChange so React skips
  re-renders when the error message hasn't actually changed

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread x-pack/platform/plugins/shared/fleet/public/hooks/use_input.ts Outdated
juliaElastic and others added 3 commits April 30, 2026 12:19
…enders

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…urce form

- Add validateSslPathInputSecret to ssl_form_validators.ts for validating
  secret-stored paths (skips { id } references, validates plain strings)
- Wire it into the download source sslKeySecretInput which previously had
  no validator, allowing whitespace paths to pass through undetected
- Fix Logstash submit validation: use plain-vs-secret mutual exclusion
  (sslKeyInput.value ? plain : secret) instead of OR, so a valid existing
  secret no longer bypasses whitespace validation of the plain key field
- Align isDisabled for both output and download source forms to use the
  same plain-vs-secret discriminator pattern

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…th-like strings

'cert authorities' and 'ES cert authorities' contain whitespace, which
now fails the SSL path validation added in this branch. Replace with
'/path/to/cert-authority' and '/path/to/es-cert-authority'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@vishaangelova vishaangelova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from a docs perspective

Copy link
Copy Markdown
Contributor

@Supplementing Supplementing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally and changes look good 🚀

I did notice something we might want to look at as a follow up (unrelated to your changes). When you submit and theres issue with fields outside of scroll view, theres no clear indication of whats wrong to the user, you click and the button doesnt change, the scroll doesnt go back up to the 'errored' fields, theres no toast, etc. I ended up clicking save twice before I realized I was missing the host URL. Something to consider, it's probably an issue on many forms.

Edit: we could also add them to be required here before the button is enabled since the error shows them as required anyway. Might be easier?

@Ikuni17 Ikuni17 added v9.3.5 and removed v9.3.4 labels May 1, 2026
Prevents silent submit failures by adding value-presence checks to
isDisabled in all four settings flyout form hooks:

- Download source: name, host, auth credentials per selected auth type
- Fleet proxy: name, url
- Fleet server hosts: name, at least one non-empty host URL
- Output: name, hosts per output type (ES, Remote ES, Logstash, Kafka)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@juliaElastic
Copy link
Copy Markdown
Contributor Author

Edit: we could also add them to be required here before the button is enabled since the error shows them as required anyway. Might be easier?

Thanks for raising, added validation that the required fields are filled before enabling the Save button: c2221be

…n disable change

Name and host are now required before Save is enabled, so fill them in
before clicking; remove the name/host required-error assertions that
no longer appear post-submit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kibanamachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
fleet 2112 2114 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 2.7MB 2.7MB +1.2KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 203.9KB 204.2KB +295.0B

History

@juliaElastic juliaElastic merged commit 23a7837 into elastic:main May 1, 2026
28 checks passed
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 9.3, 9.4

https://github.com/elastic/kibana/actions/runs/25211572404

@kibanamachine
Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
9.3 Backport failed because of merge conflicts
9.4 Backport failed because of merge conflicts

You might need to backport the following PRs to 9.4:
- [OAS][Detection Engine] Clean up remaining errors (#266228)
- Fix OAS validation errors in entity analytics privilege monitoring APIs && Risk Score APIs (#265470)
- [OAS] Improve spec descriptions, summaries, examples (#265153)
- [Cases][Docs] - Add additional examples and cleanup to openApi (#263617)
- [Entity Analytics] Add missing OpenAPI descriptions and examples to p… (#264778)
- [Osquery] Enables OpenAPI documentation for 5 new Osquery 9.4 API endpoints (#262810)

Manual backport

To create the backport manually run:

node scripts/backport --pr 266365

Questions ?

Please refer to the Backport tool documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team v9.4.0 v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Fleet]: Fleet Server and Agent become unhealthy when TLS certificate path under agent binary contains spaces

5 participants