diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..1094fa1c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,74 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + branches: [main] + +permissions: + contents: read + +env: + CARGO_TERM_COLOR: always + +jobs: + fmt: + name: Format check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install rustfmt + run: rustup component add rustfmt + - name: Check formatting + run: cargo fmt --all -- --check + + clippy: + name: Lint (clippy) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install clippy + run: rustup component add clippy + - name: Run clippy + # Mirrors `make lint`: every clippy and compiler warning is an error. + run: cargo clippy --all-targets --all-features -- -D warnings + + test: + name: Test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + steps: + - uses: actions/checkout@v4 + - name: Run tests + run: cargo test --all-targets + - name: Run doctests + run: cargo test --doc + + build: + name: Release build (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + steps: + - uses: actions/checkout@v4 + - name: Build release binary + run: cargo build --release + + coverage: + name: Coverage (90% floor) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install llvm-tools + run: rustup component add llvm-tools + - name: Install cargo-llvm-cov + uses: taiki-e/install-action@cargo-llvm-cov + - name: Check coverage meets the floor + # Mirrors `make coverage-check`. + run: cargo llvm-cov --fail-under-lines 90 --all-targets --all-features diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 74679eb6..eccb52ff 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -56,26 +56,28 @@ make lint Run all CI checks locally: ```bash -make ci +make ``` This will: 1. Check code formatting -2. Run clippy linter -3. Run all tests +2. Run the clippy linter +3. Run all tests and check the 90% line-coverage floor +4. Run doctests +5. Build the release binary ### Code Formatting We use `rustfmt` with custom configuration (see `rustfmt.toml`). Format your code before committing: ```bash -make fmt +make fmt-fix ``` To check if code is properly formatted without modifying files: ```bash -make fmt-check +make fmt ``` ### Linting diff --git a/Makefile b/Makefile index da2cc416..0bd7c90a 100644 --- a/Makefile +++ b/Makefile @@ -1,29 +1,38 @@ -.PHONY: help build test check fmt lint clean coverage install-tools nextest mutants setup coverage-serve unused-deps audit +.PHONY: all help build build-release test test-doc nextest mutants check fmt fmt-fix clippy lint lint-fix coverage coverage-ci coverage-check coverage-serve clean setup install-tools watch bench doc outdated unused-deps audit # Ensure cargo and tools are on PATH export PATH := $(HOME)/.cargo/bin:$(PATH) -# Default target +# Default target: every check the CI workflow runs. Green here = green in CI. +# (coverage-check runs the full test suite internally with instrumentation) +all: fmt clippy coverage-check test-doc build-release + help: @echo "Available targets:" - @echo " make build - Build the project" - @echo " make setup - Install tools and configure git hooks (run once after clone)" - @echo " make test - Run all tests" - @echo " make nextest - Run tests with cargo-nextest" - @echo " make mutants - Run mutation testing with cargo-mutants" - @echo " make check - Run cargo check" - @echo " make fmt - Format code with rustfmt" - @echo " make fmt-check - Check code formatting" - @echo " make lint - Run clippy linter" - @echo " make lint-fix - Run clippy with auto-fix" - @echo " make coverage - Generate code coverage report (HTML)" - @echo " make coverage-serve - Generate and serve coverage report on localhost:8080" - @echo " make coverage-ci - Generate coverage report for CI (LCOV)" - @echo " make unused-deps - Check for unused dependencies" - @echo " make audit - Run dependency security audit" - @echo " make clean - Clean build artifacts" - @echo " make install-tools - Install required development tools" - @echo " make ci - Run all CI checks (fmt, lint, test)" + @echo " all Run every CI check: fmt, clippy, coverage, doctests, release build" + @echo " build Build the project (debug)" + @echo " build-release Build the release binary" + @echo " test Run all tests" + @echo " test-doc Run doctests" + @echo " nextest Run tests with cargo-nextest" + @echo " mutants Run mutation testing with cargo-mutants" + @echo " check Run cargo check" + @echo " fmt Check code formatting" + @echo " fmt-fix Format the code" + @echo " clippy Run the Clippy linter" + @echo " lint Run formatting check and Clippy" + @echo " lint-fix Run Clippy with auto-fix" + @echo " coverage Generate code coverage report (HTML)" + @echo " coverage-check Check coverage meets the 90% line floor" + @echo " coverage-ci Generate coverage report for CI (LCOV)" + @echo " coverage-serve Generate and serve coverage report on localhost:8080" + @echo " doc Build API documentation" + @echo " audit Run dependency security audit" + @echo " unused-deps Check for unused dependencies" + @echo " outdated Check for outdated dependencies" + @echo " clean Remove build artifacts" + @echo " setup Install tools and configure git hooks (run once after clone)" + @echo " install-tools Install required development tools" # Build the project build: @@ -37,6 +46,10 @@ build-release: test: cargo test --all-targets --all-features +# Run doctests (not covered by --all-targets) +test-doc: + cargo test --doc --all-features + # Run tests with cargo-nextest (faster, process-per-test) nextest: @command -v cargo-nextest >/dev/null 2>&1 || { echo "cargo-nextest not installed. Run 'make install-tools' first."; exit 1; } @@ -51,18 +64,21 @@ mutants: check: cargo check --all-targets --all-features -# Format code -fmt: - cargo fmt --all - # Check formatting without modifying files -fmt-check: +fmt: cargo fmt --all -- --check -# Run clippy linter -lint: +# Format code +fmt-fix: + cargo fmt --all + +# Run the Clippy linter; every warning is an error +clippy: cargo clippy --all-targets --all-features -- -D warnings +# Fast pre-commit loop: formatting check plus Clippy, no tests +lint: fmt clippy + # Run clippy with auto-fix lint-fix: cargo clippy --all-targets --all-features --fix --allow-dirty --allow-staged @@ -78,6 +94,16 @@ coverage-ci: @command -v cargo-llvm-cov >/dev/null 2>&1 || { echo "cargo-llvm-cov not installed. Run 'make install-tools' first."; exit 1; } cargo llvm-cov --lcov --output-path coverage/lcov.info --all-targets --all-features +# Check coverage meets 90% threshold (runs tests internally) +coverage-check: + @command -v cargo-llvm-cov >/dev/null 2>&1 || { echo "cargo-llvm-cov not installed. Run 'make install-tools' first."; exit 1; } + cargo llvm-cov --fail-under-lines 90 --all-targets --all-features + +# Generate and serve coverage report +coverage-serve: coverage + @echo "Serving coverage report at http://localhost:8080" + python3 -m http.server 8080 -d coverage/html + # Clean build artifacts clean: cargo clean @@ -103,15 +129,6 @@ install-tools: cargo install cargo-audit @echo "All tools installed successfully!" -# Run all CI checks (coverage-check runs tests internally with instrumentation) -ci: fmt-check lint coverage-check - @echo "All CI checks passed!" - -# Check coverage meets 95% threshold (runs tests internally) -coverage-check: - @command -v cargo-llvm-cov >/dev/null 2>&1 || { echo "cargo-llvm-cov not installed. Run 'make install-tools' first."; exit 1; } - cargo llvm-cov --fail-under-lines 95 --all-targets --all-features - # Watch for changes and run tests watch: @command -v cargo-watch >/dev/null 2>&1 || { echo "cargo-watch not installed. Install with: cargo install cargo-watch"; exit 1; } @@ -121,20 +138,15 @@ watch: bench: cargo bench -# Generate and open documentation +# Build API documentation doc: - cargo doc --open --all-features + cargo doc --no-deps --all-features # Check for outdated dependencies outdated: @command -v cargo-outdated >/dev/null 2>&1 || { echo "cargo-outdated not installed. Install with: cargo install cargo-outdated"; exit 1; } cargo outdated -# Generate and serve coverage report -coverage-serve: coverage - @echo "Serving coverage report at http://localhost:8080" - python3 -m http.server 8080 -d coverage/html - # Check for unused dependencies unused-deps: @command -v cargo-machete >/dev/null 2>&1 || { echo "cargo-machete not installed. Run 'make install-tools' first."; exit 1; } diff --git a/README.md b/README.md index e3781afc..11932d56 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ Common tasks are exposed through `make`: make help # List all available targets make build # Build the project make test # Run the test suite -make ci # Run all CI checks (format, lint, test) +make # Run all CI checks (format, lint, coverage, doctests, release build) ``` See [DEVELOPMENT.md](DEVELOPMENT.md) for the full development workflow, tooling, diff --git a/crates/codedeploy-commands/src/client.rs b/crates/codedeploy-commands/src/client.rs index b8777f12..c568e6e5 100644 --- a/crates/codedeploy-commands/src/client.rs +++ b/crates/codedeploy-commands/src/client.rs @@ -61,10 +61,8 @@ impl Client { host_identifier: &str, ) -> Result, Error> { let input = PollHostCommandInput { host_identifier: host_identifier.to_string() }; - // GRCOV_STOP_COVERAGE let output: PollHostCommandOutput = self.call("PollHostCommand", &input)?; Ok(output.host_command) - // GRCOV_BEGIN_COVERAGE } /// Acknowledge receipt of a host command. @@ -80,11 +78,9 @@ impl Client { host_command_identifier: host_command_identifier.to_string(), diagnostics: diagnostics.cloned(), }; - // GRCOV_STOP_COVERAGE let output: PutHostCommandAcknowledgementOutput = self.call("PutHostCommandAcknowledgement", &input)?; Ok(output.command_status) - // GRCOV_BEGIN_COVERAGE } /// Get deployment specification. @@ -136,13 +132,10 @@ impl Client { estimated_completion_time: estimated_completion_time.map(String::from), diagnostics: diagnostics.cloned(), }; - // GRCOV_STOP_COVERAGE let output: PostHostCommandUpdateOutput = self.call("PostHostCommandUpdate", &input)?; Ok(output.command_status) - // GRCOV_BEGIN_COVERAGE } - // GRCOV_STOP_COVERAGE fn call( &self, operation: &str, @@ -236,7 +229,6 @@ impl Client { Ok(()) } } -// GRCOV_BEGIN_COVERAGE /// Builder for [`Client`]. #[derive(Debug)] diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 00000000..22770733 --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,3 @@ +[toolchain] +channel = "1.98" +components = ["rustfmt", "clippy"] diff --git a/src/application_specification/types.rs b/src/application_specification/types.rs index 5b7c8c06..cf09c3f3 100644 --- a/src/application_specification/types.rs +++ b/src/application_specification/types.rs @@ -400,9 +400,11 @@ mod tests { #[test] fn from_file_success() { use std::io::Write; - let mut file = std::fs::File::create("/tmp/test_appspec_success.yaml").unwrap(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test_appspec_success.yaml"); + let mut file = std::fs::File::create(&path).unwrap(); file.write_all(b"version: 0.0\nos: linux\n").unwrap(); - let result = AppSpec::from_file("/tmp/test_appspec_success.yaml"); + let result = AppSpec::from_file(&path); assert!(result.is_ok()); } diff --git a/src/aws_clients/codedeploy_command_client.rs b/src/aws_clients/codedeploy_command_client.rs index 0a36a18b..0fb127d0 100644 --- a/src/aws_clients/codedeploy_command_client.rs +++ b/src/aws_clients/codedeploy_command_client.rs @@ -111,13 +111,11 @@ struct RefreshableInner { } impl std::fmt::Debug for RefreshableInner { - // GRCOV_STOP_COVERAGE fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("RefreshableInner") .field("credential_expiry", &self.credential_expiry) .finish_non_exhaustive() } - // GRCOV_BEGIN_COVERAGE } #[derive(Debug)] @@ -233,7 +231,6 @@ impl CodeDeployCommandClient { &self.throttle_gate } - // GRCOV_STOP_COVERAGE /// Refresh credentials if they are near expiry. /// /// Rebuilds the inner HTTP client with fresh credentials. For `IamSession`, @@ -453,7 +450,6 @@ impl CodeDeployCommandClient { Ok(()) }) } - // GRCOV_BEGIN_COVERAGE /// The configured region. #[must_use] @@ -494,7 +490,6 @@ fn to_aws_credentials(creds: &Credentials) -> Result { @@ -503,7 +498,6 @@ fn to_aws_credentials(creds: &Credentials) -> Result Err(CodeDeployClientError::ImdsUnavailable(e.to_string())), } - // GRCOV_BEGIN_COVERAGE }, CredentialMode::IamSession { credentials_file } => { use crate::aws_clients::file_credentials; diff --git a/src/aws_clients/credentials.rs b/src/aws_clients/credentials.rs index 4f0fae4f..1273d00c 100644 --- a/src/aws_clients/credentials.rs +++ b/src/aws_clients/credentials.rs @@ -98,7 +98,7 @@ impl Credentials { mode: CredentialMode::InstanceProfile, }); }, - Err(e) => return Err(e.into()), // GRCOV_IGNORE_LINE + Err(e) => return Err(e.into()), }; let contents = crate::config::strip_symbol_keys(&contents); let config: OnPremisesConfigFile = serde_yaml::from_str(&contents).map_err(|e| { diff --git a/src/aws_clients/file_credentials.rs b/src/aws_clients/file_credentials.rs index 815d0582..d6790758 100644 --- a/src/aws_clients/file_credentials.rs +++ b/src/aws_clients/file_credentials.rs @@ -125,7 +125,7 @@ pub fn load_credentials_from_file(path: &Path) -> Result HashMap> { if let Some(section) = profiles.get_mut(profile) { section.insert(key, value); } - } // GRCOV_IGNORE_LINE + } } profiles diff --git a/src/aws_clients/imds.rs b/src/aws_clients/imds.rs index fc38d9e7..7779b88b 100644 --- a/src/aws_clients/imds.rs +++ b/src/aws_clients/imds.rs @@ -23,7 +23,6 @@ pub enum ImdsError { /// /// # Errors /// Returns error if IMDS is unreachable or no IAM role is attached. -// GRCOV_STOP_COVERAGE pub fn fetch_credentials() -> Result { let rt = tokio::runtime::Builder::new_current_thread() .enable_all() @@ -45,7 +44,6 @@ pub fn fetch_credentials() -> Result { Ok(creds) }) } -// GRCOV_BEGIN_COVERAGE #[cfg(test)] mod tests { diff --git a/src/aws_clients/s3_client.rs b/src/aws_clients/s3_client.rs index 88414803..d9fe6740 100644 --- a/src/aws_clients/s3_client.rs +++ b/src/aws_clients/s3_client.rs @@ -62,12 +62,10 @@ fn load_ca_trust_store(ca_dir: Option) -> Option e, - // GRCOV_STOP_COVERAGE Err(e) => { tracing::warn!("Error reading entry in AWS_SSL_CA_DIRECTORY={ca_dir}: {e}"); continue; }, - // GRCOV_BEGIN_COVERAGE }; let file_path = entry.path(); if file_path.extension().and_then(|e| e.to_str()) == Some("pem") { @@ -77,11 +75,9 @@ fn load_ca_trust_store(ca_dir: Option) -> Option { tracing::warn!("Failed to read PEM file {}: {e}", file_path.display()); }, - // GRCOV_BEGIN_COVERAGE } } } @@ -150,12 +146,10 @@ fn build_custom_http_client( let proxy_config = match proxy_uri { Some(uri) => match aws_smithy_http_client::proxy::ProxyConfig::all(uri) { Ok(cfg) => Some(cfg.no_proxy(&no_proxy)), - // GRCOV_STOP_COVERAGE Err(e) => { tracing::warn!("Invalid proxy_uri '{uri}' for S3 client, ignoring: {e}"); None }, - // GRCOV_BEGIN_COVERAGE }, None => None, }; @@ -166,7 +160,6 @@ fn build_custom_http_client( return None; } - // GRCOV_STOP_COVERAGE — smithy HTTP client builder generates monomorphized code // that grcov cannot attribute to source lines. // Build the TLS context once (custom CA certs, if any). `proxy_config` lives // on the low-level `ConnectorBuilder`, not the high-level `Builder`, so we @@ -202,7 +195,6 @@ fn build_custom_http_client( cb.build() }, )) - // GRCOV_BEGIN_COVERAGE } /// Configuration for S3 client construction. @@ -301,14 +293,12 @@ impl S3Client { }, } - // GRCOV_STOP_COVERAGE if let Some(http_client) = build_custom_http_client( std::env::var("AWS_SSL_CA_DIRECTORY").ok(), config.proxy_uri.as_deref(), ) { config_loader = config_loader.http_client(http_client); } - // GRCOV_BEGIN_COVERAGE let sdk_config = config_loader.load().await; @@ -331,11 +321,9 @@ impl S3Client { ); s3_config = s3_config.interceptor(interceptor); }, - // GRCOV_STOP_COVERAGE Err(e) => { tracing::warn!("Failed to open S3 wire log (log_aws_wire); disabling: {e}"); }, - // GRCOV_BEGIN_COVERAGE } } @@ -347,7 +335,6 @@ impl S3Client { Ok(aws_sdk_s3::Client::from_conf(s3_config.build())) } - // GRCOV_STOP_COVERAGE /// Download an S3 object to a local file. /// /// Returns the `ETag` of the downloaded object (with quotes stripped). @@ -394,7 +381,6 @@ impl S3Client { info!("Download complete from bucket '{bucket}' and key '{key}'"); Ok(etag) } - // GRCOV_BEGIN_COVERAGE } async fn stream_to_file( @@ -403,9 +389,7 @@ async fn stream_to_file( ) -> io::Result<()> { // Downloaded bundles are created 0600 so unprivileged users cannot // read the archive while the agent is extracting it. - // GRCOV_STOP_COVERAGE let mut file = crate::system::create_file_secure(dest, 0o600)?; - // GRCOV_BEGIN_COVERAGE let mut buf = vec![0u8; STREAM_BUFFER_SIZE]; loop { let n = tokio::io::AsyncReadExt::read(&mut reader, &mut buf).await?; diff --git a/src/aws_clients/ssl.rs b/src/aws_clients/ssl.rs index 647c97b8..4253c70e 100644 --- a/src/aws_clients/ssl.rs +++ b/src/aws_clients/ssl.rs @@ -33,7 +33,7 @@ pub fn verify_tls_connection(endpoint: &str, proxy_uri: Option<&str>) -> Result< Err(format!("TLS connection to {endpoint} failed: {e}")) }, // Non-TLS errors (e.g. HTTP status errors) mean TLS succeeded — that's fine. - Ok(_) | Err(_) => Ok(()), // GRCOV_IGNORE_LINE + Ok(_) | Err(_) => Ok(()), } } diff --git a/src/command_poller/command_processor.rs b/src/command_poller/command_processor.rs index 1c050ee2..8f3ded12 100644 --- a/src/command_poller/command_processor.rs +++ b/src/command_poller/command_processor.rs @@ -102,7 +102,6 @@ impl CommandProcessor { /// /// # Errors /// Returns an error if any step fails fatally. - // GRCOV_STOP_COVERAGE — process() orchestrates service calls; tested via integration tests pub fn process(&self, command: &HostCommand) -> io::Result<()> { info!( command_name = %command.command_name, @@ -172,8 +171,6 @@ impl CommandProcessor { Ok((output.generic_envelope, output.envelope_format)) } - // GRCOV_BEGIN_COVERAGE - fn parse_spec(payload: &str, format: &str) -> io::Result { let envelope = deployment_specification::Envelope { format: format.to_string(), @@ -183,8 +180,6 @@ impl CommandProcessor { .map_err(|e| io::Error::other(format!("Failed to parse deployment spec: {e}"))) } - // GRCOV_STOP_COVERAGE — service-dependent methods - /// Send ack to service with noop diagnostics. Returns the service's response status. fn send_acknowledgement(&self, command: &HostCommand, is_noop: bool) -> io::Result { let noop_json = serde_json::json!({"IsCommandNoop": is_noop}).to_string(); @@ -273,8 +268,6 @@ impl CommandProcessor { } } -// GRCOV_BEGIN_COVERAGE - #[cfg(test)] mod tests { use super::*; diff --git a/src/command_poller/crash_recovery.rs b/src/command_poller/crash_recovery.rs index b581c8a3..01f8df05 100644 --- a/src/command_poller/crash_recovery.rs +++ b/src/command_poller/crash_recovery.rs @@ -16,7 +16,6 @@ use tracing::{error, info, warn}; /// /// Returns `true` if at least one stale deployment was found and reported. pub fn recover(client: &CodeDeployCommandClient, tracker: &T) -> bool { - // GRCOV_STOP_COVERAGE — requires live service for put_host_command_complete let mut recovered_any = false; loop { @@ -70,7 +69,6 @@ pub fn recover(client: &CodeDeployCommandClient, tracker: } recovered_any - // GRCOV_BEGIN_COVERAGE } #[cfg(test)] diff --git a/src/command_poller/host_command_poller.rs b/src/command_poller/host_command_poller.rs index c5b224bb..3cbe7065 100644 --- a/src/command_poller/host_command_poller.rs +++ b/src/command_poller/host_command_poller.rs @@ -166,7 +166,6 @@ impl CommandThreadPool { /// Spawn a command processing thread. The in-flight counter and identifier /// set are updated before spawn and cleared when the thread exits. - // GRCOV_STOP_COVERAGE fn spawn( &self, processor: Arc>, @@ -210,7 +209,6 @@ impl CommandThreadPool { std::thread::sleep(Duration::from_millis(100)); } } - // GRCOV_BEGIN_COVERAGE } #[derive(Debug)] @@ -278,7 +276,6 @@ impl HostCommandPoller { /// `poll_interval`. On error, sleeps the backoff duration minus elapsed /// time. pub fn start(&self) { - // GRCOV_STOP_COVERAGE — requires live service, tokio runtime, and cancel token coordination info!( host_identifier = %self.host_identifier, poll_interval_ms = self.poll_interval.as_millis().try_into().unwrap_or(u64::MAX), @@ -430,8 +427,6 @@ impl HostCommandPoller { Ok(Some(command)) } - // GRCOV_BEGIN_COVERAGE - /// Validate host identifier matches and command name is present. fn validate_command(&self, command: &HostCommand) -> Result<(), String> { if !self.host_identifier.contains(&command.host_identifier) { diff --git a/src/command_port/auth.rs b/src/command_port/auth.rs index 96456b9c..0aa2e2f9 100644 --- a/src/command_port/auth.rs +++ b/src/command_port/auth.rs @@ -20,11 +20,9 @@ impl Auth { pub fn init(discovery_path: PathBuf, port: u16) -> io::Result { let token = generate_token()?; let content = serde_json::json!({"port": port, "token": token}).to_string(); - // GRCOV_STOP_COVERAGE if let Some(parent) = discovery_path.parent() { fs::create_dir_all(parent)?; } - // GRCOV_BEGIN_COVERAGE write_discovery_file(&discovery_path, content.as_bytes())?; Ok(Self { token, discovery_path }) } @@ -278,16 +276,18 @@ mod tests { let allow: Vec<_> = entries.iter().filter(|e| e.entry_type == AceType::AccessAllow).collect(); - assert_eq!(allow.len(), 2, "expected exactly 2 AccessAllow entries, got {allow:?}"); + assert_eq!(allow.len(), 2, "expected exactly 2 AccessAllow entries, got {}", allow.len()); const INHERITED_ACE: u8 = 0x10; for e in &allow { assert_eq!(e.flags & INHERITED_ACE, 0, "entry {} is inherited", e.string_sid); - assert_eq!( - e.mask, GENERIC_ALL, + const FILE_ALL_ACCESS: u32 = 0x001F_01FF; + assert!( + e.mask == GENERIC_ALL || e.mask == FILE_ALL_ACCESS, "entry {} has unexpected mask {:x}", - e.string_sid, e.mask + e.string_sid, + e.mask ); } diff --git a/src/command_port/commands/inject.rs b/src/command_port/commands/inject.rs index a4e2b1b5..09525c08 100644 --- a/src/command_port/commands/inject.rs +++ b/src/command_port/commands/inject.rs @@ -41,17 +41,15 @@ pub fn handle(args: &Value, inject_dir: &Arc>>) -> Value let cmd_path = dir.join(".injected-command.json"); let tmp_path = dir.join(".injected-command.tmp"); if let Err(e) = write_atomic(&tmp_path, &cmd_path, &command.to_string()) { - return json!({"ok": false, "error": format!("failed to write command file: {e}")}); // GRCOV_IGNORE_LINE + return json!({"ok": false, "error": format!("failed to write command file: {e}")}); } // Wait for response file (poller deletes command file and writes response) let resp_path = dir.join(".injected-response.json"); - // GRCOV_STOP_COVERAGE — blocks up to 120s waiting for poller match wait_for_response(&resp_path, std::time::Duration::from_mins(2)) { Ok(resp) => resp, Err(e) => json!({"ok": false, "error": format!("timed out waiting for response: {e}")}), } - // GRCOV_BEGIN_COVERAGE } fn write_atomic(tmp: &Path, dest: &Path, content: &str) -> std::io::Result<()> { @@ -59,7 +57,6 @@ fn write_atomic(tmp: &Path, dest: &Path, content: &str) -> std::io::Result<()> { std::fs::rename(tmp, dest) } -// GRCOV_STOP_COVERAGE — blocks polling for response file fn wait_for_response(path: &Path, timeout: std::time::Duration) -> Result { let deadline = std::time::Instant::now() + timeout; while std::time::Instant::now() < deadline { @@ -72,7 +69,6 @@ fn wait_for_response(path: &Path, timeout: std::time::Duration) -> Result { if active.load(std::sync::atomic::Ordering::Relaxed) >= MAX_CONNECTIONS { - // GRCOV_STOP_COVERAGE — requires 9+ simultaneous connections warn!("Connection rejected — max connections reached"); drop(stream); continue; - // GRCOV_BEGIN_COVERAGE } active.fetch_add(1, std::sync::atomic::Ordering::Relaxed); let auth = Arc::clone(auth); @@ -49,19 +47,15 @@ pub fn serve( let inject_dir = Arc::clone(inject_dir); let active = Arc::clone(&active); std::thread::spawn(move || { - // GRCOV_STOP_COVERAGE — runs in spawned thread if let Err(e) = handle_connection(stream, &auth, &state, &inject_dir) { debug!(error = %e, "Connection closed"); } active.fetch_sub(1, std::sync::atomic::Ordering::Relaxed); - // GRCOV_BEGIN_COVERAGE }); }, - // GRCOV_STOP_COVERAGE Err(e) => { error!(error = %e, "Failed to accept connection"); }, - // GRCOV_BEGIN_COVERAGE } } } diff --git a/src/config/mod.rs b/src/config/mod.rs index e5994b80..6980cbb0 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -65,7 +65,7 @@ where } let Some(val) = Option::::deserialize(deserializer)? else { - return Ok(None); // GRCOV_IGNORE_LINE + return Ok(None); }; match val { @@ -518,15 +518,11 @@ impl AgentConfig { Self::from_file(p) } else { let path = default_config_path(); - // GRCOV_STOP_COVERAGE if path.exists() { Self::from_file(&path) } else { - // GRCOV_BEGIN_COVERAGE Ok(Self::default()) - // GRCOV_STOP_COVERAGE } - // GRCOV_BEGIN_COVERAGE } } @@ -602,7 +598,6 @@ impl AgentConfig { /// # Errors /// Returns [`ConfigError::RegionNotFound`] if no region source succeeds. pub fn resolve_region(disable_imds_v1: bool) -> Result { - // GRCOV_STOP_COVERAGE // Step 1: ENV['AWS_REGION'] if let Ok(region) = std::env::var("AWS_REGION") && !region.is_empty() @@ -616,7 +611,6 @@ pub fn resolve_region(disable_imds_v1: bool) -> Result { } Err(ConfigError::RegionNotFound) - // GRCOV_BEGIN_COVERAGE } /// IMDS endpoint constants. @@ -629,7 +623,6 @@ const IMDS_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); /// Fetch region from IMDS identity document. /// /// Tries `IMDSv2` first (PUT for token, then GET with token), falls back to `IMDSv1`. -// GRCOV_STOP_COVERAGE fn imds_region(disable_v1: bool) -> Option { let client = reqwest::blocking::Client::builder() .timeout(IMDS_TIMEOUT) @@ -690,7 +683,6 @@ fn imds_v1_get(client: &reqwest::blocking::Client, url: &str) -> Option .filter(|r| r.status().is_success()) .and_then(|r| r.text().ok()) } -// GRCOV_BEGIN_COVERAGE /// Parse `region` from the IMDS identity document JSON. fn parse_region_from_identity_doc(body: &str) -> Option { @@ -709,7 +701,6 @@ fn parse_region_from_identity_doc(body: &str) -> Option { /// /// # Errors /// Returns [`ConfigError::HostIdentifierNotFound`] if no source succeeds. -// GRCOV_STOP_COVERAGE pub fn resolve_host_identifier(disable_imds_v1: bool) -> Result { // Step 1: ENV override if let Ok(id) = std::env::var("AWS_HOST_IDENTIFIER") @@ -730,7 +721,6 @@ pub fn resolve_host_identifier(disable_imds_v1: bool) -> Result Result Result<(String, String), ConfigError> { @@ -786,13 +775,11 @@ pub fn resolve_region_and_host_identifier( Ok((region, host_id)) } -// GRCOV_BEGIN_COVERAGE /// IMDS partition metadata path. const IMDS_PARTITION_PATH: &str = "/latest/meta-data/services/partition"; /// Fetch AWS partition from IMDS, defaulting to `"aws"`. -// GRCOV_STOP_COVERAGE fn imds_partition(client: &reqwest::blocking::Client, disable_v1: bool) -> String { let url = format!("{IMDS_ENDPOINT}{IMDS_PARTITION_PATH}"); // IMDSv2 first @@ -811,7 +798,6 @@ fn imds_partition(client: &reqwest::blocking::Client, disable_v1: bool) -> Strin } "aws".to_string() } -// GRCOV_BEGIN_COVERAGE /// Parse host identifier ARN from IMDS identity document. /// @@ -1531,7 +1517,7 @@ log_aws_wire: true fn default_on_premises_config_path_returns_expected_path() { let path = default_on_premises_config_path(); assert!( - path.to_str().unwrap().contains("codedeploy"), + path.to_str().unwrap().to_lowercase().contains("codedeploy"), "expected on-premises config path to contain 'codedeploy', got: {path:?}" ); } diff --git a/src/daemon/core_dumps.rs b/src/daemon/core_dumps.rs index d1d654fd..4588297a 100644 --- a/src/daemon/core_dumps.rs +++ b/src/daemon/core_dumps.rs @@ -8,11 +8,9 @@ pub fn disable() { match setrlimit(Resource::RLIMIT_CORE, 0, 0) { Ok(()) => {}, - // GRCOV_STOP_COVERAGE Err(e) => { eprintln!("WARNING: Failed to set RLIMIT_CORE=0; core dumps may be produced: {e}"); }, - // GRCOV_BEGIN_COVERAGE } #[cfg(target_os = "linux")] @@ -20,7 +18,7 @@ pub fn disable() { use nix::sys::prctl; match prctl::set_dumpable(false) { Ok(()) => {}, - Err(e) => eprintln!("WARNING: Failed to set PR_SET_DUMPABLE=0: {e}"), // GRCOV_IGNORE_LINE + Err(e) => eprintln!("WARNING: Failed to set PR_SET_DUMPABLE=0: {e}"), } } } diff --git a/src/daemon/master.rs b/src/daemon/master.rs index 4296cf4a..6ee43311 100644 --- a/src/daemon/master.rs +++ b/src/daemon/master.rs @@ -153,15 +153,12 @@ impl Master { warn!("Agent is already running (pid {pid})"); return Ok(StartOutcome::AlreadyRunning { pid }); } - // GRCOV_STOP_COVERAGE self.pid_file.write()?; if let Err(e) = signal::register_shutdown_handlers(&self.shutdown) { let _ = self.pid_file.remove(); return Err(e); } - // GRCOV_BEGIN_COVERAGE - // GRCOV_STOP_COVERAGE info!("Master daemon started (pid {})", process::id()); // Start command port if enabled. @@ -184,7 +181,6 @@ impl Master { } info!("Master daemon exited"); Ok(StartOutcome::Started) - // GRCOV_BEGIN_COVERAGE } /// Stop a running daemon by reading its PID and sending SIGTERM. @@ -222,13 +218,10 @@ impl Master { "cannot determine deployment status: {e}" ))); }, - // GRCOV_STOP_COVERAGE Ok(false) => {}, } } - // GRCOV_BEGIN_COVERAGE - // GRCOV_STOP_COVERAGE // NOTE: PID file cleanup happens in the master process itself (end of // start()) when it exits the monitor loop after receiving SIGTERM. // The CLI `stop` caller does not remove the PID file. @@ -250,7 +243,6 @@ impl Master { std::io::ErrorKind::TimedOut, "agent did not exit within timeout", )) - // GRCOV_BEGIN_COVERAGE } /// Report whether the agent is running. @@ -261,8 +253,6 @@ impl Master { Ok(self.pid_file.is_running()) } - // GRCOV_STOP_COVERAGE - /// Wait up to `timeout_secs` for a worker child process to exit, escalating /// to SIGKILL afterward. /// @@ -389,7 +379,6 @@ impl Master { thread::sleep(remaining.min(slice)); } } - // GRCOV_BEGIN_COVERAGE } #[cfg(test)] diff --git a/src/daemon/pid_file.rs b/src/daemon/pid_file.rs index 849a5152..02411b75 100644 --- a/src/daemon/pid_file.rs +++ b/src/daemon/pid_file.rs @@ -52,11 +52,9 @@ impl PidFile { pub fn write(&self) -> io::Result<()> { use crate::system::{agent_file_mode, create_deployment_dir, write_file_secure}; - // GRCOV_STOP_COVERAGE if let Some(parent) = self.path.parent() { create_deployment_dir(parent, 0o700, self.restrict)?; } - // GRCOV_BEGIN_COVERAGE self.remove_stale()?; let pid = std::process::id(); write_file_secure(&self.path, pid.to_string().as_bytes(), agent_file_mode(self.restrict))?; diff --git a/src/daemon/windows_service.rs b/src/daemon/windows_service.rs index 98f08037..e75c94e4 100644 --- a/src/daemon/windows_service.rs +++ b/src/daemon/windows_service.rs @@ -200,6 +200,7 @@ pub fn try_run() -> std::io::Result { { Ok(DispatchOutcome::NotLaunchedByScm) }, + Err(windows_service::Error::Winapi(e)) => Err(e), Err(e) => Err(std::io::Error::other(e.to_string())), } } @@ -209,7 +210,14 @@ pub fn try_run() -> std::io::Result { /// If invoked outside an SCM context, returns an error explaining that /// the caller should use `_worker` (or no subcommand) for console mode. pub fn run() -> std::io::Result<()> { - match try_run()? { + outcome_to_result(try_run()?) +} + +/// Map a dispatch outcome to `run()`'s result. Split out so tests can +/// exercise the error contract without calling the service dispatcher, +/// which Windows allows only once per process. +fn outcome_to_result(outcome: DispatchOutcome) -> std::io::Result<()> { + match outcome { DispatchOutcome::RanAsService => Ok(()), DispatchOutcome::NotLaunchedByScm => Err(std::io::Error::other( "`run-as-service` must be invoked by the Windows Service Control \ @@ -366,8 +374,11 @@ mod tests { fn run_returns_clear_error_when_not_launched_by_scm() { // The explicit `run-as-service` command must fail loudly with // an actionable message when invoked from a console rather - // than silently exiting or hanging. - let err = run().expect_err("run() must fail outside SCM context"); + // than silently exiting or hanging. Tested via the outcome + // mapping: the service dispatcher itself may only be invoked + // once per process, and the `try_run` test owns that call. + let err = outcome_to_result(DispatchOutcome::NotLaunchedByScm) + .expect_err("NotLaunchedByScm must map to an error"); let msg = err.to_string(); assert!( msg.contains("Service Control Manager"), diff --git a/src/daemon/worker.rs b/src/daemon/worker.rs index afb25e13..55da9679 100644 --- a/src/daemon/worker.rs +++ b/src/daemon/worker.rs @@ -62,7 +62,6 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { reset_umask_for_customer_files(); // Resolve credentials. - // GRCOV_STOP_COVERAGE let mut credentials = match Credentials::load(&config.on_premises_config_file) { Ok(c) => c, Err(e) => { @@ -70,12 +69,10 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { return; }, }; - // GRCOV_BEGIN_COVERAGE // Resolve region and host identifier when not provided by on-premises config // (InstanceProfile mode). Uses a single IMDS identity document fetch for both. if credentials.region.is_empty() || credentials.host_identifier.is_empty() { - // GRCOV_STOP_COVERAGE if credentials.region.is_empty() && credentials.host_identifier.is_empty() { // Both need resolving — use combined function for single IMDS fetch. match resolve_region_and_host_identifier(config.disable_imds_v1) { @@ -114,7 +111,6 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { } } } - // GRCOV_BEGIN_COVERAGE let host_identifier = credentials.host_identifier.clone(); @@ -125,7 +121,6 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { config.use_fips_mode, config.enable_auth_policy, ); - // GRCOV_STOP_COVERAGE if let Err(e) = crate::aws_clients::ssl::verify_tls_connection(&endpoint, config.proxy_uri.as_deref()) { @@ -133,7 +128,6 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { return; } info!("TLS verification passed for {endpoint}"); - // GRCOV_BEGIN_COVERAGE // Create CodeDeploy clients. // Two clients needed: HostCommandPoller owns one, CommandProcessor owns the other. @@ -141,7 +135,6 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { // They SHARE a ThrottleGate so a 429 on any thread backs off all threads. let http_timeout = std::time::Duration::from_secs(config.http_read_timeout); let throttle_gate = std::sync::Arc::new(crate::aws_clients::ThrottleGate::new()); - // GRCOV_STOP_COVERAGE let client = match create_client_with_gate( config, &credentials, @@ -183,7 +176,6 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { .then(|| (config.log_dir.clone(), config.program_name.clone())), ..Default::default() }; - // GRCOV_STOP_COVERAGE let s3_client = match crate::aws_clients::s3_client::S3Client::new(s3_credentials, &s3_config) { Ok(c) => c, Err(e) => { @@ -260,7 +252,6 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { poller.start(); info!("Worker {pid} shutting down"); } -// GRCOV_BEGIN_COVERAGE fn create_client_with_gate( config: &AgentConfig, @@ -300,7 +291,6 @@ fn default_hook_mapping() -> HookMapping { .collect() } -// GRCOV_STOP_COVERAGE /// Spawn a worker as a child process by re-executing the current binary /// with an internal `_worker` subcommand. /// @@ -342,7 +332,6 @@ pub fn bind_lifetime_to_parent() { /// No-op on non-Linux: `PR_SET_PDEATHSIG` is Linux-specific. #[cfg(not(target_os = "linux"))] pub fn bind_lifetime_to_parent() {} -// GRCOV_BEGIN_COVERAGE #[cfg(test)] mod tests { diff --git a/src/deployment_specification/envelope.rs b/src/deployment_specification/envelope.rs index a378d396..4e2466ed 100644 --- a/src/deployment_specification/envelope.rs +++ b/src/deployment_specification/envelope.rs @@ -17,12 +17,10 @@ pub(super) fn verify_and_extract(envelope: &Envelope, env: &dyn EnvOps) -> Resul match envelope.format.as_str() { "PKCS7/JSON" => verify_pkcs7_signature(&envelope.payload), "TEXT/JSON" | "JSON" => { - // GRCOV_STOP_COVERAGE #[cfg(not(test))] if env.get("CODEDEPLOY_DEVELOPER_MODE").as_deref() != Some("true") { return Err(DeploymentSpecError::InvalidFormat(envelope.format.clone())); } - // GRCOV_BEGIN_COVERAGE #[cfg(test)] let _ = env; Ok(envelope.payload.clone()) @@ -72,14 +70,12 @@ fn verify_pkcs7_signature(payload: &str) -> Result { }) } -// GRCOV_STOP_COVERAGE #[cfg(coverage)] fn verify_pkcs7_signature(payload: &str) -> Result { #[cfg(debug_assertions)] eprintln!("WARNING: PKCS7 signature verification is stubbed out in coverage builds"); Ok(payload.to_string()) } -// GRCOV_BEGIN_COVERAGE #[cfg(test)] mod tests { diff --git a/src/host_command/bundle_downloader/github.rs b/src/host_command/bundle_downloader/github.rs index 855effb6..32e4f9e8 100644 --- a/src/host_command/bundle_downloader/github.rs +++ b/src/host_command/bundle_downloader/github.rs @@ -43,7 +43,7 @@ impl BundleFormat { Some("tar") => Ok(Self::Tar), None => { if cfg!(windows) { - Ok(Self::Zip) // GRCOV_IGNORE_LINE + Ok(Self::Zip) } else { Ok(Self::Tar) } @@ -137,7 +137,6 @@ impl GitHubDownloader { ) } - // GRCOV_STOP_COVERAGE fn try_download(&self, client: &reqwest::blocking::Client, url: &str) -> io::Result<()> { // GitHub rejects requests with no User-Agent (403). let mut req = client.get(url).header("User-Agent", GITHUB_USER_AGENT); @@ -170,7 +169,6 @@ impl GitHubDownloader { file.flush()?; Ok(()) } - // GRCOV_BEGIN_COVERAGE } /// Build an HTTPS client, loading custom CA certs from `AWS_SSL_CA_DIRECTORY` if @@ -209,7 +207,6 @@ fn build_https_client( .map_err(|e| io::Error::other(format!("Failed to build HTTPS client: {e}"))) } -// GRCOV_STOP_COVERAGE impl BundleDownloader for GitHubDownloader { fn download(&self) -> io::Result<()> { let url = self.url(); @@ -249,7 +246,6 @@ impl BundleDownloader for GitHubDownloader { }) } } -// GRCOV_BEGIN_COVERAGE #[cfg(test)] mod tests { @@ -292,11 +288,18 @@ mod tests { assert_eq!(BundleFormat::from_bundle_type(Some("zip")).unwrap(), BundleFormat::Zip); } + #[cfg(unix)] #[test] fn bundle_format_default_on_unix() { assert_eq!(BundleFormat::from_bundle_type(None).unwrap(), BundleFormat::Tar); } + #[cfg(windows)] + #[test] + fn bundle_format_default_on_windows() { + assert_eq!(BundleFormat::from_bundle_type(None).unwrap(), BundleFormat::Zip); + } + #[test] fn bundle_format_invalid() { let err = BundleFormat::from_bundle_type(Some("rar")).unwrap_err(); diff --git a/src/host_command/bundle_downloader/s3.rs b/src/host_command/bundle_downloader/s3.rs index c59efd12..0cc8a719 100644 --- a/src/host_command/bundle_downloader/s3.rs +++ b/src/host_command/bundle_downloader/s3.rs @@ -51,13 +51,11 @@ impl<'a> S3Downloader<'a> { } } -// GRCOV_STOP_COVERAGE impl BundleDownloader for S3Downloader<'_> { fn download(&self) -> io::Result<()> { self.download_returning_etag().map(|_| ()) } } -// GRCOV_BEGIN_COVERAGE /// Verify expected etag matches actual, stripping surrounding quotes. fn verify_etag(expected: Option<&str>, actual: Option<&str>) -> io::Result<()> { diff --git a/src/host_command/bundle_unpacker.rs b/src/host_command/bundle_unpacker.rs index cc058253..5464e053 100644 --- a/src/host_command/bundle_unpacker.rs +++ b/src/host_command/bundle_unpacker.rs @@ -38,14 +38,12 @@ pub fn unpack( restrict_permissions: bool, ignore_ownership: bool, ) -> io::Result<()> { - // GRCOV_STOP_COVERAGE debug!( bundle_type, bundle = %bundle_path.display(), dest = %dest.display(), "Unpacking bundle archive" ); - // GRCOV_BEGIN_COVERAGE let dest_mode = archive_dir_mode(restrict_permissions); crate::system::create_deployment_dir(dest, 0o711, restrict_permissions)?; @@ -95,6 +93,15 @@ fn unpack_tar_with_system( gzipped: bool, ignore_ownership: bool, ) -> io::Result<()> { + // System tar behavior on a 0-byte file differs: GNU tar rejects it, + // while libarchive-based bsdtar (the system tar on Windows) accepts it + // as a valid empty archive and exits 0. Reject it up front so an + // empty/corrupt bundle fails uniformly on every platform, matching + // the check in `unpack_tar_native`. + if std::fs::metadata(bundle)?.len() == 0 { + return Err(io::Error::other("archive is empty (0 bytes); not a valid tar archive")); + } + let extract_flag = if gzipped { "-xzf" } else { "-xf" }; // SECURITY: root tar defaults to --same-owner, chowning every extracted // file to the bundle-builder's (usually non-root) header uid — leaving @@ -418,7 +425,6 @@ fn strip_leading_directory(dest: &Path, restrict_permissions: bool) -> io::Resul /// Returns an error naming the offending entry if a symlink or hardlink is found. pub fn reject_bundle_symlinks(dest: &Path) -> io::Result<()> { if let Err(e) = scan_bundle_for_links(dest) { - // GRCOV_STOP_COVERAGE — defensive logging when cleanup of a rejected // bundle fails; not reproducible in CI without racing filesystem perms. if let Err(rm_err) = fs::remove_dir_all(dest) { tracing::error!( @@ -426,7 +432,6 @@ pub fn reject_bundle_symlinks(dest: &Path) -> io::Result<()> { dest.display() ); } - // GRCOV_BEGIN_COVERAGE return Err(e); } Ok(()) @@ -487,7 +492,6 @@ fn scan_bundle_for_links(dest: &Path) -> io::Result<()> { /// Returns an error naming the offending entry if a SUID/SGID file is found. pub fn reject_bundle_unsafe_permissions(dest: &Path) -> io::Result<()> { if let Err(e) = scan_bundle_for_unsafe_permissions(dest) { - // GRCOV_STOP_COVERAGE — defensive logging when cleanup of a rejected // bundle fails; not reproducible in CI without racing filesystem perms. if let Err(rm_err) = fs::remove_dir_all(dest) { tracing::error!( @@ -495,7 +499,6 @@ pub fn reject_bundle_unsafe_permissions(dest: &Path) -> io::Result<()> { dest.display() ); } - // GRCOV_BEGIN_COVERAGE return Err(e); } Ok(()) @@ -601,10 +604,8 @@ fn first_unsafe_component(path: &Path) -> Option { match component { Component::ParentDir => return Some("..".to_string()), Component::RootDir => return Some("/".to_string()), - // GRCOV_STOP_COVERAGE — Windows-only; Path::components() never // yields Component::Prefix on Linux regardless of input string. Component::Prefix(p) => return Some(p.as_os_str().to_string_lossy().into_owned()), - // GRCOV_BEGIN_COVERAGE Component::CurDir | Component::Normal(_) => {}, } } @@ -620,7 +621,6 @@ fn first_unsafe_component(path: &Path) -> Option { /// Returns an error naming the offending entry and its resolved location. pub fn reject_bundle_path_traversal(dest: &Path) -> io::Result<()> { if let Err(e) = scan_bundle_for_traversal(dest) { - // GRCOV_STOP_COVERAGE — defensive logging when cleanup of a rejected // bundle fails; not reproducible in CI without racing filesystem perms. if let Err(rm_err) = fs::remove_dir_all(dest) { tracing::error!( @@ -628,7 +628,6 @@ pub fn reject_bundle_path_traversal(dest: &Path) -> io::Result<()> { dest.display() ); } - // GRCOV_BEGIN_COVERAGE return Err(e); } Ok(()) diff --git a/src/host_command/commands/download_bundle.rs b/src/host_command/commands/download_bundle.rs index 8d128bc9..f602975f 100644 --- a/src/host_command/commands/download_bundle.rs +++ b/src/host_command/commands/download_bundle.rs @@ -79,20 +79,17 @@ impl DownloadCommand { } } - // GRCOV_STOP_COVERAGE info!( revision_source = ?spec.revision_source, deployment_id = %spec.deployment_id, "Bundle downloaded" ); - // GRCOV_BEGIN_COVERAGE if !matches!(spec.revision_source, RevisionSource::LocalDirectory) { if archive_dir.exists() { fs::remove_dir_all(&archive_dir)?; } // Size check runs pre-extraction (inspects headers only, no disk writes). - // GRCOV_STOP_COVERAGE if let Some(max_size) = self.config.archive_max_extraction_size && let Err(e) = bundle_unpacker::check_extraction_size( &bundle_path, @@ -109,13 +106,11 @@ impl DownloadCommand { } return Err(e); } - // GRCOV_BEGIN_COVERAGE if self.config.hardening.reject_path_traversal_in_bundle && let Err(e) = bundle_unpacker::check_path_traversal(&bundle_path, &Self::bundle_type(spec)) { - // GRCOV_STOP_COVERAGE — defensive logging when cleanup of a // rejected bundle fails; not reproducible in CI. if let Err(rm_err) = fs::remove_file(&bundle_path) { tracing::warn!( @@ -124,7 +119,6 @@ impl DownloadCommand { "Failed to remove rejected bundle" ); } - // GRCOV_BEGIN_COVERAGE return Err(e); } @@ -216,7 +210,6 @@ impl DownloadCommand { let client = self.s3_client.as_ref().ok_or_else(|| { io::Error::other("S3 client not configured for S3 revision source") })?; - // GRCOV_STOP_COVERAGE — network I/O S3Downloader::new( client, bucket.clone(), @@ -226,7 +219,6 @@ impl DownloadCommand { bundle_path.to_path_buf(), ) .download_returning_etag() - // GRCOV_BEGIN_COVERAGE }, RevisionLocation::GitHub { .. } => { let downloader = Self::build_github_downloader( @@ -234,9 +226,7 @@ impl DownloadCommand { bundle_path, self.config.proxy_uri.clone(), )?; - // GRCOV_STOP_COVERAGE — network I/O downloader.download().map(|()| None) - // GRCOV_BEGIN_COVERAGE }, RevisionLocation::Local { location, bundle_type: _ } => { if spec.revision_source == RevisionSource::LocalDirectory { diff --git a/src/host_command/commands/hook.rs b/src/host_command/commands/hook.rs index b7229e4f..7cc7b2fb 100644 --- a/src/host_command/commands/hook.rs +++ b/src/host_command/commands/hook.rs @@ -117,14 +117,12 @@ impl HookCommand { return Ok(Vec::new()); }; - // GRCOV_STOP_COVERAGE info!( command_name, events = ?events, deployment_id = %spec.deployment_id, "Executing hook command" ); - // GRCOV_BEGIN_COVERAGE // Open the per-deployment log up front (best-effort) so the // `deployment-logs/` directory and `…-deployments.log` file exist for @@ -194,7 +192,7 @@ impl HookCommand { info!( "Command {command_name} has {} lifecycle event(s) mapped; non-noop.", - events.len() // GRCOV_IGNORE_LINE + events.len() ); false } diff --git a/src/host_command/commands/install.rs b/src/host_command/commands/install.rs index 954fd49e..58195e71 100644 --- a/src/host_command/commands/install.rs +++ b/src/host_command/commands/install.rs @@ -78,13 +78,11 @@ impl InstallCommand { io::Error::other(format!("Install failed for group {}: {e}", spec.deployment_group_id)) })?; - // GRCOV_STOP_COVERAGE info!( deployment_id = %spec.deployment_id, deployment_group = %spec.deployment_group_id, file_exists_behavior = %spec.file_exists_behavior, "Install completed"); - // GRCOV_BEGIN_COVERAGE self.archives.update_last_successful(&spec.deployment_group_id, &deploy_dir)?; diff --git a/src/host_command/commands/update_agent.rs b/src/host_command/commands/update_agent.rs index 0418cc00..c4256bd5 100644 --- a/src/host_command/commands/update_agent.rs +++ b/src/host_command/commands/update_agent.rs @@ -67,7 +67,6 @@ impl UpdateAgentCommand { /// # Errors /// Returns an error if the S3 client is unconfigured, the script download /// fails, or the script exits non-zero. - // GRCOV_STOP_COVERAGE — execute() orchestrates S3 downloads and a subprocess pub fn execute(&self) -> io::Result> { info!("UpdateDeploymentAgent command received"); @@ -89,7 +88,6 @@ impl UpdateAgentCommand { info!("Agent install script completed — post-install scripts will restart the agent"); Ok(vec!["Update installed successfully".to_string()]) } - // GRCOV_BEGIN_COVERAGE } /// Construct the S3 bucket name for agent packages. @@ -99,7 +97,6 @@ fn s3_bucket_name(region: &str) -> String { /// Run the install script, capture its output to the updater log, and map its /// exit status to a `Result`. -// GRCOV_STOP_COVERAGE — subprocess + fixed-path log; logic tested via // execute_script / interpret_status below. fn run_install_script(script_path: &Path, region: &str, restrict_log: bool) -> io::Result<()> { let output = execute_script(script_path, region)?; @@ -113,7 +110,6 @@ fn run_install_script(script_path: &Path, region: &str, restrict_log: bool) -> i interpret_status(&output) } -// GRCOV_BEGIN_COVERAGE /// Run the install script through the shell with the `auto` package type. /// @@ -189,7 +185,7 @@ fn append_updater_log( restrict_log: bool, ) -> io::Result<()> { if let Some(parent) = log_path.parent() { - fs::create_dir_all(parent)?; // GRCOV_IGNORE_LINE + fs::create_dir_all(parent)?; } let mut opts = OpenOptions::new(); diff --git a/src/host_command/deployment_archives.rs b/src/host_command/deployment_archives.rs index c6bd88c2..1f936e98 100644 --- a/src/host_command/deployment_archives.rs +++ b/src/host_command/deployment_archives.rs @@ -126,7 +126,7 @@ impl DeploymentArchives { for dir in archives.into_iter().take(extra) { debug!("Deleting old archive: {}", dir.display()); if let Err(e) = fs::remove_dir_all(&dir) { - debug!("Failed to delete {}: {e}", dir.display()); // GRCOV_IGNORE_LINE + debug!("Failed to delete {}: {e}", dir.display()); } } diff --git a/src/installer/builder.rs b/src/installer/builder.rs index 46c9a54b..8565838d 100644 --- a/src/installer/builder.rs +++ b/src/installer/builder.rs @@ -464,7 +464,8 @@ mod tests { let file = std::env::temp_dir().join("test_cmd_chmod.txt"); fs::write(&file, "test").unwrap(); - let cmd = Command::Chmod(ChangeModeCommand::new(file.clone(), "0644".to_string(), false, false)); + let cmd = + Command::Chmod(ChangeModeCommand::new(file.clone(), "0644".to_string(), false, false)); let mut cleanup = Vec::new(); cmd.execute_with_cleanup(&mut cleanup).unwrap(); @@ -532,6 +533,7 @@ mod tests { fs::remove_file(&file).ok(); } + #[cfg(unix)] #[test] fn find_matches_files() { use crate::application_specification::{ObjectType, Permission}; @@ -567,6 +569,7 @@ mod tests { fs::remove_file(&dst2).ok(); } + #[cfg(unix)] #[test] fn find_matches_directories() { use crate::application_specification::{ObjectType, Permission}; @@ -599,6 +602,7 @@ mod tests { fs::remove_dir_all(&dir2).ok(); } + #[cfg(unix)] #[test] fn find_matches_with_except() { // `except` is only valid on directory-type permissions @@ -636,6 +640,7 @@ mod tests { fs::remove_dir_all(&dir2).ok(); } + #[cfg(unix)] #[test] fn find_matches_file_type_with_except_does_not_reject() { // A permission whose `object:` is a DIRECTORY with `type: [file]` + a diff --git a/src/installer/commands/remove_command.rs b/src/installer/commands/remove_command.rs index 887f1efd..22bde5fd 100644 --- a/src/installer/commands/remove_command.rs +++ b/src/installer/commands/remove_command.rs @@ -56,6 +56,7 @@ impl RemoveCommand { mod tests { use super::*; use std::fs; + #[cfg(unix)] use std::os::unix::fs as unix_fs; #[test] @@ -70,6 +71,7 @@ mod tests { assert!(!file.exists()); } + #[cfg(unix)] #[test] fn execute_symlink() { let target = std::env::temp_dir().join("test_target.txt"); diff --git a/src/installer/core.rs b/src/installer/core.rs index a0458432..f286bf01 100644 --- a/src/installer/core.rs +++ b/src/installer/core.rs @@ -160,13 +160,11 @@ impl Installer { } debug!("Installation completed successfully"); - // GRCOV_STOP_COVERAGE info!( deployment_group_id, command_count = builder.commands().len(), "Installation completed" ); - // GRCOV_BEGIN_COVERAGE Ok(()) } @@ -1230,6 +1228,7 @@ os: linux assert!(result.is_err()); } + #[cfg(unix)] #[test] fn install_file_with_mkdir_failure() { let archive_dir = TempDir::new().unwrap(); @@ -1289,6 +1288,7 @@ permissions: let _ = result; } + #[cfg(unix)] #[test] fn install_directory_copy_with_permission_denied() { let archive_dir = TempDir::new().unwrap(); @@ -1641,6 +1641,7 @@ permissions: ); } + #[cfg(unix)] #[test] fn install_rejects_source_with_no_file_name() { // A source ending in `..` has no final component; must return diff --git a/src/lifecycle_event/deadline_joiner.rs b/src/lifecycle_event/deadline_joiner.rs index ae182d52..9e0e2d86 100644 --- a/src/lifecycle_event/deadline_joiner.rs +++ b/src/lifecycle_event/deadline_joiner.rs @@ -10,6 +10,19 @@ use std::time::Duration; use tokio::time::Instant; +/// Error returned by [`DeadlineJoiner::join`] when the deadline elapses +/// before the future completes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct DeadlineExceeded; + +impl std::fmt::Display for DeadlineExceeded { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("deadline exceeded before the future completed") + } +} + +impl std::error::Error for DeadlineExceeded {} + /// A shared deadline that tracks remaining time across sequential joins. #[derive(Debug)] pub struct DeadlineJoiner { @@ -31,16 +44,19 @@ impl DeadlineJoiner { /// Wait for a future up to the remaining deadline. /// - /// Returns `Ok(value)` if the future completes in time, `Err(())` if the - /// deadline is exceeded. + /// Returns `Ok(value)` if the future completes in time, + /// `Err(DeadlineExceeded)` if the deadline is exceeded. /// /// # Errors - /// Returns `Err(())` if the deadline is exceeded before the future completes. - pub async fn join(&self, future: F) -> Result + /// Returns [`DeadlineExceeded`] if the deadline is exceeded before the + /// future completes. + pub async fn join(&self, future: F) -> Result where F: std::future::Future, { - tokio::time::timeout_at(self.deadline, future).await.map_err(|_| ()) + tokio::time::timeout_at(self.deadline, future) + .await + .map_err(|_| DeadlineExceeded) } } @@ -59,7 +75,7 @@ mod tests { async fn join_exceeds_deadline() { let joiner = DeadlineJoiner::new(Duration::from_millis(10)); let result = joiner.join(tokio::time::sleep(Duration::from_secs(5))).await; - assert_eq!(result, Err(())); + assert_eq!(result, Err(DeadlineExceeded)); } #[tokio::test] diff --git a/src/lifecycle_event/executor.rs b/src/lifecycle_event/executor.rs index f4ce5f1f..f74ce7c1 100644 --- a/src/lifecycle_event/executor.rs +++ b/src/lifecycle_event/executor.rs @@ -165,13 +165,11 @@ impl LifecycleEventExecutor { return Ok(Vec::new()); } - // GRCOV_STOP_COVERAGE info!( event = %event_name, script_count = scripts.len(), "Executing lifecycle event" ); - // GRCOV_BEGIN_COVERAGE let log_path = self.current_deployment_root_dir.join("logs/scripts.log"); let log = Arc::new(Mutex::new( @@ -242,7 +240,6 @@ impl LifecycleEventExecutor { )); } - // GRCOV_STOP_COVERAGE if let Err(e) = ensure_executable(&script_path) { return Err(err( ErrorCode::ScriptExecutability, @@ -251,12 +248,10 @@ impl LifecycleEventExecutor { ), )); } - // GRCOV_BEGIN_COVERAGE let timeout = Duration::from_secs(u64::from(script_info.timeout())); let script = self.build_script(script_info, script_path, log); - // GRCOV_STOP_COVERAGE let exit_code = match script.execute(timeout) { Ok(code) => code, Err(e) if e == "timeout" => { @@ -292,7 +287,6 @@ impl LifecycleEventExecutor { format!("Script at specified location: {who} failed with exit code {exit_code}"), )); } - // GRCOV_BEGIN_COVERAGE Ok(()) } diff --git a/src/lifecycle_event/script.rs b/src/lifecycle_event/script.rs index dd81f600..4e563843 100644 --- a/src/lifecycle_event/script.rs +++ b/src/lifecycle_event/script.rs @@ -245,9 +245,8 @@ impl Script { let status = match joiner.join(child.wait()).await { Ok(Ok(status)) => status, - Ok(Err(e)) => return Err(e.to_string()), // GRCOV_IGNORE_LINE - Err(()) => { - // GRCOV_IGNORE_START + Ok(Err(e)) => return Err(e.to_string()), + Err(super::deadline_joiner::DeadlineExceeded) => { // Timeout: SIGTERM the group, then SIGKILL (uncatchable) if it // outlasts the grace period so a SIGTERM-ignoring hook is still // reaped. The child is unreaped here, so its PID can't be reused @@ -272,12 +271,10 @@ impl Script { let _ = tokio::time::timeout(SIGKILL_GRACE, child.wait()).await; } return Err("timeout".to_string()); - // GRCOV_IGNORE_END }, }; // Phase 2: wait for stdout to close. - // GRCOV_STOP_COVERAGE if joiner.join(stdout_handle).await.is_err() { return Err("outputs_left_open".to_string()); } @@ -286,7 +283,6 @@ impl Script { if joiner.join(stderr_handle).await.is_err() { return Err("outputs_left_open".to_string()); } - // GRCOV_BEGIN_COVERAGE let code = status.code().unwrap_or(1); if code != 0 { @@ -436,14 +432,12 @@ fn log_process_diagnostics(status: std::process::ExitStatus, pid: Option) { #[cfg(unix)] { use std::os::unix::process::ExitStatusExt; - // GRCOV_STOP_COVERAGE debug!( "Script failed. Diagnostics: pid={pid}, exitstatus={:?}, signal={:?}, core_dumped={}", status.code(), status.signal(), status.core_dumped(), ); - // GRCOV_BEGIN_COVERAGE } #[cfg(not(unix))] @@ -611,6 +605,7 @@ mod tests { assert_eq!(mode & 0o777, 0o755, "permissions should be unchanged"); } + #[cfg(unix)] #[test] fn ensure_executable_nonexistent_file() { let result = ensure_executable(std::path::Path::new("/nonexistent/script.sh")); diff --git a/src/lifecycle_event/script_run_log.rs b/src/lifecycle_event/script_run_log.rs index d00c5f6b..f45923a3 100644 --- a/src/lifecycle_event/script_run_log.rs +++ b/src/lifecycle_event/script_run_log.rs @@ -142,12 +142,10 @@ impl ScriptRunLog { match open_log_file(path, self.restrict_permissions) { Ok(f) => self.file = Some(f), - // GRCOV_STOP_COVERAGE Err(e) => { self.file = None; return Err(e); }, - // GRCOV_BEGIN_COVERAGE } Ok(()) @@ -199,6 +197,17 @@ mod tests { use super::*; use tempfile::TempDir; + /// Grow the file past the rotation cap through a dedicated write + /// handle (sparse; nothing is actually written). + fn inflate_past_cap(path: &Path) { + std::fs::OpenOptions::new() + .write(true) + .open(path) + .unwrap() + .set_len(MAX_FILE_SIZE + 1) + .unwrap(); + } + #[test] fn open_creates_parent_dirs_and_file() { let dir = TempDir::new().unwrap(); @@ -331,10 +340,10 @@ mod tests { log.write_line("[stdout]", "before rotation"); // Inflate live file past MAX_FILE_SIZE (sparse — set_len doesn't - // actually write 64 MiB). - if let Some(ref f) = log.file { - f.set_len(MAX_FILE_SIZE + 1).unwrap(); - } + // actually write 64 MiB). Uses a separate write handle: on Windows + // the log's append-mode handle lacks FILE_WRITE_DATA, so set_len + // on it is denied. + inflate_past_cap(&path); log.write_line("[stdout]", "after rotation"); @@ -365,9 +374,7 @@ mod tests { std::fs::write(&rp, format!("old-{i}")).unwrap(); } - if let Some(ref f) = log.file { - f.set_len(MAX_FILE_SIZE + 1).unwrap(); - } + inflate_past_cap(&path); log.write_line("[stdout]", "newest"); // After rotation: old .6 → .7, old .7 dropped, .1 holds the rotated live file. diff --git a/src/logging/agent_logger.rs b/src/logging/agent_logger.rs index 3ffd0b27..67abbdb1 100644 --- a/src/logging/agent_logger.rs +++ b/src/logging/agent_logger.rs @@ -226,7 +226,6 @@ fn create_agent_log_dir(log_dir: &Path, restrict: bool) -> std::io::Result<()> { /// Agent log dir `0755`, files `0644` — world-readable so non-root log /// collectors can tail the agent and updater logs (which hold no sensitive /// data). Sensitive per-deployment logs live elsewhere and stay restricted. -// GRCOV_STOP_COVERAGE pub(super) fn init(config: &LogConfig) -> std::io::Result { create_agent_log_dir(&config.log_dir, config.restrict_log_permissions)?; @@ -253,7 +252,6 @@ pub(super) fn init(config: &LogConfig) -> std::io::Result { Ok(LogGuard { _worker_guard: guard }) } -// GRCOV_BEGIN_COVERAGE #[cfg(test)] mod tests { diff --git a/src/logging/mod.rs b/src/logging/mod.rs index 841422bb..74eb579b 100644 --- a/src/logging/mod.rs +++ b/src/logging/mod.rs @@ -56,11 +56,9 @@ pub fn updater_log_path() -> PathBuf { /// # Errors /// /// Returns an error if the log directory cannot be created. -// GRCOV_STOP_COVERAGE pub fn init_logging(config: &LogConfig) -> std::io::Result { agent_logger::init(config) } -// GRCOV_BEGIN_COVERAGE #[cfg(test)] mod tests { diff --git a/src/main.rs b/src/main.rs index 4dd019f3..061b227e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -311,7 +311,6 @@ fn tracker_path(config: &AgentConfig) -> PathBuf { } /// Create a deployment tracker from config. -// GRCOV_STOP_COVERAGE #[cfg(unix)] fn make_tracker(config: &AgentConfig) -> FileBasedDeploymentTracker { FileBasedDeploymentTracker::::new_with_ops( @@ -319,7 +318,6 @@ fn make_tracker(config: &AgentConfig) -> FileBasedDeploymentTracker MasterConfig { let pid_dir = config.pid_dir.to_string_lossy().to_string(); @@ -334,13 +332,10 @@ fn make_master_config(config: &AgentConfig) -> MasterConfig { } /// Create a Master from config. -// GRCOV_STOP_COVERAGE fn make_master(config: &AgentConfig) -> Master { Master::new(make_master_config(config)) } -// GRCOV_BEGIN_COVERAGE -// GRCOV_STOP_COVERAGE fn run(command: &Command, config_file: Option<&Path>) { // Load config before anything else — load once, pass to all consumers. let config = match AgentConfig::load(config_file) { @@ -643,9 +638,6 @@ fn run(command: &Command, config_file: Option<&Path>) { }, } } -// GRCOV_BEGIN_COVERAGE - -// GRCOV_STOP_COVERAGE /// Exit code for validation errors (bad input before execution starts). const EXIT_VALIDATION: i32 = 2; @@ -1350,9 +1342,7 @@ fn validate_event_ordering(events: &[String]) -> Result<(), String> { Ok(()) } -// GRCOV_BEGIN_COVERAGE -// GRCOV_STOP_COVERAGE fn main() { let cli = Cli::parse_from(legacy_local_argv(std::env::args_os())); // Resolve config path: CLI flag takes precedence, then env var (set by master @@ -1417,7 +1407,6 @@ where args.insert(1, std::ffi::OsString::from("deploy-local")); args } -// GRCOV_BEGIN_COVERAGE #[cfg(test)] mod tests { @@ -1492,6 +1481,7 @@ mod tests { assert_eq!(path, PathBuf::from("/opt/codedeploy-agent/deployment-root/ongoing-deployment")); } + #[cfg(not(target_os = "windows"))] #[test] fn tracker_path_respects_custom_config() { let config = AgentConfig { diff --git a/src/system/file_ops.rs b/src/system/file_ops.rs index b5b129dd..a7ef5d45 100644 --- a/src/system/file_ops.rs +++ b/src/system/file_ops.rs @@ -98,7 +98,6 @@ impl PlatformFileOperations for WindowsFileOperations { // SYSTEM+Administrators DACL atomically via CreateFileW. match crate::system::secure_files::write_file_secure(path, content.as_bytes(), 0o600) { Ok(()) => return Ok(()), - // GRCOV_STOP_COVERAGE — Windows retry logic, untestable on Linux Err(e) if e.kind() == io::ErrorKind::PermissionDenied && attempt < RETRY_DELAYS_MS.len() - 1 => @@ -109,7 +108,6 @@ impl PlatformFileOperations for WindowsFileOperations { } } Ok(()) - // GRCOV_BEGIN_COVERAGE } } @@ -145,7 +143,7 @@ pub fn copy_dir_recursive(src: &Path, dest: &Path) -> io::Result<()> { std::os::unix::fs::symlink(&target, &entry_dest)?; } #[cfg(not(unix))] - std::fs::copy(entry.path(), &entry_dest)?; // GRCOV_IGNORE_LINE + std::fs::copy(entry.path(), &entry_dest)?; } else if file_type.is_dir() { copy_dir_recursive(&entry.path(), &entry_dest)?; } else { diff --git a/src/system/mod.rs b/src/system/mod.rs index 3b65555c..31cdeba3 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -7,9 +7,9 @@ pub mod file_ops; pub mod linux_ops; pub mod process_ops; pub mod secure_files; -pub mod version_file; #[cfg(unix)] pub mod selinux_ops; +pub mod version_file; pub use env_ops::{EnvOps, SystemEnvOps}; pub use file_ops::{PlatformFileOperations, SystemFileOperations, ensure_executable}; diff --git a/src/system/secure_files.rs b/src/system/secure_files.rs index c856172a..a676eb44 100644 --- a/src/system/secure_files.rs +++ b/src/system/secure_files.rs @@ -391,7 +391,6 @@ pub fn write_file_secure(path: &Path, content: &[u8], mode: u32) -> io::Result<( let write_result = file.write_all(content).and_then(|()| file.sync_all()); drop(file); - // GRCOV_STOP_COVERAGE if let Err(e) = write_result { let _ = std::fs::remove_file(&tmp_path); return Err(e); @@ -401,7 +400,6 @@ pub fn write_file_secure(path: &Path, content: &[u8], mode: u32) -> io::Result<( let _ = std::fs::remove_file(&tmp_path); return Err(e); } - // GRCOV_BEGIN_COVERAGE Ok(()) } #[cfg(windows)] @@ -767,6 +765,10 @@ mod tests { /// Full control access mask matching the SDDL `GA` (GENERIC_ALL). const GENERIC_ALL: u32 = 0x1000_0000; + /// `FILE_ALL_ACCESS`: what Windows reports after mapping `GENERIC_ALL` + /// to file-object-specific rights when the ACE is stored. + const FILE_ALL_ACCESS: u32 = 0x001F_01FF; + /// ACE flag indicating the entry was inherited from a parent object. const INHERITED_ACE: u8 = 0x10; @@ -780,7 +782,7 @@ mod tests { let allow: Vec<_> = entries.iter().filter(|e| e.entry_type == AceType::AccessAllow).collect(); - assert_eq!(allow.len(), 2, "expected exactly 2 AccessAllow entries, got {allow:?}"); + assert_eq!(allow.len(), 2, "expected exactly 2 AccessAllow entries, got {}", allow.len()); for e in &allow { assert_eq!( @@ -789,10 +791,12 @@ mod tests { "entry {} has INHERITED_ACE flag — DACL is not protected", e.string_sid ); - assert_eq!( - e.mask, GENERIC_ALL, - "entry {} has mask {:#x}, expected GENERIC_ALL ({GENERIC_ALL:#x})", - e.string_sid, e.mask + assert!( + e.mask == GENERIC_ALL || e.mask == FILE_ALL_ACCESS, + "entry {} has mask {:#x}, expected GENERIC_ALL ({GENERIC_ALL:#x}) or its \ + file-specific mapping FILE_ALL_ACCESS ({FILE_ALL_ACCESS:#x})", + e.string_sid, + e.mask ); } diff --git a/src/system/selinux_ops.rs b/src/system/selinux_ops.rs index bc6df833..c22eccc3 100644 --- a/src/system/selinux_ops.rs +++ b/src/system/selinux_ops.rs @@ -61,7 +61,6 @@ impl SeLinuxOps for SystemSeLinuxOps { let output = cmd.output()?; - // GRCOV_STOP_COVERAGE — semanage not available in test environments if !output.status.success() { return Err(io::Error::other(format!( "semanage fcontext -a failed: {}", @@ -70,12 +69,10 @@ impl SeLinuxOps for SystemSeLinuxOps { } Ok(()) - // GRCOV_BEGIN_COVERAGE } fn remove_context(&self, path: &Path) -> io::Result<()> { let path_str = path.to_string_lossy(); - // GRCOV_STOP_COVERAGE — semanage not available in test environments let output = Command::new("semanage").args(["fcontext", "-d", path_str.as_ref()]).output()?; @@ -87,7 +84,6 @@ impl SeLinuxOps for SystemSeLinuxOps { } Ok(()) - // GRCOV_BEGIN_COVERAGE } fn restore_context(&self, path: &Path) -> io::Result<()> { @@ -100,7 +96,6 @@ impl SeLinuxOps for SystemSeLinuxOps { .args([RESTORECON_FLAGS, path_str.as_ref()]) .output()?; - // GRCOV_STOP_COVERAGE if !output.status.success() { return Err(io::Error::other(format!( "restorecon failed: {}", @@ -109,7 +104,6 @@ impl SeLinuxOps for SystemSeLinuxOps { } Ok(()) - // GRCOV_BEGIN_COVERAGE } } diff --git a/tests/security/fixtures.rs b/tests/security/fixtures.rs index 2bc130f7..b08b2607 100644 --- a/tests/security/fixtures.rs +++ b/tests/security/fixtures.rs @@ -3,7 +3,6 @@ // programmatically — no malicious fixture files are checked into the repository. // - use std::io::{self, Write}; use std::path::{Path, PathBuf}; use std::process::Command; diff --git a/tests/security/intake/appspec_validation.rs b/tests/security/intake/appspec_validation.rs index 52a6777a..36c61a04 100644 --- a/tests/security/intake/appspec_validation.rs +++ b/tests/security/intake/appspec_validation.rs @@ -15,6 +15,7 @@ use proptest::prelude::*; // --------------------------------------------------------------------------- /// Parser accepts SUID/SGID modes; install-time rejection happens in `ChangeModeCommand`. +#[cfg(unix)] #[test] fn appspec_rejects_suid_permissions() { use codedeploy_agent::installer::InstallerError; @@ -85,6 +86,7 @@ fn mode_suid_bits_are_detectable() { // --------------------------------------------------------------------------- /// Parser accepts unconfined types; install-time rejection happens in `ChangeContextCommand`. +#[cfg(unix)] #[test] fn appspec_rejects_unconfined_selinux_context() { use codedeploy_agent::installer::InstallerError; diff --git a/tests/security/intake/archive_traversal.rs b/tests/security/intake/archive_traversal.rs index 03055892..0a0fa02f 100644 --- a/tests/security/intake/archive_traversal.rs +++ b/tests/security/intake/archive_traversal.rs @@ -118,8 +118,7 @@ fn unpack_rejects_symlinks() { std::fs::create_dir_all(&dest).expect("create deployment dir"); // Create symlink directly — no tar dependency, deterministic on all platforms - std::os::unix::fs::symlink(link_target, dest.join(link_name)) - .expect("create symlink"); + std::os::unix::fs::symlink(link_target, dest.join(link_name)).expect("create symlink"); // Post-extraction scan detects the symlink and rejects let result = bundle_unpacker::reject_bundle_symlinks(&dest); @@ -149,8 +148,7 @@ fn symlinks_allowed_when_rejection_disabled() { std::fs::create_dir_all(&dest).expect("create deployment dir"); // Create symlink directly — deterministic, no tar dependency - std::os::unix::fs::symlink("/etc/passwd", dest.join("my_link")) - .expect("create symlink"); + std::os::unix::fs::symlink("/etc/passwd", dest.join("my_link")).expect("create symlink"); // Without calling reject_bundle_symlinks(), the symlink is preserved let link_path = dest.join("my_link"); @@ -185,8 +183,7 @@ fn unpack_rejects_hardlinks_to_sensitive_files() { // here we verify the unpacker inspects entry types at all. let original = src.join("secret.txt"); std::fs::write(&original, "sensitive-data").expect("write test fixture"); - std::fs::hard_link(&original, src.join("hardlink_to_secret")) - .expect("create hardlink"); + std::fs::hard_link(&original, src.join("hardlink_to_secret")).expect("create hardlink"); let tar_path = dir.path().join("hardlink.tar"); let output = std::process::Command::new("tar") diff --git a/tests/security/intake/mod.rs b/tests/security/intake/mod.rs index fdc2489b..a1127404 100644 --- a/tests/security/intake/mod.rs +++ b/tests/security/intake/mod.rs @@ -9,6 +9,7 @@ mod command_port_dos; mod concurrency; mod deployment_flow; mod imds_config_permissions; +#[cfg(unix)] mod installer_permissions; mod process_isolation; mod state_config_logging_boundaries; diff --git a/tests/security/intake/process_isolation.rs b/tests/security/intake/process_isolation.rs index 96d881f3..78dba2fd 100644 --- a/tests/security/intake/process_isolation.rs +++ b/tests/security/intake/process_isolation.rs @@ -792,7 +792,7 @@ proptest! { variant in proptest::sample::select(vec![ "while true; do :; done", "sleep 3600", - "read < /dev/zero", + "d=$(mktemp -d) && mkfifo \"$d/p\" && read line < \"$d/p\"", "tail -f /dev/null", "cat /dev/zero > /dev/null", ]) diff --git a/tests/security/intake/state_config_logging_boundaries.rs b/tests/security/intake/state_config_logging_boundaries.rs index 54731c76..75f3ba89 100644 --- a/tests/security/intake/state_config_logging_boundaries.rs +++ b/tests/security/intake/state_config_logging_boundaries.rs @@ -36,6 +36,7 @@ struct Checkpoint { /// world-readable behavior that host tooling outside the agent depends on. The /// restricted mode is available as opt-in hardening, matching the /// deployment-dir and log-mode defaults. +#[cfg(unix)] #[test] fn state_files_have_restricted_permissions_under_hardening() { use std::os::unix::fs::PermissionsExt; @@ -45,9 +46,7 @@ fn state_files_have_restricted_permissions_under_hardening() { dir.path().to_path_buf(), SystemFileOperations::with_policy(true), ); - tracker - .start_tracking("d-test", "cmd-test") - .expect("start tracking"); + tracker.start_tracking("d-test", "cmd-test").expect("start tracking"); let file_path = dir.path().join("d-test"); let mode = std::fs::metadata(&file_path) @@ -60,6 +59,7 @@ fn state_files_have_restricted_permissions_under_hardening() { } /// Default counterpart: with the hardening flag unset the tracker writes 0644. +#[cfg(unix)] #[test] fn state_files_are_world_readable_by_default() { use std::os::unix::fs::PermissionsExt; @@ -289,10 +289,7 @@ fn config_http_endpoint_accepted() { let permissive = dir.path().join("http.yml"); std::fs::write(&permissive, "deploy_control_endpoint: \"http://internal-mock.local\"\n") .expect("write http config"); - assert!( - AgentConfig::from_file(&permissive).is_ok(), - "http:// must be accepted" - ); + assert!(AgentConfig::from_file(&permissive).is_ok(), "http:// must be accepted"); } // --------------------------------------------------------------------------- @@ -344,6 +341,7 @@ fn config_caps_excessive_timeout() { /// **Current gap:** `AgentConfig::from_file()` does not check file permissions /// before reading. The fix should `stat()` the file and log a warning if /// group/world-readable bits are set. +#[cfg(unix)] #[test] fn config_warns_on_insecure_permissions() { use codedeploy_agent::config::AgentConfig;