Fixes broken cargo audit ci #4589
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4589 +/- ##
=======================================
Coverage 80.69% 80.69%
=======================================
Files 48 48
Lines 5217 5217
=======================================
Hits 4210 4210
Misses 1007 1007 Continue to review full report at Codecov.
|
| # `localtime_r` could segfault, our code base doesn't trigger this, there is a PR on chrono that should | ||
| # fix this: https://github.com/chronotope/chrono/pull/578 | ||
| # note that both the Nimbus-SDK and glean use chrono, so if we would like to move away from it, both projects | ||
| # need to do that before we can remove the ignores (assuming `chrono` doesn't release a fixed version) |
There was a problem hiding this comment.
This is a bit unfortunate :( But yeah, this seems a pragmatic solution. Should we get an issue on file to resolve this after some reasonable time in the future? (which might mean dropping or upgrading chrono depending on the resolution) eg, if I'm reading time-rs/time#293 (comment) correctly, time-rs isn't impacted in practice as that function will return an Err?
There was a problem hiding this comment.
(well, maybe isn't impacted on the latest version? Or only on main? Blah)
There was a problem hiding this comment.
Yeah, I think using time directly will be OK, because we can use 0.3 and that isn't impacted according to the advisory... on Nimbus I could see us doing that, the use-case we have is simple enough to replicate using time (famous last words 😅) But if chrono releases a fixed version that may be the best way for both us and glean, I filed #4590
There was a problem hiding this comment.
Maintainer of time here. Time 0.2.23 and up (including all of 0.3) is patched. This just amounts to returning the Err value on the affected operating systems while still doing the "useful" thing on Windows, wasm, etc.
Fixes #4583
Our dependency CI keeps failing with
cargo audit, look at #4583 for more informationThe fix here isn't the prettiest, we should avoid ignoring advisories whenever we can... I'm opting to do that here, because the check is becoming noisy and we don't invoke the risky API. (at least not in the Nimbus SDK) and as far as I can tell glean has been using that version of chrono for a while after the advisory was released.
A better fix, which I'll file a backlog issue for, is to use the
timedependency directly instead, which is more maintained and has a fixed version (and should be able to satisfy all the use cases we need, at least innimbus)