Skip to content

fix: The EchartsRander component injects stored XSS via Eval#4957

Merged
shaohuzhang1 merged 1 commit intov2from
pr@v2@fix_xss
Mar 26, 2026
Merged

fix: The EchartsRander component injects stored XSS via Eval#4957
shaohuzhang1 merged 1 commit intov2from
pr@v2@fix_xss

Conversation

@shaohuzhang1
Copy link
Contributor

fix: The EchartsRander component injects stored XSS via Eval

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Mar 26, 2026

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Mar 26, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

></iframe>
<div ref="chartsRef" :style="style" v-resize="onResize"></div>
</div>
</template>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code appears to be well-formed and does not contain any significant irregularities. However, there are several potential areas where improvements can be made:

  1. Sandboxing: The sandbox attribute is already present on the <iframe>, which restricts certain behavior within the frame. Ensure that this is appropriate based on your application's security requirements.

  2. Performance Considerations:

    • Iframe SrcDoc: If you're embedding content from an external source using ${baseURL}${chart}?${params}, ensure that this approach doesn't introduce performance bottlenecks or CORS issues.
    • On Resize Event: The use of v-resize seems to be custom Vue directive, ensuring it exists somewhere else in your code base. If needed, consider optimizing how resize events affect performance if they aren't crucial immediately after change.
  3. Styling Flexibility:

    • The default flexbox properties (d-flex justify-content-between align-items-center) might need adjustment depending on your layout goals. Review CSS to confirm these styles fit logically with the parent container structure.
  4. Error Handling:

    • Add event listeners (such as @error) to handle cases where the iframe fails to load due to network errors or other issues.
  5. Responsive Design:

    • Check if the responsive design implemented by resizing works correctly across different browsers and devices. Test various layouts and ensure the div containing charts remains flexible and visually appealing.
  6. Documentation:

    • Document changes made to improve future maintenance and understanding by colleagues who will work on this component later.
  7. Security Best Practices:

    • Double-check that the sandbox settings do not inadvertently expose sensitive information or allow unnecessary access features like popups or camera access without user consent.

Overall, the current setup looks functional and efficient for most basic use cases, but refining it further should enhance its robustness and maintainability.

@shaohuzhang1 shaohuzhang1 merged commit 34fb95b into v2 Mar 26, 2026
3 of 4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@v2@fix_xss branch March 26, 2026 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant