fedimint / fedimint

Federated E-Cash Mint
https://fedimint.org/
MIT License
536 stars 209 forks source link

daily dependency audit #887

Closed NicolaLS closed 1 year ago

NicolaLS commented 1 year ago

cargo audit is very usefull (install it with cargo install cargo-audit) we should also add it to the CI on push (if .toml) changes but now fedimint is not regarded usable anyway so this would just slow down development

dpc commented 1 year ago

Too late. https://github.com/fedimint/fedimint/blob/a05542ad84aae617d638f12099213c253e42f6c1/.github/workflows/ci-nix.yml#L59

dpc commented 1 year ago

Though from time to time we should run nix flake lock --update-input advisory-db to update the locked advisory db.

NicolaLS commented 1 year ago

Though from time to time we should run nix flake lock --update-input advisory-db to update the locked advisory db.

oh ok did not see that

NicolaLS commented 1 year ago

Though from time to time we should run nix flake lock --update-input advisory-db to update the locked advisory db.

can we automate this ? also I don't think we should fail CI for someone opening a PR because a vulnerability was found in an older dep..maybe it would make sense to remove it from nix and put it in daily with actions-rs/audit-check@v1..?

dpc commented 1 year ago

can we automate this ? also I don't think we should fail CI for someone opening a PR because a vulnerability was found in an older dep

Seems valuable to me, but we can deal with it if it ever happens.

dpc commented 1 year ago

can we automate this

Would be great if we could teach dependabot to do it for us. :)

dpc commented 1 year ago

For now: https://github.com/fedimint/fedimint/pull/890

NicolaLS commented 1 year ago

if we don't update the advisory-db every time CI runs it does not make sense to have it there. If we do CI will block development just because the audit failed even if its unrelated to the PRs (example) Running this daily makes us aware of problems but doesn't block anything

codecov-commenter commented 1 year ago

Codecov Report

Base: 60.30% // Head: 60.23% // Decreases project coverage by -0.07% :warning:

Coverage data is based on head (15d33a2) compared to base (a05542a). Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #887 +/- ## ========================================== - Coverage 60.30% 60.23% -0.08% ========================================== Files 110 110 Lines 16995 17028 +33 ========================================== + Hits 10249 10256 +7 - Misses 6746 6772 +26 ``` | [Impacted Files](https://codecov.io/gh/fedimint/fedimint/pull/887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint) | Coverage Δ | | |---|---|---| | [gateway/cli/src/main.rs](https://codecov.io/gh/fedimint/fedimint/pull/887/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint#diff-Z2F0ZXdheS9jbGkvc3JjL21haW4ucnM=) | `0.69% <0.00%> (-0.14%)` | :arrow_down: | | [fedimintd/src/bin/configgen.rs](https://codecov.io/gh/fedimint/fedimint/pull/887/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint#diff-ZmVkaW1pbnRkL3NyYy9iaW4vY29uZmlnZ2VuLnJz) | `1.61% <0.00%> (-0.09%)` | :arrow_down: | | [fedimint-server/src/consensus/mod.rs](https://codecov.io/gh/fedimint/fedimint/pull/887/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint#diff-ZmVkaW1pbnQtc2VydmVyL3NyYy9jb25zZW5zdXMvbW9kLnJz) | `90.67% <0.00%> (+0.02%)` | :arrow_up: | | [fedimint-api/src/core.rs](https://codecov.io/gh/fedimint/fedimint/pull/887/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint#diff-ZmVkaW1pbnQtYXBpL3NyYy9jb3JlLnJz) | `72.52% <0.00%> (+0.07%)` | :arrow_up: | | [modules/fedimint-mint/src/lib.rs](https://codecov.io/gh/fedimint/fedimint/pull/887/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint#diff-bW9kdWxlcy9mZWRpbWludC1taW50L3NyYy9saWIucnM=) | `91.23% <0.00%> (+0.07%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fedimint)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

dpc commented 1 year ago

I don't know...

The problem with daily is that people don't pay attention to it. It's been failing for a while now.

Also, with latest advisory db the check is passing failing and I'm waiting for https://github.com/djc/askama/issues/738 to fix it.

NicolaLS commented 1 year ago

eck is passing and I'm wait

why is the check passing ? it should fail until they release a new version and we bump or no ?

dpc commented 1 year ago

why is the check passing ?

Sorry, I didn't had my morning coffee. The check is failing, and we need a new release of this dependency to have a fix.

elsirion commented 1 year ago

I'm actually seeing daily failures in my e-mail, it just wasn't highest priority so far to fix. I think it would still be nice to have a check in daily CI that tells you what could use some attention.