fix(security): backport SQL injection + auth fixes to 26.04.28 (hotfix)#35608
fix(security): backport SQL injection + auth fixes to 26.04.28 (hotfix)#35608sfreudenthaler wants to merge 11 commits into
Conversation
Replaces the manually quoted IN-clause with bound parameters so caller-supplied bundle ids cannot break out of the SQL literal. Also guards against null/empty input (which previously NPE'd or produced invalid `IN ()` SQL), and corrects the catch-block logger to use this class and ERROR level (matching the sibling deletePublishAuditStatus method). Refs: dotCMS/private-issues#581
…erization Adds four integration tests for getPublishAuditStatuses(List<String>): - happy path returns the requested rows and excludes others - empty list returns an empty result without producing invalid `IN ()` SQL - null list returns an empty result without NPE - bundle ids containing SQL meta-characters (single quotes, OR 1=1, '; DROP TABLE; --, comment terminators, escaped quotes) are bound as parameters and produce no rows; the audit table remains intact Refs: dotCMS/private-issues#581
Co-authored-by: semgrep-code-dotcms-test[bot] <183154938+semgrep-code-dotcms-test[bot]@users.noreply.github.com>
…pply The "Apply suggestion" action on the Semgrep bot's review (commit f68ff49) appended the bot's suggested code on top of the existing parameterized fix instead of replacing it, leaving two `placeholders` declarations in the same method (compile error: "variable placeholders is already defined"). The bot's suggestion was functionally identical to the committed code; the existing parameterized fix is correct. Removing the duplicate block.
…blishingResource Adds WebResource.InitBuilder authentication checks to GET /api/auditPublishing/get and POST /api/auditPublishing/getAll. Both endpoints previously skipped WebResource.init entirely, so an unauthenticated caller could enumerate publish audit status for any bundle id (or list of ids) — the SQL injection in the underlying DB method was the worst-case shape, but even after parameterization the data exposure remains until the resource is gated. Both endpoints now require: - a backend (CMS) user, not a frontend user - the `publishing-queue` portlet permission - rejection when no user is present (no anonymous access) Addresses review feedback from @wezell.
…eTest The auth check throws com.dotcms.rest.exception.SecurityException (WebApplicationException subclass), not java.lang.SecurityException. @test(expected = ...) defaults to the imported class, so a no-arg import was matching the wrong type and the test errored even though the auth worked correctly.
…issing token Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
❌ Issue Linking RequiredThis 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)
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 |
Backports the LTS workflow fix from main (PR #35612) to enable manual triggering with a version override and dynamic version resolution from .mvn/maven.config. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
no need to merge. just did this pr to test and for review |
Summary
Security hotfix for customers running
v26.04.28-02. Cherry-picks 9 commits from PR #35553 onto the26.04.28release line — no other changes frommainare included.Do not merge this PR. It exists to run CI and collect reviews. The release (
26.04.28-03) is being cut directly from this branch.What's included
PublishAuditAPIImpl.getPublishAuditStatuses()now uses bound parameters instead of string-concatenatedIN (...)clause (Refs: dotCMS/private-issues#581)/api/auditPublishing/getand/getAllnow require an authenticated backend user withpublishing-queueportlet permission (previously unauthenticated)WebResource.InitBuilderto PP auth inAuditPublishingResourceCherry-picked from
PR #35553 (merged to
mainon 2026-05-06)Commits cherry-picked (merge-from-main commits excluded):
dc515d99fix(publisher): parameterize getPublishAuditStatuses bundle-id queryf783f9ddtest(publisher): regression tests for parameterizationf68ff49fApply suggestions from code review947fda39fix: remove duplicate placeholders block10dcb623fix(rest): require backend user + publishing-queue portlet on AuditPublishingResource15b6b8c3test: use dotCMS-specific SecurityException381d83dfadd AuditPublishingResourceTest to MainSuite2a13f70a5euse PP authorization in AuditPublishingResource071f4da8fix(publisher): add PP auth header to audit poll and prevent 500 on missing token