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

Release 15.10.1 #7050

Closed
5 tasks done
jeddy3 opened this issue Jul 5, 2023 · 21 comments
Closed
5 tasks done

Release 15.10.1 #7050

jeddy3 opened this issue Jul 5, 2023 · 21 comments
Labels
status: wip is being worked on by someone

Comments

@jeddy3
Copy link
Member

jeddy3 commented Jul 5, 2023

  • stylelint release
  • stylelint-config-recommended update/release
  • stylelint-config-standard update/release
  • stylelint.io update
  • stylelint-demo check
  • tweet
  • add patched version to the draft advisory and publish

For #7043.

Let's add a manual entry to the changelog for #5042 to https://github.com/stylelint/stylelint/pull/7048/files just before we merge:

I'm out for the rest of the day, but if anyone fancies releasing... go for it!

@mattxwang
Copy link
Member

Happy to kickstart the release process!

mattxwang added a commit that referenced this issue Jul 6, 2023
@mattxwang mattxwang added status: wip is being worked on by someone and removed status: needs discussion triage needs further discussion labels Jul 6, 2023
mattxwang added a commit that referenced this issue Jul 6, 2023
Manually adds changelog entry; ref: #7050

---------

Co-authored-by: Matthew Wang <matt@matthewwang.me>
@mattxwang
Copy link
Member

I've published the new version to NPM, have verified the new release, and have opened stylelint/stylelint.io#334. It's a bit past midnight for me now, so I might have to drop off before stylelint/stylelint.io#334 gets reviewed. In that case, happy to have someone else drive home the rest of the release; if not, I'll be up in about ~ 6-7 hours and can wrap the rest of it up.

@ybiquitous
Copy link
Member

@mattxwang No problems. I'll take over soon. 👍🏼

@ybiquitous
Copy link
Member

All done! 🎉

I've published the advisory: GHSA-f7xj-rg7h-mc87

Thank you so much for your effort! 👏🏼

@jeddy3
Copy link
Member Author

jeddy3 commented Jul 6, 2023

@mattxwang Thank you for getting the ball rolling!
@ybiquitous And thank you for finishing up!

I've edited the advisory to:

  • improve the description for users
  • mark @romainmenke as the remediation developer

@ntwb
Copy link
Member

ntwb commented Jul 6, 2023

Thanks everyone 💯

@ybiquitous
Copy link
Member

@jeddy3 Thanks for your follow-up!

@G-Rath
Copy link

G-Rath commented Jul 9, 2023

@ybiquitous unless there's an actual vulnerability in stylelint itself you should revoke GHSA-f7xj-rg7h-mc87 as the semver advisory will cover the dependency and so your advisory only adds artificial constraints to addressing the real advisory.

i.e. the patch for semver is going to be backported soon to older versions, at which point the advisory should then be resolvable without needing to upgrade stylelint but because you've published your own advisory usages of stylelint will still be flagged as having to upgrade.

@ybiquitous
Copy link
Member

@G-Rath Thanks for the feedback. I'm glad to hear that semver will be backported. To be honest, I'm unfamiliar with GitHub advisory reports. Can we revoke the already published advisory?

@G-Rath
Copy link

G-Rath commented Jul 10, 2023

@ybiquitous you can, but you have to contact GitHub support.

If you're having trouble, I have a few contacts on the GitHub security team I can reach out to to see if they can help.

@ybiquitous
Copy link
Member

@G-Rath Thanks. I've contacted GitHub support. I'll be sure to share the info if the request proceeds.

@stylelint/owners You can see details of the request with our shared email address.

@ybiquitous
Copy link
Member

@G-Rath @stylelint/owners

I've received a response from GitHub support. They recommend keeping the advisory instead of withdrawing it because Stylelint users can get information about the semver vulnerability through this Stylelint advisory.

Here's an excerpt of the response comment:

Maintainers of a package alerting users to a vulnerable dependency, as you have done, is a valid use of a GitHub Security Advisory.

So, I've updated the advisory description, following the advice. Please check out the added section "Security fix backported to older semver versions", and let me know if you find anything in it. 🙏🏼

@G-Rath
Copy link

