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

Implemented SRI check on the external babel links #6152

Merged

Conversation

nelsonuprety1
Copy link
Member

@nelsonuprety1 nelsonuprety1 commented Jan 25, 2024

Fixes #6120

What changes did you make?

  • The script tag in the default-markdown.html and default.html files was updated with integrity and crossorigin attributes.

Why did you make the changes (we will use this info to test)?

  • The changes were made to address CodeQL alerts 36 and 37, which will enhance the script integrity and mitigate potential vulnerabilities.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied Screenshot 2024-01-24 at 18 23 30
Visuals after changes are applied Screenshot 2024-01-24 at 18 17 05 Screenshot 2024-01-24 at 18 17 57 Screenshot 2024-01-24 at 18 18 25 Screenshot 2024-01-24 at 18 18 46

Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b nelsonuprety1-implement-sri-check-6120 gh-pages
git pull https://github.com/nelsonuprety1/website.git implement-sri-check-6120

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/nelsonuprety1/website/blob/implement-sri-check-6120/CONTRIBUTING.md  

@github-actions github-actions bot added role: front end Tasks for front end developers Complexity: Medium size: 1pt Can be done in 4-6 hours Feature: Code Alerts labels Jan 25, 2024
@freaky4wrld freaky4wrld self-requested a review January 25, 2024 05:26
@freaky4wrld
Copy link
Member

ETA: EoD 1/25/12
Availability: Evenings

Copy link
Member

@freaky4wrld freaky4wrld left a comment

Choose a reason for hiding this comment

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

Hey @nelsonuprety1 thanks for taking up this issue, these are my observations:

  • the to/from branch is correct
  • the linked issue is correct as well
  • the issue correctly states the changes made and the reason to make those changes
  • both the attributes integrity and crossorigin are added and with a correct sha-hash
  • on manual testing there's no error generated in the browser-console
  • also the babel.js gets loaded correctly, thanks for providing the screenshot of pages using the file default-markdown.html and default.html

I noticed you wanted to do default.html instead of 'default.html' (you need to use backticks the key on top of tab)

PR approved !!!

@nelsonuprety1
Copy link
Member Author

@roslynwythe please check the PR thank you

Copy link
Member

@roslynwythe roslynwythe left a comment

Choose a reason for hiding this comment

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

Thank you @nelsonuprety1 for completing this issue which improves the security of our website. The branches are setup correctly, your code changes are correct and clean, and you described your work. Great job!

@t-will-gillis t-will-gillis merged commit 4473ccf into hackforla:gh-pages Jan 28, 2024
11 checks passed
@nelsonuprety1 nelsonuprety1 deleted the implement-sri-check-6120 branch January 28, 2024 21:45
@roslynwythe roslynwythe mentioned this pull request Mar 20, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Medium Feature: Code Alerts role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement SRI check on the external babel links
4 participants