Repository navigation
chore(security): update dependencies for open Dependabot alerts - #381
Conversation
samgbafa
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES
Reviewed at 4bf41de20834592fff0273ee800ad3b5842801e6. One blocking finding (1): the S3 client runs the SDK 1.x checksum defaults. Everything else is non-blocking or confirmed.
Findings
-
Medium, blocking. Checksum defaults change the wire format.
tinycloud-node-server/src/storage/s3.rs:73-86(new_client) sets neitherrequest_checksum_calculationnorresponse_checksum_validation, so aws-sdk-s3 1.129 usesWhenSupportedfor both. 0.19 sent no checksums and validated none. I pointed the ported client at a loopback S3 stub and recorded what it sent:- PutObject with an in-memory body (
MemoryStaging) addsx-amz-sdk-checksum-algorithm: CRC32andx-amz-checksum-crc32. - PutObject with
ByteStream::from_path(theTempFileSystemStagepath,s3.rs:391-396and:433-438) switches tocontent-encoding: aws-chunked,x-amz-content-sha256: STREAMING-AWS4-HMAC-SHA256-PAYLOAD-TRAILERand anx-amz-trailerchecksum. That's a different upload encoding, not just an extra header, and several S3-compatible providers and older MinIO releases reject it. - GetObject sends
x-amz-checksum-mode: ENABLED. When the stub returned a wrongx-amz-checksum-crc32,read()streamed all of the body and then failed withstreaming errorat EOF. So a mismatch aborts a response that is already partly sent; it doesn't reject the read up front.
Ruling: the proposed fix is required. Set
RequestChecksumCalculation::WhenRequiredandResponseChecksumValidation::WhenRequiredon the S3 config builder innew_client. That restores the 0.19 behaviour, because PutObject, GetObject, HeadObject, DeleteObject and ListObjectsV2 don't require checksums. The endpoint is configured by the operator, and dev/CI run against MinIO, so the port shouldn't depend on which provider is behind it. Mirroring the setting in the harness client (tests/at_rest_harness.rs:142-160) is optional but keeps it consistent. After the push, the same probe should show nox-amz-checksum-*headers, noaws-chunkedand nox-amz-checksum-mode. - PutObject with an in-memory body (
-
Low, non-blocking: alerts the description claims as fixed stay open.
rustls-webpki 0.101.7(Cargo.lock:7622) andrustls 0.21.12are still in the node graph, throughrocket_http 0.5.1andreqwest 0.11.27, which comes from ssidid-web/did-ion/did-tz. Alerts #73 and #74 (>=0.101.0, <0.103.12) and #90 (<0.103.13) cover 0.101.7, so they won't close. Please move them to "Not fixed". #90 is a panic while parsing a CRL; I found no CRL configuration in the node, core or the vendored sqlx (inference: no reachable path). -
Low, non-blocking:
BehaviorVersion::latest()(s3.rs:34, harness:142) together with thebehavior-version-latestfeatures (Cargo.toml:27-28). A future lockfile refresh can change SDK defaults without any change to the code. Consider pinningBehaviorVersion::v2026_01_12(). -
Info: timeouts and retries. 0.49 configured no timeouts ("If no timeout is specified, requests will never time out"); its retry was standard with 3 attempts. 1.x with this behaviour version has a 3.1 s connect timeout, no operation timeout, standard retry with 3 attempts, and stalled-stream protection on uploads and downloads (20 s grace). The stall detector doesn't count time when the consumer isn't polling, so slow downstream clients are fine. These bound the time a request can hang, and I'd accept them as they are. Retrying PutObject is safe because keys are content-addressed.
-
Info: credential chain unchanged.
aws_config::defaultsuses the same order as 0.49from_env: env, then profile/SSO/credential_process, web identity, ECS, IMDS.docker-compose.dstack-postgres.yaml:37-39sets staticAWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY/AWS_DEFAULT_REGION, so the env provider wins. IMDS is reached only if those are empty, as before. Optional hardening: addAWS_EC2_METADATA_DISABLED=trueto the compose (tests/s3_block_store.rs:31already does this). -
Info: region, endpoint and path style are correct. The 1.x region chain still reads
AWS_DEFAULT_REGIONafterAWS_REGION. The production compose always setsTINYCLOUD_STORAGE_BLOCKS_ENDPOINT(:36, from theS3_ENDPOINTsecret), soforce_path_style(true)always applies there. The probe confirmed requests go to/<bucket>/<key>, and the trailing/thatUri::to_stringadds is handled. SDK 0.19 had no virtual-host addressing, so this matches the old behaviour. AWS S3, MinIO and R2 all accept path style. The compose and env names are AWS-style and don't identify the provider; earlier infrastructure analysis found production is not R2 and is most likely AWS S3. A config without anendpointnow gets virtual-host style; that only matters off production. -
Info: error mapping is equivalent. HeadObject
NotFoundreturnsNone/false, GetObjectNoSuchKeyreturnsNone, andInvalidRangegoes through HeadObject toUnsatisfiable. A missingContent-Lengthbecomes 0, the same as 0.19's default. No caller matches the boxedS3StoreError::S3. S3 errors are outside the TC-1054 retryable/outcome-unknown classification, which covers database errors only (routes/mod.rs:160-190), both before and after this PR, so nothing regresses there. Multipart isn't used. The range-read path is ported as it was. -
Confirmed: TLS.
cargo tree -i aws-lc-rsand-i aws-lc-sysfind nothing, with default features,--all-features,--all-features --target alland--workspace.webpkiandring 0.16are gone.rustls 0.23.45has only theringprovider feature. The S3 client builds on an explicit ring provider (s3.rs:25-31), and Postgres TLS (the vendored sqlx_tls-rustls-ring-*) and the link proxy use ring explicitly. The root store hasn't changed: native roots before (hyper-rustls 0.22with_native_roots) and after (rustls-native-certs 0.8), and the image copiesca-certificates.crt. The egress and hook lib tests ran without a CryptoProvider panic. -
Confirmed: other crypto bumps are patch-level. The node's direct
hmac 0.12/sha2 0.10are unchanged.sha2 0.11andhmac 0.13are used only byaws-sigv4 1.6andaws-smithy-checksums.keccak 0.1.6reaches SIWE/CACAO throughsha3; the siwe, cacaos and siwe-recap tests pass (17).openssl 0.10.80is reached only through native-tls inreqwest 0.11(DID resolution). No source changes were needed outside the S3 port. -
Confirmed: deferrals.
alloy-dyn-abiis inCargo.lock:189but isn't compiled:cargo tree -e features -i alloy-dyn-abiis empty fortinycloud-nodewith default features and with--all-features. The m1/w5 lock refreshes only touch test crates.
Local verification (no Docker)
cargo build passes. Storage lib tests pass (3/3). at_rest_harness non-ignored tests pass (11). s3_block_store compiles; its one test is ignored because it needs MinIO. The siwe/cacao tests pass. The egress/hook lib tests: 79 pass, and the 2 that fail need TINYCLOUD_TEST_POSTGRES_URL, which wasn't set (environment, not this PR).
Must be green before merge (after the fix in 1)
- Docker image build (alpine/musl, ring via aws-smithy-http-client).
at-rest harness (native PostgreSQL + MinIO): the green checks, pluss3_block_store -- --ignoredagainst MinIO (PUT/GET/HEAD/DELETE/LIST, path style, SigV4 1.6 signing).- M1 real-data and W5 policy-runtime e2e jobs (
--locked). - Full workspace tests with PostgreSQL.
No CI job covers the production bucket (signing and region against the real endpoint). Run a KV put/get smoke right after the first deploy.
samgbafa
left a comment
There was a problem hiding this comment.
APPROVE (delta from 4bf41de)
Reviewed at f9dd41eacf29ba60bdae97b866dd42da37ae21f7. Finding 1 from the previous review is resolved, and findings 2 and 3 are addressed.
Scope of the delta
One commit. It touches only tinycloud-node-server/src/storage/s3.rs, tinycloud-node-server/tests/at_rest_harness.rs and tinycloud-node-server/Cargo.toml, where the behavior-version-latest features are dropped. Cargo.lock is unchanged. cargo tree -i aws-lc-rs is still empty with --all-features --target all.
Checksum behaviour (re-ran the loopback S3 stub probe)
- PutObject, in-memory body (through
persistwithMemoryStaging): sendsx-amz-content-sha256: <hex>and a plain body. Nox-amz-checksum-*and nox-amz-sdk-checksum-algorithm. - PutObject,
ByteStream::from_path: sendscontent-length: 9,x-amz-content-sha256: UNSIGNED-PAYLOADand a raw body. Noaws-chunked, noSTREAMING-*-TRAILERand nox-amz-trailer. - GetObject: no
x-amz-checksum-mode. The stub's deliberately wrongx-amz-checksum-crc32is ignored and the body reads in full (11 bytes), as with 0.19. - ListObjectsV2 and HeadObject: no checksum headers. Requests stay path-style (
/<bucket>/<key>).
WhenRequired is set on the S3 config builder after Builder::from(&sdk_config), so it overrides any value taken from shared config or environment. The harness client mirrors it.
Other items
- Behaviour version:
BehaviorVersionis pinned tov2026_01_12throughs3::behavior_version(), and the harness uses the same function. Resolves finding 3. - PR description: it now lists
rustls-webpki 0.101.7(viarocket_httpandreqwest 0.11) as not fixed, with #90, #73 and #74 still open. Resolves finding 2.
Local verification (no Docker)
cargo buildpasses.- Storage lib tests pass (3/3).
at_rest_harnessnon-ignored tests pass (11).s3_block_storecompiles; its one test is ignored because it needs MinIO.
Merge prerequisites
- CI green:
- Docker image build;
at-rest harness (native PostgreSQL + MinIO), includings3_block_store -- --ignored;- M1 real-data and W5 policy-runtime e2e jobs;
- full workspace tests with PostgreSQL.
- After the first deploy, run a KV put/get smoke against the real production bucket. No CI job covers signing, region or endpoint against it.
|
APPROVE (delta review at exact head 5634de6). The approved change set from f9dd41e is carried forward byte-identical: this delta is a merge of
Merge prerequisites stand: a fresh green CI at this head (Docker build, MinIO at-rest harness + |
Summary
Updates dependencies for open Dependabot alerts on the root
Cargo.lock, and migrates the S3 storage backend fromaws-sdk-s30.19 to 1.x (the only way to leaveaws-smithy-json0.49 andwebpki0.21.4). Toolchain pin isrust-toolchain.toml1.97.1; norust-version/MSRV is declared in the workspace.Fixed (root Cargo.lock, shipped)
aws-sdk-dynamodbwas unused and is removedThe ring 0.16 / rustls 0.19 / sct / untrusted 0.7 chain is also gone (it came from aws-sdk 0.19).
ring0.17.14 is unchanged.Code change (
tinycloud-node-server/src/storage/s3.rs,tests/at_rest_harness.rs): SDK 1.x API port. Behaviour notes:request_checksum_calculation/response_checksum_validationtoWhenRequired(SDK 1.x defaults add CRC32 trailers, aws-chunked uploads and checksum-mode GETs, which the 0.19 SDK did not send and some S3-compatible providers reject).BehaviorVersionis pinned tov2026_01_12instead oflatest().endpointnow usesendpoint_url+force_path_style(true)(the old immutable endpoint was path-style).ringprovider this leaves rustls with no unambiguous defaultCryptoProvider(the egress/hooks tests panicked). The SDK is built withdefault-features = falseand an explicitaws-smithy-http-clientonrustls-ring(s3::http_client()), so aws-lc-rs is not in the build.S3StoreError::S3is boxed (clippyresult_large_err).test/m1-realdata-e2eandtest/w5-policy-runtime-node-e2elockfiles refreshed (CI runs them--lockedand they depend on tinycloud-node);metadata --lockedpasses for both.Not fixed (root Cargo.lock)
0.101.7(andrustls 0.21.12) remain in the graph viarocket_http 0.5.1andreqwest 0.11.27(from ssi did-web/did-ion/did-tz), and those alerts cover 0.101.x. Only the 0.103 line is updated. fix: distinguish epoch-insert DB errors from missing spaces #90 is a CRL-parsing panic; no CRL configuration was found in the node, core or vendored sqlx.alloyfeature ofdependencies/siwe(alloy = "0.2"). No workspace crate enables that feature, so it is not compiled into the node. Fixing needs an alloy 0.2 → 0.8+ port of the vendored siwe crate; a patched 0.7.x does not exist.prometheus; major bump.Deferred: test-fixture only, can wait
test/m1-realdata-e2e/Cargo.lock: 34 alerts (partially refreshed here for the SDK change)test/w5-policy-runtime-node-e2e/Cargo.lock: 32 (same)test/load/signer/Cargo.lock: 22 (untouched)These are not in the node binary or Docker image.
Security-sensitive bumps (for review)
rustls 0.23.36 → 0.23.45; rustls-webpki 0.103.9 → 0.103.15; openssl 0.10.75 → 0.10.80; keccak 0.1.6; rand 0.8.6 / 0.9.3; new aws-sigv4 1.6.0 (SigV4 signing for S3) and sha2/hmac/digest 0.11/0.13 (aws-lc-free SDK graph); ring 0.16 and webpki 0.21 removed.
Verification
Local, Rust 1.97.1:
cargo fmt --check,RUSTFLAGS=-Dwarnings cargo clippy --workspace --all-targets,cargo test --workspaceagainst a local PostgreSQL 14 (no Docker access), duckdb test commands from CI, wasm32 build of sdk-wasm/verifier-wasm: pass. Not run locally: Docker build;postgres_space_filter_survives_a_collation_that_ignores_the_sentinel(needs PG16 ICU rules);blackhole_*pool-timeout tests (iptables); at-rest MinIO harness, M1/W5 e2e. CI will cover these.