[piscisaureus]

• vnotify works on an aws_sdk_s3::Client its caller hands it --
  Watcher::new takes one, and nothing here builds a client or loads
  SDK config. It needs types and operations, nothing else. Taking
  aws-sdk-s3's default features therefore picks a TLS and HTTP stack
  on behalf of every consumer.
• Those defaults include rustls, which chains to
  aws-smithy-runtime/tls-rustls -> aws-smithy-http-client's
  legacy-rustls-ring + connector-hyper-0-14-x, pulling hyper 0.14,
  hyper-rustls 0.24, rustls 0.21 and rustls-webpki 0.101. Cargo unions
  features across a graph, so a consumer that has moved to rustls 0.23
  cannot opt out again -- it compiles both stacks and keeps the
  rustls-webpki advisory GHSA-82j2-j2ch-gfr8 that 0.101 carries. This is
  currently blocking exactly that in denoland/lscache.
• The defaults also drag default-https-client, and with it aws-lc-sys
  and a cmake build dependency nothing here uses.
• tokio gains rt because JoinSet lives behind it. That was always
  required; it merely happened to be satisfied because aws-sdk-s3's
  default rt-tokio turned on tokio/rt transitively, so the manifest
  never had to admit it. Narrowing the SDK features removes that
  accident -- the build fails without this -- and declaring it is right
  regardless.
• Net effect: the dependency tree now contains no rustls, no hyper and
  no aws-lc at all, and Cargo.lock loses 472 lines. cargo check --all-targets is clean.

[avocet-bot]

Review: #1 — "fix: stop selecting the caller's TLS and runtime stack"

Reviewed head SHA: b7017f0b65952d0588fbbccee3e6379be01d9063 (verified as the live PR head immediately before posting)
Verdict: ✅ Approve — clean, correct dependency-hygiene fix. No blocking or non-blocking defects found.

Review skill evidence

This review was produced via the Claude Code pr-review-toolkit:code-reviewer subagent (invoked through the Task tool), which cloned the repo at the PR head, read src/lib.rs, ran cargo tree, and compiled the crate. I then independently re-verified its key claims against the cloned source and lockfile (grep for client/config/TLS construction, ByteStream method usage, tokio feature usage, and vulnerable-crate presence) and re-checked the live head SHA before posting.

---

Background: what this crate is and what the PR changes

vnotify is a small Rust library that provides a cache/coordination layer on top of an Amazon S3 bucket. The important architectural fact for this review is that vnotify does not own an AWS client: the caller constructs an aws_sdk_s3::Client (with whatever credentials, region, HTTP connector, and TLS stack the caller wants) and hands it to vnotify — VnotifyCache::new(client: aws_sdk_s3::Client, config: Config) at src/lib.rs:104. Everything vnotify does with S3 is calling operations on that injected client (get_object, put_object, head_object, list_objects_v2).

In the Rust/Cargo ecosystem, a crate's Cargo features are unified across the whole dependency graph: if vnotify enables aws-sdk-s3's default features, every downstream consumer inherits them and — critically — cannot turn them back off, because Cargo takes the union of all requested features. The aws-sdk-s3 default feature set drags in a concrete TLS + HTTP runtime stack (rustls → aws-smithy-runtime/tls-rustls → legacy-rustls-ring + connector-hyper-0-14-x, i.e. hyper 0.14, hyper-rustls 0.24, rustls 0.21, rustls-webpki 0.101), plus default-https-client (aws-lc-sys + a cmake build dep). Since vnotify only needs S3 types and operations, forcing that stack on all consumers is both unnecessary bloat and an active problem: rustls-webpki 0.101 carries advisory GHSA-82j2-j2ch-gfr8, and a consumer that has already migrated to rustls 0.23 gets both stacks compiled and cannot escape the vulnerable one. The PR description notes this is currently blocking exactly that in denoland/lscache.

The fix is two lines in Cargo.toml:

