Stream: git-wasmtime

Topic: wasmtime / PR #13894 wasi: do not panic on extreme file t...

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:24):

SebTardif opened PR #13894 from SebTardif:fix/datetime-from-no-unwrap to bytecodealliance:main:

Summary

DescriptorStat::new converted SystemTime to WASI Datetime with Datetime::try_from(t).unwrap(). When a timestamp falls outside the representable i64 second range, conversion returns Err and the host panics while building file stats.

This change maps conversion failure to None (missing timestamp), which matches the existing Option<Datetime> fields on DescriptorStat and the prior "metadata access failed" path (now combined via ok().and_then(...)).

Failure scenario

  1. Guest has write access to a preopened directory.
  2. Guest sets extreme atime/mtime via set_times / path_filestat_set_times (or the host FS already has extreme mtimes).
  3. Guest (or host code) stats the path.
  4. Host panics in datetime_from instead of returning a stat with missing timestamp fields.

Bug origin

The unwrap path was introduced with the cap-primitives migration in #13872 (Alex Crichton, 2026-07), including a FIXME that conversion should be infallible or handle errors properly.

Test

cargo test -p wasmtime-wasi --lib datetime_from_tests

Unit test constructs a far-past SystemTime that fails Datetime::try_from and asserts .ok() is None (the old .unwrap() would panic on the same value).

Related

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:24):

SebTardif requested pchickey for a review on PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:24):

SebTardif requested wasmtime-wasi-reviewers for a review on PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:34):

SebTardif updated PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:44):

:memo: alexcrichton submitted PR review:

Thanks for the PR, but please be sure you review our AI tool usage policy. You own the code and you own the responsibility for being correct here, and for example the analysis of when this was introduced was wrong. The PR referred there was a refactor and did not introduce this.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:44):

:speech_balloon: alexcrichton created PR review comment:

Given how small this is now, could this get inlined directly?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:44):

:speech_balloon: alexcrichton created PR review comment:

