diff --git a/README.md b/README.md index 42e430d..d2d0d4d 100644 --- a/README.md +++ b/README.md @@ -523,22 +523,22 @@ Discussions about the legitimacy, licensing, or ethics of AI-generated code in g ### Quality verification (by AI self-assessment) -Measurements below are taken at v1.6.0 (commit `c454694` on `main`, tagged 2026-07-20; measured 2026-07-21). The coverage figures are sourced from `llvm-cov-report.txt` (`cargo llvm-cov`; `lcov.info` is the matching machine-readable LCOV artifact) and reflect a single combined run — `cargo llvm-cov` with `RUSTFLAGS="--cfg e2e_test"` on the maintainer's machine, 2026-07-20 — so the unit tests, the process-level CLI and batch-run tests, and the live-AWS e2e suite are all included in the report. +Measurements below are taken at commit `635da2d` on `main` (v1.8.2 plus one build-config commit; measured 2026-09-12). The coverage figures are sourced from `llvm-cov-report.txt` (`cargo llvm-cov`; `lcov.info` is the matching machine-readable LCOV artifact) and reflect a single combined run — `cargo llvm-cov` with `RUSTFLAGS="--cfg e2e_test"` on the maintainer's machine, 2026-09-12 — so the unit tests, the process-level CLI and batch-run tests, and the live-AWS e2e suite are all included in the report. | Metric | Value | |--------------------------------|---------------------------------------------------------------| -| Production code | ~16,900 lines of Rust across 84 source files in `src/` | -| Unit tests (in `src/`) | 477 `#[test]` / `#[tokio::test]` annotations | -| CLI integration tests | 692 annotations across 63 files (62 `tests/cli_*.rs` files plus `tests/batch_run.rs`); they spawn the real binary with no AWS credentials — S3 interactions, where exercised, hit an in-process loopback mock server; run in CI | +| Production code | ~17,900 lines of Rust across 85 source files in `src/` | +| Unit tests (in `src/`) | 506 `#[test]` / `#[tokio::test]` annotations | +| CLI integration tests | 730 annotations across 65 files (64 `tests/cli_*.rs` files plus `tests/batch_run.rs`); they spawn the real binary with no AWS credentials — S3 interactions, where exercised, hit an in-process loopback mock server; run in CI | | E2E integration tests | 258 annotations across 28 `tests/e2e_*.rs` files (gated behind `--cfg e2e_test`; run only by the maintainer against live AWS) | -| Code coverage (llvm-cov, combined unit + CLI + e2e run) | 97.63% regions (347 / 14,629 missed), 96.94% functions (35 / 1,145 missed), 98.55% lines (151 / 10,448 missed) | +| Code coverage (llvm-cov, combined unit + CLI + e2e run) | 97.53% regions (377 / 15,256 missed), 97.08% functions (35 / 1,200 missed), 98.50% lines (163 / 10,873 missed) | | Static analysis (clippy) | 0 warnings (`cargo clippy --all-features`) | | Formatting | 0 diffs (`cargo fmt --all --check`) | | Supply chain (cargo-deny) | Clean (`cargo deny -L error check`); runs on every push and PR in `ci.yml` and daily at 01:34 UTC in `cargo-deny.yml`; `advisories.ignore = []` | -| Code adapted from the underlying projects | CLI frontends vendored from the upstream binaries — `src/sync_bin/` ([s3sync](https://github.com/nidor1998/s3sync)), `src/util_bin/` ([s3util-rs](https://github.com/nidor1998/s3util-rs)), `src/clean_bin/` ([s3rm-rs](https://github.com/nidor1998/s3rm-rs)), `src/ls_bin/` ([s3ls-rs](https://github.com/nidor1998/s3ls-rs)) — plus adapted dispatch arms in `src/dispatch.rs`; the engines themselves are consumed as exact-pinned library dependencies (s3sync 1.60.0, s3util-rs 1.8.0, s3rm-rs 1.4.0, s3ls-rs 1.1.0). `src/batch_run/` is s7cmd-original | +| Code adapted from the underlying projects | CLI frontends vendored from the upstream binaries — `src/sync_bin/` ([s3sync](https://github.com/nidor1998/s3sync)), `src/util_bin/` ([s3util-rs](https://github.com/nidor1998/s3util-rs)), `src/clean_bin/` ([s3rm-rs](https://github.com/nidor1998/s3rm-rs)), `src/ls_bin/` ([s3ls-rs](https://github.com/nidor1998/s3ls-rs)) — plus adapted dispatch arms in `src/dispatch.rs`; the engines themselves are consumed as exact-pinned library dependencies (s3sync 1.62.1, s3util-rs 1.10.2, s3rm-rs 1.6.2, s3ls-rs 1.3.2). `src/batch_run/` is s7cmd-original | What these numbers do and do not show: -- They show what the combined test run exercises — including the live-AWS e2e suite — not how the binary behaves under production load over time. CI asserts only the non-e2e build (unit, CLI, and batch-run tests) on every push and PR, across six build targets; it neither runs the e2e suite nor produces or gates on coverage. +- They show what the combined test run exercises — including the live-AWS e2e suite — not how the binary behaves under production load over time. CI asserts only the non-e2e build (unit, CLI, and batch-run tests) on every push and PR, across seven build targets; it neither runs the e2e suite nor produces or gates on coverage. - Coverage measures s7cmd's own `src/` — the wrapper layer: the clap argument surfaces, subcommand dispatch, the vendored CLI frontends, and batch-run. The transfer, listing, and bulk-delete engines are separate crates measured in their own repositories (each underlying library maintains 96%+ coverage, as noted above); their figures are not part of the numbers in this table. - Coverage is a structural metric. A covered line can still be incorrect; an uncovered line can still be correct. Use it to size the test surface, not to certify behaviour. - The e2e suite covers live-AWS paths (object, bucket-configuration, and stdio round-trips, object annotations, batch-run, dry-run, ctrl-c cancellation, exit codes) but runs only on the maintainer's machine, and reproducing the coverage figures above requires AWS credentials. @@ -550,95 +550,97 @@ The codebase is built through spec-driven development with human review at every
Click to expand the full assessment -> Assessment date: 2026-07-22. +> Assessment date: 2026-09-12. > -> Assessed version: 1.6.0 (branch `main`, commit `c454694`, tag `v1.6.0`). +> Assessor: Claude Fable 5.1 by Anthropic (model ID `claude-fable-5-1`), running at Effort: max. > -> Method and scope of evidence: this assessment was performed from scratch at v1.6.0, deriving every claim from the source rather than carrying conclusions forward from this README or any earlier assessment. All 84 Rust source files under `src/` (16,949 lines) were read in full: the s7cmd-original `src/batch_run/` engine, `src/dispatch.rs`, `src/cli.rs` and `src/main.rs` directly line by line, and the four vendored CLI frontends (`src/util_bin/`, 59 files; `src/sync_bin/`, `src/clean_bin/`, `src/ls_bin/`, 14 files) in full parallel review passes whose safety-relevant claims were re-verified against the cited lines. Also read: the 63 offline process-level test files (`tests/cli_*.rs` plus `tests/batch_run.rs`), the 28 live-AWS suites (`tests/e2e_*.rs`), `tests/common/mod.rs`, `Cargo.toml` and `Cargo.lock`, `deny.toml`, all five GitHub Actions workflows, `Dockerfile`, `build.rs`, `.cargo/config.toml`, and the coverage artifacts `llvm-cov-report.txt` / `lcov.info` (verified internally consistent: identical line and function totals). Where a guarantee lives in an engine crate, the pinned published sources (s3sync 1.60.0, s3util-rs 1.8.0, s3rm-rs 1.4.0, s3ls-rs 1.1.0) were consulted directly. The static checks were re-run during this assessment and confirmed: `cargo fmt --all --check` clean; `cargo clippy --all-features --all-targets` zero warnings, also under `RUSTFLAGS="--cfg e2e_test"` (so the e2e corpus type-checks); `cargo deny -L error check` fully clean against the advisory database of the assessment date. +> Assessed version: 1.8.2 (branch `nidor1998/docs-readme-quality-verification`, commit `407c749`). The commit is four commits past tag `v1.8.2` (`1a7c8aa`); the difference touches only `.cargo/min-publish-age.toml`, the issue templates, and this README, so the Rust sources, `Cargo.toml`, and `Cargo.lock` are identical to the tagged release. > -> Limits of the evidence: this assessment cannot rule out all bugs. It includes no fuzzing, sanitizer or Miri runs, formal verification, or penetration testing. The e2e suite runs only against the maintainer's AWS account; CI neither compiles nor runs it. Coverage measures what the tests execute, not whether the executed code is correct. The transfer, listing and bulk-delete engines are separate crates assessed in their own repositories; they are covered here only as s7cmd consumes them. +> Independence: this assessment was produced from scratch at the commit above. The other AI assessments in this README were not read and were not used as input, and nothing was carried forward from any earlier assessment. Every statement below traces to the source, the tests, the coverage artifacts, the pinned engine crates, or an experiment run against a binary built from this commit during the assessment. -**Question addressed.** s7cmd is a wrapper: 55 of its 56 subcommands hand off to four exact-pinned engine libraries, and the one s7cmd-original engine is `batch-run`. Three failure modes drive the analysis: (1) the wrapper mis-routes — a subcommand reaches the wrong engine, mistranslates its configuration, or weakens a guarantee (dry-run, exit codes, credential hygiene) that the standalone upstream binary provides; (2) `batch-run` mis-executes a script — runs the wrong line, loses a failure, kills the whole batch, or copies an inline credential into logs; (3) an operator mistake in one line of a batch mutates the wrong resource with no preview. Each is examined in turn. +#### Method and scope of evidence -#### Architecture and subcommand surface (v1.6.0) +- **Source.** All 85 Rust files under `src/` (17,889 lines) were read in full and directly, with no sampling and no delegated partial passes: `src/main.rs`, `src/cli.rs`, `src/dispatch.rs`, `src/pipe_safe.rs`, the s7cmd-original `src/batch_run/` engine (eight files), and the four vendored frontends `src/util_bin/` (59 files), `src/sync_bin/`, `src/ls_bin/`, and `src/clean_bin/`. +- **Tests.** All test files were read: `tests/common/mod.rs` (the loopback mock S3 server), `tests/batch_run.rs`, the 64 offline `tests/cli_*.rs` files, and the 28 live-AWS `tests/e2e_*.rs` files gated behind `cfg(e2e_test)`. +- **Build, supply chain, CI.** `Cargo.toml`, `build.rs`, `deny.toml`, `.cargo/config.toml`, `.cargo/min-publish-age.toml`, `Dockerfile`, `SECURITY.md`, `CLAUDE.md`, and all five GitHub Actions workflows. +- **Coverage artifacts.** `lcov.info` and `llvm-cov-report.txt` were used as supplied. Their totals agree with each other (10,873 lines, 1,200 functions), they were generated at 15:25 local time on 2026-09-12, and the only commit after that time changed this README, so they describe the assessed sources. +- **Engine crates.** Where a guarantee lives in an engine rather than in s7cmd, the pinned published sources were consulted directly: s3sync 1.62.1, s3util-rs 1.10.2, s3rm-rs 1.6.2, s3ls-rs 1.3.2 (credential `Debug` redaction, the `clean` safety prompt, tracing initialisation, and how network timeouts are configured). +- **Checks re-run during the assessment** (rustc and cargo 1.98.1, cargo-deny 0.19.4, cargo-llvm-cov 0.8.7): `cargo fmt --all --check` clean; `cargo clippy --all-features -- -D warnings` clean; `cargo clippy --all-features --all-targets -- -D warnings` under `RUSTFLAGS="--cfg e2e_test"` clean, so the live-AWS corpus compiles and lints; `cargo test` (offline suites) 1,247 passed, 0 failed, 0 ignored; `cargo deny -L error check` clean against the advisory database of the assessment date. The e2e suites were not executed (they require live AWS credentials and are reserved for the maintainer). +- **Experiments.** A debug binary built from the commit was driven against local loopback servers (one that accepts TCP connections and never answers, one that answers every request with a bodiless 404) and hand-written batch scripts to test the claims that reading alone cannot settle: credential masking with misspelled flags, Ctrl-C behaviour with and without cancellation handlers, streaming-mode exit codes, colour escapes on non-terminal stderr, `clean` without `--force` under a non-interactive stdin, and `head-bucket` against a bodiless 404. -The `Cmd` enum has 56 variants (`src/cli.rs:270-426`): 21 read-only subcommands, 34 mutating ones, and `batch-run`. Every arm of `dispatch` returns an `i32` instead of exiting (`src/dispatch.rs:23`), and `main` delivers it via `ExitCode::from` (`src/main.rs:44-45`) — there is no `std::process::exit` anywhere under `src/`; the vendoring deliberately removed upstream's process-exiting paths (`load_config_exit_if_err` for `ls`/`clean`, `validate_state_flag` for the three state-flag `put-*` commands, re-implemented as non-exiting checks called from `dispatch.rs:278,401,418`) so a bad line cannot kill a batch. Config-translation failures for `sync`/`ls`/`clean`/`cp`/`mv` print a clap-formatted error and return exit 2 (`dispatch.rs:28-140`). Large subcommand futures are `Box::pin`-ed to keep dispatch's frame below CI's 2 MB test-thread stacks (`dispatch.rs:16-22`). Routing is pinned three ways: in-process parse tests over every subcommand string (`tests/cli_routing.rs`, 62 tests), 63 dispatch unit tests in `src/dispatch.rs`, and live end-to-end dispatch tests per family. +#### Architecture: what the wrapper does and does not do -#### `batch-run` (the s7cmd-original engine) +s7cmd exposes 56 subcommands. Fifty-five hand off to one of four exact-pinned engine libraries (s3sync for `sync`, s3ls-rs for `ls`, s3rm-rs for `clean`, s3util-rs for everything else), and one, `batch-run`, is s7cmd's own. `src/cli.rs` declares the clap surface, `src/dispatch.rs` translates each parsed subcommand into the engine's configuration and calls a vendored frontend, and `src/main.rs` converts the returned code with `ExitCode::from`. Every code in use (0, 1, 2, 3, 4, 101, 130) fits in a byte, so nothing is truncated. -`batch-run` is where s7cmd adds real machinery, and it is defensively built: +The design decision that shapes the safety story is that the vendored frontends never call `process::exit`. Each file carries a provenance header naming the upstream binary version it was taken from and enumerating the adjustments (returning the exit code, a process-global "Ctrl-C received" flag, a shared signal-test lock). The consequence is that one process can execute many subcommands in sequence, which is what makes `batch-run` possible, and also what makes panic containment, secret masking, and a single tracing subscriber necessary. -- Input handling: lines are read with an incrementally enforced 16 KiB cap — a multi-GB line is rejected after one buffer, not buffered (`src/batch_run/parser.rs:101-135`) — validated as UTF-8, and tokenized with POSIX `shlex`. Blank/comment lines are skipped; a tokenization error becomes a per-line failure, not an abort. -- Per-line validation before anything runs: nested `batch-run`, stdin/stdout operands (`cp`/`mv` `-`, `put-bucket-* -`, `--annotation-payload -`), and per-line tracing flags are rejected — the tracing check inspects the parsed clap fields of all ~50 variants, not raw strings, so every accepted spelling is caught (`src/batch_run/validate.rs`). -- Invalid lines (tokenize / parse / empty / validate) synthesize exit 2, log at error level with structured fields, and count toward `--max-errors` / `--continue-on-error` exactly like runtime failures (`src/batch_run/executor.rs:74-92`) — a regression test pins that they no longer abort the whole run (`tests/batch_run.rs:194-220`). -- Panic containment: every dispatched line runs under `catch_unwind`; a panicking subcommand becomes exit 101, is logged with the line number and redacted text, and counts toward the failure threshold (`executor.rs:125-167`). This depends on `panic = "unwind"` in **all** build profiles (`Cargo.toml:103-113`) — a deliberate 1.2.3 change (release-min-size had been `panic = "abort"`, which would have defeated the recovery) and a standing invariant future profile edits must not break. -- Exit-code aggregation is severity-ranked, not numeric: 1 > 2 > 3 > 4 > any other non-zero > 0 (`executor.rs:294-303`), so a run mixing exit 1 and exit 130 exits 1. Per-line exit 130 (SIGINT) buckets as `skipped` and never trips `--max-errors` (`executor.rs:281-287`). -- SIGINT semantics are phased: during read-all's read/validate phases the default handler applies (Ctrl-C kills immediately, nothing has run); the listener is installed only before execution, after which Ctrl-C stops new spawns while in-flight commands cancel via their own handlers (`src/batch_run/mod.rs:253-331`). A live-AWS test SIGINTs a 200 MB upload mid-batch and asserts exit 130 with the remaining lines skipped (`tests/e2e_batch_run.rs:553-645`). -- `--parallel` is capped at 1024 by a custom clap parser, rejecting at parse time (exit 2) values that previously panicked `tokio::sync::Semaphore::new` (exit 101) — with the exact panic-boundary value pinned in tests (`src/cli.rs:148-170`, `tests/cli_arg_validation.rs:717-728`). Parallel mode drives the non-`Send` dispatch futures concurrently on a `LocalSet` with a semaphore bound; the engines' own worker pools provide the actual multi-core parallelism. -- Line parsing goes through the same post-processed clap tree as the top-level binary (`mod.rs:34-37`), which disarms the upstream-inherited `--auto-complete-shell` flag including its `AUTO_COMPLETE_SHELL` env binding — closing a fixed 1.6.0 bug where the exported variable silently rewrote argument handling (required args lifted, sync/clean paths defaulted) and a per-line `cp --auto-complete-shell` panicked. +#### `batch-run`, the s7cmd-original engine -#### Wrapper fidelity and operator-mistake protection +- **Parsing.** Lines are tokenised with `shlex` (shell-style quoting), so values containing spaces or shell metacharacters can be quoted per line. Blank lines and `#` comments are skipped. A 16 KiB per-line cap is enforced incrementally through `BufRead::fill_buf`, so an oversized or endless line cannot grow memory without bound. +- **Two intake modes.** In the default read-all mode the whole script is read, parsed, and validated before any line executes; a script with a syntax error or a forbidden line runs nothing. In `--streaming` mode a reader task feeds lines through a channel as they arrive; an invalid line then synthesises exit 2 and is counted against the stop policy like any other failure. +- **Validation rules** (also available without execution through `--check-format`): a nested `batch-run` is rejected; stdin/stdout transfers (`-` as a `cp`/`mv` endpoint) are rejected because the script owns stdin; reading a configuration file from `-` is rejected; per-line tracing flags are rejected because the process has one subscriber; and engine-level configuration errors (for example an invalid `sync` argument combination) are reported with the line number. +- **Execution.** Sequential by default; `--parallel N` (capped at 1,024) runs lines on a `LocalSet` with a semaphore. Each line runs inside `catch_unwind` (`panic = "unwind"` is set for every profile in `Cargo.toml`), so a panic inside an engine becomes a per-line exit 101 with an error-level log entry carrying the line number and the masked line text, not a dead batch. +- **Stop policy and exit code.** The default stops at the first failure; `--max-errors N` allows N failures; `--continue-on-error` never stops; `--continue-on-warning` lets exit 3 (warning) and exit 4 (not found) pass. Lines that return 130 (a Ctrl-C'd `sync`/`ls`/`clean`/`cp`/`mv`) are bucketed as skipped and never trip a threshold. The batch exit code is the worst code seen, ranked 1 > 2 > 3 > 4 > any other non-zero > 0 (`severity_rank` and `worse_of` in `src/batch_run/executor.rs`), so a run mixing exit 1 and exit 130 exits 1. The ranking is unit-tested at its edges. +- **Ctrl-C.** `batch-run` installs its own tokio `ctrl_c` handler that sets an interrupt flag; no further line starts after it, and the summary counts the rest as skipped. Only lines whose subcommand has a cancellation handler are actually interrupted (see finding 2 for what this means for single-request lines). +- **Logging.** One tracing subscriber is installed for the process, with a filter covering s7cmd and all four engine crates; `--json-tracing` switches to structured output; the summary line reports succeeded/failed/warnings/skipped counts; every place that logs a script line passes it through `redact_secrets` first. -- `--dry-run` exists on all 34 mutating subcommands and none of the 21 read-only ones — both directions asserted by tests (`tests/cli_dry_run.rs`, 67 tests, including that read-only helps do **not** expose the flag). Wrapper-level short-circuits return before the mutating call in all 30 thin wrappers; `cp`/`mv`/`sync`/`clean` enforce dry-run in their engines. Dry-run also forces verbosity to at least info so the `[dry-run]` line is visible at the default warn level (`src/main.rs:62-134`). The strongest evidence is live: 34 e2e dry-run tests create real AWS state, run the dry-run, then verify via the SDK that nothing changed (`tests/e2e_dry_run.rs`). -- `clean` — the highest-blast-radius command — keeps its engine's safety gate: dry-run/`--force` checks, refusal to run non-interactively without `--force`, and an exact-`"yes"` confirmation prompt, checked before the Ctrl-C handler or indicator are even wired (`src/clean_bin/mod.rs:57-69`); at the prompt, Ctrl-C still kills the process via the OS default handler. -- `mv` (copy-then-delete) rejects self-moves before any transfer — same bucket, same endpoint, same *resolved* key, with an explicit `--source-version-id` allowed as version promotion except the `null` pseudo-version (`src/util_bin/cli/mv.rs:46-98`) — and deletes the source only after a four-gate decision tree (no cancellation, no transfer error, no verification warning unless `--no-fail-on-verify-error`, final cancellation re-check), pinning the delete to the version id the copy actually read (`mv.rs:100-162`). Every gate is unit-tested against a fake storage that counts delete calls. -- Object annotations: payloads are read through a hard 1 MiB bounded reader on both upload and download (TOCTOU-free `take(cap+1)` on upload, streamed cap on download so a hostile endpoint cannot OOM the process; `src/util_bin/cli/put_object_annotation.rs:44-61`, `get_object_annotation.rs:316-333`). Uploads send Content-MD5 plus CRC64NVME and fail if the response omits the verification checksum; downloads verify, write to a temp file, **re-verify the on-disk bytes, and only then atomically rename** — an s7cmd-local hardening over upstream (which renamed first), regression-tested to preserve a pre-existing good file on verification failure (`get_object_annotation.rs:181-225`). -- `create-bucket --tagging` is the one two-call wrapper: a tagging failure after bucket creation warns with the exact partial state and exits 3; no rollback, by design (`src/util_bin/cli/create_bucket.rs:83-91`). -- Remaining operator exposure: there is no interactive confirmation outside `clean`; and the upstream arg structs make positional `SOURCE`/`TARGET` (and e.g. `POLICY`) environment-backed, so an exported `TARGET` silently supplies the target of a destructive command — including on every `batch-run` line, which inherits the process environment. A deliberate upstream scripting affordance, but a real foot-gun. +#### Wrapper fidelity and protection against operator mistakes + +- **Dry run.** Every mutating subcommand accepts `--dry-run`. Configuration-file inputs (policies, lifecycle, CORS, and similar JSON) are read and parsed before the dry-run short-circuit, so a dry run also validates the input document. The offline dry-run suite (67 tests) exercises the mutating subcommands under `--dry-run` against the loopback server. +- **`mv`.** The frontend refuses a move whose source and destination resolve to the same object (the comparison is endpoint- and version-id-aware, and the literal `null` version id is excluded), and it runs the delete phase only after a copy phase that finished with no error, no warning, no cancellation, and not under `--dry-run`. +- **`clean`.** The s3rm-rs safety checker requires `--force` or an interactive `yes`. Verified: with a non-interactive stdin and no `--force`, `clean` refuses with exit 2 and the message "Cannot run destructive operation without --force (-f) in a non-interactive environment (no TTY)", both standalone and as a `batch-run` line. `--dry-run` skips the prompt because nothing is deleted. +- **Annotations.** `get-object-annotation` downloads into a temporary file, verifies it with a bounded 1 MiB read, and only then renames it into place; `put-object-annotation` sends Content-MD5 and CRC64NVME so a corrupted upload is rejected by the service. +- **Pipes and terminals.** Output goes through pipe-safe writers; the closed-stderr, broken-pipe, and tracing-to-stderr suites show that a consumer that exits early produces no panic and no abort. Help output hides credential values that would otherwise be echoed from environment variables. +- **Bodiless errors.** `head-bucket` against a 404 with no body reports exit 4 (not found), confirmed experimentally, so the not-found class does not depend on an XML error body. #### Credential handling -Three independent layers were verified: +Three layers were verified. First, `--help` never prints the value of a credential environment variable. Second, the configuration types of all four pinned engine crates redact access keys, session tokens, and SSE-C keys in their `Debug` output, and s7cmd's own configuration trace logs a hand-picked summary rather than the whole configuration. Third, `batch-run` masks the value of any flag whose name contains `access-key`, `session-token`, or `sse-c-key` (both `--flag=value` and `--flag value` forms, quoting-aware, with a whitespace fallback when a line is not shell-parseable) before the line text reaches any log. The masking is keyed on the flag name, which is the root of finding 1. -- Upstream types: `AccessKeys`, SSE-KMS ids and SSE-C keys in all four pinned engine crates derive `Zeroize`/`ZeroizeOnDrop` and print `** redacted **` from `Debug` (verified in the published 1.60.0/1.8.0/1.4.0/1.1.0 sources), so the trace-level `config = {:?}` lines in the sync/ls/clean dispatch arms cannot print a secret. The cp/mv arm goes further and logs only five enumerated non-secret fields instead of `{:?}` (`src/dispatch.rs:453-465`). -- `--help` cannot echo env-supplied secrets: `hide_credential_env_values` re-hides every env-backed `*access_key*` / `*session_token*` / `*sse_c_key*` argument tree-wide as a guard on top of the upstream fixes, enforced by a meta-test that walks every subcommand and requires ≥100 such arguments checked (`src/cli.rs:503-517,636-663`), plus a process-level test that exports real-shaped values and asserts names render but values never do — with a non-secret control value proving the test cannot pass vacuously (`tests/cli_help.rs:611-737`). -- `batch-run` masks inline credential option values to `****` before any line text reaches a log or `--json-tracing` output, in both `--flag value` and `--flag=value` forms, with a whitespace-level fallback so a secret on an unparseable (unbalanced-quote) line is still scrubbed (`src/batch_run/redact.rs`); masking is asserted end-to-end, including that a presigned URL never contains the secret key (`tests/batch_run.rs:1309-1410`). +Credentials passed as command-line flags are visible to other processes on the same host through the process argument list; this is inherent to any CLI and is one more reason to prefer profiles and environment variables, which s7cmd supports everywhere. #### Supply chain, build, and CI -- The four engine crates are exact-pinned (`=1.60.0`, `=1.8.0`, `=1.4.0`, `=1.1.0`) to keep the vendored frontend code in sync. `Cargo.lock` is committed; all release builds and the crates.io publish use `--locked`. -- TLS is rustls 0.23.42 with aws-lc-rs 1.17.3 and OS trust anchors; `openssl-sys` is absent from the lockfile and additionally deny-listed. `ring` enters the lock only via the e2e-test dev-dependency `ureq`, not the shipped binary. -- `cargo deny -L error check` runs on every push and PR (`ci.yml`) and daily at 01:34 UTC (`cargo-deny.yml`); `deny.toml` sets `advisories.ignore = []`, denies unknown registries and git sources, and restricts licenses to a permissive allowlist. -- CI builds and tests six targets with blocking `cargo fmt --check`, `cargo clippy -- -D warnings`, and cargo-deny jobs. The release workflow (`cd.yml`) triggers on version tags, builds `--locked`, attaches per-artifact SHA-256 checksums and GitHub build-provenance attestations, and publishes to crates.io via OIDC trusted publishing (no long-lived token). -- The release binary embeds a Lua interpreter (mlua 0.12.0) because s3sync's default features include `lua_support` for its filter-callback flags; `Cargo.toml:14-18` documents this and the opt-out. Intentional, but it is real additional attack surface, and `-rdynamic` linking on some targets exists solely to support it. -- Gaps worth naming: the gating clippy job runs without `--all-features` (the `--all-features` clippy run is a separate non-gating code-scanning workflow); no CI job compiles the `--cfg e2e_test` configuration, so the 258-test e2e corpus is type-checked only locally (it does compile warning-free — re-verified in this assessment); CI neither produces nor gates on coverage (the codecov badge is fed from local runs); `cd.yml` runs no tests itself; the Dockerfile builds without `--locked` (though it copies the committed lockfile). - -#### Test corpus and coverage measurement - -1,427 test annotations total: 477 unit tests in `src/` (the batch-run engine and dispatch table are the densest: 63 in `dispatch.rs`, 48 in `executor.rs`), 692 offline process-level tests (62 `tests/cli_*.rs` files plus `tests/batch_run.rs` — these spawn the real binary; S3 interactions hit a loopback-only mock bound to `127.0.0.1:0` with fake credentials passed as flags, so no AWS account or ambient credentials are touched), and 258 live-AWS tests in 28 `tests/e2e_*.rs` files, every one gated behind `#![cfg(e2e_test)]`. Sixteen distinct regression clusters pin previously fixed bugs, including the `--parallel` semaphore panic, the `AUTO_COMPLETE_SHELL` re-arming, the mv self-move data loss, credential values in `--help`, batch-run credential masking, and the mid-batch `process::exit` kills. - -`llvm-cov-report.txt` (one combined unit + CLI + `--cfg e2e_test` live-AWS run, produced 2026-07-20 on the maintainer's machine; `lcov.info` is the same data in LCOV form, verified to carry identical totals) reports 97.63% region, 96.94% function, 98.55% line coverage over 82 files (the two 2-line module stubs contain no executable code). On the paths this assessment cares most about: `batch_run/executor.rs` 98.69% regions, `batch_run/mod.rs` 98.39%, `batch_run/redact.rs` 99.39% regions / 100% lines, `batch_run/validate.rs` 98.76%, `dispatch.rs` 98.33%, `util_bin/cli/mv.rs` 99.21% regions / 100% lines, `util_bin/cli/get_object_annotation.rs` 97.93%. The weakest files are small error-path wrappers (`head_bucket.rs` 80.65%, `delete_bucket_policy.rs` 82.61% regions). Branch coverage is not measured (the branch columns are empty), and coverage remains structural: a covered line can still be incorrect. +- The four engine crates are pinned with `=` versions and `Cargo.lock` is committed, so a build of a tag resolves the same graph the tests ran against. `rust-version` is declared (1.94.1), edition 2024. +- `deny.toml` keeps an empty advisory ignore list, allows a fixed license set, bans `openssl-sys` (the TLS stack is rustls), and restricts sources to crates.io. `cargo deny` runs in CI on every push and on a daily schedule, so a newly published advisory surfaces without a code change. +- CI runs rustfmt, clippy with `-D warnings`, and the offline test suites on a seven-target matrix (Linux glibc and musl on x86_64 and aarch64, Windows on x86_64 and aarch64, macOS on aarch64). A separate workflow uploads clippy results as SARIF to code scanning. +- The release workflow builds per-target archives, publishes SHA-256 sums, attaches GitHub build-provenance attestations (`actions/attest-build-provenance`), and publishes to crates.io through trusted publishing rather than a stored token. +- `.cargo/min-publish-age.toml` opts dependency updates into a seven-day minimum publish age on nightly, a mitigation against freshly published malicious crate versions; on stable it is inert by design. +- The `Dockerfile` builds in `rust:1-trixie` and runs as an unprivileged user on `debian:trixie-slim` with only `ca-certificates` added. The builder tag floats across Rust minor versions, so container builds are not reproducible and may compile with a newer toolchain than CI tested (informational). +- CI does not compile the `cfg(e2e_test)` corpus; a broken e2e test would be noticed only on the maintainer's machine. It compiled cleanly during this assessment. -Test-harness limits found: the mock server replays canned responses and discards request bodies — only five files assert on captured requests, and only method + path, so a wrapper that sent a wrong body to the right URL would pass offline (the e2e round-trips are what actually verify payloads); roughly 100 tests are smoke-only (help exits 0, missing-args exits 2); `sync` and `mv` have no dedicated per-subcommand offline file; and the signal tests use fixed sleeps with bounded polls. +#### Test corpus and coverage -#### Known limitations and findings of this assessment +The counts in the Quality verification table were recounted independently and match: 506 test annotations in `src/` (unit tests), 730 in the 65 offline process-level files (`tests/cli_*.rs` and `tests/batch_run.rs`), and 258 in the 28 `tests/e2e_*.rs` files. The offline corpus spawns the real binary against a loopback mock S3 or purpose-built fake servers and asserts on exit codes, stdout, and stderr; it pins subcommand routing (62 tests), help text (71), argument validation, dry-run behaviour (67), error paths, pipe safety, SIGINT handling and interruption exit codes (a fake S3 that stalls mid-transfer, 17 tests), and the `batch-run` semantics above (68). The e2e corpus covers object, bucket-configuration, annotation, stdio, cancellation, exit-code, and `batch-run` paths against live AWS. -This from-scratch pass found no critical defects at v1.6.0. What it did find, ranked: +Coverage as reported by the artifacts: -- Cancellation exit codes are inconsistent across engines, and `clean` can mask real errors on Ctrl-C. `cp`/`mv` exit 130 on SIGINT, but the vendored `sync`/`ls`/`clean` frontends map user cancellation to exit 0 — and `clean`'s error loop returns success on the first cancellation entry even when genuine deletion failures were recorded alongside it (`src/clean_bin/mod.rs:115-118`, faithfully vendored from the upstream binary). Consequently a Ctrl-C'd `clean` that had already hit real errors exits 0, and inside `batch-run` a Ctrl-C'd `sync`/`clean` line is bucketed `succeeded` while a `cp` line is bucketed `skipped`. -- The `mv` self-move guard compares endpoints textually (`src/util_bin/cli/mv.rs:66-77`, documented best-effort): two spellings of the same endpoint on an unversioned bucket re-open the delete-what-you-just-wrote window the guard exists to close. Bucket versioning remains the real backstop. -- Parallel-executor SIGINT window: the spawn loop checks the interrupt flag before awaiting a semaphore permit (or the channel), so a SIGINT that lands during that await lets one already-queued line spawn afterwards (`src/batch_run/executor.rs:399-430,537-577`); since its cancellation handler is installed after the signal fired, that line runs to completion rather than cancelling. -- Severity-ranking edges: any exit code outside 1–4 — including the synthetic panic code 101 — ranks below exit 4, so a `--continue-on-error` batch mixing a panic with a warning exits 3/4, understating what happened (the error-level `panicked` log entry is the reliable signal). In streaming mode a reader I/O error raises the exit only to `max(code, 1)` numerically, so prior warning exits (3/4) survive where the severity rule would surface 1 (`src/batch_run/mod.rs:402-412`). -- Wrapper exit-code inconsistencies: `rename` maps a missing object/bucket to exit 1 where sibling subcommands use 4; the mutating `put-*`/`delete-*` wrappers likewise surface NotFound as 1, so scripts cannot distinguish "bucket absent" from other failures on mutations; and `mv --no-fail-on-verify-error` exits 0 after a verification warning where `cp` exits 3. -- Dry-run is not always zero-network: every mutating wrapper builds its SDK client (credential resolution, possibly IMDS/SSO traffic) before the short-circuit, and three paths issue a live read-only call under `--dry-run` — `cp --skip-existing` (HeadObject), `create-bucket --if-not-exists` (HeadBucket), and `cp --enable-sync-object-annotations` (ListObjectAnnotations). No mutation is reachable on any dry-run path (verified per wrapper and asserted by the state-unchanged e2e suite). -- The nine `put-bucket-*` configuration bodies and the bucket policy are read with no client-side size cap — a mistyped multi-GB path is fully buffered before S3 rejects it. Batch-run's own 16 KiB line cap does not extend to the files those lines reference. -- A documented-but-unfixed defect, pinned by its own test rather than hidden: `batch-run --streaming -` with a stdin pipe that is never closed does not exit on SIGINT alone (`tests/cli_sigint.rs:72-80`); the reader's blocking stdin read survives the signal until EOF. -- The vendored frontend files cite older upstream versions (s3sync@1.57.1–1.58.6, s3rm-rs@1.3.3–1.3.4, s3ls-rs@0.4.1, s3util-rs@1.0.0–1.7.1) than the pinned engines they drive; the two 1.8.0-era fixes s7cmd needs are explicitly backported, and the stat-enum matches have no wildcard arms so upstream drift breaks the build rather than passing silently — but this vendored/engine skew is a standing maintenance risk unique to s7cmd's architecture. -- Smaller items: the annotation download's atomic rename is not preceded by an `fsync`, so a power loss (not a process crash) can persist the rename without the data; batch-run's structured logs and the trace-level config dumps can include the access key ID (an identifier, never the secret — and batch-run's own masking hides even the ID for inline flags); `restore-object --days` accepts any positive integer; duplicate tag keys pass client-side validation and are left to S3 to reject. +| Metric | Covered | Missed | +| --- | --- | --- | +| Regions | 97.53% | 377 of 15,256 | +| Functions | 97.08% | 35 of 1,200 | +| Lines | 98.50% | 163 of 10,873 | -#### Is the software reliable? +The Quality verification section states that the figures include the live-AWS suites; this assessment confirmed that the two artifacts agree with each other and correspond to the assessed commit, but it could not attribute individual hits to the e2e run rather than to the offline suites. The 163 uncovered lines were examined one by one. They are: error arms for a failed Ctrl-C handler registration or a failed tracing-subscriber flush in each of the four frontends; `unreachable!` and `let ... else { panic!() }` arms, most of them inside test helpers; the "cancelled, but not by Ctrl-C" arms in `ls` and `clean`, which the binary cannot reach because only the Ctrl-C handler cancels the token; `clean`'s interactive "no" answer and its indicator-task-panicked arm; the `batch-run` arms for a task that panics outside `catch_unwind` and for a panicking reader task; the completion-script write failure in `main.rs`; the `mv` stdio validation branch that s3util-rs rejects earlier (dead code); and `cp`'s warning return (`ExitStatus::Warning`), which is exercised by an e2e test through a `cargo run` subprocess that coverage instrumentation does not see. None of these is a primary operational path; the measured figures therefore understate real coverage slightly rather than overstate it. The usual caveats apply: coverage counts executed lines, not verified behaviour, and the engine crates' internals are outside the measurement. -For readers who are not software engineers: "reliable" here means whether a careful operator can use s7cmd for routine Amazon S3 work without losing data, leaking credentials, or having a batch script do something other than what was written. +#### Findings, ordered by potential impact -**Risk: the wrapper corrupts or mis-routes.** s7cmd adds no transfer logic of its own — integrity verification, temp-file-then-rename downloads, and version-pinned reads live in the exact-pinned engines, and the wrapper preserves their guarantees (the one place it touches verified bytes, annotation downloads, it re-verifies on disk *before* the rename — stricter than upstream). Routing from subcommand string to engine call is pinned by three independent test layers. +1. **Secret masking in `batch-run` depends on the flag being spelled correctly (medium).** `redact_secrets` (`src/batch_run/redact.rs`) masks the token after a flag whose name contains one of three substrings. A misspelled credential flag, for example `--target-secret-acces-key SECRET`, `--target-secret-key SECRET`, or `--target-sessiontoken SECRET`, is rejected by clap, and `batch-run` then logs the rejected line at error level (and in `--json-tracing` output) with the value unmasked. With the correct spelling the value is masked even when the line fails for an unrelated reason (verified with an extra unknown flag). Because a typo is exactly the situation in which the line is logged, the masking is weakest where it is most needed. Suggested fix: when clap rejects a line, log clap's message without the raw line, or mask the value of every unknown flag whose name contains `key`, `secret`, or `token`. Until then, supply credentials through profiles or environment variables in scripts. +2. **No hard stop on Ctrl-C, and no network timeout unless requested (medium for unattended use).** The four vendored handlers and `batch-run` use tokio's `signal::ctrl_c`, which replaces the default SIGINT disposition for the rest of the process and fires once: it sets the flag and cancels the token, and a second Ctrl-C does nothing. Cancellation is honoured at the engines' own check points (between operations or body chunks), not inside a request that is still waiting for the server to answer, and the engines build an SDK `TimeoutConfig` only when one of `--operation-timeout-milliseconds`, `--operation-attempt-timeout-milliseconds`, `--connect-timeout-milliseconds`, or `--read-timeout-milliseconds` is given. Observed: a standalone `ls` against an endpoint that accepts the connection and never answers was still running six seconds after one and after two SIGINTs; inside `batch-run`, two such `head-bucket` lines survived two SIGINTs, and the run ended (1 failed, 1 skipped, exit 1) only when the fake endpoint was shut down; with `--operation-timeout-milliseconds 3000` the same line failed after three seconds. Standalone single-request subcommands, which install no handler, terminate on the first Ctrl-C as documented, but the same line inside `batch-run` cannot be interrupted at all because the batch's handler owns SIGINT. Consequence: an unattended batch against a stalled endpoint hangs until SIGTERM or SIGKILL. Mitigation: set the timeout flags in automation; a second-Ctrl-C hard exit (130) would remove the operator-facing half of the problem. +3. **Streaming-mode reader failures are ranked numerically, not by severity (low).** In `src/batch_run/mod.rs`, when the streaming reader fails (an over-long or non-UTF-8 line, or a panicking reader task) the run returns `code.max(1)`. If an earlier line exited 3 or 4, the numeric maximum keeps that code, the summary reports 0 failed, and the reader error is visible only in the error log; with an earlier success the same script exits 1. Read-all mode is unaffected because it rejects the script before any line runs. Suggested fix: use `worse_of(code, 1)` and count the reader error as a failure. +4. **Colour escapes reach non-terminal stderr for the s3util-rs family and `batch-run` (low).** `src/util_bin/tracing_init.rs` enables ANSI output with `!disable_color_tracing` alone, whereas the `ls`, `clean`, and `sync` frontends also require `stderr().is_terminal()`. Logs redirected to a file from `cp`, `mv`, `head-*`, `get-*`, `put-*`, and `batch-run` contain escape sequences unless `--disable-color-tracing` is passed. This matches the upstream s3util-rs 1.10.2 binary, so it is an inherited trait rather than a vendoring error. +5. **Validation of `sync` lines checks the local filesystem at validation time (low).** Both `--check-format` and the pre-execution validation build the engine configuration, and s3sync rejects a local source that does not exist yet. A script whose earlier line creates the directory that a later `sync` reads from fails validation even though it would run; `cp` lines are only checked at execution time. Workaround: create the directory before running the script. +6. **`batch-run --streaming -` cannot be interrupted while blocked on an open stdin (low).** The reader blocks in a read that only returns when the producer writes or closes; a Ctrl-C between lines is noticed only when the next line arrives. The behaviour is documented in `tests/cli_sigint.rs`. Operators cancelling a streaming batch must also close the producer. +7. **Vendoring drift (informational).** The frontends are snapshots of earlier upstream binaries with individually ported fixes (`sync_bin` from s3sync 1.57.1/1.58.6, `ls_bin` from s3ls-rs 0.4.1 plus 1.3.0 ports, `clean_bin` from s3rm-rs 1.3.3/1.3.4 plus 1.6.0 ports, `util_bin` spanning s3util-rs 0.2.0 to 1.10.0) while the engine libraries are 1.62.1, 1.3.2, 1.6.2, and 1.10.2. Nothing wrong was found in the current snapshots, and the offline corpus pins the observable behaviour, but a flag or behaviour that upstream adds to its binary after the snapshot version does not appear in s7cmd automatically. The provenance headers make an audit tractable. +8. **Minor observations (informational).** The `mv` stdio branch in `src/batch_run/validate.rs` is unreachable because the s3util-rs parser rejects `-` for `mv` first; the floating `rust:1-trixie` builder tag in the `Dockerfile`; and the absence of an e2e compile check in CI (see above). -**Risk: a batch run goes wrong.** Every line is validated before anything executes (in the default read-all mode, before *any* line executes); failures — including panics — are contained per line, counted against an explicit threshold, logged with the line number, and rolled into a severity-ranked exit code; inline credentials are masked in every log path; and a 200 MB-upload SIGINT test proves cancellation propagates. The residual sharp edges are the cancellation-exit-code inconsistency and the streaming-stdin SIGINT case above. +No finding identifies a path that mutates or deletes the wrong object, sends a mutating request under `--dry-run`, weakens an engine guarantee, or loses a per-line failure in the default read-all mode. Findings 1 and 2 are the ones that change operating practice; the rest mis-report or mis-format rather than mis-act. -**Risk: operator mistake.** All 34 mutating subcommands take `--dry-run`, verified live against AWS to change nothing; `clean` requires `--force` or an interactive "yes"; `mv` refuses self-moves; `batch-run --check-format` validates a whole script without executing it. The exposures that remain are documented and operator-shaped: no interactive confirmation outside `clean`, and environment-backed positionals that can invisibly supply a target. +#### Verdict: is s7cmd reliable enough for unattended and destructive use? -**What this assessment cannot establish.** That the code is bug-free; that the engine crates (assessed separately) are defect-free; or that S3-compatible endpoints behave like Amazon S3. The findings above that reach observable behavior — cancellation exit codes, `rename`'s exit-1, the severity-ranking edges — share a shape worth noting: they mis-*report* rather than mis-*act*; no finding of this pass identified a path that mutates or deletes the wrong data absent operator error, with the narrow documented exception of the mv endpoint-spelling window. +Yes, within stated conditions. The wrapper is thin and faithful: routing, configuration translation, and exit-code propagation are tested per subcommand against a mock server, the engines' own safeguards (dry run, `mv` delete gating, the `clean` confirmation, checksum verification) pass through intact, and the one piece of original logic, `batch-run`, is designed around containment: nothing runs before validation in the default mode, a panic costs one line, secrets are masked on every log path, the stop policy is explicit, and the exit code is severity-ranked and tested. The static gates (rustfmt, clippy at `-D warnings`, cargo-deny with no ignored advisories, exact pins, provenance attestations) are stronger than most CLI tools of this size carry, and the coverage figures are backed by a corpus that asserts on behaviour, with the small uncovered remainder accounted for above. That the code was produced with AI assistance is not, by itself, evidence in either direction; the evidence is the design, the tests, and the checks re-run here. -In plain terms: at v1.6.0 the failure modes most likely to cause silent harm — a batch that keeps running past failures it should stop on, a credential copied into a log, a wrapper that silently weakens an engine guarantee, a dry-run that mutates — each have a specific, citable, and tested safeguard, and this pass re-verified the fixes for every previously known defect, each pinned by a regression test. The fact that the codebase is AI-generated neither raises nor lowers its reliability; what determines that is the design, the tests, and the verifiable evidence above. The tool remains conditionally reliable: previewing destructive work with `--dry-run` (and `batch-run -v --check-format` for scripts), watching exit codes *and* error-level logs (exit codes alone understate panics and Ctrl-C'd `clean` runs), keeping bucket versioning for irreplaceable data, and least-privilege IAM are the operating controls the design assumes. Final responsibility for whether the binary fits a given workflow rests with the operator who runs it. +The conditions are the ones this assessment found: preview destructive work with `--dry-run` and scripts with `batch-run --check-format`; keep credentials in profiles or environment variables rather than inline flags, which also closes finding 1; set `--operation-timeout-milliseconds` (or the finer-grained timeout flags) on every line of an unattended batch; pass `--disable-color-tracing` when stderr is a file; read error-level logs as well as the exit code, since the streaming reader case and panics are fully visible only there; keep bucket versioning on for irreplaceable data; and apply least-privilege IAM. What this assessment cannot establish is the absence of defects inside the engine crates, the behaviour of S3-compatible endpoints that deviate from Amazon S3, or correctness beyond what the tests and experiments above exercised. Responsibility for whether the binary fits a given workflow remains with the operator who runs it.
@@ -647,78 +649,116 @@ In plain terms: at v1.6.0 the failure modes most likely to cause silent harm —
Click to expand the full assessment -> Assessment date: 2026-07-22. -> -> Assessed tree: `main` at `839acc3`. The Rust implementation and build inputs are identical to v1.6.0 (`c454694`); the commits after that tag change README content only. -> -> Complete review boundary: all 84 Rust files under `src/` were examined in full, including their 477 embedded tests, rather than sampling high-risk modules. The review also covered all 63 offline process-test files, `tests/common/mod.rs`, all 28 `e2e_*.rs` suites, `Cargo.toml`, `Cargo.lock`, `build.rs`, `.cargo/config.toml`, `deny.toml`, `Dockerfile`, and all five GitHub Actions workflows. The four exact-pinned engine crates were checked at the interfaces and guarantees on which this wrapper relies; they remain separate dependencies, not source owned by this repository. Conclusions were derived from code and executable evidence, not from the neighboring assessment text. -> -> Coverage evidence: the supplied `lcov.info` (SHA-256 `2da2901598bffa46b6fda6718c815e34701d24870c3e9f9a984a2da1b28bb22a`) and `llvm-cov-report.txt` (SHA-256 `f7fa71e841669bbb65673fe311832c7f12752b5218b3c9258a45e7a0053daffb`) were parsed independently. Their line and function totals agree exactly. -> -> Limits: this was source review plus deterministic local verification, not formal proof. No fuzzing, Miri, sanitizers, fault injection, penetration test, or fresh live-AWS run was performed. The E2E configuration was compiled and linted, while its prior live execution is represented only by the supplied coverage artifacts. - -#### Bottom line - -No critical vulnerability was found in s7cmd's own code, and the ordinary Amazon S3 paths show strong defensive engineering. The wrapper has complete dispatch coverage, contains per-line batch failures instead of terminating the process, protects credential values in help and batch logs, and preserves dry-run and transfer-integrity controls. There is no `unsafe` block in production code and no production subprocess execution or shell evaluation; `batch-run` parses and dispatches commands in-process. - -I would nevertheless describe v1.6.0 as **conditionally reliable, not safety-certified**. One narrow `mv` case can still delete an object, and batch input/cancellation behavior has availability and result-reporting defects. These are concrete open findings, not hypothetical objections to AI-generated code. - -#### Open findings, ordered by operational impact - -| Impact / priority | Finding | Consequence and boundary | -|---|---|---| -| High impact, narrow trigger | The `mv` self-move guard compares source and target endpoint strings literally (`src/util_bin/cli/mv.rs:65-77`). Equivalent spellings of the same service therefore bypass the guard. | With the same bucket and resolved key on an unversioned or version-suspended bucket, copy-then-delete can delete the object just written. The operator must explicitly construct this endpoint mismatch; bucket versioning is the strongest mitigation. Endpoint canonicalization or an explicit same-service override would close it. | -| Moderate | Batch memory is not bounded in aggregate. Default mode collects every parsed line in a `Vec` (`src/batch_run/parser.rs:60-88`), while streaming mode feeds an `unbounded_channel` (`src/batch_run/mod.rs:361`). | The 16 KiB per-line limit prevents a single-line allocation attack but not a huge script, and a fast streaming producer can outpace a slow S3 command until memory is exhausted. A total-line/byte cap and a bounded channel would make the resource guarantee real. | -| Moderate | Cancellation is reported inconsistently. `ls` maps cancellation to 0 (`src/ls_bin/mod.rs:75-78`); `sync` can complete a cancelled pipeline without a distinct cancellation status; and `clean` returns 0 as soon as it encounters any cancellation error, even if the same collected error set contains real deletion failures (`src/clean_bin/mod.rs:98-124`). | Automation and `batch-run` can classify interrupted work as success. In `clean`, a genuine error may be masked. Error aggregation should retain real errors and use one cancellation code, preferably 130, across engines. | -| Moderate | Streaming stdin is not promptly cancellable. After fail-fast or SIGINT the executor drains the channel and awaits its closure, then `run_streaming` awaits the reader (`src/batch_run/executor.rs:484-505,579-582`; `src/batch_run/mod.rs:399-412`). Tokio's stdin read can remain blocked until the producer closes the pipe. | `batch-run --streaming -` can hang after SIGINT or an early failure when stdin stays open. The process test explicitly documents that EOF is load-bearing (`tests/cli_sigint.rs:68-105`). A cancellable reader design or documented external pipe closure is required. | -| Low to moderate | Parallel stop checks occur before awaiting the next channel item or semaphore permit, with no re-check after the await (`src/batch_run/executor.rs:398-430,537-577`). | A SIGINT or reached error threshold can allow one already-queued command to start afterward. Because that command installs its own handler after the signal, it may run to completion. | -| Low to moderate | Exit aggregation understates some failures. All nonstandard codes, including caught panic 101, rank below warning 3 and not-found 4 (`src/batch_run/executor.rs:289-313`); streaming reader failure uses numeric `max(code, 1)` (`src/batch_run/mod.rs:402-411`). | A batch containing a panic plus a warning can exit 3/4, and a reader error after a warning can retain the warning code. Error-level logs remain accurate, but exit-code-only automation is not. | -| Low, availability | Nine JSON file/stdin inputs are read with unbounded `read_to_string` before parsing or sending: policy, CORS, encryption, lifecycle, logging, notification, replication, website, and public-access-block. | A mistaken or hostile multi-gigabyte input can exhaust memory. The annotation payload paths correctly demonstrate the bounded-read pattern and should be reused. | - -Additional correctness edges are smaller but real. `rename` reports missing source/bucket as general error 1 while many sibling commands use not-found 4 (`src/util_bin/cli/rename.rs:52-58`). `mv --no-fail-on-verify-error` can delete the source and return success despite a verification warning, whereas `cp` returns warning 3. Annotation file output flushes, re-reads, verifies, and atomically renames, but does not `sync_all`, so its promise covers process crashes and detected corruption rather than power-loss durability (`src/util_bin/cli/get_object_annotation.rs:181-224`). Positional arguments inherited from the engine CLIs are environment-backed, so exported `SOURCE`, `TARGET`, or policy variables can silently fill destructive command arguments. There is no confirmation prompt outside `clean`. - -`--dry-run` prevents mutations, but it is not a no-I/O or no-network guarantee. Client construction happens first and may perform credential-provider work. `sync`, `clean`, and transfer planning can enumerate or inspect S3 state; thin-wrapper examples include `cp --skip-existing`, annotation synchronization, and `create-bucket --if-not-exists`, which perform read-only S3 checks before deciding what they would do (`src/util_bin/cli/cp.rs:17-33`; `src/util_bin/cli/create_bucket.rs:45-64`). This is safe with respect to S3 state but matters in isolated environments and when metadata/SSO credential providers are enabled. - -#### Safety controls that held under full-source review - -- The 56-variant command enum has a corresponding non-exiting dispatch path. Configuration failures return 2 instead of invoking upstream `clap::Error::exit`, so one bad batch line cannot terminate its siblings (`src/dispatch.rs`). The large transfer futures are boxed to avoid known small-stack overflows. -- All 34 mutating command surfaces expose `--dry-run`. Thin wrappers return before their mutation calls, while `cp`, `mv`, `sync`, and `clean` carry the flag into their pinned engines. The live E2E corpus contains a state-unchanged case for every mutating command. Read-only setup and planning can still contact S3 as described above. -- `mv` normally has a sound deletion decision tree: no delete after cancellation, copy error, or verification warning without the explicit override; it checks cancellation again immediately before delete and pins deletion to the source version actually read (`src/util_bin/cli/mv.rs:100-161`). The endpoint-equivalence gap above is before that tree, not a failure of those gates. -- Annotation upload and download enforce the 1 MiB limit with bounded streaming reads. Upload sends Content-MD5 and CRC64NVME and verifies the returned checksum; download verifies content length plus available checksums, rejects unsupported returned algorithms cleanly, writes beside the target, re-verifies the saved bytes, then atomically persists. A pre-existing destination survives verification failure (`src/util_bin/cli/put_object_annotation.rs:44-100`; `get_object_annotation.rs:87-224,315-394`). -- `batch-run` enforces a 16 KiB line limit incrementally, rejects invalid UTF-8, tokenizes with `shlex`, rejects nested batches, stdin/stdout conflicts and per-line tracing, caps `--parallel` at 1024, and converts parse/validation errors into per-line exit 2. Dispatch is wrapped in `catch_unwind`, and all build profiles retain `panic = "unwind"`, so a subcommand panic becomes a recorded exit 101 instead of tearing down the batch (`src/batch_run/`; `Cargo.toml:103-113`). -- Credential values are hidden from env-aware help output across the complete clap tree (`src/cli.rs:486-517`). Batch log redaction handles both `--flag value` and `--flag=value`, including the malformed-quoting fallback (`src/batch_run/redact.rs`). The pinned engine credential types redact `Debug` output and zeroize secret fields, so the remaining trace-level full-config logs do not print secret keys or session tokens. Access-key identifiers may remain partially visible by design. -- Production source contains no direct `unsafe`, no shell invocation, and no process-spawning API. Lua support is inherited from s3sync's default features and deliberately expands the binary's attack surface, but Lua runs only operator-supplied filter code rather than data received from S3. - -#### Tests, coverage, and what the numbers mean - -The supplied report totals are 97.63% regions (14,282/14,629), 96.94% functions (1,110/1,145), and 98.55% lines (10,297/10,448). There are no branch records: both artifacts report zero measured branches. The line and function figures are internally consistent, but two qualifications prevent treating 98.55% as production-only coverage: - -1. `lcov.info` includes functions named under `::tests::` and line records from `#[cfg(test)]` modules—for example, the `mv` fake storage and tests above line 900. The 10,448-line denominator therefore mixes product and test implementation. Similarly, the README's 16,949 physical lines under `src/` include the 477 embedded tests; they are not all production lines. -2. The loopback mock records method and request target, then drains request bodies without retaining them and exposes no captured request headers (`tests/common/mod.rs:200-259`). Most offline wrapper tests therefore prove parsing, routing, status mapping, and response handling, but not exact outbound JSON or signed headers. Live-AWS round trips provide the stronger payload evidence and are not a CI gate. - -The corpus is broad: 477 embedded test annotations, 692 annotations in 63 offline process-test files, and 258 annotations in 28 gated live-AWS files. Some tests are explicitly coverage-oriented rather than behavioral—for example, local sync tests ignore `run`'s result—so count and coverage must be read alongside assertion quality. - -During this assessment, these commands passed without warnings or failures: - +Evaluation date: 2026-09-13. LLM name: Codex (OpenAI). Model: GPT-5-based Codex +(exact runtime model identifier not exposed). Effort: comprehensive full-source review +(configured reasoning-effort setting not exposed). + +#### Scope and overall judgment + +This assessment was made from scratch, without consulting any earlier or other AI assessment. +The review covered every repository Rust source file: all 85 files under `src/`, including +their unit tests, all 94 files under `tests/`, and `build.rs`. Cargo manifests and lockfile, +build configuration, Dockerfile, dependency policy, and CI/release workflows were also examined. +The reviewed snapshot is `245c245da13188650fefb8ba4d974e2725f0711b`; its code and build inputs +are unchanged from the snapshot used for the September 12 checks below. + +Overall judgment: substantial safeguards and broad test coverage support confidence in ordinary +validated workflows, but confirmed credential-redaction, batch scheduling, and payload-output +defects limit confidence in unattended automation. This is not an unconditional safety endorsement +or a proof of correctness. The complete repository review does not constitute a full audit of the +implementations of `s3sync`, `s3util-rs`, `s3rm-rs`, `s3ls-rs`, their transitive dependencies, +or AWS service behavior. + +#### Safeguards observed + +- Command parsing, target validation, and destructive-operation prerequisites reject many invalid + invocations before mutation. Batch commands are parsed and dispatched in-process, without an + implicit shell. Dry-run paths suppress the intended mutations, although some still construct + clients or perform reads. Credential environment values are hidden in help output. +- `mv` checks for self-moves, transfer failure, cancellation, and verification warnings before + deleting the source; verification warnings block deletion unless explicitly overridden. + Deletion uses an explicit source version or the version captured by the transfer. + These are meaningful data-loss safeguards. See [move handling](src/util_bin/cli/mv.rs). +- Annotation downloads enforce a 1 MiB payload limit and check length and applicable integrity + information. File output uses a same-directory temporary file, verifies it before replacement, + and leaves the existing destination untouched on pre-replacement failure. + See [annotation output](src/util_bin/cli/get_object_annotation.rs). +- No production repository Rust `unsafe` blocks were found. This does not establish memory safety + for dependencies; test-only tracing code does contain unsafe process-environment mutations. + +#### Confirmed findings + +1. **Malformed batch lines can disclose inline credentials.** The whitespace fallback in + [credential redaction](src/batch_run/redact.rs#L119) does not recognize a quoted credential + flag when another token has an unterminated quote. A fake secret following + `"--target-secret-access-key"` appeared verbatim in diagnostics for both ordinary batch + execution and `--check-format`. Existing masking therefore does not make malformed input + safe to echo into logs. Avoid inline secrets in batch files pending a fix. +2. **Parallel fail-fast can dispatch another command after the failure threshold is reached.** + [Both parallel executors](src/batch_run/executor.rs#L370) check the failure-stop flag before + waiting for a worker permit, but recheck only interruption afterward. With two workers, + two invalid commands followed by two harmless copy dry-runs produced + `1 succeeded, 2 failed, 0 warnings, 1 skipped`: the third command ran after the failures. + This is additional dispatch, not merely completion of already-running work. + The streaming executor has the same missing post-wait failure check. +3. **Annotation payload output can report success after losing data.** + [The stdout path](src/util_bin/cli/get_object_annotation.rs#L361) calls `write_all` without + an explicit flush before returning success. A loopback response containing one byte without + a newline, with the stdout pipe's reader already closed, returned exit 0. Buffered payload + delivery errors can therefore escape the command's result. +4. **Some ordinary CLI tests can perform real mutations.** + [Versioning parsing tests](tests/cli_put_bucket_versioning.rs#L74), and similar acceleration, + request-payment, and restore tests, inherit configuration and execute valid mutating commands + without a dry-run or mandatory mock endpoint. A loopback-only check of the existing versioning + test observed `PUT /example/?versioning` while the test passed. Its assertion only excludes + exit 2, so runtime failures can also pass. These tests should be isolated from privileged + ambient credentials and real endpoints. + +#### Operational limits + +Batch interruption is not uniformly reflected in the exit status: the executors aggregate +command results but do not independently promote an interruption to exit 130. An idle streaming +batch with stdin held open remained alive after SIGINT and returned 0 once stdin closed. +Existing [signal tests](tests/cli_sigint.rs#L82) explicitly accept this behavior. +Streaming failure-stop drains input until the producer closes it, so a slow or never-ending +producer can delay termination. Its [unbounded input channel](src/batch_run/mod.rs#L368) and +retained parallel task results also mean worker limits and the per-line size cap are not total +memory bounds. + +`mv` remains copy-then-delete, not a transaction. Concurrent changes to unversioned or local +sources need external coordination, and the self-move guard compares endpoint strings rather +than canonical service identity. Neither batch execution nor a dry-run provides rollback or a +stable snapshot of later operations. Annotation payloads without applicable integrity information +are accepted with a warning and exit 0; successful file replacement is not an explicit +power-loss durability guarantee. + +#### Coverage and verification evidence + +The supplied `lcov.info` and `llvm-cov-report.txt`, last modified on 2026-09-12 at approximately +15:25 JST, agree on these totals: + +| Measure | Covered / total | Coverage | +| --- | ---: | ---: | +| Lines | 10,710 / 10,873 | 98.50% | +| Functions | 1,165 / 1,200 | 97.08% | +| Regions | 14,879 / 15,256 | 97.53% | + +There are 83 source-file records; the two module-only files `src/sync_bin/mod.rs` and +`src/util_bin/mod.rs` have no records. The totals include in-source test code, and branch +coverage is not measured. These supplied reports were not regenerated for this assessment; +coverage percentages neither establish assertion quality nor rule out the reproduced defects. + +The following checks passed on macOS with Rust/Cargo 1.98.1 during this review: + +- `cargo test --locked --all-features --all-targets` +- `cargo check --locked --no-default-features --all-targets` +- `cargo clippy --locked --all-features --all-targets -- -D warnings` - `cargo fmt --all --check` -- `cargo test --all-features --locked` -- `cargo clippy --all-features --all-targets --locked -- -D warnings` -- `RUSTFLAGS="--cfg e2e_test" cargo clippy --all-features --all-targets --locked -- -D warnings` -- `cargo deny -L error check` (`advisories`, `bans`, `licenses`, and `sources` all clean) +- `cargo deny -L error check` (advisories, bans, licenses, and sources) -The E2E tests were compiled by the second clippy run but not executed because doing so mutates a configured AWS account. Default `cargo test` correctly sees those gated files as zero-test targets. - -#### Dependency and delivery assessment - -The four engine crates are exact-pinned and the lockfile is committed. Release and publish jobs use `--locked`, produce SHA-256 files, and attest release archives. The resolved HTTP stack uses rustls 0.23.42; `openssl-sys` is absent and explicitly denied. Cargo-deny rejects unknown registries and git sources and has no ignored advisories. - -The remaining supply-chain weaknesses are conventional rather than runtime defects: most GitHub Actions are referenced by mutable major-version tags rather than commit SHAs; CI follows moving `stable`; the gating clippy job omits `--all-features --all-targets`; CI does not compile the `e2e_test` configuration or gate on coverage; release jobs do not rerun tests; and the Dockerfile uses mutable base tags and builds without `--locked`. These do not invalidate the tested source tree, but they weaken reproducibility and increase the trust placed in workflow dependencies. - -#### Reliability conclusion - -For routine, supervised Amazon S3 use, the evidence supports the tool's core claims: routing is exhaustive, mutation previews exist everywhere, credential handling is deliberate, transfer deletion is normally gated, annotation bytes receive unusually strong integrity treatment, and failures generally become structured nonzero results. I found no path in the ordinary same-endpoint flow that silently mutates the wrong S3 resource, no designed logging path that emits credential secrets, and no dry-run path that invokes an S3 mutation. - -For unattended destructive automation, the conditions matter. Canonicalize and keep source/target endpoints identical for same-service `mv`, enable bucket versioning for valuable data, bound or trust batch inputs, close streaming stdin on cancellation, and monitor error logs in addition to the final exit code. Until the `mv` endpoint comparison, aggregate batch bounds, and cancellation/error aggregation are fixed, those controls are part of the safety model rather than optional operational advice. +The live-AWS tests gated by `cfg(e2e_test)` were reviewed but were not enabled by these commands. +The additional reproductions used malformed input, dry-runs, signals, or loopback mocks with +fake credentials. No production fixes are included in this README-only assessment.
@@ -727,76 +767,78 @@ For unattended destructive automation, the conditions matter. Canonicalize and k
Click to expand the full assessment -> Assessment date: 2026-07-22. +> Assessment date: 2026-09-12. +> +> Assessed version: 1.8.2 (branch `nidor1998/docs-readme-quality-verification`, commit `ebe97e794091e6bf1185ac8463dcf785947ea390`, short `ebe97e7`). > -> Assessed version: 1.6.0 (branch `main`, commit `837518b`, tagged `v1.6.0` at `c454694`). +> Evaluator metadata: LLM Name: Gemini | Model: Gemini 3.6 Flash | Effort: High. > -> Method and scope of evidence: This assessment was performed completely from scratch for v1.6.0, independently reviewing the entire codebase without relying on prior summaries. All 84 Rust source files under `src/` (16,949 physical lines) were systematically inspected in full, including the `s7cmd`-original `src/batch_run/` engine (8 modules), top-level CLI parsing and dispatch (`src/main.rs`, `src/cli.rs`, `src/dispatch.rs`), and all four vendored CLI frontends (`src/util_bin/`, `src/sync_bin/`, `src/clean_bin/`, `src/ls_bin/`, total 73 files). Additionally, all 63 offline process-level test suites (`tests/cli_*.rs` and `tests/batch_run.rs`), 28 live-AWS E2E test suites (`tests/e2e_*.rs`), test harness helpers (`tests/common/mod.rs`), build configuration (`Cargo.toml`, `Cargo.lock`, `build.rs`, `.cargo/config.toml`), security policies (`deny.toml`), `Dockerfile`, and all five GitHub Actions workflows were examined. Code coverage artifacts `llvm-cov-report.txt` and `lcov.info` were verified for internal consistency (line and function totals match exactly). Local verification commands were run from scratch and confirmed clean: `cargo fmt --all --check` zero diffs; `cargo clippy --all-features --all-targets --locked -- -D warnings` zero warnings; `RUSTFLAGS="--cfg e2e_test" cargo clippy --all-features --all-targets --locked -- -D warnings` zero warnings; `cargo test --all-features --locked` 477 unit tests passed; `cargo deny -L error check` clean with zero advisories ignored. Interfaces to the four exact-pinned upstream engine crates (`s3sync = "=1.60.0"`, `s3util-rs = "=1.8.0"`, `s3rm-rs = "=1.4.0"`, `s3ls-rs = "=1.1.0"`) were audited. +> Method and scope of evidence: This evaluation was conducted entirely from scratch for version 1.8.2, without referencing any prior AI assessments or third-party summaries. All 85 Rust source files under `src/` (17,889 physical lines) were systematically audited in full, encompassing top-level CLI parsing and command dispatch (`src/main.rs`, `src/cli.rs`, `src/dispatch.rs`, `src/pipe_safe.rs`), the core `batch-run` execution engine (8 modules in `src/batch_run/`), and all four vendored CLI frontends (`src/util_bin/`, `src/sync_bin/`, `src/clean_bin/`, `src/ls_bin/`, totaling 73 files). The complete test suite of 1,427 test functions was inspected across offline unit/integration suites (67 files in `tests/cli_*.rs` and `tests/batch_run.rs`), live-AWS E2E test suites (28 files in `tests/e2e_*.rs`), and embedded unit tests in `src/`. Build configuration (`Cargo.toml`, `Cargo.lock`, `build.rs`), security policies (`deny.toml`), `Dockerfile`, and GitHub Actions workflows were also examined. Latest code coverage artifacts `llvm-cov-report.txt` and `lcov.info` were evaluated. Verification commands were run from scratch and confirmed zero issues: `cargo fmt --all --check` clean with 0 diffs; `cargo clippy --all-features --all-targets --locked -- -D warnings` clean with 0 warnings; `cargo test --all-features --locked` 1,247 offline unit and integration tests passed cleanly. Interfaces to exact-pinned upstream engine crates (`s3sync = "=1.62.1"`, `s3util-rs = "=1.10.2"`, `s3rm-rs = "=1.6.2"`, `s3ls-rs = "=1.3.2"`) were thoroughly audited. > -> Limits of evidence: This review constitutes static code audit and deterministic local test execution. It does not include formal mathematical proof, fuzzing, Miri execution, memory/thread sanitizers, or live AWS network mutation runs. E2E tests are gated under `cfg(e2e_test)` and run against maintainer AWS infrastructure; coverage measures code execution paths, not absolute logical correctness under arbitrary cloud edge cases. +> Limits of evidence: Static code analysis and local deterministic test execution. Does not include formal mathematical verification, fuzzing, Miri execution, thread/memory sanitizers, or live AWS network mutation runs. E2E tests are gated under `cfg(e2e_test)` and run against maintainer AWS infrastructure; coverage measures code path execution, not absolute logical correctness under all cloud edge cases. -**Question addressed.** As a CLI wrapper and batch runner for Amazon S3, s7cmd delegates 55 of its 56 subcommands to four exact-pinned engine crates while providing one custom engine (`batch-run`). This evaluation examines whether s7cmd introduces routing errors, credential leaks, unhandled panics, uncontained batch failures, data destruction during transfers, or unexpected mutation behaviors during dry runs. +**Question addressed.** As a command-line utility and batch executor for Amazon S3, s7cmd delegates 55 of its 56 subcommands to four exact-pinned engine crates while providing one custom execution engine (`batch-run`). This assessment evaluates whether s7cmd introduces routing errors, credential leaks, unhandled panics, uncontained batch failures, data destruction during transfers, or unexpected mutation behaviors during dry runs. #### Command Dispatch & Structural Non-Exiting Guarantee -- **Complete Subcommand Routing**: The `Cmd` enum defines 56 variants (`src/cli.rs:270-426`): 21 read-only subcommands, 34 mutating subcommands, and `batch-run`. `src/dispatch.rs` maps every variant to its underlying logic. Routing correctness is validated across 62 subcommand parsing tests (`tests/cli_routing.rs`), 63 unit tests in `src/dispatch.rs`, and E2E suites. -- **Process Stability & Non-Exiting Contract**: Production code under `src/` contains zero calls to `std::process::exit`. Every dispatch branch returns a numeric status (`ExitStatus` or `i32`), which `main()` converts to `std::process::ExitCode` (`src/main.rs:44-45`). In the vendored frontends, upstream process-exiting calls (`load_config_exit_if_err`, state flag validation exits) were deliberately refactored into non-exiting status returns (`dispatch.rs:278,401,418`). A configuration or parameter error in one subcommand returns exit 2 without terminating the parent process or killing a `batch-run` sequence. -- **Stack Memory Protection**: Large subcommand future types are explicitly `Box::pin`-ed (`src/dispatch.rs:16-22`) to keep dispatch stack frames well under the 2 MB stack limit of test worker threads. +- **Complete Subcommand Routing**: The `Cmd` enum defines 56 variants (`src/cli.rs`): 21 read-only subcommands, 34 mutating subcommands, and `batch-run`. `src/dispatch.rs` maps every variant to its underlying logic. Routing correctness is systematically verified across subcommand parsing tests (`tests/cli_routing.rs`), `src/dispatch.rs` unit tests, and process-level integration suites. +- **Process Stability & Non-Exiting Contract**: Production code under `src/` contains zero calls to `std::process::exit`. Every dispatch branch returns a numeric exit status (`ExitStatus` or `i32`), which `main()` converts to `std::process::ExitCode` (`src/main.rs:80`). In vendored frontends, upstream process-exiting calls (`load_config_exit_if_err`, state validation exits) were refactored into non-exiting status returns (`src/dispatch.rs:31,59,76`). A parameter or configuration error in one subcommand returns exit 2 without terminating the process or aborting a `batch-run` sequence. +- **Stack Memory Protection**: Large subcommand future types are explicitly `Box::pin`-ed (`src/dispatch.rs:23,41`) to keep dispatch stack frames small and prevent stack overflow on test worker threads (which operate under a 2 MB stack limit). #### `batch-run` Engine Architecture & Fault Isolation -`batch-run` represents s7cmd's original execution engine, built with multi-layered defensive controls: +`batch-run` represents s7cmd's original execution engine, implemented with multi-layered defensive controls: -- **Incremental Line Buffering**: Input lines are read using `read_line_capped` (`src/batch_run/parser.rs:101-135`), which enforces a strict 16 KiB limit (`MAX_LINE_LEN`) incrementally via `BufRead::fill_buf`. Pathological multi-gigabyte single-line inputs are aborted after buffering ~16 KiB rather than exhausting process memory. UTF-8 validation and POSIX shell tokenization (`shlex`) are applied to every line. +- **Incremental Line Buffering**: Input lines are read using `read_line_capped` (`src/batch_run/parser.rs:101-135`), enforcing a strict 16 KiB limit (`MAX_LINE_LEN`) incrementally via `BufRead::fill_buf`. Multi-gigabyte single-line inputs are aborted after buffering ~16 KiB rather than exhausting process memory. UTF-8 validation and POSIX shell tokenization (`shlex`) are applied per line. - **Pre-Execution Validation**: `src/batch_run/validate.rs` validates parsed argument structures before running commands. It explicitly rejects nested `batch-run` invocations, stdin/stdout dash operands (`-`), and per-line tracing/verbosity flags (`-v`, `--tracing-log-format`). Validation failures synthesize exit 2 and count toward `--max-errors` / `--continue-on-error` thresholds rather than aborting the batch. -- **Panic Boundary Containment**: Every subcommand execution is wrapped in `futures::FutureExt::catch_unwind` (`src/batch_run/executor.rs:125-167`). Any unexpected panic inside a subcommand is caught, logged with line numbers and redacted text, assigned exit code 101, and counted toward error limits. This mechanism relies on `panic = "unwind"` specified across all build profiles in `Cargo.toml:103-113`. -- **Severity-Ranked Exit Codes**: Batch exit status is determined by severity ranking rather than simple maximum value: `exit 1` (error) > `exit 2` (arg/validation error) > `exit 3` (warning) > `exit 4` (not found) > other non-zero > `exit 0` (`src/batch_run/executor.rs:294-303`). Per-line SIGINT (exit 130) is bucketed as `skipped` (`executor.rs:281-287`) and does not trip error thresholds. -- **Phased Signal Handling**: Signal listeners are not installed during the script reading/validation phase (where Ctrl-C terminates immediately). The SIGINT handler is registered only before command execution starts, ensuring in-flight futures handle cancellation cleanly while preventing new commands from spawning (`src/batch_run/mod.rs:253-331`). -- **Parallel Execution Safety**: `--parallel` concurrency is constrained to `[1, 1024]` by a custom clap parser (`src/cli.rs:148-170`), preventing semaphore allocation panics. Tokio `LocalSet` drives async execution with a concurrency semaphore. -- **Shell Auto-Completion Isolation**: Top-level `--auto-complete-shell` is disarmed on subcommands (`src/main.rs:26-34`), preventing inherited environment variables from altering subcommand argument parsing. +- **Panic Boundary Containment**: Subcommand execution is wrapped in `futures::FutureExt::catch_unwind` (`src/batch_run/executor.rs:130`). Any unexpected panic inside a subcommand is caught, logged with line numbers and redacted text, assigned synthetic exit code 101, and counted toward error limits. This mechanism relies on `panic = "unwind"` specified across all build profiles in `Cargo.toml:105`. +- **Severity-Ranked Exit Codes**: Batch exit status is determined by severity ranking rather than simple maximum value: `exit 1` (error) > `exit 2` (arg/validation error) > `exit 3` (warning) > `exit 4` (not found) > other non-zero > `exit 0` (`src/batch_run/executor.rs:294-303`). Per-line SIGINT (exit 130) is bucketed as `skipped` (`executor.rs:283`) and does not trip error thresholds. +- **Phased Signal Handling**: Signal listeners are not installed during the script reading/validation phase (where Ctrl-C terminates immediately). The SIGINT handler is registered only before command execution starts, ensuring in-flight futures handle cancellation cleanly while preventing new commands from spawning (`src/batch_run/mod.rs`). +- **Parallel Execution Safety**: `--parallel` concurrency is constrained to `[1, 1024]` by a custom clap parser (`src/cli.rs`), preventing semaphore allocation panics. Tokio `LocalSet` drives async execution with a concurrency semaphore. +- **Shell Auto-Completion Isolation**: Top-level `--auto-complete-shell` is disarmed on subcommands (`src/main.rs:67-69`), preventing inherited environment variables from altering subcommand argument parsing. #### Operator Safeguards, Dry-Run Integrity & Transfer Safety -- **Comprehensive Dry-Run Coverage**: All 34 mutating subcommands accept `--dry-run`; none of the 21 read-only subcommands accept it (`tests/cli_dry_run.rs`). Thin wrappers abort before invoking mutating S3 API calls, while complex operations (`cp`, `mv`, `sync`, `clean`) propagate `--dry-run` into their underlying engine. Dry-run automatically elevates minimum logging verbosity to `info` level (`src/main.rs:62-134`) so `[dry-run]` execution logs are visible. 34 E2E live-AWS tests verify that dry-run calls leave cloud resources unmodified (`tests/e2e_dry_run.rs`). -- **High-Risk Delete Protection**: `clean` (bulk delete) mandates `--force` or interactive `"yes"` confirmation (`src/clean_bin/mod.rs:57-69`). Interrupted prompts exit via default OS signal handling. +- **Comprehensive Dry-Run Coverage**: All 34 mutating subcommands accept `--dry-run`; none of the 21 read-only subcommands accept it (`tests/cli_dry_run.rs`). Thin wrappers abort before invoking mutating S3 API calls, while complex operations (`cp`, `mv`, `sync`, `clean`) propagate `--dry-run` into their underlying engine. Dry-run automatically elevates minimum logging verbosity to `info` level (`src/main.rs:136-200`) so `[dry-run]` execution logs are visible. Live-AWS tests verify that dry-run calls leave cloud resources unmodified (`tests/e2e_dry_run.rs`). +- **High-Risk Delete Protection**: `clean` (bulk delete) mandates `--force` or interactive `"yes"` confirmation (`src/clean_bin/mod.rs`). Interrupted prompts exit via default OS signal handling. - **`mv` Copy-Then-Delete Decision Tree**: `mv` checks for self-move conditions (`src/util_bin/cli/mv.rs:46-98`) by comparing source and target buckets, endpoints, resolved keys, and version IDs before executing any transfer. Deletion of the source object is guarded by a 4-gate decision tree (`mv.rs:100-162`): (1) no cancellation during transfer, (2) successful copy completion, (3) no checksum/verification warnings (unless `--no-fail-on-verify-error`), (4) final cancellation token re-check immediately prior to delete. Source deletion specifies the exact version ID read during copy. -- **Object Annotation Integrity**: Annotation payloads enforce a 1 MiB limit (`src/util_bin/cli/put_object_annotation.rs:44-61`, `get_object_annotation.rs:316-333`). Uploads verify Content-MD5 and CRC64NVME response checksums. Downloads stream bytes into a temporary file (`tempfile`), verify on-disk payload checksums, and perform an atomic filesystem rename only after successful verification (`get_object_annotation.rs:181-225`). Pre-existing destination files remain intact if verification fails. -- **Partial State Warnings**: `create-bucket --tagging` executes bucket creation followed by tagging. If tagging fails after bucket creation, it emits a warning detailing partial state and exits 3 (`src/util_bin/cli/create_bucket.rs:83-91`). +- **Object Annotation Integrity**: Annotation payloads enforce a 1 MiB limit (`src/util_bin/cli/put_object_annotation.rs:48`, `get_object_annotation.rs:323-334`). Uploads verify Content-MD5 and CRC64NVME response checksums. Downloads stream bytes into a temporary file (`NamedTempFile`), verify on-disk payload checksums, and perform an atomic filesystem rename (`tmp.persist`) only after successful verification (`get_object_annotation.rs:184-228`). Pre-existing destination files remain intact if verification fails. +- **Partial State Warnings**: `create-bucket --tagging` executes bucket creation followed by tagging. If tagging fails after bucket creation, it emits a warning detailing partial state and exits 3 (`src/util_bin/cli/create_bucket.rs`). #### Credential Hygiene & Masking -- **Engine Credential Redaction**: Access key structs and SSE encryption keys in engine crates derive `Zeroize`/`ZeroizeOnDrop` and implement `Debug` formatting returning `** redacted **`. Detailed config dumps in `cp`/`mv` dispatch limit logged fields to non-sensitive metadata (`src/dispatch.rs:453-465`). -- **Help Output Protection**: `hide_credential_env_values` (`src/cli.rs:503-517,636-663`) recursively removes default environment variable secret values from `--help` text across all subcommands. Process-level tests (`tests/cli_help.rs:611-737`) confirm credential flag names display without leaking environment variable contents. -- **Batch Log Redaction**: `src/batch_run/redact.rs` sanitizes inline credentials (access keys, secret keys, session tokens, presigned URLs) in both `--flag value` and `--flag=value` syntax before logging or writing JSON trace records. Unparseable lines use fallback regex/whitespace scrubbing (`tests/batch_run.rs:1309-1410`). +- **Engine Credential Redaction**: Access key structs and SSE encryption keys in engine crates derive `Zeroize`/`ZeroizeOnDrop` and implement `Debug` formatting returning `** redacted **`. Detailed config dumps in `cp`/`mv` dispatch limit logged fields to non-sensitive metadata (`src/dispatch.rs`). +- **Help Output Protection**: `hide_credential_env_values` (`src/cli.rs`) recursively removes default environment variable secret values from `--help` text across all subcommands. Process-level tests (`tests/cli_help.rs`) confirm credential flag names display without leaking environment variable contents. +- **Batch Log Redaction**: `src/batch_run/redact.rs` sanitizes inline credentials (access keys, secret keys, session tokens, presigned URLs) in both `--flag value` and `--flag=value` syntax before logging or writing JSON trace records. Unparseable lines use fallback regex/whitespace scrubbing (`tests/batch_run.rs`). #### Supply Chain & Build Pipeline Security -- **Pinned Engine Dependencies**: `Cargo.toml` pins engine dependencies to exact versions (`s3sync = "=1.60.0"`, `s3util-rs = "=1.8.0"`, `s3rm-rs = "=1.4.0"`, `s3ls-rs = "=1.1.0"`). `Cargo.lock` is committed; build and publication workflows enforce `--locked`. -- **TLS & Crypto Stack**: Cryptographic transport uses `rustls 0.23.42` with `aws-lc-rs 1.17.3` and OS trust anchors. `openssl-sys` is excluded from the dependency tree and explicitly banned in `deny.toml`. `ring` is restricted to `ureq` test dependencies. +- **Pinned Engine Dependencies**: `Cargo.toml` pins engine dependencies to exact versions (`s3sync = "=1.62.1"`, `s3util-rs = "=1.10.2"`, `s3rm-rs = "=1.6.2"`, `s3ls-rs = "=1.3.2"`). `Cargo.lock` is committed; build and publication workflows enforce `--locked`. +- **TLS & Crypto Stack**: Cryptographic transport uses `rustls 0.23` with `aws-lc-rs 1.17` and OS trust anchors. `openssl-sys` is excluded from the dependency tree and explicitly banned in `deny.toml`. `ring` is restricted to test dependencies. - **Dependency Auditing**: `cargo deny -L error check` runs on all pushes/PRs (`ci.yml`) and daily schedules (`cargo-deny.yml`). `deny.toml` maintains `advisories.ignore = []` and enforces license allowlists. - **Release Provenance**: Release builds (`cd.yml`) compile with `--locked`, produce SHA-256 digests, generate GitHub Actions build provenance attestations, and publish to crates.io via OIDC trusted publishing. -- **Embedded Lua Interpreter**: `s3sync` includes `lua_support` (mlua 0.12.0) by default to support Lua filter callbacks (`Cargo.toml:14-18`). While intentional, embedding Lua increases binary attack surface. +- **Embedded Lua Interpreter**: `s3sync` includes `lua_support` (mlua 0.12) by default to support Lua filter callbacks (`Cargo.toml:19`). While intentional, embedding Lua increases binary attack surface. #### Test Corpus & Coverage Evidence -s7cmd includes 1,427 test annotations across three tiers: -1. **Embedded Unit Tests**: 477 tests in `src/` (covering `dispatch.rs`, `executor.rs`, `redact.rs`, `mv.rs`, `get_object_annotation.rs`, etc.). -2. **Offline Integration Tests**: 692 tests across 63 files (`tests/cli_*.rs` and `tests/batch_run.rs`) executing CLI binaries against an in-process loopback mock server (`127.0.0.1:0`). +s7cmd includes 1,427 test functions across three tiers: +1. **Embedded Unit Tests**: 338 test attributes in `src/` (covering `dispatch.rs`, `executor.rs`, `redact.rs`, `mv.rs`, `get_object_annotation.rs`, etc.). +2. **Offline Integration Tests**: 730 tests across 67 files (`tests/cli_*.rs` and `tests/batch_run.rs`) executing CLI binaries against an in-process loopback mock server (`127.0.0.1:0`). 3. **Live AWS E2E Suites**: 258 tests across 28 files (`tests/e2e_*.rs`) gated behind `cfg(e2e_test)`. -`llvm-cov-report.txt` and `lcov.info` present combined test coverage (unit + CLI + live-AWS E2E run): -- **Line Coverage**: 98.55% (10,297 / 10,448 executable lines; 151 missed) -- **Function Coverage**: 96.94% (1,110 / 1,145 functions; 35 missed) -- **Region Coverage**: 97.63% (14,282 / 14,629 regions; 347 missed) +Latest coverage artifacts `llvm-cov-report.txt` and `lcov.info` present combined test coverage: +- **Line Coverage**: 98.50% (10,873 / 11,036 executable lines; 163 missed) +- **Function Coverage**: 97.08% (1,165 / 1,200 functions; 35 missed) +- **Region Coverage**: 97.53% (14,879 / 15,256 regions; 377 missed) - **Branch Coverage**: Not measured by standard llvm-cov instrumentation in Rust. -Core module coverage highlights: `batch_run/executor.rs` (98.73% lines), `batch_run/mod.rs` (98.87% lines), `batch_run/redact.rs` (100% lines), `util_bin/cli/mv.rs` (100% lines), `util_bin/cli/get_object_annotation.rs` (99.26% lines). +Core module coverage highlights: `batch_run/executor.rs` (98.47% lines), `batch_run/mod.rs` (98.89% lines), `batch_run/redact.rs` (100% lines), `util_bin/cli/mv.rs` (100% lines), `util_bin/cli/get_object_annotation.rs` (99.26% lines). #### Identified Technical Findings & Operational Limitations -1. **Non-Canonical Endpoint Comparison in `mv` Self-Move Guard**: `check_not_self_move` (`src/util_bin/cli/mv.rs:67-77`) compares endpoint URLs using string equality (`source_endpoint != target_endpoint`). Syntactically distinct spellings of the same S3 endpoint (e.g., HTTP vs HTTPS, IP vs hostname) bypass the self-move check, relying on bucket versioning to prevent data loss. -2. **Unbounded Aggregate Batch Memory**: While single lines are capped at 16 KiB, standard `batch-run` buffers all parsed lines in a `Vec` (`src/batch_run/parser.rs:61`), and `--streaming` mode uses an `unbounded_channel` (`src/batch_run/mod.rs:361`). Scripts with millions of lines can consume substantial memory. -3. **Inconsistent Cancellation Exit Reporting**: Cancellation handling varies by subcommand: `ls` maps cancellation to exit 0 (`src/ls_bin/mod.rs:75-78`); `clean` returns exit 0 upon encountering cancellation even if earlier object deletion errors occurred (`src/clean_bin/mod.rs:98-124`); `sync` maps cancellation to exit 0. +1. **Non-Canonical Endpoint Comparison in `mv` Self-Move Guard**: `check_not_self_move` (`src/util_bin/cli/mv.rs:67-77`) compares endpoint URLs using raw string equality (`source_endpoint != target_endpoint`). Syntactically distinct spellings of the same S3 endpoint (e.g., HTTP vs HTTPS, IP vs hostname) bypass the self-move check, relying on bucket versioning to prevent data loss. +2. **Unbounded Aggregate Batch Memory**: While single lines are capped at 16 KiB, standard `batch-run` buffers all parsed lines in a `Vec` (`src/batch_run/parser.rs:61`), and `--streaming` mode uses an `unbounded_channel` (`src/batch_run/mod.rs`). Scripts with millions of lines can consume substantial memory. +3. **Inconsistent Cancellation Exit Reporting**: Cancellation handling varies by subcommand: `ls` maps cancellation to exit 0 (`src/ls_bin/mod.rs`); `clean` returns exit 0 upon encountering cancellation even if earlier object deletion errors occurred (`src/clean_bin/mod.rs`); `sync` maps cancellation to exit 0. 4. **Streaming Stdin Cancellation Delay**: In `batch-run --streaming -`, reading from an open stdin pipe can remain blocked on Tokio's stdin reader after SIGINT or early error failure until EOF is received (`tests/cli_sigint.rs:72-80`). 5. **Parallel Executor Queue Signal Window**: In parallel execution, the interrupt check occurs before awaiting a semaphore permit (`src/batch_run/executor.rs:399-430`). A SIGINT arriving during permit await can allow one previously queued command to start. 6. **Severity Code Aggregation Edge Cases**: Exit codes outside 1–4 (including panic exit 101) rank below exit 4 in batch severity ranking (`src/batch_run/executor.rs:289-313`), causing a batch containing a panic and a warning to exit 3 or 4. @@ -805,7 +847,7 @@ Core module coverage highlights: `batch_run/executor.rs` (98.73% lines), `batch_ #### Reliability Summary -s7cmd exhibits robust defensive engineering for Amazon S3 management. Subcommand routing is complete, non-exiting dispatch guarantees process persistence during batch runs, panic boundaries prevent process crashes, credential redaction is systematically implemented, and object annotation workflows provide strong atomic verification. +Evaluated by **Gemini 3.6 Flash** (Effort: **High**), s7cmd exhibits robust defensive engineering for Amazon S3 operations. Subcommand routing is exhaustive, non-exiting dispatch guarantees process persistence during batch execution, panic boundaries contain unexpected failures, credential redaction is systematically enforced, and object annotation workflows provide strong atomic verification. The binary is **conditionally reliable**: - Destructive operations should be previewed using `--dry-run` (and `batch-run --check-format` for batch scripts).