Skip to content

fix: resolve security vulnerabilities and normalize tests for CI#44

Merged
EffortlessSteven merged 5 commits into
mainfrom
fix/main-ci-and-audit
Apr 15, 2026
Merged

fix: resolve security vulnerabilities and normalize tests for CI#44
EffortlessSteven merged 5 commits into
mainfrom
fix/main-ci-and-audit

Conversation

@EffortlessSteven

Copy link
Copy Markdown
Member

Summary of Changes

This Pull Request addresses several critical maintenance tasks to ensure the stability, security, and cross-platform reliability of the Shipper workspace.

Security Hardening

  • Vulnerability Remediation: Updated �ws-lc-sys,
    ustls-webpki, and
    and to versions that resolve 6 vulnerabilities reported by cargo audit.
  • Dependency Modernization: Replaced the unmaintained and unsound �tty crate with the standard library's std::io::IsTerminal in the shipper-progress crate.
  • Dependency Refresh: Updated workspace dependencies including hmac, sha2, serde, and okio, effectively superseding several open Dependabot PRs.

CI & Cross-Platform Stability

  • Snapshot Normalization: Updated insta snapshots across the workspace to handle Windows-specific binary names and path slashes, ensuring consistent test behavior between local development (Windows) and CI runners (Linux).
  • Test Verification: Successfully verified all 4010 unit, property, BDD, and documentation tests.
  • Reproducible Builds: Pinned core dependencies in the root Cargo.toml to ensure consistent builds across environments.

Status

  • Build: Passing
  • Clippy/Fmt: Passing
  • Tests: Passing
  • Security: Audit Clean

- Update aws-lc-sys, rustls-webpki, and rand to fix cargo audit findings
- Replace unmaintained 'atty' with 'std::io::IsTerminal' in shipper-progress
- Normalize path slashes and binary names in snapshots for Windows/CI stability
- Pin dependencies in root Cargo.toml for reproducible builds
@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@EffortlessSteven has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 43 minutes and 22 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 22 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c619f9d0-ad96-4e49-a350-18a994db69bf

📥 Commits

Reviewing files that changed from the base of the PR and between bcafbf1 and bee9aa2.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • crates/shipper-cli/Cargo.toml
  • crates/shipper-cli/tests/e2e_expanded.rs
  • crates/shipper-plan/Cargo.toml
  • crates/shipper-plan/src/lib.rs

Walkthrough

The PR updates dependency versions across the workspace and individual crates (serde, proptest, insta, sha2, rand, hmac), replaces the atty crate with Rust's standard library std::io::IsTerminal, and enhances test infrastructure with path-resolution helpers and improved snapshot normalization functions for platform-specific line endings and path separators.

Changes

