Repository navigation
Conversation
The s3s-fs binary took its credentials only as --access-key and --secret-key, which puts the secret in the process argument list where any local user can read it. Both flags now fall back to S3S_FS_ACCESS_KEY and S3S_FS_SECRET_KEY, and --help does not print their values. The flags still win when given, and giving only one of the pair is still an error. Signed-off-by: Panagiotis Moustafellos <pmoust@nofire.ai>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrow configuration changes preserve existing credential behavior, with no blocking issues identified.
No open issues 🎉
What changed in this PR
Adds environment-variable credentials to s3s-fs, allowing users to avoid exposing secrets in process arguments.
Changes:
- Adds
S3S_FS_ACCESS_KEYandS3S_FS_SECRET_KEYfallbacks while preserving command-line precedence and paired-credential validation. - Hides environment values from help output and enables clap’s
envfeature.
| File | Description |
|---|---|
crates/s3s-fs/src/main.rs |
Configures credential environment variables and hides their values in help. |
crates/s3s-fs/Cargo.toml |
Enables environment support on the optional clap dependency. |
Review effort: Balanced
pmoust
added a commit
to pmoust/ClickBench
that referenced
this pull request
Oct 26, 2026
Ravel reads cold statements as ranged GETs. On a single drive RustFS serves each ranged GET by reading the whole 1 MiB erasure block around it, about 1.5 MB of disk per 10 KB range measured on c6a.4xlarge, so the cold figures measured the store's read amplification more than the database. s3s-fs serves a range with one file read. Both are single-node S3 servers over the machine's disk; the entry stays a local-disk entry with nothing required from the operator. ./install downloads the s3s-fs binary for the host architecture from a release on the NOFireAI/s3s fork (s3s-fs-0.17.0-ravel.1: the v0.17.0 release plus listing ETags, s3s-project/s3s#1013, which Ravel's store qualification requires, and keys read from the environment, s3s-project/s3s#1014, so no key sits in a process argument list) and verifies it against that release's SHA256SUMS before moving it into place, the way Ravel's own binaries are installed. When the release is unreachable it builds the same pinned commit with cargo install --locked, so what runs is fixed by content either way; the commit installed is recorded beside the binary. RustFS is gone from the entry: nothing downloads it. rustfs-start becomes store-start, the store directory is store/ and the generated keys are STORE_ACCESS_KEY and STORE_SECRET_KEY. The README gains a section on the store that states the swap and the measured reason for it.
pmoust
added a commit
to pmoust/ClickBench
that referenced
this pull request
Oct 26, 2026
Ravel reads cold statements as ranged GETs. On a single drive RustFS serves each ranged GET by reading the whole 1 MiB erasure block around it, about 1.5 MB of disk per 10 KB range measured on c6a.4xlarge, so the cold figures measured the store's read amplification more than the database. s3s-fs serves a range with one file read. Both are single-node S3 servers over the machine's disk; the entry stays a local-disk entry with nothing required from the operator. ./install downloads the s3s-fs binary for the host architecture from a release on the NOFireAI/s3s fork (s3s-fs-0.17.0-ravel.1: the v0.17.0 release plus listing ETags, s3s-project/s3s#1013, which Ravel's store qualification requires, and keys read from the environment, s3s-project/s3s#1014, so no key sits in a process argument list) and verifies it against that release's SHA256SUMS before moving it into place, the way Ravel's own binaries are installed. When the release is unreachable it builds the same pinned commit with cargo install --locked, so what runs is fixed by content either way; the commit installed is recorded beside the binary. RustFS is gone from the entry: nothing downloads it. rustfs-start becomes store-start, the store directory is store/ and the generated keys are STORE_ACCESS_KEY and STORE_SECRET_KEY. The README gains a section on the store that states the swap and the measured reason for it.
This branch has not been deployed
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
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.
The
s3s-fsbinary takes its credentials only as--access-keyand--secret-key. That puts the secret key in the process argument list, which any local user can read throughpsor/proc.With this change, both flags fall back to the environment variables
S3S_FS_ACCESS_KEYandS3S_FS_SECRET_KEY.hide_env_valueskeeps their values out of--help. Nothing else about the flags changes:The only build change is enabling clap's
envfeature on the crate's optionalclapdependency, which adds no crates to the lock file.This is separate from #1013, the listing fixes, so each can be reviewed on its own.