Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Idea: safety badges #150

Closed
mleonhard opened this issue Nov 27, 2020 · 15 comments
Closed

Idea: safety badges #150

mleonhard opened this issue Nov 27, 2020 · 15 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@mleonhard
Copy link

I want crate readme files to show their unsafe-ness.

  • badge unsafe
    for crates without #![forbid(unsafe_code)]
  • badge unsafe-deps
    for crates with deps that lack #![forbid(unsafe_code)]
  • badge safe
    for crates with #![forbid(unsafe_code)]

Clicking on the badge would show the cargo-geiger report for that version of the crate.

Is anyone else interested in setting up something like this?

SVG sources: badges-svg.zip

@8573
Copy link

8573 commented Nov 28, 2020

@mleonhard wrote—

I want crate readme files to show their unsafe-ness.

  • badge unsafe
    for crates without #![forbid(unsafe_code)]
  • badge unsafe-deps
    for crates with deps that lack #![forbid(unsafe_code)]

I suppose such badges could be provided by a website such as that proposed in #108.

@mleonhard wrote—

  • badge safe
    for crates with #![forbid(unsafe_code)]

The current de-facto standard for this in the orbit of the Secure Code Working Group seems to be the following badge, as can be seen in the READMEs of cargo-geiger, cargo-audit, and Abscissa:

unsafe forbidden

It links to Safety Dance; I suppose linking to a cargo-geiger report (or the line in the source code that has #![forbid(unsafe_code)]) makes more sense, although how to continue to promote Safety Dance would be then an open question (although I imagine a textual note promoting it might be more useful than a badge whose text doesn't mention the name "Safety Dance").

@anderejd
Copy link
Contributor

There's another class of libraries that would be most interesting in my opinion, the crates that declare [forbid(unsafe_code)] and have all their dependencies declare [forbid(unsafe_code)].

@tarcieri
Copy link
Collaborator

@anderejd sure wish there were some sort of attribute which enforced that at the compiler level

@anderejd
Copy link
Contributor

anderejd commented Dec 4, 2020

@tarcieri That would be neat. I wonder if cargo-deny would be interested in providing something like that...

@tarcieri
Copy link
Collaborator

tarcieri commented Dec 4, 2020

My understanding of cargo-deny is they explicitly don't want to do source code analysis, only dependency analysis (see this comment).

Seems like cargo-geiger itself is well equipped to do that sort of linting, though.

@anderejd
Copy link
Contributor

anderejd commented Dec 4, 2020

We could probably add a "transitive forbid unsafe" / "deep safe" metric quite easily. Should be as simple as traversing the dependency tree from the leaf nodes and up, propagating a "all dependencies forbid unsafe" flag upwards.

@tarcieri
Copy link
Collaborator

tarcieri commented Dec 4, 2020

That sounds super helpful.

Perhaps this is already possible and I just don't know how to do it, but it'd be nice if cargo-geiger had a --check option like rustfmt for this sort of thing, which exits with 0 if all dependencies are are unsafe-free and/or #![forbid(unsafe)], and 1 if there are unsafe dependencies.

Bonus points if you could include a whitelist of "trusted" unsafe crates and ensure that no new unsafe dependencies are added.

@Shnatsel
Copy link

Shnatsel commented Dec 5, 2020

I am not convinced that a marker for safe-only dependency tree is worth the trouble, because it's just so rare in practice. There are a scant few crates that can claim that. Although I have authored a few such crates, and could put it on some of my stuff.

This also further entrenches the special status of the standard library, which might not be such a great thing. It's important to recognize that libstd also contains unsafe code.

@Shnatsel
Copy link

Shnatsel commented Dec 5, 2020

Finally, it's impossible to actually adhere to this badge because you usually don't control your dependencies, so there may be newer versions that added unsafe code, but the badge is still present (and is now misleading) because installations from crates.io do not respect Cargo.lock by default

@anderejd
Copy link
Contributor

anderejd commented Dec 6, 2020

Finally, it's impossible to actually adhere to this badge because you usually don't control your dependencies, so there may be newer versions that added unsafe code, but the badge is still present (and is now misleading) because installations from crates.io do not respect Cargo.lock by default

@Shnatsel That Cargo.lock is ignored when installing from crates.io is news to me, is it done to allow using bug fix and patch releases of dependencies or some other reason?

You bring up an interesting perspective in this comment, that cargo-geiger may possibly be useful for end-users installing an executable from crates.io, I had not thought about this at all until reading your comment. Previously my idea of cargo-geiger was strictly a tool for developers, intended to help with auditing and dependency evaluation. I opened #160 for this discussion.

@anderejd
Copy link
Contributor

anderejd commented Dec 6, 2020

Perhaps this is already possible and I just don't know how to do it, but it'd be nice if cargo-geiger had a --check option like rustfmt for this sort of thing, which exits with 0 if all dependencies are are unsafe-free and/or #![forbid(unsafe)], and 1 if there are unsafe dependencies.

Bonus points if you could include a whitelist of "trusted" unsafe crates and ensure that no new unsafe dependencies are added.

@tarcieri There is no --check mode yet, but the --forbid-only mode: https://github.com/rust-secure-code/cargo-geiger/blob/a7e902dfe6ca4da8fa81d8712fc034a3fca55021/cargo-geiger/src/args.rs#L53 was added as a first step towards this feature, including the whitelist/allow list.

@anderejd anderejd added enhancement New feature or request question Further information is requested labels Dec 6, 2020
@8573
Copy link

8573 commented Dec 6, 2020

@Shnatsel wrote—

Finally, it's impossible to actually adhere to this badge because you usually don't control your dependencies, so there may be newer versions that added unsafe code, but the badge is still present (and is now misleading) […]

Isn't the situation the same as for CI badges, which are commonly used even though they provide only a ‘best-effort’ indication of whether software builds successfully, as, even if the badge is green, there may have been a newer version of a dependency that breaks the build released since the last time CI ran?

@anderejd
Copy link
Contributor

Pie in the sky: Having crates.io run the scan and choose which badge to display.

@pinkforest
Copy link
Collaborator

pinkforest commented Dec 16, 2021

Great for making those badges - I shall be dishing out these badges in geiger.rs people can embed in their repo README.md :)

One thing though - Currently we are pretty strict about what is considered unsafe -

Should we have some type of grading e.g. instead of declaring something unsafe just because they don't declare the no unsafe allowed or should we be more proactive ❓ and do a bit conditional 🥕 thing?

@pinkforest
Copy link
Collaborator

I'll move this into discussions

@geiger-rs geiger-rs locked and limited conversation to collaborators May 17, 2022
@pinkforest pinkforest converted this issue into discussion #310 May 17, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants