Skip to content

USWDS - File Input: Replace inline javascript to meet content security policy (CSP) standards #5997

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 6, 2024
Merged

USWDS - File Input: Replace inline javascript to meet content security policy (CSP) standards #5997

merged 1 commit into from
Nov 6, 2024

Conversation

jeffpw-goog
Copy link
Contributor

@jeffpw-goog jeffpw-goog commented Jul 26, 2024 •
edited by mahoneycm
Loading

Summary

Fixed the file input component to meet content security policy standards. Inline Javascript is forbidden by typical content security policies (CSPs), and this replaces several inline onerror handlers in the file input component with addEventListener to avoid that violation.

Breaking change

This is not a breaking change.

Related issue

Closes #5990

Related pull requests

Changelog: uswds/uswds-site#2912

Preview link

None

Problem statement

The USWDS library should be compatible with sites that apply a content security policy (CSP). Currently the file input component uses onerror inline event handlers, which are prohibited by CSP (source), so the component does not work correctly under a CSP.

Solution

  • Replace onerror with calls to addEventListener.
  • Add a helper method setPreviewFallback to remove code duplication
  • Update file extension parsing logic to parse the file extension at the end of the filename rather than anywhere in the filename

Major changes

None

Testing and review

Steps to test:

  1. Run npm start
  2. Open http://localhost:6006/?path=/story/components-form-inputs-file-input--default
  3. Create a text file and upload it to the file input component with the following filenames. In each case confirm that the expected preview icon is shown.
    • file.txt (or any extension not in the list below)
    • file.pdf
    • file.doc
    • file.pages
    • file.xls
    • file.numbers
    • file.mov
    • file.mp4

Steps to test with a CSP policy:

Tip

To streamline testing, I copied @jeffpw-goog demo repo work in our sandbox repo so we can utilize the preview build!

CSP Preview link →

You can use this link and complete the steps about without reproducing a CSP policy demo via the steps below.

Leaving the steps below which outline how Jeff made the testing repo
Added by @mahoneycm

  1. Clone uswds-sandbox
  2. Update src/_includes/head.html:
    • Add <meta http-equiv="Content-Security-Policy" content="script-src 'nonce-mynonce' 'strict-dynamic' https:" />
    • Add nonce="mynonce" to the <link rel="preload" ... as="script"> tag
  3. Update src/_layouts/default.html:
  4. Run npm install "https://github.com/jeffpw-goog/uswds/tree/file-input-csp" --save
  5. Run npm install && npm init && npm start
  6. Load http://localhost:8080 in your browser
  7. Repeat the test cases from above

@jeffpw-goog jeffpw-goog marked this pull request as ready for review July 29, 2024 22:37
@jeffpw-goog jeffpw-goog mentioned this pull request Aug 1, 2024
Closed
@mahoneycm mahoneycm mentioned this pull request Aug 30, 2024
Closed
2 tasks
mahoneycm
mahoneycm reviewed Oct 7, 2024
View reviewed changes
@jeffpw-goog jeffpw-goog force-pushed the file-input-csp branch 2 times, most recently from 24bf5d2 to 91937e9 Compare October 10, 2024 15:11
@jeffpw-goog jeffpw-goog requested a review from mahoneycm October 10, 2024 15:12
@jeffpw-goog
Copy link
Contributor Author

Sorry @mahoneycm I didn't realize there was a formatting issue in my last push. Fixed now!

@mahoneycm mahoneycm mentioned this pull request Oct 21, 2024
Draft
mahoneycm
mahoneycm approved these changes Oct 21, 2024
View reviewed changes
Copy link
Contributor

@mahoneycm mahoneycm left a comment

Choose a reason for hiding this comment

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

Hey there @jeffpw-goog

Thanks for your continued work here! The PR description and testing steps were great.

This is working like a charm and follows standard USWDS code practices. Flagging others for additional review now!

@mahoneycm
Copy link
Contributor

Testing repo

To streamline testing, I copied @jeffpw-goog demo repo work in our repo so we can utilize the preview build!

CSP Preview link →

You can use this link and complete 1-3 without the second group of instructions to recreate the CSP policy demo.

CC: @mejiaj @amyleadem @cathybaptista

@jeffpw-goog jeffpw-goog mentioned this pull request Oct 25, 2024
Merged
cathybaptista
cathybaptista approved these changes Oct 25, 2024
View reviewed changes
Copy link
Contributor

@cathybaptista cathybaptista left a comment

Choose a reason for hiding this comment

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

I tested this with different file types and did not have any problems. .doc and .pages had the same icon (W), .xls and .numbers had the same icon (X) and movie and .mp4 used the same camera. all filenames are correct.

@mejiaj mejiaj mentioned this pull request Oct 29, 2024
Draft
mejiaj

This comment was marked as outdated.

Sign in to view

@mejiaj mejiaj dismissed their stale review October 29, 2024 15:47

Forgot a comment

mejiaj
mejiaj reviewed Oct 29, 2024
View reviewed changes
Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution. This is a good security fix in general.

I've tested on the website comparing develop with this branch on uswds/uswds-site#2923. I did discover some small differences.

Before After
live feature

All variants using different types of files. Also used the file selector and drag and drop.

@jeffpw-goog jeffpw-goog requested a review from mejiaj October 29, 2024 16:42
mejiaj
mejiaj approved these changes Oct 29, 2024
View reviewed changes
Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

Thanks @jeffpw-goog! Confirming that xlsx and doc icons are appearing.

@mejiaj mejiaj requested a review from thisisdano October 29, 2024 17:29
@gwendolyngoetz gwendolyngoetz mentioned this pull request Oct 30, 2024
Closed
1 task
@amyleadem amyleadem mentioned this pull request Nov 5, 2024
Merged
23 tasks
@amyleadem amyleadem mentioned this pull request Nov 5, 2024
Closed
thisisdano
thisisdano approved these changes Nov 6, 2024
View reviewed changes
Copy link
Contributor

@thisisdano thisisdano 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!

@thisisdano thisisdano merged commit 329d840 into uswds:develop Nov 6, 2024
4 checks passed
@thisisdano thisisdano mentioned this pull request Nov 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amyleadem amyleadem Awaiting requested review from amyleadem

4 more reviewers

@mejiaj mejiaj mejiaj approved these changes

@thisisdano thisisdano thisisdano approved these changes

@mahoneycm mahoneycm mahoneycm approved these changes

@cathybaptista cathybaptista cathybaptista approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

USWDS - Bug: file upload component is incompatible with a typical content security policy (CSP)
5 participants
@jeffpw-goog @mahoneycm @mejiaj @thisisdano @cathybaptista