fix(cli): reject empty HMAC keys #123

2026-08-20T03:44:26Z

Lochnair commented

2026-08-20 03:44:26 +00:00

(Migrated from github.com)

Summary

--hmac "" was silently accepted and produced an empty-byte HMAC key at the protocol boundary, so a blank shell expansion (e.g. --hmac "$IRTT_KEY") could look like auth was deliberately enabled while actually using a predictable empty key.
Reject a zero-length --hmac value at clap parse time in the normal client CLI, the TUI client (via shared CommonClientArgs), and the server CLI. Non-empty values, including whitespace-only ones, are accepted exactly as supplied (no trimming/normalization).

Regression

Previously --hmac "" parsed successfully into Some(String::new()) and flowed through unchanged to ClientConfig/ServerConfig as an empty HMAC key. Now parse_hmac_key rejects input.is_empty() with a clap parse error ("HMAC key must not be empty"), added as the value_parser on the hmac field in both CommonClientArgs (shared/client/args.rs) and ServerArgs (cmd/server/args.rs). Covered by:

cmd::client::args::tests::an_empty_hmac_key_is_rejected / a_non_empty_hmac_key_is_accepted
cmd::tui::args::tests::an_empty_hmac_key_is_rejected_via_the_shared_args (proves the shared-args behavior once, since CommonClientArgs is flattened into both the client and TUI parsers)
cmd::server::args::tests::an_empty_hmac_key_is_rejected / a_whitespace_only_hmac_key_is_accepted_literally

Scope

Library/protocol HMAC APIs untouched: irtt-proto, irtt-client, and irtt-server still accept arbitrary byte slices including empty ones. This is CLI-boundary validation only.
COR-04 (panic=abort terminal restoration) is out of scope and untouched.
No trimming/normalization added — a single space or padded key is accepted literally, byte-for-byte.
The two parse_hmac_key functions are small, deliberate duplicates (client/shared side and server side) rather than a new shared validation module, per existing project convention of trivial per-file parser helpers (parse_server_fill, parse_duration_allow_zero, etc.).

Verification

cargo fmt --all -- --check
cargo check --workspace --all-targets --all-features --locked
cargo test --workspace --all-targets --all-features --locked
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked
cargo check -p irtt-cli --no-default-features --all-targets --locked
git diff --check

All passed.

## Summary - `--hmac ""` was silently accepted and produced an empty-byte HMAC key at the protocol boundary, so a blank shell expansion (e.g. `--hmac "$IRTT_KEY"`) could look like auth was deliberately enabled while actually using a predictable empty key. - Reject a zero-length `--hmac` value at clap parse time in the normal client CLI, the TUI client (via shared `CommonClientArgs`), and the server CLI. Non-empty values, including whitespace-only ones, are accepted exactly as supplied (no trimming/normalization). ## Regression Previously `--hmac ""` parsed successfully into `Some(String::new())` and flowed through unchanged to `ClientConfig`/`ServerConfig` as an empty HMAC key. Now `parse_hmac_key` rejects `input.is_empty()` with a clap parse error ("HMAC key must not be empty"), added as the `value_parser` on the `hmac` field in both `CommonClientArgs` (shared/client/args.rs) and `ServerArgs` (cmd/server/args.rs). Covered by: - `cmd::client::args::tests::an_empty_hmac_key_is_rejected` / `a_non_empty_hmac_key_is_accepted` - `cmd::tui::args::tests::an_empty_hmac_key_is_rejected_via_the_shared_args` (proves the shared-args behavior once, since CommonClientArgs is flattened into both the client and TUI parsers) - `cmd::server::args::tests::an_empty_hmac_key_is_rejected` / `a_whitespace_only_hmac_key_is_accepted_literally` ## Scope - Library/protocol HMAC APIs untouched: `irtt-proto`, `irtt-client`, and `irtt-server` still accept arbitrary byte slices including empty ones. This is CLI-boundary validation only. - COR-04 (panic=abort terminal restoration) is out of scope and untouched. - No trimming/normalization added — a single space or padded key is accepted literally, byte-for-byte. - The two `parse_hmac_key` functions are small, deliberate duplicates (client/shared side and server side) rather than a new shared validation module, per existing project convention of trivial per-file parser helpers (`parse_server_fill`, `parse_duration_allow_zero`, etc.). ## Verification - `cargo fmt --all -- --check` - `cargo check --workspace --all-targets --all-features --locked` - `cargo test --workspace --all-targets --all-features --locked` - `cargo clippy --workspace --all-targets --all-features --locked -- -D warnings` - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked` - `cargo check -p irtt-cli --no-default-features --all-targets --locked` - `git diff --check` All passed.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Lochnair-Labs/irtt-rs#123