G-Rath commented Jul 11, 2023

@ybiquitous in that case can you update the advisory to reflect the versions of Stylelint that are able to use the patched versions of semver? (v5, v6, and v7 branches).

It's not enough to just add a comment, because security tools like dependabot and osv-detector will falsely flag applications as being vulnerable based on the ranges.

@ybiquitous
Copy link
Member

can you update the advisory to reflect the versions of Stylelint that are able to use the patched versions of semver? (v5, v6, and v7 branches).

Umm, it seems difficult. 🤔
Stylelint 15.10.1 was released as a patched version. Users can select either way, updating stylelint or semver.

I'll ask GitHub Support for more information.

@G-Rath
Copy link

G-Rath commented Jul 11, 2023

This is why I recommend just withdrawing the advisory completely 😅

What GitHub support have recommended is technically correct but it means you have to do the work of maintaining the stylelint advisory to ensure it is 100% accurate which now there are backports for semver, is not true - this has to be done manually because there is no relationship between your advisory and the semver one; your one just says "any version of Stylelint below 15.10.1 has a vulnerability" regardless of what version semver is actually installed.

If you withdraw your advisory though, the semver advisory (which reflects the exact versions that are vulnerability) will still be flagged for stylelint users but you won't have to maintain the stylelint advisory to reflect any changes that happen to the semver advisory...

@ybiquitous
Copy link
Member

@G-Rath I asked GitHub support again, but unfortunately, it told me there was no way to suppress such alerts. 😓

As you commented, the current situation is not ideal, but I think we must tolerate it. Updating stylelint in addition to semver seems to bother us, but there should not be considerable harm.

Instead, withdrawing the advisory published once may be a big problem. I'd like to see how it goes for a while.

@G-Rath
Copy link

G-Rath commented Jul 11, 2023

@ybiquitous you need to update the advisory to reflect the versions of stylelint that can pull in vulnerable versions of semver - I've just checked and it looks like no versions do that because of the backporting meaning that the advisory should be withdrawn since it not longer relevant at all.

Updating stylelint in addition to semver seems to bother us, but there should not be considerable harm.

No but it does create more work for folks like myself - this advisory has resulted in dozens of our repositories being flagged even though we can now patch semver, and upgrading to stylelint v15 is a major change so we can't just do it for most of these repositories.

Instead, withdrawing the advisory published once may be a big problem

Withdrawing advisories is a well supported part of the system - there will be no problems in doing so.

@ybiquitous
Copy link
Member

@G-Rath I'm afraid about your inconvenience, but I'd like to follow the recommendation by GitHub Support that we should not withdraw the advisory.

this advisory has resulted in dozens of our repositories being flagged even though we can now patch semver, and upgrading to stylelint v15 is a major change so we can't just do it for most of these repositories.

Just to confirm, can you close such alerts instead of accepting them? Not upgrading stylelint seems like no problem.

@G-Rath
Copy link

G-Rath commented Jul 12, 2023

I'd like to follow the recommendation by GitHub Support that we should not withdraw the advisory.

Their recommendation is based on the fact that there was a "vulnerability" - feel free to ask them what they think they should do now that the advisory no longer affects any version of stylelint.

Alternatively I can suggest an improvement to the advisory on the GHADB updating the affected ranges if you like, and we can go from there. I've opened github/advisory-database#2491 for this.

Just to confirm, can you close such alerts instead of accepting them? Not upgrading stylelint seems like no problem.

While we do have that capability in our system for the short-term, it comes with a lot of red tape since we don't have a single client - so we've got a number of different security levels and folks that we engage with and discuss these things with.

Given that this advisory is incorrect, I'd prefer to get that corrected to help not just myself but the ecosystem as a whole since that's really the issue here anyway.

@ybiquitous
Copy link
Member

@G-Rath Thanks for opening the Pull Request. I didn't know of the repository. Let's continue the discussion on that PR.

@ntwb
Copy link
Member

ntwb commented Jul 12, 2023

Thanks @G-Rath & @ybiquitous

I think we've all learnt a few things following this discussion, albeit retroactively

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone
Development

No branches or pull requests

5 participants