Cohort / File(s) Summary
Workspace & Crate Dependency Updates
Cargo.toml, crates/shipper-plan/Cargo.toml, crates/shipper/Cargo.toml
Bumped workspace-level dependencies (serde 1.0→1.0.228, proptest 1.10.0→1.11.0, insta 1→1.47.2) and crate-specific versions (sha2 0.10→0.11 in shipper-plan, rand 0.10.0→0.10.1 and HMAC/sha2 stack in shipper).
TTY Detection Modernization
crates/shipper-progress/Cargo.toml, crates/shipper-progress/src/lib.rs
Removed atty dependency and replaced atty::is(Stream::Stdout) with std::io::stdout().is_terminal() using Rust standard library IsTerminal trait.
Test Infrastructure Enhancements
crates/shipper-cli/tests/cli_snapshots.rs, crates/shipper-cli/tests/e2e_expanded.rs, crates/shipper-plan/src/lib.rs, crates/shipper-state/src/lib.rs
Added and updated normalization helpers to handle Windows line endings (\r\n\n), platform-specific binary names, version redaction, temp directory path substitution, and backslash-to-forward-slash conversions; updated snapshot assertions to use new normalization functions.
BDD Workflow Test Setup Refactoring
crates/shipper-cli/tests/bdd_workflow.rs
Added helper functions for executable discovery (find_executable_on_path, resolve_tool_path), tool proxy generation (create_tool_proxy, create_failing_tool_proxy), and Cargo filtering (path_entry_has_cargo); refactored bdd_doctor_missing_cargo test to use filtered PATH, resolved tool paths via environment variables, and generated shim scripts instead of empty directories.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Dependencies dance, new versions take their place,
atty bids farewell—the standard library's grace,
Paths normalized, helpers aligned with care,
Tests now robust across platforms everywhere!
Hop along, dear changes—you're looking quite fair!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 74.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title directly reflects the two main objectives of the changeset: resolving security vulnerabilities through dependency updates and normalizing tests for cross-platform CI compatibility.
Description check ✅ Passed The pull request description is comprehensive and clearly related to the changeset, detailing security hardening, CI/cross-platform stability improvements, and reproducible builds with specific examples and test verification results.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/main-ci-and-audit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request performs a significant dependency update across the workspace, including replacing the 'atty' crate with the standard library's 'IsTerminal' trait. It also introduces comprehensive path and output normalization helpers in the test suite to ensure snapshot stability across different platforms. Feedback includes addressing version mismatches for cryptographic libraries ('hmac' and 'sha2') to prevent duplicate dependencies and improving the executable search helper to include the '.com' extension on Windows for consistency.

Comment thread crates/shipper/Cargo.toml
Comment on lines +64 to +65
hmac = "0.13"
sha2 = "0.11"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The hmac and sha2 dependencies have been updated to 0.13 and 0.11 respectively in this crate, but other crates in the workspace (like shipper-webhook) are still pinned to the older versions (0.12 and 0.10). This causes duplicate versions of these cryptographic libraries to be included in the dependency graph, increasing binary size and potentially causing type mismatches if types from these crates are shared. Please update all crates in the workspace to use the same versions.

Comment on lines +145 to +161
fn find_executable_on_path(program: &str) -> Option<PathBuf> {
let path_var = std::env::var_os("PATH")?;

#[cfg(windows)]
let candidates = [
format!("{program}.exe"),
format!("{program}.cmd"),
format!("{program}.bat"),
program.to_string(),
];
#[cfg(not(windows))]
let candidates = [program.to_string()];

std::env::split_paths(&path_var)
.flat_map(|dir| candidates.iter().map(move |candidate| dir.join(candidate)))
.find(|candidate| candidate.is_file())
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The find_executable_on_path helper on Windows only checks for .exe, .cmd, and .bat extensions. However, the path_entry_has_cargo helper on line 235 also checks for .com. For consistency and better cross-platform reliability in tests, find_executable_on_path should also include .com in its candidate list.

Suggested change
fn find_executable_on_path(program: &str) -> Option<PathBuf> {
let path_var = std::env::var_os("PATH")?;
#[cfg(windows)]
let candidates = [
format!("{program}.exe"),
format!("{program}.cmd"),
format!("{program}.bat"),
program.to_string(),
];
#[cfg(not(windows))]
let candidates = [program.to_string()];
std::env::split_paths(&path_var)
.flat_map(|dir| candidates.iter().map(move |candidate| dir.join(candidate)))
.find(|candidate| candidate.is_file())
}
fn find_executable_on_path(program: &str) -> Option<PathBuf> {
let path_var = std::env::var_os("PATH")?;
#[cfg(windows)]
let candidates = [
format!("{program}.exe"),
format!("{program}.cmd"),
format!("{program}.bat"),
format!("{program}.com"),
program.to_string(),
];
#[cfg(not(windows))]
let candidates = [program.to_string()];
std::env::split_paths(&path_var)
.flat_map(|dir| candidates.iter().map(move |candidate| dir.join(candidate)))
.find(|candidate| candidate.is_file())
}

@EffortlessSteven EffortlessSteven merged commit 6b6f278 into main Apr 15, 2026
33 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant