Skip to content

USWDS-Site: HTML-proofer and snyk fixes #3068

Merged
thisisdano merged 15 commits into
mainfrom
al-html-proofer-jan-2
Jan 23, 2025
Merged

USWDS-Site: HTML-proofer and snyk fixes #3068
thisisdano merged 15 commits into
mainfrom
al-html-proofer-jan-2

Conversation

@amyleadem
Copy link
Copy Markdown
Contributor

@amyleadem amyleadem commented Jan 21, 2025

Summary

  • Fixed 404 links throughout the site.
  • Updated snyk ignore

Important

We should confirm changelog dates before merge

Related issue

N/A

Preview links

  • Link component page
    • Link text: “OMB M-23-22”
  • Together page
    • Link text: "“Priority 2 Executive Order”, “Executive Order 13985”, “Executive Order 14058”
  • Design principles
    • Link text: “Delivering Excellent, Equitable, and Secure Federal Services and Customer Experience”
  • Patterns - Inclusive design
    • Links: "Executive Order 13985", “Priority 2 Executive Order”
  • Patterns - Gender identity and sex
    • Link text: ”Executive Order 14075”, "Recommendations on the best practices for the collection of sexual orientation and gender identity”, url: https://www.state.gov/x-gender-marker-available-on-u-s-passports-starting-april-11/,
    • Link: X gender marker available on U.S. Passports starting April 11. (March 31, 2022) Retrieved on July 20, 2022, from url: https://www.state.gov/x-gender-marker-available-on-u-s-passports-starting-april-11/ [This link is no longer active. Archived copy on state.gov]
  • Patterns - Preferred language
    • Link text: url https://digital.gov/resources/delivering-digital-first-public-experience/

Problem statement

HTML-proofer failures

Received the following HTML-proofer build errors in CircleCI:

For the Links > External check, the following failures were found:

* At ./_site/components/link/index.html:3943:

  External link https://www.whitehouse.gov/omb/management/ofcio/delivering-a-digital-first-public-experience/ failed (status code 404)

* At ./_site/design-principles/index.html:2849:

  External link https://www.performance.gov/pma/cx/ failed (status code 404)

* At ./_site/patterns/create-a-user-profile/gender-identity-and-sex/index.html:2847:

  External link https://www.whitehouse.gov/briefing-room/presidential-actions/2022/06/15/executive-order-on-advancing-equality-for-lesbian-gay-bisexual-transgender-queer-and-intersex-individuals/ failed (status code 404)

* At ./_site/patterns/create-a-user-profile/gender-identity-and-sex/index.html:2849:

  External link https://www.whitehouse.gov/wp-content/uploads/2023/01/SOGI-Best-Practices.pdf failed (status code 404)

* At ./_site/patterns/create-a-user-profile/gender-identity-and-sex/index.html:3335:

  External link https://www.whitehouse.gov/wp-content/uploads/2023/01/SOGI-Best-Practices.pdf failed (status code 404)

* At ./_site/patterns/create-a-user-profile/gender-identity-and-sex/index.html:3337:

  External link https://www.state.gov/x-gender-marker-available-on-u-s-passports-starting-april-11/ failed (status code 404)

* At ./_site/patterns/inclusive-design/index.html:2501:

  External link https://www.performance.gov/pma/cx/ failed (status code 404)

* At ./_site/patterns/inclusive-design/index.html:2501:

  External link https://www.performance.gov/equity/ failed (status code 404)

* At ./_site/patterns/select-a-language/language-preferences/index.html:3004:

  External link https://www.whitehouse.gov/wp-content/uploads/legacy_drupal_files/omb/memoranda/2017/m-17-06.pdf failed (status code 404)

* At ./_site/together/index.html:429:

  External link https://www.performance.gov/pma/cx/ failed (status code 404)

* At ./_site/together/index.html:435:

  External link https://www.performance.gov/pma/cx/ failed (status code 404)

* At ./_site/together/index.html:435:

  External link https://www.whitehouse.gov/briefing-room/presidential-actions/2021/12/13/executive-order-on-transforming-federal-customer-experience-and-service-delivery-to-rebuild-trust-in-government/ failed (status code 404)

* At ./_site/together/index.html:435:

  External link https://www.performance.gov/equity/ failed (status code 404)

Snyk failures

Issues with no direct upgrade or patch:
  ✗ Insecure Randomness [High Severity][https://security.snyk.io/vuln/SNYK-JS-UNDICI-8641354] in undici@6.20.1
    introduced by @uswds/compile@1.2.1 > gulp-svgstore@9.0.0 > cheerio@1.0.0 > undici@6.20.1
  This issue was fixed in versions: 5.28.5, 6.21.1, 7.2.3

Solution

HTML-proofer

Updated links according to the decisions in the January 2025 broken links spreadsheet (Google Sheets 🔒)

Snyk

Ran the following commands to update snyk ignore. (Reference: Snyk process doc, Google docs 🔒)

Note

I included INFLIGHT here as well because we just updated that one 2 weeks ago and I want to keep the ignores in sync as much as possible.

npx snyk ignore --id="SNYK-JS-INFLIGHT-6095116" --reason="No available upgrade or patch" 
npx snyk ignore --id="SNYK-JS-UNDICI-8641354" --reason="No available upgrade or patch" 

Testing and review

  • Confirm the link replacements are appropriate and work as expected
  • Confirm there is no need to add or change related content on these pages
  • Confirm changelogs are present and accurate
  • Confirm the snyk ignore updates are appropriate

Comment thread _together/01-summary.md
Copy link
Copy Markdown
Contributor Author

@amyleadem amyleadem Jan 21, 2025

Choose a reason for hiding this comment

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

question: We do not currently have changelogs on these Together pages. Do we want to add them? It should be a fairly low lift.

@thisisdano @annepetersen

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, we won't be adding changelogs to these pages

Copy link
Copy Markdown
Contributor Author

@amyleadem amyleadem Jan 21, 2025

Choose a reason for hiding this comment

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

question: This page is currently excluded from the side nav. Should we add it in, maybe below the "Introducing patterns" link?

@annepetersen @thisisdano

image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, we won't be adding this to the sitenav

@amyleadem amyleadem marked this pull request as ready for review January 21, 2025 23:30
@amyleadem amyleadem requested a review from a team as a code owner January 21, 2025 23:30
Comment thread pages/patterns/select-language/language-preferences.md Outdated
Comment thread _components/link/guidance/additional-information.md Outdated
@amyleadem
Copy link
Copy Markdown
Contributor Author

Update: New snyk failures popping up. Going to include a fix for that here.

@amyleadem amyleadem changed the title USWDS-Site: HTML-proofer fixes USWDS-Site: HTML-proofer and snyk fixes Jan 22, 2025
Comment thread pages/patterns/create-a-profile/gender-identity-and-sex.md Outdated
Comment thread pages/patterns/create-a-profile/gender-identity-and-sex.md Outdated
@amyleadem amyleadem requested a review from finekatie January 22, 2025 17:06
Comment thread pages/patterns/create-a-profile/gender-identity-and-sex.md Outdated
Copy link
Copy Markdown
Contributor

@finekatie finekatie left a comment

Choose a reason for hiding this comment

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

This all looks good to me!

@amyleadem amyleadem requested review from mahoneycm and mejiaj January 22, 2025 18:00
@amyleadem
Copy link
Copy Markdown
Contributor Author

@mejiaj and @mahoneycm tagging you for a quick review here to confirm the changes on the dev side of the house, particularly the snyk updates since (as far as I know) UNDICI is a new dependency for the snyk ignore. Just wanting to confirm there are no issues with ignoring that one.

@mahoneycm
Copy link
Copy Markdown
Contributor

@amyleadem the undici vulnerability can be resolved by running npm audit fix! I was going to have @cathybaptista resolve this in #3063 but if it's preventing builds, we can go ahead and update here as well.

Noting it will also resolve https://github.com/uswds/uswds-site/security/dependabot/92

Copy link
Copy Markdown
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 updating. The replacement links look good. Minor, non-blocking, comment on display of OMB M-23-22 text.

Comment thread _components/link/guidance/additional-information.md
Copy link
Copy Markdown
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.

LGTM! I left one unblocking question about some potentially unnecessary changelog entries. I trust your judgement but let me know if you want a second pair of eyes to compare content!

Comment thread .snyk
Comment on lines +3543 to +3547
SNYK-JS-UNDICI-8641354:
- '*':
reason: No available upgrade or patch
expires: 2025-02-21T15:55:42.507Z
created: 2025-01-22T15:55:42.548Z
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: The snyk warning says this may be deriving from a sub-dependency of USWDS-Compile. I'm curious if other projects that use Compile are seeing this vulnerability.

If we continue to see this issue, we may want to test Compile's develop branch after uswds/uswds-compile#143 gets merged to see if it resolves the issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question: Are any of these links simply destination changes where the content is not changing? in the past, we've opted to not add changelogs for links that have the same content.

From previous link update discussions:

If the link points to new or different content, we should add a changelog. If it is just a new url for the same content, leave it as is

I trust your judgement here though!

Copy link
Copy Markdown
Contributor Author

@amyleadem amyleadem Jan 22, 2025

Choose a reason for hiding this comment

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

IMO, these types of link updates (related to executive orders and administration changes) warrant a changelog. These updates reflect to a meaningful change to me, more than a basic domain name change/site re-org would.

We will be working on an SOP for this later on, so assuming team agreement on this stance, we can make sure to include this caveat there.

Curious to hear what everyone thinks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Definitely need a changelog. Thanks!

@finekatie
Copy link
Copy Markdown
Contributor

finekatie commented Jan 23, 2025

Hey @amyleadem , we've already been working on a PR to update links on the Design Principles page. One of the broken links fixed here was not noted in the original PR (was underway before Monday 1/20). Jacline also found another one in the same section. Do I need to update them both in my PR too so that the old link doesn't override what you've corrected here?

@amyleadem
Copy link
Copy Markdown
Contributor Author

amyleadem commented Jan 23, 2025

@finekatie It looks like the other PR is not touching the link that this PR fixes, so there shouldn't be any conflicts between them. Depending on which PR gets merged first, we'll just need to update the main branch in the other PR to get everything in sync (We can walk through that together when it is time if you want). No action needed atm 👍

@thisisdano thisisdano merged commit 086f98d into main Jan 23, 2025
@thisisdano thisisdano deleted the al-html-proofer-jan-2 branch January 23, 2025 20:52
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.

6 participants