[9.3] [Fleet] Validate SSL certificate paths for whitespace#267277
[9.3] [Fleet] Validate SSL certificate paths for whitespace#267277juliaElastic wants to merge 1 commit intoelastic:9.3from
Conversation
Fixes elastic#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. **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. 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](https://claude.com/claude-code) <img width="536" height="829" alt="image" src="https://github.com/user-attachments/assets/5489927a-b71f-4625-9812-15bfea234f06" /> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Mason Herron <46727170+Supplementing@users.noreply.github.com>
botelastic
Bot
added
the
Team:Fleet
|
Pinging @elastic/fleet (Team:Fleet) |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟢 Low
The validate function references usernameInput, passwordInput, passwordSecretInput, apiKeyInput, apiKeySecretInput, headersInput, and authTypeInput at lines 105-180, but none of these variables are defined in the hook. Calling validate() throws ReferenceError: usernameInput is not defined.
const sslKeySecretInput = useSecretInput(
(downloadSource as DownloadSourceBase)?.secrets?.ssl?.key,
validateSslPathInputSecret,
undefined
);
+ // Auth inputs not yet implemented - stubbed for future use
+ const authTypeInput = { value: 'none' as const };
+ const usernameInput = { validate: () => true, value: '', setErrors: () => {} };
+ const passwordInput = { validate: () => true, value: '', setErrors: () => {} };
+ const passwordSecretInput = { validate: () => true, value: undefined, setErrors: () => {} };
+ const apiKeyInput = { validate: () => true, value: '', setErrors: () => {} };
+ const apiKeySecretInput = { validate: () => true, value: undefined, setErrors: () => {} };
+ const headersInput = { validate: () => true };
+
const inputs = {Also found in 1 other location(s)
x-pack/platform/plugins/shared/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/use_output_form.tsx:1182
Reference to undefined variable
remoteElasticsearchUrlInputon line 1182 will cause aReferenceErrorat runtime. The variable is never defined inuseOutputForm- onlyelasticsearchUrlInputexists (defined at line 262). When a user edits or creates a Remote Elasticsearch output (isRemoteElasticsearchis true), this code path executes and crashes.
🤖 Copy this AI Prompt to have your agent fix this:
In file x-pack/platform/plugins/shared/fleet/public/applications/fleet/sections/settings/components/download_source_flyout/use_download_source_flyout_form.tsx around lines 45-93:
The `validate` function references `usernameInput`, `passwordInput`, `passwordSecretInput`, `apiKeyInput`, `apiKeySecretInput`, `headersInput`, and `authTypeInput` at lines 105-180, but none of these variables are defined in the hook. Calling `validate()` throws `ReferenceError: usernameInput is not defined`.
Evidence trail:
File: x-pack/platform/plugins/shared/fleet/public/applications/fleet/sections/settings/components/download_source_flyout/use_download_source_flyout_form.tsx at REVIEWED_COMMIT. Lines 57-87 define the hook's inputs (nameInput, defaultDownloadSourceInput, hostInput, proxyIdInput, sslCertificateAuthoritiesInput, sslCertificateInput, sslKeyInput, sslKeySecretInput). Lines 105-113 reference usernameInput.validate(), passwordInput.validate(), passwordSecretInput.validate(), apiKeyInput.validate(), apiKeySecretInput.validate(), headersInput.validate() — none of which are defined. Line 117 references authTypeInput.value — also not defined. Lines 253-256 reference the same undefined variables outside useCallback. The git_diff between MERGE_BASE and REVIEWED_COMMIT confirms all these references were introduced by this PR.
Also found in 1 other location(s):
- x-pack/platform/plugins/shared/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/use_output_form.tsx:1182 -- Reference to undefined variable `remoteElasticsearchUrlInput` on line 1182 will cause a `ReferenceError` at runtime. The variable is never defined in `useOutputForm` - only `elasticsearchUrlInput` exists (defined at line 262). When a user edits or creates a Remote Elasticsearch output (`isRemoteElasticsearch` is true), this code path executes and crashes.
ApprovabilityVerdict: Needs human review An unresolved review comment identifies runtime-crashing bugs where undefined variables (usernameInput, passwordInput, etc.) are referenced in the validation logic, which would cause ReferenceErrors. Additionally, the author does not own any of the 24 changed files, and the PR introduces validation behavior changes across multiple Fleet features. You can customize Macroscope's approvability policy. Learn more. |
|
Seeing there were many conflicts in 9.3 and the original issue was reported in 9.4, decided to not backport to 9.3 |
There was a problem hiding this comment.
Backport Review: found 2 issue(s). See inline comments for details.
Share feedback in the #appex-qa channel.
Posted via Macroscope — Backport Review
| }) | ||
| .expect(400); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Auth-related test cases in PUT section also test features not on 9.3
Same issue as the POST section above — these authHeaderConflictCases and auth switching/clearing/preserving tests reference auth, secrets.auth, and headers fields that are not part of the 9.3 download source schema.
See details
This block adds ~250 lines of tests for auth-related update operations (switching from username/password to api_key, converting plain text to secrets, updating headers, replacing auth with headers-only, clearing auth via null, preserving auth/SSL secrets on partial updates, etc.).
These all depend on the auth features from PRs #250557 and #255209, which have not been backported to 9.3.
The original PR #266365 only changed certificate_authorities test values from 'cert authorities' to '/path/to/cert-authority' — those are the only changes that should be kept here.
Posted via Macroscope — Backport Review
| const authHeaderConflictCases: Array<[string, { auth?: {}; secrets?: {} }]> = [ | ||
| ['username/password', { auth: { username: 'testuser', password: 'testpassword' } }], | ||
| [ | ||
| 'username/password as secret', | ||
| { auth: { username: 'testuser' }, secrets: { auth: { password: 'testpassword' } } }, | ||
| ], | ||
| ['api_key', { auth: { api_key: 'my-api-key' } }], | ||
| ['api_key as secret', { secrets: { auth: { api_key: 'my-api-key' } } }], | ||
| ]; | ||
|
|
||
| describe('fleet_download_sources_crud', function () { |
There was a problem hiding this comment.
~500 lines of auth-related tests for features that do not exist on 9.3
These test cases (auth headers, username/password, API key, auth secrets, clearing auth) test download source auth features introduced by PRs #250557 and #255209 on main. The 9.3 branch does not have these features — the download source schema on 9.3 has no auth field at all (see server/types/models/download_sources.ts). These tests will fail at runtime because the API will reject or ignore auth payloads.
See details
The added block spans from the authHeaderConflictCases fixture through all auth test cases in both the POST and PUT describe blocks. This includes tests like:
should allow creating a download source with username/password authshould allow creating a download source with api_key authshould not allow both username/password and api_keyshould delete password secret when switching from username/password to api_keyshould clear auth and headers when setting auth to null- All
authHeaderConflictCasesparameterized tests - etc.
On main, these tests pass because the download source schema includes auth fields. On 9.3, DownloadSourceSchema only has id, name, host, is_default, proxy_id, ssl, and secrets.ssl. There is no auth field.
This appears to be scope creep from the conflict resolution — the PR description mentions "took incoming new test cases". These auth tests should be removed from this backport, keeping only the SSL cert path validation changes (the 'cert authorities' → '/path/to/cert-authority' replacements) from the original PR.
Posted via Macroscope — Backport Review
💔 Build Failed
Failed CI StepsMetrics [docs]Module Count
Async chunks
Page load bundle
History |
2 participants
Backport
This is a backport of PR #266365 to
9.3.Original PR description
Validates SSL certificate path fields for whitespace characters to prevent
silent failures when saving Fleet settings flyouts.
Conflict resolution
oas_docs/output/kibana.serverless.yamlandkibana.yaml: accepted 9.3 version (generated files)x-pack/platform/plugins/shared/fleet/common/services/index.ts: added only thevalidateSslCertPathexport; excluded exports for modules that don't exist on 9.3 (yaml_utils,var_group_helpers,cloud_connectors,otelcol_helpers)x-pack/platform/plugins/shared/fleet/public/applications/fleet/sections/settings/components/download_source_flyout/use_download_source_flyout_form.tsx: took incoming auth validation logicx-pack/platform/plugins/shared/fleet/public/applications/fleet/sections/settings/components/download_source_flyout/use_download_source_flyout_form.test.tsx: took incoming importsx-pack/platform/plugins/shared/fleet/server/routes/download_source/handler.ts: took incomingvalidateDownloadSourcefunctionx-pack/platform/plugins/shared/fleet/server/types/models/output.ts: took incomingcompression_levelschema fixx-pack/platform/test/fleet_api_integration/apis/download_sources/crud.ts: took incoming new test cases