Redact sensitive duplicate query parameter values in ApiServlet logs by dheeraj12347 · Pull Request #13677 · apache/cloudstack

dheeraj12347 · 2026-07-23T08:48:21Z

Description

This PR manually backports the fix from #13559 to the 4.20 LTS branch.

Changes

Redact duplicate values for sensitive query parameters before logging in ApiServlet.
Add helper methods to identify sensitive parameters and format logged values.
Backport the corresponding unit tests.

This backport includes the changes applicable to the 4.20 branch.

Fixes #13311

dheeraj12347 · 2026-07-23T08:50:49Z

@DaanHoogland

As discussed in #13559, this is the manual backport of the fix for the 4.20 LTS branch.

DaanHoogland

some linting but clgtm still

DaanHoogland · 2026-07-24T10:40:59Z

+private static final String REDACTED = "REDACTED";
+private static final String LOGGER_REPLACEMENTS = "[\n\r\t]";
+
+private static final Set<String> SENSITIVE_PARAMETER_KEYWORDS = Set.of(
+    "password",
+    "privatekey",
+    "accesskey",
+    "secretkey",
+    "apikey",
+    "signature",
+    "sessionkey",
+    "token"
+);


indent seems off

DaanHoogland · 2026-07-24T10:41:25Z

+}
+
+static String formatValuesForLog(String parameterName, String[] values) {
+    if (!isSensitiveParameter(parameterName)) {
+        return Arrays.toString(values);
+    }
+
+    String[] masked = new String[values.length];
+    Arrays.fill(masked, REDACTED);
+    return Arrays.toString(masked);
+}


indent is off for this bit as well

DaanHoogland · 2026-07-24T10:44:35Z

@dheeraj12347 , you should not have to open a new PR (next time) just rebase your fix with git rebase --onto <new branch> <old base>

I think we can close the one on main now and it looks like it won’t give major conflicts when merging forwards.

codecov · 2026-07-24T10:46:30Z

Codecov Report

❌ Patch coverage is 68.42105% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.35%. Comparing base (e8df87e) to head (e9a1890).
⚠️ Report is 35 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
server/src/main/java/com/cloud/api/ApiServer.java	0.00%	3 Missing ⚠️
server/src/main/java/com/cloud/api/ApiServlet.java	81.25%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #13677      +/-   ##
============================================
+ Coverage     16.26%   16.35%   +0.08%     
- Complexity    13435    13578     +143     
============================================
  Files          5667     5669       +2     
  Lines        500731   501421     +690     
  Branches      60803    60907     +104     
============================================
+ Hits          81430    81992     +562     
- Misses       410197   410243      +46     
- Partials       9104     9186      +82

