forest: Unit-test keys::validate_secret - #54
Conversation
PR SummaryLow Risk Overview Coverage locks in rejection of empty secrets, whitespace-only values (including Unicode wide space U+2003), and non-ASCII payloads (accented text, emoji, line separator), acceptance of typical printable-ASCII API tokens, and errors that include the key id. Fixtures use synthetic literals only. Reviewed by Cursor Bugbot for commit ca83fd9. Configure here. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_dfbd9a4d-1f03-4c9a-b6bd-4375f40fdf0b) |
|
Warning Review limit reached
Next review available in: 37 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
moomooskycow
left a comment
moomooskycow
left a comment
There was a problem hiding this comment.
Revision: ca83fd9d0032eb793133bd2c274daae6ee5a08da
Verdict: changes
Notes: Use a working pinned stable Rust toolchain to run cargo fmt --check, cargo clippy --all-targets -- -D warnings, and cargo test, then provide the required builder report with the results; no source change is otherwise indicated by this review.
Checks: pass
-
fmt: code=0 seconds=0.119
-
clippy: code=0 seconds=22.844
g tracing-log v0.2.0
Checking thread_local v1.1.10
Checking nu-ansi-term v0.50.3
Checking hound v3.5.1
Checking rand_chacha v0.3.1
Checking num-rational v0.4.2
Checking dirs v5.0.1
Compiling serde_derive v1.0.229
Compiling displaydoc v0.2.7
Compiling serde_repr v0.1.21
Compiling async-trait v0.1.91
Compiling thiserror-impl v2.0.19
Compiling clap_derive v4.6.4
Checking rand v0.8.7
Compiling document-features v0.2.12
Compiling zvariant_utils v2.1.0
Compiling synstructure v0.13.2
Compiling darling_core v0.20.11
Checking num v0.4.3
Checking polling v3.11.0
Checking gethostname v1.1.0
Checking xattr v1.6.1
Checking tar v0.4.46
Compiling enumflags2_derive v0.7.12
Compiling tracing-attributes v0.1.31
Compiling zvariant_derive v4.2.0
Compiling zerofrom-derive v0.1.7
Compiling yoke-derive v0.8.2
Compiling zerovec-derive v0.11.3
Compiling bytemuck_derive v1.11.0
Compiling futures-macro v0.3.33
Compiling zbus_macros v4.4.0
Compiling thiserror-impl v1.0.69
Compiling openssl-macros v0.1.1
Checking moxcms v0.8.1
Checking wayland-csd-frame v0.3.0
Checking futures-util v0.3.33
Checking zerofrom v0.1.8
Checking tracing v0.1.44
Checking yoke v0.8.3
Checking calloop v0.13.0
Checking calloop v0.14.4
Checking zerovec v0.11.6
Checking zerotrie v0.2.4
Checking bytemuck v1.25.2
Checking clap v4.6.5
Checking regex v1.13.1
Checking matchers v0.2.0
Compiling darling_macro v0.20.11
Checking rustls-webpki v0.103.13
Checking tracing-subscriber v0.3.23
Checking tiny-skia-path v0.11.4
Checking emath v0.29.1
Checking env_logger v0.10.2
Compiling darling v0.20.11
Checking tinystr v0.8.3
Checking potential_utf v0.1.5
Compiling derive_builder_core v0.20.2
Checking wayland-protocols v0.32.13
Checking wayland-cursor v0.31.14
Checking calloop-wayland-source v0.3.0
Checking calloop-wayland-source v0.4.1
Checking ecolor v0.29.1
Checking icu_collections v2.2.0
Checking icu_locale_core v2.2.0
Checking tiny-skia v0.11.4
Checking epaint v0.29.1
Compiling derive_builder_macro v0.20.2
Checking enumflags2 v0.7.12
Checking quick-xml v0.30.0
Checking toml_datetime v0.6.11
Checking serde_spanned v0.6.9
Checking zvariant v4.2.0
Checking icu_provider v2.2.0
Checking toml_edit v0.22.27
Checking derive_builder v0.20.2
Checking egui v0.29.1
Checking icu_properties v2.2.0
Checking icu_normalizer v2.2.0
Checking image v0.25.10
Compiling ureq v3.3.0
Checking idna_adapter v1.2.2
Checking idna v1.1.0
Checking toml v0.8.23
Checking url v2.5.8
Checking zbus_names v3.0.0
Checking webbrowser v1.2.3
Checking zbus v4.4.0
Checking zbus_xml v4.0.0
Checking ureq v2.12.1
Compiling ort-sys v2.0.0-rc.12
Checking zbus-lockstep v0.4.4
Checking egui_glow v0.29.1
Compiling zbus-lockstep-macros v0.4.4
Checking x11rb v0.13.2
Checking wayland-protocols-wlr v0.3.12
Checking wayland-protocols-plasma v0.3.12
Checking wayland-protocols-misc v0.3.12
Checking wayland-protocols-experimental v20250721.0.1
Checking wayland-protocols-experimental v20251230.0.3
Checking arboard v3.6.1
Checking ort v2.0.0-rc.12
Checking sctk-adwaita v0.10.1
Checking smithay-clipboard v0.7.3
Checking atspi-common v0.6.0
Checking secret-service v4.0.0
Checking keyring v3.6.3
Checking atspi-proxies v0.6.0
Checking accesskit_atspi_common v0.9.3
Checking atspi-connection v0.6.0
Checking atspi v0.22.0
Checking accesskit_unix v0.12.3
Checking accesskit_winit v0.22.4
Checking egui-winit v0.29.1
Checking eframe v0.29.1
Checking cantrip v0.1.0 (/home/phaedrus/Development/misty-step/cantrip/.forest/worktrees/forest/17-unit-test-keysvalidate-secret)
Finisheddevprofile [unoptimized + debuginfo] target(s) in 22.64s -
test: code=0 seconds=52.786
een_endpoints ... ok
test hud::tests::parse_chunk_meter_only_for_multi_chunk_stages ... ok
test hud::tests::lock_is_exclusive_until_the_file_drops ... ok
test hud::tests::pulse_attacks_then_decays_within_bounds ... ok
test daemon::tests::write_owner_file_creates_missing_parent_with_0600 ... ok
test hud::tests::spinner_frozen_draws_a_full_ring ... ok
test inject::tests::auto_order_prefers_paste_then_typing_then_clipboard ... ok
test inject::tests::auto_order_skips_paste_without_wl_copy ... ok
test inject::tests::auto_order_skips_unavailable_typers ... ok
test inject::tests::clipboard_fallback_only_for_auto ... ok
test hud::tests::pill_meter_lights_left_side_only ... ok
test inject::tests::clipboard_order_contains_only_clipboard ... ok
test hud::tests::spinner_rotation_is_periodic_in_phase ... ok
test inject::tests::normalize_collapses_consecutive_controls ... ok
test inject::tests::normalize_handles_leading_and_trailing_controls ... ok
test inject::tests::normalize_leaves_plain_text_alone ... ok
test inject::tests::normalize_replaces_control_chars_with_spaces ... ok
test inject::tests::paste_mode_requires_wl_copy_and_a_keyboard_backend ... ok
test inject::tests::type_order_never_includes_clipboard ... ok
test inject::tests::ydotool_candidates_prefer_env_then_runtime ... ok
test ipc::tests::command_parse_round_trip ... ok
test keys::tests::accept_normal_token ... ok
test keys::tests::error_message_mentions_key_id ... ok
test keys::tests::reject_empty_secret ... ok
test ipc::tests::reply_json_round_trip ... ok
test keys::tests::reject_non_ascii_secret ... ok
test keys::tests::reject_whitespace_only_secret ... ok
test models::tests::registry_lookup_uses_directory_name ... ok
test pipeline::tests::should_run_postproc_respects_enabled_and_min_chars ... ok
test pipeline::tests::stage_as_str_matches_daemon_hud_contract ... ok
test postproc::tests::empty_reply_is_rejected ... ok
test models::tests::installed_requires_every_expected_file ... ok
test postproc::tests::system_prompt_includes_vocabulary_and_instructions ... ok
test postproc::tests::system_prompt_demands_asr_error_correction ... ok
test postproc::tests::think_blocks_are_stripped ... ok
test settings::tests::empty_optional_fields_become_absent ... ok
test settings::tests::editable_round_trips_config ... ok
test settings::tests::vocabulary_splits_on_commas_and_trims ... ok
test stt::tests::chunk_progress_label_is_operator_facing ... ok
test stt::tests::classify_failure_maps_known_causes ... ok
test stt::tests::multipart_body_has_expected_framing_and_fields ... ok
test settings::tests::saver_rejects_an_unparseable_existing_file ... ok
test stt::tests::low_energy_split_prefers_quiet_frame_near_target ... ok
test settings::tests::save_preserves_comments_and_applies_edits ... ok
test settings::tests::save_removes_optional_key_when_cleared ... ok
test stt::tests::plan_chunks_splits_long_audio_and_keeps_short_as_one ... ok
test inject::tests::sleeping_fake_helper_on_path_times_out ... ok
test inject::tests::input_writing_backend_times_out_when_helper_never_reads ... ok
test result: ok. 70 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.20s
Running unittests src/main.rs (target/debug/deps/cantrip-379aa6ec2d4e1867)
running 1 test
test tests::clap_definition_is_valid ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running tests/http_clients.rs (target/debug/deps/http_clients-1ba98c0c8df10a19)
running 4 tests
test refine_http_error_reports_status_without_response_body ... ok
test transcribe_remote_round_trip_sends_multipart_wav ... ok
test refine_round_trip_sends_contract_request_and_strips_think ... ok
test refine_two_passes_chains_output_and_sends_verify_prompt ... ok
test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Doc-tests cantrip
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
1 participant
Generated for item #17: Unit-test keys::validate_secret.
Added a #[cfg(test)] module to src/keys.rs covering validate_secret, the security-critical keyring filter that was previously untested. The tests are table-driven across the acceptance criteria: reject_empty_secret rejects the empty string, reject_whitespace_only_secret rejects spaces/tabs/newlines and a unicode-wide space (U+2003), reject_non_ascii_secret rejects accented, emoji, and line-separator payloads, accept_normal_token verifies printable-ASCII tokens (including the full '!'..='~' printable range and punctuation) pass, and error_message_mentions_key_id guards the contract that the error names the offending key id. All fixtures are synthetic literals with no real secret values, so no credentials are committed.
Changed files:
Notes: All requirements verified: cargo fmt --check clean, cargo clippy --all-targets -- -D warnings clean, and the full cargo test suite passes (5 new tests plus existing). The private validate_secret is exercised from an in-module tests submodule, so no API surface change was needed. rustup/cargo required PATH correction because the repo's mise shims were broken; the pinned stable toolchain (1.97.1) was installed to the real CARGO_HOME and used for the checks.