feat: add OpenDAL S3 and filesystem remotes#593
Merged
Conversation
jleni
marked this pull request as ready for review
July 25, 2026 12:42
jleni
added a commit
that referenced
this pull request
Jul 25, 2026
…ixes (#600) * fix(remote): never fail a build on remote config, restore legacy prefixes Follow-up hardening for the OpenDAL remote (#593). Blocking issues: - Remote config errors no longer fail the build. `load_remote_config` became fallible in #593 and `Config::load` propagated it with `?`, which `run_wrapper_mode` also propagates — so a bad remote killed every compiler invocation instead of costing cache hits. The reason is now recorded in `Config::remote_error`, the build continues local-only, and commands that exist to use the remote surface it via `require_remote()`. `kache status` reports MISCONFIGURED rather than "not configured". - Legacy prefixes work again. `prefix = "team/"`, `/team`, `a//b` and an empty `KACHE_S3_PREFIX` were all accepted before #593 and became hard errors. They now normalize (with a warning that objects move once); only `..`, `.` and backslashes stay errors. An empty prefix addresses the bucket root, which needs `join_remote_key` so keys do not gain a leading slash. - A present-but-empty `KACHE_S3_BUCKET` disables the remote instead of silently falling back to the file-configured bucket, which pointed jobs at a different remote than they asked for. - The S3 HTTP client sets connect and read timeouts again. Only `pool_idle_timeout` was configured, so a silent endpoint hung head/get/put forever with RetryLayer unable to fire. 3.1s matches the AWS SDK default this replaced. Transport and filesystem: - `credential_process` is re-lexed with shlex-style rules and executed directly instead of being rejoined and handed to `sh -c` / `cmd.exe /C`. reqsign splits on whitespace without stripping quotes, so the quoting must be reapplied, but a shell also expands globs, `$(...)`, `%VAR%` and treats `&`/`|`/`^` as operators — and under cmd.exe single quotes do not group at all. - `get` rejects a body shorter than its advertised length. - `list` gains a total deadline and an entry cap; the per-entry timeout only caught a stalled lister. - Filesystem writes verify the target resolves inside the configured root. OpenDAL canonicalizes only the root, so a symlink planted below it redirected writes out of the cache. Best-effort: TOCTOU remains. - Keys reject control characters, and trailing dots/spaces on filesystem remotes, which Windows strips (two keys would collide). - `atomic_write_dir` is rejected when on another filesystem (rename would EXDEV on every write) or inside the object prefix (staging files would list as cached objects). Smaller items: - `[cache.remote]` rejects unknown keys, so `bukcet = "..."` is reported. - `default-features = false` on the remaining OpenDAL crates (resolved graph unchanged; the manifest now matches the stated policy). - The JSON report carries `network.backend` so consumers can tell whether the HTTP-named metrics describe a network or a local mount. - `sync --workspace` validates the workspace before resolving credentials. * fix(remote): correct staging check, shlex on Windows, and list bounds Follow-up from a cross-model review of the previous commit. - The staging-directory check compared against the whole object root, so with an empty prefix the documented default `<path>/.kache-tmp` sits under it and every empty-prefix filesystem remote was declared unusable. Compare against the object tree kache actually lists (`<prefix>/v3`) instead. - Move the same-filesystem (EXDEV) check out of `Config::load` and into `create_filesystem_operator`. It needs real syscalls, and `Config::load` runs on the rustc-wrapper hot path where stat-ing an unavailable network mount could stall the compiler — the exact failure this series exists to prevent. - `shlex_split` treated every backslash as a POSIX escape, so a perfectly ordinary `credential_process = C:\tools\aws-creds.exe` became `C:toolsaws-creds.exe`. Backslash is a path separator on Windows. - `verify_write_containment` started at the parent, so an existing symlink AT the destination was never inspected. Start at the leaf via `symlink_metadata`. - The LIST total deadline was only checked before waiting, so a stall could overshoot it by a whole progress timeout. Wait for whichever comes first. Also bound retained key bytes: entry count alone does not bound memory. - An empty `KACHE_S3_BUCKET` now disables the remote consistently, instead of erroring only when `type = "s3"` was explicit while the file bucket was fine. - Revert `deny_unknown_fields` on `[cache.remote]`. It fails the whole `FileConfig` parse, which silently reverts every other setting (`key_salt`, `cache_dir`, ...) to its default — a cache-key shift is far worse than an ignored typo. - Extract the body-length guard into `verify_complete_body` so it is directly testable; over HTTP the transport rejects a short body first, so the previous test could not exercise it. The report field is documented as the backend configured at report time; it is not per-transfer, so a window spanning a backend switch carries one label. * docs(remote): prefix normalization and filesystem staging rules The S3 page still stated that a non-canonical prefix is rejected; it is now normalized with a warning, and an empty prefix stores at the bucket root. Document that objects move once, and that a rejected prefix costs cache hits rather than the build. The filesystem page gains the staging-directory containment rule and the shared-directory trust boundary the write-containment check defends. * fix(remote): give the credential_process child the selected profile `ProfileSelectingEnv` only redirects reqsign's own in-process environment reads. A `credential_process` helper is a separate process that inherits kache's environment, so `reqsign-command-execute-tokio` spawned it with the ambient `AWS_PROFILE` still set. A helper that shells out to AWS tooling could therefore return credentials for a different account than `profile` selected, with no indication anything was wrong. Spawn the child directly and set `AWS_PROFILE` on it when a profile is configured. The process environment is still never mutated, and with no profile configured the child keeps inheriting the ambient value. This drops the last use of `reqsign-command-execute-tokio`, so the direct dependency goes too. Also record the measured blast radius of `deny_unknown_fields` in the comment explaining why it is not used: one typo'd key resolves `key_salt` to None, which shifts the cache key and rebuilds the workspace, and leaves no reason to report. * test(remote): isolate new config tests from ambient KACHE_S3_* overrides CI runs under kache-action, which exports `KACHE_S3_PREFIX=artifacts` and `KACHE_S3_REGION`. Environment overrides win over file config, so the new prefix-normalization test read `artifacts` instead of the prefix it had just written and failed on Linux, taking twelve more config tests with it through the poisoned `config_path_lock`. Not a race: the tests validated clean locally because no `KACHE_S3_*` is set here, and `--test-threads=1` also hid the resulting cascade. Reproduced with `KACHE_S3_PREFIX=artifacts cargo test`, which fails without this change and passes with it. Add `remove_env_var_for_test` and `isolate_s3_env` (restoring on drop, unlike the existing tests that remove permanently) and use them in every new test that asserts on file config. Also stop the credential-process test from mutating `AWS_PROFILE` on the whole process. It now asserts the configured profile reaches the child and that the ambient value passes through when none is configured, without writing to the environment at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
The remote cache path was coupled directly to the AWS SDK and S3 configuration. That prevented adding other storage implementations without duplicating the upload, download, retry, listing, and validation logic.
This change introduces one backend seam and uses it for the two requested services:
Filesystem writes use a same-filesystem staging directory and atomic rename. The default staging path is
<path>/.kache-tmp.Compatibility
KACHE_S3_BUCKET-only setups still select S3KACHE_S3_*overrides remain supportedreqsign-aws-v4now provides SigV4 signingThe documented AWS SDK migration boundaries are assume-role profile chains, modern
sso-sessionprofiles, profile-based web identity, SigV4a/MRAP, and profile-only endpoint configuration. S3-compatible services should configure their endpoint explicitly.OpenDAL 0.58 does not yet expose per-operator AWS profile selection (apache/opendal#7944). Kache works around that with a released-reqsign credential chain and a per-operator environment view:
profileapplies to shared files, SSO, andcredential_process, overrides ambientAWS_PROFILE, and never mutates process-global environment. Replacing this with the native builder API after apache/opendal#7949 lands in a release requires regression checks for quotedcredential_processcommands and credential-source precedence.Other OpenDAL services are not automatically enabled: each needs its OpenDAL service feature plus Kache configuration/builder integration.
Security / release note
Kache uses the separately published OpenDAL 0.58 core, S3, filesystem, retry, and reqwest-transport crates. It does not depend on the yanked 0.58 umbrella facade or its affected auto-registration constructor (apache/opendal#7923); default features are disabled and Kache installs its custom reqwest transport explicitly.
OpenDAL 0.58 plus
reqsign-aws-v43.0.2 now resolve toquick-xml0.41.0. The temporary exceptions forRUSTSEC-2026-0194andRUSTSEC-2026-0195have therefore been removed.Validation
cargo test -q -p kache --locked— 1,308 unit tests plus all integration suites passedcargo clippy -q -p kache --all-targets --locked -- -D warningscargo +1.95 check -p kache --all-targets --lockedcargo package -p kache --allow-dirty --lockednix build .#kache --no-linkcargo fmt --all -- --checkcargo deny check bans licenses sourcesgit diff --checkAddresses #586.