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

[docs] Add yank log and update yank policy #722

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Dec 13, 2023

Closes #238
Closes #679

Comment on lines +104 to +107
- The bug contradicts documented behavior
- The bug contradics widely assumed behavior
- The bug is a regression (code exists which would succeed on a version X and
would fail on a version Y > X)

Choose a reason for hiding this comment

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

As I have explained in #679, I think these last three are not good reasons for yanking (at least, usually they won't be).

Additinally I think this policy is not going to be workable because it calls for you to submit a RustSec advisory for all issues you consider yankworthy. But many of the issues caught by this policy will not be considered advisory-worthy by RustSec. For example, RustSec will not want to issue an advisory about something just because it's a regression.

I think you should resolve this dilemma by not yanking for non-advisory-worthy issues.

I'm still struggling to see the positive benefit of yanking so often.

Copy link
Member Author

@joshlf joshlf Dec 14, 2023

Choose a reason for hiding this comment

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

Our perspective is that there's no way for us to know whether any given user cares about any given bug. For any given bug, some users may prefer to have their compilation fail rather than allow a bug to manifest at runtime, especially if a fix has been published so it's always possible to get back to a good state by updating to the latest version. If we opt not to yank/publish a rustsec advisory for some bugs, then we're saving some users (who wouldn't have cared about the bug) from having to update to the latest version, but we're also exposing the users who would care to a bug that they would prefer to avoid. Given that there's no way for us to cause problems only for the users who care (since we don't know a prior who they are), we have to choose between requiring some users to update more frequently than they'd like and exposing some users to more bugs than they'd like. Our perspective is that, especially for a crate whose sole purpose is to allow users to write correct, secure code, the latter would not be an acceptable choice.

Aso, FWIW, this is just a codification of what we've been doing implicitly for a long time, and in practice we yank very infrequently - in my rough estimation, no more than five times in zerocopy's five years of being on crates.io.

Additinally I think this policy is not going to be workable because it calls for you to submit a RustSec advisory for all issues you consider yankworthy. But many of the issues caught by this policy will not be considered advisory-worthy by RustSec. For example, RustSec will not want to issue an advisory about something just because it's a regression.

My understanding is that RustSec is not particularly opinionated about what counts as a "valid" advisory. The advisory format even supports informational notices such as "this crate is no longer maintained."

@Shnatsel, am I correct in my understanding that RustSec would be okay with publishing advisories for the following categories?

  • A bug which contradicts documented behavior of an API
  • A bug which contradicts widely assumed behavior of an API
  • A bug which is a regression (code could exist which would succeed on version X but fail on semver-compatible version Y > X)

Choose a reason for hiding this comment

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

So far we have been publishing security advisories only for bugs that can cause memory safety issues. Depending on whether or not triggering the bug requires contrived code, it would either be a regular advisory or one with the informational = "unsound" marker so that it is only ever surfaced as a warning by RustSec tooling.

As an example, reading or writing data incorrectly is something that could be yank-worthy but out of scope of a RustSec advisory.

@ijackson
Copy link

Thanks for soliciting input. I hope I'm not being too dogged, but I felt I should reply, since you asked.

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.

Yanking policy clarification/discussion Document yanked releases
3 participants