Feature: safety report in readme

[mleonhard]

I want a tool to automatically update update a safety report section in my project's readme file. Requirements:

• Run cargo-geiger on the current project
• Convert the report into Markdown format
• Look for a # Safety Report section in Readme.md and replace it with the generated report

Advanced features:

• An argument with the name of the markdown file to update
• An argument with the name of the section to replace
• Check the git repository, see if there's a tag at HEAD in the current branch, and include the tag in the report. This could be a separate program.
• An option to include the branch name in the report.
• An option to include the git commit ID in the report.
• For each unsafe crate, link to a ticket about making the crate safer. So folks can easily express their desire for more safety in code and dependencies. For crates without such tickets, link to a page explaining how to file the ticket and add the link to cargo-geiger (make a PR). When crate maintainers delete tickets that ask for more safety, include a link to a ticket in a different repository. The isaacs/github repository is an example of this technique.

Similar tools: clog-cli

[jmcconnell26]

I had an initial stab at the base feature request here #156
This seems like a really useful feature to have, and I like the sound of the advanced features as well, I can keep plugging away at them, if no one has any objections?

[8573]

@mleonhard wrote—

• For each unsafe crate, link to a ticket about making the crate safer. So folks can easily express their desire for more safety in code and dependencies. For crates without such tickets, link to a page explaining how to file the ticket and add the link to cargo-geiger (make a PR).

Rather than keeping this information inside cargo-geiger, it might be better kept in, say, a hypothetical file https://github.com/rust-secure-code/safety-dance/blob/master/crate-info.yaml containing something like—

Example

- arrayref:
    repo: "https://github.com/droundy/arrayref"
    ticket: 50
    upstream-tickets:
      - "https://github.com/droundy/arrayref/issues/18"

- clap:
    repo: "https://github.com/clap-rs/clap"
    ticket: 49

- prost:
    repo: "https://github.com/danburkert/prost"
    ticket: 68

- ureq:
    repo: "https://github.com/algesten/ureq"
    ticket: 59
    status:
      unsafe-forbidden:
        date: 2020-06-16
        see: ["https://github.com/algesten/ureq/pull/68"]

- miniz_oxide:
    repo: "https://github.com/Frommi/miniz_oxide"
    ticket: 2
    status:
      unsafe-forbidden:
        date: 2019-08-09
        see: ["https://github.com/Frommi/miniz_oxide/pull/56"]

- flate2:
    repo: "https://github.com/rust-lang/flate2-rs"
    ticket: 32
    status:
      waiting:
        - for: author-response
          since: 2019-11-01
          see: ["https://github.com/rust-lang/flate2-rs/issues/220"]
        - for: library-support
          since: 2019-11-01
          see: ["https://github.com/rust-secure-code/safety-dance/issues/32#issuecomment-548833236"]
...

The ticket numbers refer to Safety Dance tickets, not the crates' tickets.

@mleonhard wrote—

When crate maintainers delete tickets that ask for more safety, include a link to a ticket in a different repository. The isaacs/github repository is an example of this technique.

If, say, a crate is forked by people who want to make it safer,¹ someone could post this information as a comment on the crate's Safety Dance ticket. Compared to filing a pull request for a patch to a list maintained by the Secure Code Working Group of crates and their tickets, a casual comment on a ticket would both have less administrative overhead and, I suspect, especially if made by a non-member of the Working Group, have less risk of controversy for the Working Group than would linking from a Group-maintained resource to a resource that could be seen as inherently antagonistic towards the original developer.

(¹ If you do this, consider not using the "Fork" button on GitHub — if the fork is known to GitHub as a fork, the original developer can delete it by blocking you.)

[Shnatsel]

I'm hesitant to promote metrics used by cargo-geiger as the standard and give them high visibility. It is very useful to get a lay of the land, but it's not really a metric to optimize for. For example, minimizing the number of unsafe expressions in the dependency tree will incentivize hand-rolling your own unsafe code instead of reusing an existing well-vetted crate, if that crate has other functionality you don't necessarily need.

[anderejd]

I think this is a fine idea, in the same vein as publishing test coverage metrics for a library. It doesn't really guarantee anything about the quality of the code, but it's a rough indication at least. How to interpret and act on metrics about unsafe usage, test coverage, number of code lines etc. is up to each developer or developer team.

But I also see the point of not promoting the cargo geiger metrics as some sort of golden standard, it's not perfect, even if we manage to fix all the bugs and plug the holes that libraries can use to affect the scanning results. Perhaps this should be clarified further in the readme?

[anderejd]

