Skip to content

fix: open dialog when clicking on content on pending bundles (#36250)#36292

Open
gortiz-dotcms wants to merge 2 commits into
mainfrom
issue-36250-open-dialog-on-content-click-publishing-queue
Open

fix: open dialog when clicking on content on pending bundles (#36250)#36292
gortiz-dotcms wants to merge 2 commits into
mainfrom
issue-36250-open-dialog-on-content-click-publishing-queue

Conversation

@gortiz-dotcms

@gortiz-dotcms gortiz-dotcms commented Jun 23, 2026

Copy link
Copy Markdown
Member

Open edit content dialog to improve navigation on the pending bundle tab.

Grabacion.de.pantalla.2026-06-23.a.la.s.4.46.18.p.m.mov

This PR fixes: #36250

@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @gortiz-dotcms's task in 2m 28s —— View job


🔍 dotCMS Backend Review Pipeline

Tasks:

  • Initialize review pipeline
  • Load dotCMS conventions (CLAUDE.md + docs)
  • Get Java diff
  • Run parallel sub-agent reviews (Security, Database, Java Standards, REST API)
  • Consolidate findings and post review

Results

Security: 1 High finding — see review comment

Database: ✅ NO_FINDINGS

Java Standards: ✅ NO_FINDINGS

REST API: ✅ NO_FINDINGS

Note: Formal --request-changes review submission requires elevated GitHub App permissions — not submitted. The review comment with findings has been posted above.

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — deepseek.v3.2

[🟡 Medium] dotCMS/src/main/java/com/dotcms/publisher/business/PublishQueueElementTransformer.java:156 — Potential NullPointerException if contentlet is null and contentlet.getContentType() is called before the ternary check. The ternary UtilMethods.isSet(contentlet) ? contentlet.getContentType() : null is safe, but the earlier line contentlet.getContentType().name() in the original Map.of would have thrown NPE. The fix is correct, but ensure the contentlet null check is consistently applied.

[🟡 Medium] dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/view_publish_queue_list.jsp:333 — Missing null/empty check for contentTypeVariable before using it in JavaScript. If contentTypeVariable is empty, the openContentEditor call may fail or behave unexpectedly. Consider adding a fallback or validation.

[🟡 Medium] dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/view_publish_queue_list.jsp:118 — The openContentEditor function dispatches a custom event but does not handle cases where the Angular listener is not ready or missing. This could silently fail. Ensure the frontend Angular component is guaranteed to listen for this event.


Run: #28052934571 · tokens: in: 1443 · out: 284 · total: 1727

@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

🔍 dotCMS Backend Review

[🟠 High] dotCMS/src/main/webapp/html/portlet/ext/contentlet/publishing/view_publish_queue_list.jsp:339

inode and contentTypeVariable are injected raw into single-quoted JavaScript string literals inside an onclick HTML attribute with no JavaScript escaping. An attacker who can influence either value (e.g. a content type variable containing ', alert(1), ') could break out of the JS string literal and execute arbitrary JavaScript. While dotCMS content type variables are alphanumeric/underscore today and inodes are UUIDs, there is no output-layer guarantee — if a DB record is corrupted or future validation relaxes, XSS becomes exploitable. StringEscapeUtils is already imported at line 2.

<a href="javascript:void(0)" onclick="openContentEditor('<%=inode%>', '<%=contentTypeVariable%>')">

💡 Apply JS escaping first, then HTML escaping for the attribute context (both from the already-imported StringEscapeUtils):

<a href="javascript:void(0)"
   onclick="openContentEditor('<%=StringEscapeUtils.escapeHtml(StringEscapeUtils.escapeJavaScript(inode))%>',
                              '<%=StringEscapeUtils.escapeHtml(StringEscapeUtils.escapeJavaScript(contentTypeVariable))%>')">

Next steps

  • 🔴 / 🟠 Fix locally and push — these need your judgment
  • 🟡 You can ask me to handle mechanical fixes inline: @claude fix <issue description> in <File.java>
  • Every new push triggers a fresh review automatically

…entTransformer (#33645)

Contentlet.getContentType() returns null when the content type has been deleted. Calling .name() and .variable() on the result directly caused an unhandled NPE that broke the publishing queue UI for every asset in the affected bundle. Cache the result once and guard for null before use.

Refs: #33645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

feat(publishing-queue): add Back to Publishing Queue link in content editor sidebar

1 participant