Flag	Coverage Δ
uitests	`4.14% <ø> (-0.01%)`	⬇️
unittests	`17.21% <68.42%> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dheeraj12347 · 2026-08-28T15:15:38Z

Hi @DaanHoogland , I’ve closed #13559 as discussed and checked the CI run for the 4.20 backport #13677. The build, coverage, lint, Codecov and other checks are passing. The only remaining failure is the Simulator smoke/test_network_permissions job, which is failing while creating an IPv6 network because the zone has no IPv6 DNS configured. This appears unrelated to the ApiServlet change. Could you please take a look when you get a chance and let me know if anything else is needed from my side?

harikrishna-patnala · 2026-09-07T11:41:37Z

+private static final String REDACTED = "REDACTED";
+private static final String LOGGER_REPLACEMENTS = "[\n\r\t]";
+
+private static final Set<String> SENSITIVE_PARAMETER_KEYWORDS = Set.of(


cloudstack/server/src/main/java/com/cloud/api/ApiServer.java

Lines 554 to 559 in 02182a1

if(parameterMap.putIfAbsent(param.getName(), new String[]{param.getValue()}) != null) {

String message = String.format("Query parameter '%s' has multiple values [%s, %s]. Only the last value will be respected." +

"It is advised to pass only a single parameter", param.getName(), param.getValue(), parameterMap.get(param.getName()));

logger.warn(StringUtils.cleanString(message));

}

}

@dheeraj12347 there is another place with the same content exists and there it has already addressed with "cleanString()" method, which doesnot cover all the sensitive parameters you have listed. Can you please check if this can be fixed somehow.

@harikrishna-patnala Addressed this by using ApiServlet.formatValuesForLog() in the ApiServer duplicate-parameter warning, so the same sensitive-parameter redaction is applied there as well. The existing cleanString() sanitization is still retained.

thanks for the changes @dheeraj12347. I've tested it looks good from logic and logs point of view.

2026-09-08 06:25:35,022 WARN [c.c.a.ApiServlet] (qtp1404565079-346:[ctx-92464837]) (logid:4df39a1d) Query parameter 'password' has multiple values [REDACTED, REDACTED]. Only the last value will be respected.It is advised to pass only a single parameter

Can you please move these helper methods to StringUtils class ?

Redact sensitive duplicate query parameter values in ApiServlet logs

cb3c236

dheeraj12347 mentioned this pull request Jul 23, 2026

Redact sensitive duplicate query parameter values in ApiServlet logs #13559

Closed

DaanHoogland reviewed Jul 24, 2026

View reviewed changes

DaanHoogland requested review from sureshanaparti and vladimirpetrov August 28, 2026 15:46

DaanHoogland added this to CloudStack Testing Aug 31, 2026

github-project-automation Bot moved this to Backlog in CloudStack Testing Aug 31, 2026

DaanHoogland moved this from Backlog to Ready in CloudStack Testing Aug 31, 2026

DaanHoogland assigned harikrishna-patnala Sep 1, 2026

harikrishna-patnala reviewed Sep 7, 2026

View reviewed changes

harikrishna-patnala moved this from Ready to In review in CloudStack Testing Sep 7, 2026

Redact sensitive duplicate query parameter values

e9a1890

harikrishna-patnala moved this from In review to conflict/waiting in CloudStack Testing Sep 8, 2026

Navigation Menu

Uh oh!

Redact sensitive duplicate query parameter values in ApiServlet logs - #13677

Redact sensitive duplicate query parameter values in ApiServlet logs#13677
dheeraj12347 wants to merge 2 commits into
apache:4.20from
dheeraj12347:fix-redact-sensitive-duplicate-params-4.20

dheeraj12347 commented Jul 23, 2026 •

edited by DaanHoogland

Loading

Uh oh!

dheeraj12347 commented Jul 23, 2026

Uh oh!

DaanHoogland left a comment

Uh oh!

DaanHoogland Jul 24, 2026

Uh oh!

DaanHoogland Jul 24, 2026

Uh oh!

DaanHoogland commented Jul 24, 2026

Uh oh!

codecov Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

dheeraj12347 commented Aug 28, 2026

Uh oh!

harikrishna-patnala Sep 7, 2026

Uh oh!

dheeraj12347 Sep 8, 2026

Uh oh!

harikrishna-patnala Sep 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if(parameterMap.putIfAbsent(param.getName(), new String[]{param.getValue()}) != null) {
	String message = String.format("Query parameter '%s' has multiple values [%s, %s]. Only the last value will be respected." +
	"It is advised to pass only a single parameter", param.getName(), param.getValue(), parameterMap.get(param.getName()));
	logger.warn(StringUtils.cleanString(message));
	}
	}

Uh oh!

Conversation

dheeraj12347 commented Jul 23, 2026 • edited by DaanHoogland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Uh oh!

dheeraj12347 commented Jul 23, 2026

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jul 24, 2026

Uh oh!

codecov Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dheeraj12347 commented Aug 28, 2026

Uh oh!

harikrishna-patnala Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

dheeraj12347 Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

harikrishna-patnala Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dheeraj12347 commented Jul 23, 2026 •

edited by DaanHoogland

Loading

codecov Bot commented Jul 24, 2026 •

edited

Loading