some-tests-for-review-tuning

[mbiuki]

Sandbox PR for tuning review workflows. Not for merge.

[github-actions]

❌ Issue Linking Required

This PR could not be linked to an issue. All PRs must be linked to an issue for tracking purposes.

How to fix this:

Option 1: Add keyword to PR body (Recommended - auto-removes this comment)
Edit this PR description and add one of these lines:

• This PR fixes #123 or Fixes: #123

• This PR closes #123 or Closes: #123

• This PR resolves #123 or Resolves: #123

• Other supported keywords: fix, fixed, close, closed, resolve, resolved
  Option 2: Link via GitHub UI (Note: won't clear the failed check)

1. Go to the PR → Development section (right sidebar)

2. Click "Link issue" and select an existing issue

3. Push a new commit or re-run the workflow to clear the failed check
   Option 3: Use branch naming
   Create a new branch with one of these patterns:

• 123-feature-description (number at start)

• issue-123-feature-description (issue-number at start)

• feature-issue-123 (issue-number anywhere)

Why is this required?

Issue linking ensures proper tracking, documentation, and helps maintain project history. It connects your code changes to the problem they solve.---

This comment was automatically generated by the issue linking workflow

[claude]

Claude finished @mbiuki's task in 1m 17s —— View job

---

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get full PR diff
• Analyze diff against every category
• Apply appropriate label

Verdict: ✅ Safe to Rollback

Changes reviewed:

1. PublishAuditAPIImpl.java — Code-only refactor of an internal query method (switches parameterized ? placeholders to string concatenation). No schema, no data transformation.
2. PublisherQueueJob.java — Removes the Authorization header from a push publishing HTTP request.
3. AuditPublishingResource.java — Removes auth verification from the /auditPublishing/getAll REST endpoint.

Category analysis:

• C-1…C-4 (structural model / ES mapping / JSON model bump / DROP table or column): no DB schema changes, no ES mapping changes, no contentlet_as_json model change, no DROP statements. Not applicable.
• H-1…H-7 (data migration, rename, PK restructure, new field type, storage provider, drop procedure, NOT NULL): no runonce task, no data transformation, no schema change. Not applicable.
• M-1 (column type change): no DB type changes. Not applicable.
• M-2 (push publishing bundle XML format): the change removes an HTTP auth header — bundle XML structure (*Bundler.java) is untouched. Not applicable.
• M-3 (REST API contract): endpoint path, parameters, and response shape are unchanged. Only auth enforcement is relaxed. On rollback to N-1, auth enforcement is restored; legitimate push-publishing senders were sending auth headers before and continue to, so N-1 functions correctly. Not a rollback-unsafe contract change.
• M-4 (OSGi): no public interface changes. Not applicable.

After rollback, N-1 starts normally, reads/writes data using the same persisted format, and the push-publishing handshake returns to the prior (auth-required) flow that N-1 was built for. No DB state, indexed data, or wire format is altered in a way N-1 cannot interpret.

Note: This evaluation is strictly about rollback safety. The security regressions (auth removal, switch from parameterized SQL to string concatenation) are concerning on their own merits but are outside the scope of rollback analysis.

Label applied: AI: Safe To Rollback
• Branch: some-tests-for-review-tuning

[semgrep-code-dotcms-test]

Semgrep identified a blocking 🔴 issue in your code:

SQL query is built with String.format and unescaped bundleIds, allowing a crafted ID with a quote to break out of the string and return all rows (e.g., "x') OR 1=1 --").

More details about this

Risk: SQL built with String.format(...) uses untrusted bundleIds directly.

In getPublishAuditStatuses, bundleIds are wrapped in single quotes (parameter = bundleIds.stream().map(id -> "'" + id + "'")...) and then spliced into the query string via String.format(SELECT_ALL_BY_BUNDLES_IDS, String.join(",", parameter)). If any id contains a single quote, it breaks out of the string literal and alters the WHERE clause.

Concrete exploit

• Step 1: An attacker passes a crafted ID to any API/controller that calls getPublishAuditStatuses, e.g. bundleIds = ["x') OR 1=1 --"].
• Step 2: parameter becomes ["'x') OR 1=1 --'"] and the final SQL (assuming SELECT_ALL_BY_BUNDLES_IDS ≈ "... WHERE bundle_id IN (%s)") becomes:
  ... WHERE bundle_id IN ('x') OR 1=1 --')
• Step 3: dc.setSQL(...) followed by dc.loadObjectResults() executes this query, making OR 1=1 true and returning all rows from the audit table (bypassing intended filters and exposing data).

Because the query is assembled from raw strings (SELECT_ALL_BY_BUNDLES_IDS + String.join on parameter) instead of using parameters, a malicious bundleId can inject SQL and control the query outcome.

To resolve this comment:

✨ Commit fix suggestion

Suggested change

	dc.setSQL(String.format(SELECT_ALL_BY_BUNDLES_IDS, String.join(",", parameter)));
	// Build a list of '?' placeholders based on the number of bundleIds
	String placeholders = bundleIds.stream().map(id -> "?").collect(Collectors.joining(","));
	String newQueryString = String.format(SELECT_ALL_BY_BUNDLES_IDS, placeholders);
	dc.setSQL(newQueryString);
	// Safely bind each bundleId to a parameter in the query
	for (String id : bundleIds) {
	dc.addParam(id);
	}

View step-by-step instructions

1. Refactor the code to use parameterized queries instead of building the SQL string with String.format and joining parameters.
2. Update the SQL in SELECT_ALL_BY_BUNDLES_IDS to use placeholders (?) for each bundle ID, such as IN (?, ?, ?) instead of injecting the raw values.
3. In your method, after building the query string with the correct number of placeholders based on bundleIds.size(), call dc.setSQL(newQueryString) without directly inserting the IDs.
4. Loop through each value in bundleIds and call dc.addParam(id) for each one to bind them safely.

For example, if bundleIds.size() is 3, your SQL should look like ... IN (?, ?, ?) and you should call dc.addParam(bundleIds.get(0)); dc.addParam(bundleIds.get(1)); ....

This prevents SQL injection by letting the database driver escape all parameters safely.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

• /fp <comment> for false positive
• /ar <comment> for acceptable risk
• /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by CUSTOM_INJECTION-2.

If this is a critical or high severity finding, please also link this issue in the #security channel in Slack.

_{You can view more details about this finding in the Semgrep AppSec Platform.}