codeant-ai-for-open-source[bot] commented on code in PR #36606:
URL: https://github.com/apache/superset/pull/36606#discussion_r2618007904


##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx:
##########
@@ -490,7 +490,9 @@ export class TableRenderer extends Component {
               true,
             )}
           >
-            {t('Subtotal')}
+            {t('Subvalue (%(aggregatorName)s)', {

Review Comment:
   **Suggestion:** Security / XSS risk: the translated "Subvalue" string 
(including `aggregatorName`) is injected directly into the <th> without 
respecting `allowRenderHtml` or sanitization; if `aggregatorName` contains 
user-controlled markup this can lead to XSS. Use the existing `safeHtmlSpan` 
when `allowRenderHtml` is true (or otherwise escape/sanitize) before inserting 
into the DOM. [security]
   
   **Severity Level:** Critical 🚨
   ```suggestion
               {allowRenderHtml
                 ? safeHtmlSpan(
                     t('Subvalue (%(aggregatorName)s)', {
                       aggregatorName: t(this.props.aggregatorName),
                     }),
                   )
                 : t('Subvalue (%(aggregatorName)s)', {
   ```
   <details>
   <summary><b>Why it matters? ⭐ </b></summary>
   
   This is a valid security concern: the file already uses safeHtmlSpan 
elsewhere when allowRenderHtml is true.
   In renderColHeaderRow the local scope includes allowRenderHtml and 
safeHtmlSpan is imported, so switching to a conditional
   that wraps the translated string in safeHtmlSpan when allowRenderHtml is 
true fixes a real XSS/vector-injection gap.
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx
   **Line:** 493:493
   **Comment:**
        *Security: Security / XSS risk: the translated "Subvalue" string 
(including `aggregatorName`) is injected directly into the <th> without 
respecting `allowRenderHtml` or sanitization; if `aggregatorName` contains 
user-controlled markup this can lead to XSS. Use the existing `safeHtmlSpan` 
when `allowRenderHtml` is true (or otherwise escape/sanitize) before inserting 
into the DOM.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



##########
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx:
##########
@@ -703,7 +705,9 @@ export class TableRenderer extends Component {
             true,
           )}
         >
-          {t('Subtotal')}
+          {t('Subvalue (%(aggregatorName)s)', {

Review Comment:
   **Suggestion:** Security / XSS risk in the row subtotal label: the 
translated "Subvalue" is rendered directly without honoring `allowRenderHtml` 
or sanitizing the interpolated `aggregatorName`. Wrap the output with 
`safeHtmlSpan` when `allowRenderHtml` is true to avoid inserting untrusted HTML 
into the DOM. [security]
   
   **Severity Level:** Critical 🚨
   ```suggestion
             {allowRenderHtml
               ? safeHtmlSpan(
                   t('Subvalue (%(aggregatorName)s)', {
                     aggregatorName: t(this.props.aggregatorName),
                   }),
                 )
               : t('Subvalue (%(aggregatorName)s)', {
   ```
   <details>
   <summary><b>Why it matters? ⭐ </b></summary>
   
   Same issue as above in renderTableRow: the subtotal string is injected 
directly. The function has allowRenderHtml in scope and safeHtmlSpan is 
available,
   so conditionally wrapping the translated label when allowRenderHtml is true 
removes the XSS risk without wider changes.
   </details>
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** 
superset-frontend/plugins/plugin-chart-pivot-table/src/react-pivottable/TableRenderers.jsx
   **Line:** 708:708
   **Comment:**
        *Security: Security / XSS risk in the row subtotal label: the 
translated "Subvalue" is rendered directly without honoring `allowRenderHtml` 
or sanitizing the interpolated `aggregatorName`. Wrap the output with 
`safeHtmlSpan` when `allowRenderHtml` is true to avoid inserting untrusted HTML 
into the DOM.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   ```
   </details>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to