Repo hygiene, CI gate hardening, panic-path fixes, and a rand 0.9 build fix#860
Merged
Conversation
Remove a 3.9MB compiled benchmark binary (bench_security), two stale patch files whose changes are already applied to src/executor/mod.rs, a leftover cargo test log, and 799 runtime state snapshots under .rustible_state/ that were never read by tests or source. Ignore all of these going forward. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The file listed npm scripts that have never applied to this cargo project, which misleads agentic tooling into wrong build/test/lint invocations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Point issue links and Cargo.toml repository at adolago/rustible (repo transferred from kernelfirma; Cargo.toml pointed at a third, unrelated URL) - Reconcile WinRM and azure_vm/gcp_compute status with canonical docs/FEATURE_STATUS.md (beta and experimental respectively, not stub/complete) - Soften ROADMAP rollback rows to Beta quality, matching the Remaining Beta Gate section in the same file - Drop the RUSTSEC-2026-0009 audit ignore: the lock already resolves time 0.3.47 (the fixed release) since MSRV moved to 1.88 Progresses #849. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR #859 bumped rand 0.8 -> 0.9 (a breaking release) without touching code, leaving main uncompilable: OsRng moved to fallible TryRngCore, thread_rng/gen/gen_range were renamed, and StdRng::from_entropy was removed. The soft clippy/check gates let it merge; this lands the actual migration: - thread_rng() -> rng(), gen() -> random(), gen_range -> random_range - vault/vars/cli-vault crypto paths now use the rand_core 0.6 OsRng re-exported by aes-gcm's aead, matching what argon2/aes-gcm expect, instead of relying on rand's version coincidentally lining up - signing key generation uses TryRngCore::try_fill_bytes with an explicit panic on OS entropy failure - russh test key helper uses ssh-key's own rand_core OsRng (the 'seeded for reproducibility' comment was wrong: from_entropy was never reproducible) - same renames applied to tests/ and benches/ssh_comparison Progresses #850. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolves the 56 outstanding warnings so the lint gate can be enforced: sort_by_key with Reverse for descending sorts, checked_div for manual zero-guarded division, is_multiple_of, collapsed match arms, and if-let bindings replacing unwrap-after-is_some in the privilege escalation paths (local/ssh/russh/kubernetes), junos commit-confirm, and postgresql socket handling. Bench helpers used only by env-gated ssh2 benches and unit tests get explicit allow(dead_code). Behavior-neutral: identical ordering (stable sorts, tie behavior preserved), identical division results, identical branch conditions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- ci.yml: clippy now runs with --all-targets and -D warnings instead of warning-only on lib/bins. The old comment claimed tests do not compile under feature combinations; the feature_matrix job has been proving otherwise with cargo check --all-targets per bundle. - clippy.toml: allow unwrap/expect/panic in test code so the security workflow's unwrap_used/expect_used scan reports production signal instead of ~4000 test-code hits. - Cargo.toml [lints.clippy]: deny dbg_macro and todo, warn on unimplemented, so debugging leftovers cannot land on main. The two test mocks using unimplemented! switch to descriptive panics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The licenses job previously generated deny.toml on the fly and ran cargo deny with '|| true', so license/ban/source violations could never fail the workflow. The policy file is now version-controlled (same content CI used to generate) and the gate is hard. Advisory checking stays owned by cargo-audit and .cargo/audit.toml; the deny gate covers licenses, bans, and sources to avoid two advisory ignore lists drifting apart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
is_multiple_of, collapsed match guards, and rustfmt normalization in files missed by the previous lint-cleanup commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- inventory/constructed: skip hosts that compose rules removed from the inventory mid-iteration instead of unwrapping the lookup - distributed/types: a pre-epoch system clock now degrades to zeroed timestamps instead of killing the controller; the static default bind address parse documents its infallibility with expect - cli/output: JSON rendering falls back to a serialization-error object instead of panicking mid-output (new json_line/json_pretty helpers replace 18 unwrap/expect sites) - provisioning/resolver: template capture groups degrade gracefully if the regex ever changes shape, instead of unwrapping Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes the remaining alpha-readiness tracker item on vault password hygiene: the library vault already stored its password as SecretString, but the CLI VaultEngine kept a plain String alive for the whole command. The engine now uses SecretString, the password acquisition helpers return SecretString from all three sources (password file, RUSTIBLE_VAULT_PASSWORD, interactive prompt), and transient file-read buffers are zeroized after trimming. No call-site ripple: the constructor takes impl Into<SecretString>. Covered by tests/secrets_zeroization_tests.rs (49 green) and a new SecretString round-trip test in the vault unit suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cargo clippy --fix ran under default features and underscored two bindings whose only uses are behind non-default cfg gates, breaking compilation under those features: - src/cli/commands/state.rs: the `state` path in the Mv and ReplaceProvider arms is consumed only inside #[cfg(feature = "provisioning")]; it had been turned into `state: _`. Restore the binding and reference it in the not-provisioning branch so it stays used in every config. - tests/cloud_integration_tests.rs: `registry` is asserted on only under the aws/azure/gcp gates; it had become `_registry`. Restore it with a statement-level allow(unused_variables). Caught by the aggregate feature-bundle compile (experimental,full-provisioning,api,database) — the CI feature_matrix job mirrors this. Default-features clippy -D warnings stays clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code Review by Qodo
Context used 1. CLAUDE clippy command mismatched
|
CI Feedback 🧐(Feedback updated until commit f4c47ef)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| - `npm run lint` - Linting | ||
| - `cargo check` - Fast typecheck; `cargo build` - Build project | ||
| - `cargo test -- --test-threads=1` - Run tests (CI runs single-threaded) | ||
| - `cargo clippy --lib --bins -- -D warnings` - Lint |
There was a problem hiding this comment.
1. Claude clippy command mismatched 📎 Requirement gap ⚙ Maintainability
CLAUDE.md documents cargo clippy --lib --bins -- -D warnings, but CI now runs `cargo clippy --all-targets -- -D warnings`, so contributors following the documented steps may miss lint failures that block merges.
Agent Prompt
## Issue description
`CLAUDE.md` documents a clippy command (`--lib --bins`) that does not match the merge-gating CI clippy job (`--all-targets`). This breaks the goal of having locally reproducible required checks.
## Issue Context
CI was updated to run `cargo clippy --all-targets -- -D warnings`.
## Fix Focus Areas
- CLAUDE.md[4-7]
- .github/workflows/ci.yml[45-48]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+14
to
+25
| /// Serialize for line-oriented JSON output without panicking; emits a JSON | ||
| /// error object if serialization itself fails. | ||
| fn json_line<T: serde::Serialize>(value: &T) -> String { | ||
| serde_json::to_string(value) | ||
| .unwrap_or_else(|e| format!(r#"{{"error":"serialization failed: {}"}}"#, e)) | ||
| } | ||
|
|
||
| /// Pretty-printing variant of [`json_line`]. | ||
| fn json_pretty<T: serde::Serialize>(value: &T) -> String { | ||
| serde_json::to_string_pretty(value) | ||
| .unwrap_or_else(|e| format!(r#"{{"error":"serialization failed: {}"}}"#, e)) | ||
| } |
There was a problem hiding this comment.
2. Invalid json fallback 🐞 Bug ≡ Correctness
json_line/json_pretty interpolate serde_json::Error into a hand-built JSON string without escaping, so the fallback can become invalid JSON if the error contains quotes/backslashes/newlines. This breaks machine JSON output exactly in the serialization-failure scenario the helpers are meant to handle.
Agent Prompt
### Issue description
`json_line`/`json_pretty` currently build a JSON fallback via `format!(r#"{"error":"... {}"}"#, e)`, inserting the error string unescaped. This can produce invalid JSON (and/or multi-line output) when serialization fails.
### Issue Context
These helpers are used for JSON-mode CLI output, so the fallback must itself always be valid JSON.
### Fix Focus Areas
- Replace the `format!(...)` fallback with a `serde_json::json!({...})` value and serialize that (which guarantees proper escaping).
- Ensure both line-oriented and pretty variants remain single JSON values (and consider keeping the fallback schema stable, e.g. `{ "error": "serialization_failed", "details": "..." }`).
### Fix Focus Areas (code pointers)
- src/cli/output.rs[14-25]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
PR Summary by QodoFix rand 0.9 breakage, harden CI gates, and eliminate panic paths WalkthroughsDescription• Restore builds by migrating rand 0.8 usage to rand 0.9 APIs and RNG traits. • Make clippy and license compliance hard CI gates with checked-in configurations. • Remove production panic paths (JSON output, template resolution, inventory iteration, clock skew). • Clean up repo/docs pointers (repo URL, issue links, and correct Rust build commands). Diagramgraph TD
CI["ci.yml"] --> CLIP["Clippy -D warnings"] --> CODE["Core code"] --> RNG["rand 0.9"]
CLIP --> CLTOML["clippy.toml"]
SEC["security.yml"] --> DENY["cargo-deny gate"] --> DNY["deny.toml"]
CODE --> VAULT["Vault + CLI"]
CODE --> OUT["CLI JSON output"]
subgraph Legend
direction LR
_wf[[Workflow]] ~~~ _cfg["Config file"] ~~~ _mod["Module"]
end
High-Level AssessmentThe following are alternative approaches to this PR: 1. Introduce a small internal RNG facade (crate::crypto::rng)
2. Revert/pin rand to 0.8 short-term
3. Pin toolchain to stabilize clippy gate
Recommendation: Keep the PR’s approach: complete the rand 0.9 migration and harden CI/security gates so future breaking bumps can’t merge silently. Consider an RNG facade later if dependency churn becomes frequent; it’s not required to land these fixes safely. File ChangesEnhancement (1)
Bug fix (15)
Refactor (34)
Tests (6)
Documentation (4)
Other (6)
|
adolago
added a commit
that referenced
this pull request
Jun 13, 2026
test_release_docs_do_not_reference_legacy_repo_namespace forbade github.com/adolago/rustible, but adolago/rustible is now the canonical home of the repository. PR #860's docs alignment moved the issue links onto adolago/rustible, which tripped this guard and turned the entire Test matrix red on one assertion (38 passed; 1 failed). Point the guard at the actual previous owner, kernelfirma/rustible, so it still prevents stale-namespace references from creeping back in while allowing the current adolago namespace. None of the seven guarded docs reference kernelfirma anymore, so the suite is green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A codebase-improvement sweep across four areas — repo hygiene/docs accuracy, lint & CI gate hardening, targeted production panic fixes, and the beta-readiness queue — plus a critical build fix:
maindid not compile when this branch started.mainwas brokenThe dependabot PR #859 (merged after the repo ownership change) bumped
rand0.8 → 0.9 — a breaking API change — without migrating any code, leaving the crate uncompilable (11 errors:OsRngmoved to the fallibleTryRngCore,thread_rng/gen/gen_rangerenamed,StdRng::from_entropyremoved). The soft CI gates let it merge, which is exactly the gap the lint-hardening commits here close. The migration is4633b45d.What's in here (11 commits)
Repo hygiene & docs (#849)
bench_securitybinary, two already-appliedpatch*.difffiles,test_output.txt, and 799 runtime.rustible_state/snapshots; ignore them going forward.CLAUDE.mdlisted npm scripts for this cargo project — replaced with the realcargocommands.Cargo.tomlrepositoryatadolago/rustible(the repo had moved;Cargo.tomlpointed at a third, unrelated URL). Reconcile WinRM / azure / gcp / rollback status inROADMAP.mdand the winrm feature comment against the canonicaldocs/FEATURE_STATUS.md. Drop the staleRUSTSEC-2026-0009audit ignore (the lockfile already resolves the fixedtime 0.3.47).rand 0.9 migration (#850) —
4633b45dthread_rng()→rng(),gen*→random*. Crypto paths (vault, vars, CLI vault) now use the rand_core 0.6OsRngre-exported byaes-gcm'saead, matching whatargon2/aes-gcmexpect rather than relying on rand's version lining up by coincidence. Signing-key generation usesTryRngCore::try_fill_byteswith an explicit panic on OS-entropy failure. The russh test key helper uses ssh-key's ownOsRng(its "seeded for reproducibility" comment was wrong —from_entropywas never reproducible).Lint & CI gate hardening
sort_by_key+Reversefor descending sorts,checked_div,is_multiple_of, collapsed match arms, andif letreplacingunwrap-after-is_somein the privilege-escalation and DB paths).ci.ymlclippy is now a hard gate:cargo clippy --all-targets -- -D warnings(was warning-only on lib/bins, with a stale "tests don't compile" comment the feature_matrix job already disproved).clippy.tomlallows unwrap/expect/panic in tests so the security SAST scan reports production signal instead of ~4000 test hits.Cargo.toml [lints.clippy]deniesdbg_macro/todo.security.ymllicense gate is now hard:deny.tomlis checked in (was generated inline and run with|| true, so it could never fail). Advisory checks stay owned bycargo-audit/.cargo/audit.tomlto avoid two ignore-lists drifting.Production panic-path fixes —
0f9919acjson_line/json_pretty, 18 sites); resolver template captures degrade gracefully.Beta-readiness: vault zeroization —
331f45cdSecretString; the CLIVaultEnginekept a plainStringalive for the whole command. It now uses zeroizing storage from all three password sources (file,RUSTIBLE_VAULT_PASSWORD, interactive prompt), and transient read buffers are wiped. Closes the alpha-readiness tracker's vault item.Verification
Local, default features unless noted:
cargo fmt --all -- --checkcargo clippy --all-targets -- -D warningscargo test --libbash scripts/smoke_tests.shcargo test --test secrets_zeroization_testscargo check --all-targets --no-default-features --features experimental,full-provisioning,api,database(CI aggregate bundle)cargo check --features kubernetes,ssh2-backend --libThe aggregate bundle caught a real bug pre-push:
clippy --fix(run under default features) had underscored two bindings whose only uses are behind non-defaultcfggates, breaking the feature builds — fixed inf4c47efd.Notes & risks
if letrewrites are privilege-sensitive but verified via connection tests + smoke.ci.ymlclippy gate andsecurity.ymllicense gate run on this PR. Accepted ratchet: a future rolling-stable clippy may add lints and fail CI without code changes — fix-forward rather than pinning the toolchain here.allowlist rather than restoring|| true.Follow-ups (not in this PR)
security.ymlworkflow_dispatch) and reconciledocs/security/SECURITY_AUDIT_REPORT.md(tracker low-item feat: Add SSH connection pooling for 11x performance improvement #2).run.rsask-vault-pass plaintext lifetime; sweep the infalliblewriteln!-into-Stringunwraps tolet _ =.🤖 Generated with Claude Code