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

feat: add OpenSSF badge to current footer #6030

Closed
wants to merge 1 commit into from

Conversation

bmuenzenmeyer
Copy link
Collaborator

@bmuenzenmeyer bmuenzenmeyer commented Oct 19, 2023

Description

Adds the OpenSSF best practices badge to the current site's footer. This is a stop-gap measure while the new site is being designed - but furthers efforts from the @nodejs/security working group started within nodejs/security-wg#859

Validation

https://nodejs-xhjbbt8kf-openjs.vercel.app/en

image

Related Issues

closes #5432

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npx turbo lint to ensure the code follows the style guide. And run npx turbo lint:fix to fix the style errors if necessary.
  • I have run npx turbo format to ensure the code follows the style guide.
  • I have run npx turbo test to check if all tests are passing.
  • I've covered new added functionality with unit tests if necessary.

@bmuenzenmeyer bmuenzenmeyer requested a review from a team as a code owner October 19, 2023 13:41
@vercel
Copy link

vercel bot commented Oct 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Oct 19, 2023 1:41pm

@github-actions
Copy link

Unit Test Coverage Report

Lines Statements Branches Functions
Coverage: 94%
94.7% (286/302) 76.81% (53/69) 93.33% (56/60)

Unit Test Report

Tests Skipped Failures Errors Time
29 0 💤 0 ❌ 0 🔥 8.137s ⏱️

Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

I don't think we should add this to every page. IMHO it's distasteful and does not align well with the style and design of our website.

@ljharb
Copy link
Member

ljharb commented Oct 19, 2023

Why is it distasteful?

@bmuenzenmeyer
Copy link
Collaborator Author

bmuenzenmeyer commented Oct 19, 2023

I don't think we should add this to every page. IMHO it's distasteful and does not align well with the style and design of our website.

Alternatives considered:

  • The security link in the header links off the site.
  • About/Governance was discussed and determined not to be right either.

FWIW, I've never seen a badge NOT on a repository's README, but that decision has already been made.

I'm happy to adjust however we see fit but suspect there is not concensus or one singular perfect solution. If we don't value removing this roadblock for nodejs/security-wg#956 we can close. #6031 is the permanent solution

@anonrig
Copy link
Member

anonrig commented Oct 19, 2023

I know that it serves a purpose of passing the gold level openssf, but other than passing to the gold level,"openssf best practices: passing" doesn't provide a useful insight to a random website visitor.

Why is it distasteful?

Because:

  • It does not relate itself to any of the existing components from old/new designs.
  • The size of the button is bigger than more important links such as any item in the last footer horizontal list that starts with the openjs foundation | terms of use etc.
  • The green background in the badge is not the same as the green color we use in the identity of the project.

Is there a specific reason for adding this to all pages of nodejs.org compared to a single page? It seems this is a badge that should be included in security page, since it is a security related badge.

The best practices requirements state that:

The project repository front page and/or website MUST identify and hyperlink to any achievements, including this best practices badge, within 48 hours of public recognition that the achievement has been attained

As far as I can see, it doesn't state that we need to put this to every single page.

@ovflowd
Copy link
Member

ovflowd commented Oct 19, 2023

I stand with Yagiz here. This badge has absolute no meaning or value for your average visitor and even Node.js end-user. Maybe even collaborator.

It feels right to be either at the README or at the Security page, now that there's a PR open to add a security page.

@ljharb
Copy link
Member

ljharb commented Oct 19, 2023

I agree with all that, which is why it belongs on the repo readme, but the security page is fine too. It'd just be nice to not have to wait for the redesign.

@ovflowd
Copy link
Member

ovflowd commented Oct 19, 2023

Right. But afaik if we only add on the readme we don't get certification. To be frank, I find these requirements from OpenSSF quite nonsensical. I hate when other organisations impose a badge on us to gain X, or at lest they should allow us to decide where to put it.

Anyhow, sorry for my ramble. If we can wait for the other PR I mentioned to get merged, then we can move this badge to the security page of the website. Even tho that badge completely does not match anything on the website and feels misplaced.

@ljharb
Copy link
Member

ljharb commented Oct 19, 2023

@ovflowd that's incorrect; "only the readme" is completely sufficient for gold status, and is the preferred location.

@ovflowd
Copy link
Member

ovflowd commented Oct 19, 2023

@ovflowd that's incorrect; "only the readme" is completely sufficient for gold status, and is the preferred location.

I was told that it needed to be the nodejs/node readme and the node.js website readme wasn't allowed.

There was a whole debacle about that. I might be wrong, my memory fails me daily 🫠

@ljharb
Copy link
Member

ljharb commented Oct 19, 2023

oh, well yeah when i say "readme" i mean "in the project repo it's describing", websites don't have a readme in a colloquial sense :-)

putting it in the website repo's readme wouldn't make sense for a project that's not "the website".

@ovflowd
Copy link
Member

ovflowd commented Oct 19, 2023

oh, well yeah when i say "readme" i mean "in the project repo it's describing", websites don't have a readme in a colloquial sense :-)

putting it in the website repo's readme wouldn't make sense for a project that's not "the website".

Well, I am not the one who decides what goes to the nodejs readme. Yet often websites readme's are also a touchpoint to the project.

What I meant is that displaying a badge is ridiculous as a requirement. At least for me, an ignorant one (as I have little to no knowledge about anything OpenSSF). How could a badge grant "golden" security status? 😅

@ovflowd
Copy link
Member

ovflowd commented Oct 19, 2023

(Sorry for going offtopic btw)

@ljharb
Copy link
Member

ljharb commented Oct 19, 2023

Fair question, but I'm assuming it's about providing transparency so that users can validate security practices if they want.

@ovflowd
Copy link
Member

ovflowd commented Oct 19, 2023

Fair question, but I'm assuming it's about providing transparency so that users can validate security practices if they want.

If that's the goal, the badge should imo be accompanied with a text paragraph. As Yagiz mentioned, most of people visiting the website wouldnt know what OpenSSF stands for (or even is), even if they are curious about our security standards. Now, if we accompany the badge with a paragraph mentioning if one's want to get to know about some of our practices to then click the badge, that makes way much more sense for me.

@bmuenzenmeyer
Copy link
Collaborator Author

bmuenzenmeyer commented Oct 20, 2023

@anonrig

Is there a specific reason for adding this to all pages of nodejs.org compared to a single page? It seems this is a badge that should be included in security page, since it is a security related badge.

No there is no requirement, but there also is no security page. This is a stop-gap for the next couple months.


@ovflowd

at the Security page, now that there's a PR open to add a security page

?


Mock up of the footer with a paragraph explainer before the badge. Text borrowed from https://www.bestpractices.dev/en

image

The footer seems like the perfect place to me. Casual users won't look at any of its content.


If folks have other ideas, I can close this PR or hear direct suggestions. My goal was always to do something quick and unblocking, knowing that a new solution is coming. Perhaps my urgency is misplaced.

@bmuenzenmeyer
Copy link
Collaborator Author

closing - i see #6028 now that will have a security page

@ljharb ljharb deleted the 5432-cii-best-practices branch October 20, 2023 02:53
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.

CII Best practices badge addition
6 participants