1. aws-sdk-s3 = "1" → aws-sdk-s3 = { version = "1", default-features = false } — stops vnotify from selecting a TLS/HTTP stack on its dependents' behalf.
2. tokio features ["sync", "time"] → ["sync", "time", "rt"] — makes an always-required feature explicit (see below).

Cargo.lock drops ~472 lines as a consequence (rustls, hyper 0.14, hyper-rustls, tokio-rustls, ring, aws-lc, rustls-webpki, h2, mio, socket2, tokio-util, and the p256/ecdsa/elliptic-curve sigv4a crypto crates).

---

Why this is safe (verified, not assumed)

1. vnotify never builds a client, loads SDK config, or configures a connector — so nothing it uses at compile time depends on the dropped features. A grep over src/ for aws_config, from_env, SdkConfig, Client::from_conf, HttpClient, .tls, rustls returns nothing. The client arrives fully-configured from the caller (src/lib.rs:25, :104, :265, :310). The HTTP client, TLS backend, and async sleep implementation are therefore runtime concerns owned entirely by the caller, not compile-time requirements of vnotify. The dropped default-https-client/rt-tokio features are exactly those runtime concerns.

2. Object-body I/O uses only the surviving ByteStream code path. Writes use ByteStream::from(body) where body: Bytes (src/lib.rs:214/243/251); reads use res.body.collect().await followed by .into_bytes() (src/lib.rs:277-278). It never calls .into_async_read(), which is the one ByteStream method that requires the aws-smithy-types rt-tokio/tokio/tokio-util features that this PR removes. .collect()/From<Bytes> are backed by the byte-stream-poll-next feature, which survives. This was confirmed to compile.

3. The tokio rt addition is genuinely required and sufficient. vnotify uses tokio::task::JoinSet (src/lib.rs:17, :23, :122-123, :324), which lives behind tokio's rt feature. Previously this compiled only by accident — aws-sdk-s3's default rt-tokio transitively enabled tokio/rt, so the manifest never had to declare it. Narrowing the SDK features removes that accident, so declaring rt explicitly is both necessary (the build fails without it) and correct. The other tokio uses are covered by features that are kept: oneshot/mpsc need sync (src/lib.rs:27/121/156), sleep/timeout need time (src/lib.rs:330/339/480). There is no use of tokio::net, tokio::io, or tokio::fs, so dropping tokio's networking/IO features (mio, socket2) breaks nothing. vnotify does its own deadlines with tokio::time::timeout rather than relying on the SDK's default timer.

4. It actually compiles with the narrowed, locked feature set. On the repo-pinned toolchain (1.84.0 via rust-toolchain.toml): cargo check --locked --all-targets, cargo build --locked, and cargo test --locked --no-run all finished with exit 0. --locked succeeding also proves Cargo.lock is internally consistent with the new Cargo.toml (no lock drift for CI to reject). Resolved aws-sdk-s3 v1.71.0, aws-smithy-types v1.2.12.

5. The security win is real, not cosmetic. A grep over the head Cargo.lock for rustls, hyper, hyper-rustls, aws-lc, ring, webpki, tokio-rustls returns nothing, and cargo tree --locked on the compiled graph confirms none are present. So GHSA-82j2-j2ch-gfr8 (rustls-webpki 0.101) is removed from vnotify's dependency tree, not merely absent from a stale lockfile.

---

Design note (not a defect)

Building vnotify standalone now yields a crate with no HTTP client and no default async-sleep implementation. That is correct and intended: vnotify never issues requests on a client it built itself. In a real consumer binary, the application depends on aws-config/aws-sdk-s3 with an HTTP client and sleep impl, and Cargo feature unification restores those in the final artifact. The point of the fix is that vnotify stops forcing a specific TLS/runtime stack on its dependents while remaining fully functional with a caller-provided client.

Tests

No test changes are needed. The change is a build-configuration narrowing whose correctness is a compile-time/dependency-graph property; that property is verified by cargo check/build --locked succeeding and by cargo tree showing the intended graph. There is no new runtime behavior to cover.

---

Recommendation: approve and merge.