For each unsafe crate, link to a ticket about making the crate safer. So folks can easily express their desire for more safety in code and dependencies. For crates without such tickets, link to a page explaining how to file the ticket and add the link to cargo-geiger (make a PR). When crate maintainers delete tickets that ask for more safety, include a link to a ticket in a different repository. The isaacs/github repository is an example of this technique.

@mleonhard I'm not sure how this could be done automatically, seems better suited for humans to deal with. History seems to suggest that a good pinch of politeness and kindness is needed when approaching project maintainers when suggesting safety improvements, so I would not be comfortable coding that into this tool and causing some massive GitHub issue spam problem 😱

[8573]

For each unsafe crate, link to a ticket about making the crate safer. So folks can easily express their desire for more safety in code and dependencies. For crates without such tickets, link to a page explaining how to file the ticket and add the link to cargo-geiger (make a PR). […]

@mleonhard I'm not sure how this could be done automatically, […]

I don't think OP meant for the tickets to be opened automatically but rather would have a person "file the ticket and add the link to cargo-geiger (make a PR)".

History seems to suggest that a good pinch of politeness and kindness is needed when approaching project maintainers when suggesting safety improvements, […]

This is one reason I mildly might suggest primarily using Safety Dance tickets rather than primarily using upstream tickets — less risk of appearing to encourage potentially impolite people to hound upstream developers who use unsafe.

[8573]

@Shnatsel wrote—

I'm hesitant to promote metrics used by cargo-geiger as the standard and give them high visibility. It is very useful to get a lay of the land, but it's not really a metric to optimize for. For example, minimizing the number of unsafe expressions in the dependency tree will incentivize hand-rolling your own unsafe code instead of reusing an existing well-vetted crate, if that crate has other functionality you don't necessarily need.

@anderejd wrote—

But I also see the point of not promoting the cargo geiger metrics as some sort of golden standard, […]

Would the mis-incentivization problem be better (or worse?) if a report such as @mleonhard proposes were to include only qualitative information about dependencies — no optimizable metric but rather “this one is 100% safe, this one has some unsafe but has been marked as ‘well-vetted’, this one has unsafe and isn't vetted”?

[szunami]

I think one major value add of this kind of work is making it clear to library consumers what they are getting when they add a dependency. Obviously this is tricky if your library is deep down the dependency chain, but I think having libraries communicate this stuff could be really cool & helpful.

With this in mind, I think a coarse github badge with a % of code which is unsafe could be a neat thing to come out of this work, similar to code coverage.

[anderejd]

This is one reason I mildly might suggest primarily using Safety Dance tickets rather than primarily using upstream tickets — less risk of appearing to encourage potentially impolite people to hound upstream developers who use unsafe.

@8573 Would perhaps a short section in the readme with a link to the Safety Dance repository be a good idea?

[anderejd]

Would the mis-incentivization problem be better (or worse?) if a report such as @mleonhard proposes were to include only qualitative information about dependencies — no optimizable metric but rather “this one is 100% safe, this one has some unsafe but has been marked as ‘well-vetted’, this one has unsafe and isn't vetted”?

@8573 This sounds like https://crates.io/crates/cargo-crev in combination with https://crates.io/crates/cargo-audit.

As for me, I'm happy with keeping cargo-geiger focused on scanning code, gathering and presenting metrics.

[anderejd]

For example, minimizing the number of unsafe expressions in the dependency tree will incentivize hand-rolling your own unsafe code instead of reusing an existing well-vetted crate, if that crate has other functionality you don't necessarily need.

@Shnatsel Interesting point of view. If the task as hand needs to be written using unsafe code, interacting with system libraries for example, for me, I mostly prefer to use unsafe code written by other authors since I find the documentation regarding writing correct unsafe Rust to be lacking and seemingly incomplete. This could be lack of skill and courage on my part, of course.

EDIT: In short, I disagree. Especially since the output from cargo-geiger includes the top crate being scanned.

EDIT 2: But I see that a big library with a large amount unsafe code would be reported by cargo-geiger as a big source of unsafe code. I'm not sure that's a bad thing? A large pile of unsafe code, is a large pile of unsafe code...? Is it a bad thing to make that fact visible to users?

[anderejd]

This feature request is fixed by: #156 #159 #162 #163 #164.

Let's open new issues for related tweaks, similar new features or related discussions.

[mleonhard]

@anderejd Thanks so much for implementing this feature. :)

I want to try it out, but I'm having trouble building cargo-geiger from HEAD with rustc 1.49 and nightly. Do you have any idea when the next release will happen?

[anderejd]

This was all @jmcconnell26 🥇

The next release should be soonish, in a couple of days best case.