Repository navigation
Conversation
f5a58ef to
fe572b3
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes adding object storage from the CloudStack UI by switching the addObjectStoragePool API call from a GET (query-string) request to a form-encoded POST request, preventing credential leakage in URLs and ensuring compatibility with POST-enforcing configurations. It also hardens client/server diagnostic logging so that sensitive request contents are not emitted while keeping useful correlation metadata.
Changes:
- UI: submit
addObjectStoragePoolviapostAPI(URLSearchParams body) and mask the secret key field using a password input withautocomplete="off". - UI diagnostics: introduce centralized, safe Axios error logging that avoids serializing request/response bodies and query params.
- Server/API: mark
addObjectStoragePoolas carrying sensitive request info and suppress request-parameter logging (including legacy query strings) for this command; add corresponding unit tests.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/infra/AddObjectStorage.vue | Switches object storage creation to POST and masks secret key input. |
| ui/src/utils/apiError.js | Adds safe API error summarization + logging to avoid leaking sensitive data. |
| ui/src/utils/request.js | Replaces raw response logging with safe API error logging in the request error path. |
| ui/src/utils/plugins.js | Uses safe API error logging in the global notifier error handler. |
| ui/tests/unit/views/infra/AddObjectStorage.spec.js | Adds unit coverage for POST transport, body encoding, error propagation, and secret-key masking. |
| ui/tests/unit/utils/apiError.spec.js | Adds unit tests ensuring only safe scalar metadata is logged/serialized. |
| server/src/main/java/com/cloud/api/ApiServlet.java | Suppresses parameter logging (and query string logging) for sensitive commands including addObjectStoragePool. |
| server/src/test/java/com/cloud/api/ApiServletTest.java | Adds tests validating parameter/query-string suppression and ordinary-command behavior. |
| api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddObjectStoragePoolCmd.java | Marks the command request as containing sensitive information. |
| api/src/test/java/org/apache/cloudstack/api/command/admin/storage/AddObjectStoragePoolCmdTest.java | Adds a test asserting the command is annotated as sensitive. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13768 +/- ##
============================================
+ Coverage 17.69% 18.06% +0.36%
- Complexity 15835 16335 +500
============================================
Files 5925 5937 +12
Lines 533539 536800 +3261
Branches 65274 65794 +520
============================================
+ Hits 94427 96974 +2547
- Misses 428435 428758 +323
- Partials 10677 11068 +391
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18742 |
|
@blueorangutan test keepEnv |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-16674) |
|
Is being E2E tested shortly. |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-16683) |
|
Follow-up commit 4bac5d4 removes browser-console API error logging from both the shared Axios error handler and the notification helper. User-facing notifications and server-side diagnostics are unchanged; request and response objects are no longer written to the browser console. The now-unused logging helper and its tests were removed with the same commit. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18815 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-16731) |
Retain cleaned query context instead of applying POST-payload suppression to the entire query. Exclude known credential fields and indexed parameters for requests already covered by the sensitive-logging policy, without interpreting descriptors from mixed query and POST bodies. Recognize end-of-input as a details-descriptor boundary so removing a trailing credential parameter cannot defeat existing indexed-key redaction. Keep duplicate and malformed request diagnostics without raw values, and verify request data, response handling and storage discovery remain unchanged.
70545ab to
7b95398
Compare
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✖️ suse15. SL-JID 19631 |
Description
This PR fixes object storage creation from the UI when state-changing API requests are required to use POST.
The form previously submitted addObjectStoragePool using GET, placing the provider’s access and secret keys in the request URL. It now uses the existing postAPI helper to submit the same command and indexed parameters in a form-encoded request body.
The change also updates credential handling in the request’s diagnostic paths:
Uses a password input with autocomplete disabled for the object-storage secret-key field.
Marks addObjectStoragePool as carrying sensitive request information and includes it in the servlet’s existing POST-body logging suppression policy.
Retains cleaned query context in management START/END logs and the access log. Known credential and user-data fields are omitted, along with indexed parameters for requests covered by the sensitive-logging policy.
Removes the two shared raw API-error console logging paths from the request error handler and notification helper. User-facing notifications and error propagation are preserved.
Prevents duplicate-parameter and malformed-token diagnostics from printing raw values, and corrects control-character sanitization in log metadata.
Updates the existing indexed-credential cleaner to recognize a credential descriptor at the end of a query, ensuring its associated value remains redacted when the descriptor is the final parameter.
The query-cleaning helper constructs a separate diagnostic string without modifying the request parameters. Useful context for requests such as addHost, createAccount, and requests containing user data is retained while sensitive fields are removed.
Fixes #13679
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
Not applicable.