Conventionally these tests are in mod test as opposed to something more specific. Additionally we prefer to lean more heavily on integration tests exercise far more of the stack and ensure more complete fixes. Would it be possible to to model this as a test in crates/test-programs/src/bin/*.rs?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 15:44):

:speech_balloon: alexcrichton created PR review comment:

Please go through the output of your AI and consider rewording/removing comments like this. Comments like this about the state of a previous version of the tree are basically irrelevant noise as soon as the patch lands. The test here is self-explanatory and needs no comments.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:05):

github-actions[bot] added the label wasi on PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

SebTardif updated PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

SebTardif edited PR #13894:

Summary

DescriptorStat::new converted host SystemTime values to WASI Datetime with unwrap(). When conversion fails (time outside the representable i64 second range), that panics the host while building file stats.

Map conversion failure to None (missing timestamp) on the existing Option<Datetime> fields.

Failure scenario

Host Metadata can surface SystemTime values that do not fit WASI Datetime (for example extreme far-past times representable as SystemTime but not as i64 seconds). A subsequent guest or host stat path that builds DescriptorStat would panic instead of returning stats with missing timestamp fields.

Guest set_times is not the reproducer for the overflow itself: those APIs already take Datetime values within range. The panic is on host-side conversion of SystemTime into Datetime.

Origin

The previous description incorrectly attributed introduction to the cap-primitives migration (#13872). That PR refactored the conversion site; the unwrap-style failure mode is not claimed as newly introduced there. This change is a small hardening of the conversion.

Test

Unit test under crates/wasi/src/filesystem.rs (mod test) asserts that extreme SystemTime values fail Datetime::try_from and map to None without panicking.

Integration coverage of the surrounding set_times/stat stack already exists as p1_fd_filestat_set in crates/test-programs / crates/wasi/tests. A guest program cannot construct the overflowing SystemTime via the public WASI timestamp APIs, so the edge case stays as a unit test on the conversion.

cargo test -p wasmtime-wasi --lib test::datetime_try_from

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

:speech_balloon: SebTardif created PR review comment:

Inlined the Datetime::try_from(...).ok() mapping at the three call sites.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

:speech_balloon: SebTardif created PR review comment:

Renamed to mod test and dropped the narrative comments.

On modeling this in crates/test-programs/src/bin/*.rs: guest set_times / filestat APIs already take WASI Datetime (i64 seconds), so a guest cannot construct a SystemTime outside that range through the public ABI. The panic path is host Metadata -> Datetime conversion. Existing integration coverage for the surrounding stack is p1_fd_filestat_set. The remaining unit test only covers the overflow mapping that guests cannot reach.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

:speech_balloon: SebTardif created PR review comment:

Removed. Left a comment-free unit test that only asserts the conversion edge.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:23):

SebTardif commented on PR #13894:

@alexcrichton

please be sure you review our AI tool usage policy... the analysis of when this was introduced was wrong. The PR referred there was a refactor and did not introduce this.

Thanks for the review. I re-read the AI tool use policy and corrected the PR description: #13872 was a refactor of the conversion site, not the introduction of this failure mode. Sorry for the incorrect origin claim.

Addressed the inline notes in the latest commit: inlined the conversion, mod test without narrative comments, and explained why a guest test-program cannot reproduce the host SystemTime overflow (WASI timestamps are already Datetime). Happy to adjust further if you want the unit test dropped entirely and only rely on existing p1_fd_filestat_set coverage.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:49):

:memo: alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 17 2026 at 17:49):

:speech_balloon: alexcrichton created PR review comment:

Yes this can't be driven by purely guest-defined behavior, but it should be possible to prepare a filesystem on the host where there's a file with an extremely old timestamp in it which the guest attempts to stat and read. This can then assert that there's no panic and additionally the behavior of the timestamp basically just disappearing.

The test here is additionally overly-verbose and AI-generated where it checks is_err() and ok().is_none(). Please review all code generated by an AI, including tests.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 18 2026 at 11:52):

SebTardif requested wasmtime-core-reviewers for a review on PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 18 2026 at 11:52):

SebTardif updated PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 18 2026 at 11:53):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 18 2026 at 11:53):

:speech_balloon: SebTardif created PR review comment:

Done. Host prepares a file with an extreme mtime before the guest runs; the guest does path_filestat_get and reads the contents (p1_stat_extreme_host_mtime under crates/test-programs, wired through the p1 and p2 runners).

On macOS the OS clamps that extreme time into a far-past value that still fits i64 seconds, so Datetime::try_from succeeds. The remaining failure mode was converting that negative time into wasi:clocks/wall-clock (u64 seconds), which used to make the whole stat return OVERFLOW. Unrepresentable wall-clock times are now treated as missing timestamps so filestat succeeds and the file remains readable.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 17:26):

:memo: alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 17:26):

:speech_balloon: alexcrichton created PR review comment:

Can you redefine the above function in terms of this one? Otherwise this looks like duplicated functionality.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 17:26):

:speech_balloon: alexcrichton created PR review comment:

It's ok to drop this custom test module now that it's integration-tested with the additional in-wasm test

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 17:26):

:speech_balloon: alexcrichton created PR review comment:

Can the above function be implemented in terms of this one instead of duplicating it?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 17:26):

:speech_balloon: alexcrichton created PR review comment:

I might recommend extracting this to a helper shared function in store.rs to avoid the need to duplicate this across p1/p2-sync/p2-async

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 17:26):

:speech_balloon: alexcrichton created PR review comment:

Can the above function be deduplicated with this one?

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:speech_balloon: SebTardif created PR review comment:

@alexcrichton

Can you redefine the above function in terms of this one?

Done. run is now run_with_workspace_setup(path, |_| Ok(()), with_builder).

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

SebTardif updated PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:speech_balloon: SebTardif created PR review comment:

@alexcrichton

Can the above function be implemented in terms of this one?

Done for p2-sync as well: run delegates to run_with_workspace_setup with a no-op setup.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:speech_balloon: SebTardif created PR review comment:

@alexcrichton

Can the above function be deduplicated with this one?

Done for p2-async the same way.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:speech_balloon: SebTardif created PR review comment:

@alexcrichton

It's ok to drop this custom test module now that it's integration-tested

Removed the #[cfg(test)] mod test block from filesystem.rs.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:memo: SebTardif submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 18:39):

:speech_balloon: SebTardif created PR review comment:

@alexcrichton

extract this to a helper shared function in store.rs

Added store::prepare_extreme_mtime_fixture and wired p1 / p2-sync / p2-async p1_stat_extreme_host_mtime through it.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 19:08):

:thumbs_up: alexcrichton submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 19:08):

alexcrichton added PR #13894 wasi: do not panic on extreme file timestamps in DescriptorStat to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 19:32):

github-merge-queue[bot] removed PR #13894 wasi: do not panic on extreme file timestamps in DescriptorStat from the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 21:52):

SebTardif updated PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 21:52):

SebTardif commented on PR #13894:

Pushed a MinGW fix for the merge-queue failure on Test MinGW x86_64 (wasmtime-wasi).

The extreme-mtime fixture used SystemTime::UNIX_EPOCH - (i64::MAX+1), which is constructible on Unix but not on Windows FILETIME-based SystemTime, so setup panicked before the guest ran. The fixture now prefers that far-past value when available and falls back to the Windows epoch (~1601) otherwise. The guest still only asserts that path_filestat_get does not panic the host.

Happy to be re-queued when convenient.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 22:02):

SebTardif updated PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 22:12):

alexcrichton has enabled auto merge for PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 22:13):

alexcrichton added PR #13894 wasi: do not panic on extreme file timestamps in DescriptorStat to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 22:38):

:check: alexcrichton merged PR #13894.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 20 2026 at 22:38):

alexcrichton removed PR #13894 wasi: do not panic on extreme file timestamps in DescriptorStat from the merge queue.

Last updated: Jul 29 2026 at 05:03 UTC