Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Correct DOM Text Handling, Pin Tags in Workflow, and Add Permissions #93

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Yuyuutsu
Copy link

@Yuyuutsu Yuyuutsu commented Jan 21, 2025

What is the context of this PR?

Code Scanning has been enabled in some of our Github Repositories is highlighting issues.
This PR is to fix these issues:

  • DOM text reinterpreted as HTML
  • Workflow does not contain permissions
  • Unpinned tag for a non-immutable Action in workflow

How to review

Check if the changes make sense
Test Launcher locally to see if the HTML changes do not affect Launcher Visually

@Yuyuutsu Yuyuutsu marked this pull request as ready for review January 21, 2025 10:13
@Yuyuutsu Yuyuutsu requested a review from a team as a code owner January 21, 2025 10:13
@Yuyuutsu Yuyuutsu changed the title add fixes to codeql errors Fix CodeQL Alerts Jan 27, 2025
@Yuyuutsu Yuyuutsu changed the title Fix CodeQL Alerts Fix: Correct DOM Text Handling, Pin Tags in Workflow, and Add Permissions Jan 30, 2025
@@ -1,4 +1,13 @@
// uuidv4: from https://github.com/kelektiv/node-uuid
function escapeHtml(unsafe) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually used anywhere? How come this has been added?

Copy link
Author

Choose a reason for hiding this comment

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

Oops forgot to apply the escapeHTML function. Added now.

Copy link
Author

Choose a reason for hiding this comment

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

It was a suggested change so "any text taken from the DOM and used to set innerHTML is properly escaped to prevent XSS attacks".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants