diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1094fa1c..4bdc8d35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] steps: - uses: actions/checkout@v4 - name: Run tests @@ -54,7 +54,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest] + os: [ubuntu-latest, windows-latest, macos-latest] steps: - uses: actions/checkout@v4 - name: Build release binary diff --git a/CHANGELOG.md b/CHANGELOG.md index 14080264..b2ef7d19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,35 @@ +## 2.1.2 + +### Added + +* **macOS support** on EC2 Mac, macOS 15 and later (Apple silicon, aarch64). The agent ships as a `.pkg` and runs as a launchd LaunchDaemon. Deployments use AppSpec `os: linux`. AppSpec `permissions` entries that set `acls` or `context` are rejected before the install runs; `owner`, `group` and `mode` apply. Agent self-update is not available on macOS yet. +* **Optional parallel S3 bundle downloads.** Setting `s3_download_max_concurrency` above 1 (max 64) downloads a bundle in parallel HTTP range requests of `s3_download_chunk_size` bytes (default 8MiB, minimum 64KiB). Every range is pinned to the object's ETag, or to its version when one is set, so an object replaced mid-download fails the deployment. The default of 1 keeps the single-request download. + +### Fixed + +* **Deployment log format matches the 1.x agent ([#430](https://github.com/aws/aws-codedeploy-agent/issues/430)).** Each `scripts.log` line is now written to `codedeploy-agent-deployments.log` as it is produced, as `[] []`. Previously the agent copied the last 2048 bytes of script output into the log after each lifecycle event, which dropped the deployment ID, doubled the timestamp, added a blank line after each record, and lost `LifecycleEvent` headers and any older output. The failure-summary line is removed, and blank script output lines are logged as a bare `[stdout]` record in both files. +* **Command endpoints in the European Sovereign Cloud and ISO partitions.** Hosts in `eusc-*`, `us-iso-*`, `us-isob-*`, `eu-isoe-*` and `us-isof-*` regions resolved an `amazonaws.com` endpoint that does not exist. Each partition now uses its own DNS suffix. + +### Security + +* **(opt-in)** On macOS, `strip_loader_env_in_hooks` strips the `DYLD_*` loader variables from hook environments. + +## 2.1.0 + +### Added + +* **`RESTART` deployment mode.** The agent installs from the copy of the revision from the deployment group's last successful deployment already on the instance. If that copy is missing or incomplete, the agent downloads the revision. + +### Security + +* A deployment whose AppSpec file path resolves outside the application revision directory is rejected. Nested paths such as `configs/appspec.yml` are still supported. + +## 2.0.1 + +### Fixed + +* **Windows AppSpec paths with leading separators.** A `files.source` or `hooks.location` beginning with `\` (including the bare `\` form) or `/` resolved against the drive root instead of the deployment archive, failing Install events with access-denied errors. Leading separators are now stripped. + ## 2.0.0 Version 2.0.0 replaces the Ruby runtime with a single statically linked binary in Rust, completely removing the dependency on a Ruby runtime. diff --git a/Cargo.toml b/Cargo.toml index 91aca3a2..ab6bb07b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ edition = "2024" rust-version = "1.91" homepage = "https://aws.amazon.com/codedeploy/" repository = "https://github.com/aws/aws-codedeploy-agent" -version = "2.1.0" +version = "2.1.2" description = "AWS CodeDeploy agent responsible for deploying software on an individual EC2/On-prem instance" license = "Apache-2.0" authors = ["Amazon Web Services"] @@ -27,7 +27,8 @@ tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt", "regist tracing-appender = "0.2" time = { version = "0.3", features = ["macros", "formatting"] } nu-path = "0.110" -tokio = { version = "1", features = ["process", "io-util", "time", "rt", "macros", "sync"] } +# `rt-multi-thread`: parallel S3 range downloads. +tokio = { version = "1", features = ["process", "io-util", "time", "rt", "rt-multi-thread", "macros", "sync"] } chrono = "0.4" reqwest = { version = "0.12", default-features = false, features = ["blocking", "rustls-tls"] } # `vendored`: ship OpenSSL inside the binary so one artifact runs across @@ -53,7 +54,7 @@ subtle = "2" strip-ansi-escapes = "0.2" [target.'cfg(unix)'.dependencies] -nix = { version = "0.29", features = ["user", "fs", "signal", "process", "resource"] } +nix = { version = "0.29", features = ["user", "fs", "signal", "process", "resource", "event"] } signal-hook = "0.3" [target.'cfg(windows)'.dependencies] diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index eccb52ff..44b16e15 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -319,6 +319,34 @@ rustup default stable-x86_64-pc-windows-gnu cargo build --release ``` +## Building on macOS + +Target: `aarch64-apple-darwin` (Apple silicon), built with the Xcode +command-line tools. `MACOSX_DEPLOYMENT_TARGET=15.0` matches the oldest +supported macOS (15 Sequoia). + +```bash +xcode-select --install +rustup target add aarch64-apple-darwin + +export MACOSX_DEPLOYMENT_TARGET=15.0 +cargo build --release --target aarch64-apple-darwin +``` + +Package the binary as a `.pkg` (payload: binary, `codedeploy-local` symlink, +config template, LaunchDaemon plist; see `macos/pkg/`): + +```bash +macos/pkg/build-pkg.sh target/aarch64-apple-darwin/release/codedeploy-agent aarch64 2.1.1 2.1.1-1 out/ +macos/pkg/build-pkg.sh target/x86_64-apple-darwin/release/codedeploy-agent x86_64 2.1.1 2.1.1-1 out/ +sudo installer -pkg out/codedeploy-agent-2.1.1-1.aarch64.pkg -target / +``` + +The package installs `/Library/LaunchDaemons/com.amazon.aws.codedeploy-agent.plist` +(the launchd counterpart of `systemd/codedeploy-agent.service`) and starts the +agent. `sudo launchctl print system/com.amazon.aws.codedeploy-agent` shows its +state; `sudo /opt/codedeploy-agent/bin/uninstall.sh` removes it. + ## Running the Agent Locally Build and locate the binary: @@ -471,6 +499,7 @@ so `teardown` works across terminal sessions. | `e2e-ec2-iam-session.sh` | Fresh EC2 instance | On-prem registration, INI file — IMDS bypassed | | `e2e-ec2-concurrent.sh` | Fresh EC2 instance | IMDS, runs N parallel deployments (default 3) against the same instance | | `e2e-windows.sh` | Fresh Windows Server EC2 | IMDS | +| _(macOS)_ | No script: EC2 Mac requires a Dedicated Host with a 24 h minimum charge. Build the `.pkg` (see [Building on macOS](#building-on-macos)), install it on a Mac instance and run a deployment by hand. | IMDS | ### Shortest path to a green E2E run diff --git a/README.md b/README.md index 11932d56..74b44234 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,23 @@ AWS Lambda compute platform. The agent is licensed under [Apache License 2.0](LICENSE). +## Supported platforms + +The agent is built and tested on the following operating systems. Linux and +Windows ship as rpm/deb and msi; macOS ships as a `.pkg` (see +[Building on macOS](DEVELOPMENT.md#building-on-macos)). + +| Platform | Versions | Architectures | Service manager | +|---|---|---|---| +| Amazon Linux | 2, 2023 | x86_64, aarch64 | systemd | +| RHEL, Oracle Linux | 8, 9, 10 | x86_64, aarch64 | systemd | +| Rocky Linux, CentOS Stream | 9, 10 | x86_64, aarch64 | systemd | +| SLES | 15 | x86_64, aarch64 | systemd | +| Debian | 11, 12, 13 | x86_64, aarch64 | systemd | +| Ubuntu Server | 16.04–26.04 LTS, 25.04 | x86_64, aarch64 | systemd | +| Windows Server | 2016, 2019, 2022, 2025 | x86_64 | Service Control Manager | +| macOS (EC2 Mac) | 15 Sequoia, 26 Tahoe, 27 | aarch64 | launchd | + ## Getting Started ### Prerequisites diff --git a/conf/codedeployagent.yml b/conf/codedeployagent.yml index 40f6f73a..03688648 100644 --- a/conf/codedeployagent.yml +++ b/conf/codedeployagent.yml @@ -4,6 +4,8 @@ # Logging :program_name: codedeploy-agent :log_dir: /var/log/aws/codedeploy-agent +# macOS: the LaunchDaemon plist also writes codedeploy-agent.launchd.log into +# the default directory above; changing :log_dir does not move that file. :verbose: false # Capture Amazon S3 HTTP wire logs to .aws_wire.log in log_dir @@ -123,6 +125,15 @@ # :archive_max_extraction_size: "4GiB" # binary (powers of 1024) # :archive_max_extraction_size: "4GB" # SI (powers of 1000) +# Bundle download tuning (Amazon S3 revisions). By default the agent downloads +# a bundle with a single GetObject. +# s3_download_max_concurrency: parallel HTTP range requests in flight +# (default 1, max 64). Set above 1 to download in parallel ranges. +# s3_download_chunk_size: bytes per request (default 8MiB, minimum 64KiB). +# Accepts bytes or strings like "16MB". +# :s3_download_max_concurrency: 10 +# :s3_download_chunk_size: "8MiB" + # Endpoints (leave empty for defaults) # :deploy_control_endpoint: # :s3_endpoint_override: diff --git a/crates/codedeploy-commands/Cargo.toml b/crates/codedeploy-commands/Cargo.toml index 72923d57..c67f3241 100644 --- a/crates/codedeploy-commands/Cargo.toml +++ b/crates/codedeploy-commands/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codedeploy-commands" -version = "2.1.0" +version = "2.1.2" edition = "2024" rust-version = "1.91" publish = false diff --git a/crates/codedeploy-commands/src/endpoint.rs b/crates/codedeploy-commands/src/endpoint.rs index 379d77a8..f0882f8e 100644 --- a/crates/codedeploy-commands/src/endpoint.rs +++ b/crates/codedeploy-commands/src/endpoint.rs @@ -24,13 +24,25 @@ pub fn resolve( service.push_str("-fips"); } - let domain = if region.starts_with("cn-") { + format!("https://{service}.{region}.{}", partition_dns_suffix(region)) +} + +fn partition_dns_suffix(region: &str) -> &'static str { + if region.starts_with("cn-") { "amazonaws.com.cn" + } else if region.starts_with("eusc-") { + "amazonaws.eu" + } else if region.starts_with("us-iso-") { + "c2s.ic.gov" + } else if region.starts_with("us-isob-") { + "sc2s.sgov.gov" + } else if region.starts_with("eu-isoe-") { + "cloud.adc-e.uk" + } else if region.starts_with("us-isof-") { + "csp.hci.ic.gov" } else { "amazonaws.com" - }; - - format!("https://{service}.{region}.{domain}") + } } #[cfg(test)] @@ -77,6 +89,70 @@ mod tests { ); } + #[test] + fn eusc_region_endpoint() { + assert_eq!( + resolve("eusc-de-east-1", None, false, false), + "https://codedeploy-commands.eusc-de-east-1.amazonaws.eu" + ); + } + + #[test] + fn eusc_region_secure_endpoint() { + assert_eq!( + resolve("eusc-de-east-1", None, false, true), + "https://codedeploy-commands-secure.eusc-de-east-1.amazonaws.eu" + ); + } + + #[test] + fn gov_cloud_stays_on_default_domain() { + assert_eq!( + resolve("us-gov-west-1", None, false, true), + "https://codedeploy-commands-secure.us-gov-west-1.amazonaws.com" + ); + } + + #[test] + fn iso_region_endpoint() { + assert_eq!( + resolve("us-iso-east-1", None, false, false), + "https://codedeploy-commands.us-iso-east-1.c2s.ic.gov" + ); + } + + #[test] + fn iso_b_region_endpoint() { + assert_eq!( + resolve("us-isob-east-1", None, false, false), + "https://codedeploy-commands.us-isob-east-1.sc2s.sgov.gov" + ); + } + + #[test] + fn iso_e_region_endpoint() { + assert_eq!( + resolve("eu-isoe-west-1", None, false, false), + "https://codedeploy-commands.eu-isoe-west-1.cloud.adc-e.uk" + ); + } + + #[test] + fn iso_f_region_endpoint() { + assert_eq!( + resolve("us-isof-south-1", None, false, false), + "https://codedeploy-commands.us-isof-south-1.csp.hci.ic.gov" + ); + } + + #[test] + fn iso_region_secure_endpoint() { + assert_eq!( + resolve("us-iso-east-1", None, false, true), + "https://codedeploy-commands-secure.us-iso-east-1.c2s.ic.gov" + ); + } + #[test] fn custom_override_bypasses_construction() { assert_eq!( diff --git a/launchd/com.amazon.aws.codedeploy-agent.plist b/launchd/com.amazon.aws.codedeploy-agent.plist new file mode 100644 index 00000000..5aa2b75a --- /dev/null +++ b/launchd/com.amazon.aws.codedeploy-agent.plist @@ -0,0 +1,102 @@ + + + + + + Label + com.amazon.aws.codedeploy-agent + + ProgramArguments + + /bin/bash + -a + -c + [ -f /etc/profile ] && source /etc/profile; BASH_EXECUTION_STRING='[ -f /etc/profile ] && source /etc/profile; /opt/codedeploy-agent/bin/codedeploy-agent start'; exec /opt/codedeploy-agent/bin/codedeploy-agent start + + + UserName + root + + RunAtLoad + + + KeepAlive + + Crashed + + + + ThrottleInterval + 10 + + ExitTimeOut + 7200 + + + SoftResourceLimits + + NumberOfFiles + 65536 + Core + -1 + + HardResourceLimits + + NumberOfFiles + 65536 + Core + -1 + + + StandardOutPath + /var/log/aws/codedeploy-agent/codedeploy-agent.launchd.log + StandardErrorPath + /var/log/aws/codedeploy-agent/codedeploy-agent.launchd.log + + + ProcessType + Adaptive + + diff --git a/macos/pkg/Distribution.xml.in b/macos/pkg/Distribution.xml.in new file mode 100644 index 00000000..9f12bbe0 --- /dev/null +++ b/macos/pkg/Distribution.xml.in @@ -0,0 +1,24 @@ + + + + AWS CodeDeploy Agent + com.amazon.aws + + + + + + + + + + + + + + + + + + com.amazon.aws.codedeploy-agent.pkg + diff --git a/macos/pkg/build-pkg.sh b/macos/pkg/build-pkg.sh new file mode 100755 index 00000000..e3f76dea --- /dev/null +++ b/macos/pkg/build-pkg.sh @@ -0,0 +1,90 @@ +#!/bin/sh +# Build the macOS installer package (.pkg) for one architecture. +# +# Run after `cargo build --release --target `. +# Runs on macOS only (pkgbuild/productbuild are Xcode command-line tools). +# +# Usage: +# macos/pkg/build-pkg.sh +# binary path to the release codedeploy-agent binary for +# arch x86_64 | aarch64 (matches LATEST_VERSION pkg_ keys) +# version bare agent version, e.g. 2.1.1 (package version) +# full-version version-build, e.g. 2.1.1-1234 (file name only) +# out-dir directory to write codedeploy-agent-..pkg +# +# Payload (install-location /): +# /opt/codedeploy-agent/bin/codedeploy-agent 0755 +# /opt/codedeploy-agent/bin/codedeploy-local -> codedeploy-agent (symlink; see legacy_local_argv in src/main.rs) +# /private/etc/codedeploy-agent/conf/codedeployagent.yml 0644 (kept on upgrade, see scripts/) +# /Library/LaunchDaemons/com.amazon.aws.codedeploy-agent.plist 0644 +# (/etc is a symlink to /private/etc on macOS; the payload uses the real path.) +# Signing and notarization are separate steps, not done here. +set -eu + +if [ "$#" -ne 5 ]; then + sed -n '2,/^set -eu/p' "$0" | sed 's/^# \{0,1\}//' >&2 + exit 2 +fi + +BINARY=$1 +ARCH=$2 +VERSION=$3 +FULL_VERSION=$4 +OUT_DIR=$5 + +case "$ARCH" in + x86_64) HOST_ARCH=x86_64 ;; + aarch64) HOST_ARCH=arm64 ;; # Distribution.xml uses Apple's name + *) echo "arch must be x86_64 or aarch64, got '$ARCH'" >&2; exit 2 ;; +esac +[ -f "$BINARY" ] || { echo "binary not found: $BINARY" >&2; exit 2; } + +HERE=$(cd "$(dirname "$0")" && pwd) +REPO=$(cd "$HERE/../.." && pwd) +IDENTIFIER=com.amazon.aws.codedeploy-agent +MIN_OS=15.0 # oldest supported macOS; must match MACOSX_DEPLOYMENT_TARGET + +WORK=$(mktemp -d "${TMPDIR:-/tmp}/codedeploy-pkg.XXXXXX") +trap 'rm -rf "$WORK"' EXIT +ROOT="$WORK/root" + +# --- payload --------------------------------------------------------------- +install -d -m 0755 "$ROOT/opt/codedeploy-agent/bin" +install -m 0755 "$BINARY" "$ROOT/opt/codedeploy-agent/bin/codedeploy-agent" +ln -s codedeploy-agent "$ROOT/opt/codedeploy-agent/bin/codedeploy-local" +# pkg has no uninstall hook; ship the manual uninstaller next to the binary. +install -m 0755 "$HERE/uninstall.sh" "$ROOT/opt/codedeploy-agent/bin/uninstall.sh" + +install -d -m 0755 "$ROOT/private/etc/codedeploy-agent/conf" +install -m 0644 "$REPO/conf/codedeployagent.yml" "$ROOT/private/etc/codedeploy-agent/conf/codedeployagent.yml" + +install -d -m 0755 "$ROOT/Library/LaunchDaemons" +install -m 0644 "$REPO/launchd/$IDENTIFIER.plist" "$ROOT/Library/LaunchDaemons/$IDENTIFIER.plist" + +# --- component package ----------------------------------------------------- +COMPONENT="$WORK/$IDENTIFIER.pkg" +# installer silently skips non-executable scripts. +chmod 0755 "$HERE/scripts/preinstall" "$HERE/scripts/postinstall" +pkgbuild \ + --root "$ROOT" \ + --install-location / \ + --identifier "$IDENTIFIER" \ + --version "$VERSION" \ + --scripts "$HERE/scripts" \ + --ownership recommended \ + "$COMPONENT" + +# --- distribution package -------------------------------------------------- +sed -e "s|@VERSION@|$VERSION|g" \ + -e "s|@HOST_ARCH@|$HOST_ARCH|g" \ + -e "s|@MIN_OS@|$MIN_OS|g" \ + "$HERE/Distribution.xml.in" > "$WORK/Distribution.xml" + +mkdir -p "$OUT_DIR" +OUT="$OUT_DIR/codedeploy-agent-$FULL_VERSION.$ARCH.pkg" +productbuild \ + --distribution "$WORK/Distribution.xml" \ + --package-path "$WORK" \ + "$OUT" + +echo "$OUT" diff --git a/macos/pkg/scripts/postinstall b/macos/pkg/scripts/postinstall new file mode 100755 index 00000000..1e273e44 --- /dev/null +++ b/macos/pkg/scripts/postinstall @@ -0,0 +1,47 @@ +#!/bin/sh +# pkg postinstall: runs as root after the payload is written. +# +# Like rpm %post / deb postinst: restore the config preinstall saved (keeping +# the new template as .pkgnew), create the log dir launchd writes to, then +# enable and bootstrap the LaunchDaemon. +set -eu + +LABEL=com.amazon.aws.codedeploy-agent +PLIST=/Library/LaunchDaemons/$LABEL.plist +CONF=/etc/codedeploy-agent/conf/codedeployagent.yml + +if [ -f "$CONF.pkg-save" ]; then + cp -p "$CONF" "$CONF.pkgnew" + mv -f "$CONF.pkg-save" "$CONF" +fi + +install -d -m 0755 /var/log/aws/codedeploy-agent +install -d -m 0755 /opt/codedeploy-agent + +launchctl enable "system/$LABEL" || : +# bootout any stale definition (kickstart would reuse the old one), then +# bootstrap the plist just installed; a failure fails the install. +launchctl bootout "system/$LABEL" 2>/dev/null || : +# Normally nothing is running here; if something re-bootstrapped the agent +# mid-deployment, wait the same ExitTimeOut bound as preinstall. +i=0 +while launchctl print "system/$LABEL" >/dev/null 2>&1; do + i=$((i + 1)) + if [ "$i" -ge 7260 ]; then # ExitTimeOut + margin: let launchd's own SIGKILL land first + echo "$LABEL is still loaded after ${i}s (ExitTimeOut); cannot bootstrap the new plist" >&2 + exit 1 + fi + sleep 1 +done +# bootstrap can fail transiently right after a bootout; retry briefly. +i=0 +until launchctl bootstrap system "$PLIST"; do + i=$((i + 1)) + if [ "$i" -ge 5 ]; then + echo "$LABEL: launchctl bootstrap failed after $i attempts" >&2 + exit 1 + fi + sleep 1 +done + +exit 0 diff --git a/macos/pkg/scripts/preinstall b/macos/pkg/scripts/preinstall new file mode 100755 index 00000000..3feed5e0 --- /dev/null +++ b/macos/pkg/scripts/preinstall @@ -0,0 +1,38 @@ +#!/bin/sh +# pkg preinstall: runs as root before the payload is written. +# +# Like rpm %pre / deb preinst: save the operator's config for postinstall to +# restore (the payload ships the template), then stop the agent and wait for +# it to exit so the binary is not replaced under it. +set -eu + +LABEL=com.amazon.aws.codedeploy-agent +CONF=/etc/codedeploy-agent/conf/codedeployagent.yml + +if [ -f "$CONF" ]; then + # Keep an existing save: after a crashed upgrade $CONF may be the template + # and .pkg-save the only copy of the operator's config. postinstall removes + # the save on success, so a lingering one always means "restore me". + [ -f "$CONF.pkg-save" ] || cp -p "$CONF" "$CONF.pkg-save" +else + # Fresh install: a stale save must not be restored over the template. + rm -f "$CONF.pkg-save" +fi + +if launchctl print "system/$LABEL" >/dev/null 2>&1; then + launchctl bootout "system/$LABEL" || : + # bootout only sends SIGTERM; wait for the job to unload before the payload + # replaces the binary. launchd SIGKILLs at ExitTimeOut (7200 s), so a job + # still present after that means launchd is wedged: fail the install. + i=0 + while launchctl print "system/$LABEL" >/dev/null 2>&1; do + i=$((i + 1)) + if [ "$i" -ge 7260 ]; then # ExitTimeOut + margin: let launchd's own SIGKILL land first + echo "$LABEL did not exit within ExitTimeOut; aborting install" >&2 + exit 1 + fi + sleep 1 + done +fi + +exit 0 diff --git a/macos/pkg/uninstall.sh b/macos/pkg/uninstall.sh new file mode 100755 index 00000000..bd8be3ca --- /dev/null +++ b/macos/pkg/uninstall.sh @@ -0,0 +1,40 @@ +#!/bin/sh +# Uninstall the AWS CodeDeploy agent on macOS. +# +# .pkg has no uninstall hook; this is the rpm %preun / deb prerm counterpart: +# sudo /opt/codedeploy-agent/bin/uninstall.sh +# Deployment state, logs and the operator's config are left in place, as the +# Linux packages do; only the .pkgnew template copy is removed. +set -eu + +LABEL=com.amazon.aws.codedeploy-agent +PLIST=/Library/LaunchDaemons/$LABEL.plist + +if launchctl print "system/$LABEL" >/dev/null 2>&1; then + launchctl bootout "system/$LABEL" || : + # bootout only sends SIGTERM; wait (up to ExitTimeOut) for the job to + # unload before removing the binary it may still re-exec. + i=0 + while launchctl print "system/$LABEL" >/dev/null 2>&1; do + i=$((i + 1)) + if [ "$i" -ge 7260 ]; then # ExitTimeOut + margin: let launchd's own SIGKILL land first + echo "$LABEL did not exit within ExitTimeOut (7200 s); aborting uninstall" >&2 + exit 1 + fi + sleep 1 + done +fi +launchctl disable "system/$LABEL" || : + +pkgutil --forget "$LABEL" >/dev/null 2>&1 || : + +rm -f "$PLIST" +rm -f /opt/codedeploy-agent/bin/codedeploy-local +rm -f /opt/codedeploy-agent/bin/codedeploy-agent +rm -f /opt/codedeploy-agent/bin/uninstall.sh # this script; sh keeps its own fd open +rmdir /opt/codedeploy-agent/bin 2>/dev/null || : +# Keep codedeployagent.yml; drop only the .pkgnew template copy. +rm -f /etc/codedeploy-agent/conf/codedeployagent.yml.pkgnew +rmdir /etc/codedeploy-agent/conf /etc/codedeploy-agent 2>/dev/null || : + +echo "AWS CodeDeploy agent removed." diff --git a/src/application_specification/error.rs b/src/application_specification/error.rs index 58ffe5a5..1ea30d4b 100644 --- a/src/application_specification/error.rs +++ b/src/application_specification/error.rs @@ -33,6 +33,11 @@ pub enum ParseError { )] InvalidTimeout, + #[error( + "The deployment failed because the runas value ({0}) in the application specification file is not a valid user name. The name must not be empty, must not start with '-', and must not contain whitespace or control characters. Update the value, and then try again." + )] + InvalidRunas(String), + #[error( "The deployment failed because the application specification file specifies a destination file, but no source file. Update the files section of the AppSpec file, and then try again." )] @@ -267,6 +272,7 @@ mod tests { ParseError::InvalidFileExistsBehavior("bad".to_string()), ParseError::EmptyScriptLocation, ParseError::InvalidTimeout, + ParseError::InvalidRunas("-c".to_string()), ParseError::MissingSource, ParseError::MissingDestination("file".to_string()), ParseError::PermissionsOnWindows, diff --git a/src/application_specification/hooks.rs b/src/application_specification/hooks.rs index 4c799885..ce39fd32 100644 --- a/src/application_specification/hooks.rs +++ b/src/application_specification/hooks.rs @@ -87,7 +87,24 @@ impl ScriptLocation { } #[derive(Debug, Clone, PartialEq)] -pub struct Username(pub(crate) String); +pub struct Username(String); + +impl Username { + /// Reject a `runas` that `su`/`sudo` would read as an option (leading `-`) + /// or that contains whitespace/control characters. Everything else is + /// accepted (`DOMAIN\user`, `user@realm`); shell metacharacters are inert + /// because `runas` is passed as its own argv entry, never through a shell. + pub(crate) fn new(name: &str) -> Result { + let valid = !name.is_empty() + && !name.starts_with('-') + && !name.bytes().any(|b| b.is_ascii_whitespace() || b.is_ascii_control()); + if valid { + Ok(Username(name.to_string())) + } else { + Err(ParseError::InvalidRunas(name.to_string())) + } + } +} #[derive(Debug, Clone, Copy, PartialEq)] pub struct Timeout(u32); @@ -117,6 +134,7 @@ impl Default for Timeout { #[cfg(test)] mod tests { + use super::{ParseError, Username}; use crate::application_specification::AppSpec; #[test] @@ -139,4 +157,42 @@ mod tests { let scripts = spec.hooks().get("ApplicationStop"); assert!(scripts.is_empty()); } + + #[test] + fn username_accepts_real_account_names_including_domain_forms() { + for n in [ + "root", + "deploy", + "www-data", + "svc_user", + "user.name", + "host$", + "DOMAIN\\user", + "user@realm", + "a/b", + "root;id", + "$(id)", + ] { + assert!(Username::new(n).is_ok(), "{n:?} should be accepted"); + } + } + + #[test] + fn username_rejects_option_like_and_whitespace_names() { + for n in [ + "-c", + "--command=id", + "-", + "a b", + "\troot", + "root\n", + "a\0b", + "", + ] { + assert!( + matches!(Username::new(n), Err(ParseError::InvalidRunas(_))), + "{n:?} should be rejected" + ); + } + } } diff --git a/src/application_specification/parse.rs b/src/application_specification/parse.rs index 149c0d96..14a78b37 100644 --- a/src/application_specification/parse.rs +++ b/src/application_specification/parse.rs @@ -80,14 +80,10 @@ impl RawScript { None => return Err(ParseError::EmptyScriptLocation), }; - let runas = self.runas.and_then(|s| { - let trimmed = s.trim().to_string(); - if trimmed.is_empty() { - None - } else { - Some(Username(trimmed)) - } - }); + let runas = match self.runas.as_deref().map(str::trim) { + None | Some("") => None, + Some(trimmed) => Some(Username::new(trimmed)?), + }; let timeout = match self.timeout { Some(t) => Timeout::new(t)?, @@ -1059,4 +1055,10 @@ mod tests { assert_eq!(ctx.user(), Some("system_u")); assert_eq!(ctx.type_(), "object_t"); } + + #[test] + fn script_runas_option_like_value_is_rejected() { + let yaml = "version: 0.0\nos: linux\nhooks:\n ApplicationStart:\n - location: start.sh\n runas: \"-c\"\n"; + assert!(matches!(AppSpec::parse(yaml), Err(ParseError::InvalidRunas(v)) if v == "-c")); + } } diff --git a/src/application_specification/selinux.rs b/src/application_specification/selinux.rs index e99cf7f0..1f1c6396 100644 --- a/src/application_specification/selinux.rs +++ b/src/application_specification/selinux.rs @@ -28,7 +28,9 @@ impl SeLinuxContext { /// /// This is only used for testing the role validation error path. /// In production, role is always None. - #[cfg(test)] + // SELinux is Linux-only; macOS is unix but not linux. The only caller is the + // Linux-gated ChangeContextCommand test. + #[cfg(all(test, target_os = "linux"))] pub(crate) fn new_with_role( user: Option, role: Option, diff --git a/src/aws_clients/s3_client.rs b/src/aws_clients/s3_client.rs index 7606162d..b3e918ee 100644 --- a/src/aws_clients/s3_client.rs +++ b/src/aws_clients/s3_client.rs @@ -3,9 +3,23 @@ //! Wraps `aws-sdk-s3` with agent-specific configuration: endpoint override, //! FIPS mode, proxy, and signature v4 (SDK default). //! -//! Downloads use a single `GetObject` request with streaming body. -//! For very large bundles, consider switching to `aws-s3-transfer-manager` -//! which splits downloads into parallel range requests for higher throughput. +//! ## Parallel downloads +//! +//! When `download_max_concurrency` is above 1, a download is split into +//! `download_chunk_size` range requests (default 8 MiB) with up to that many in +//! flight, each written to its own offset of the destination file. +//! +//! - The first request fetches the first chunk and reads the object size from +//! `Content-Range`, so a small object still costs one request. +//! - Later ranges send `If-Match` with the first `ETag` (and the `versionId`, if +//! pinned), so an object replaced mid-download fails instead of mixing two +//! revisions. +//! - Each range retries a body that fails mid-stream, which the SDK does not. +//! - The download falls back to one unranged `GetObject` when the endpoint +//! refuses ranges, the object is empty, there is no strong `ETag` or version +//! to pin the object, or a response without `Content-Range` may be truncated. +//! +//! The default concurrency, 1, downloads with one unranged `GetObject`. //! //! ## Wire logging (`log_aws_wire`) //! @@ -30,12 +44,35 @@ use super::aws_client::AwsClient; use super::credentials::{CredentialMode, Credentials}; -use std::io::{self, Write}; +use std::fs::File; +use std::io; use std::path::Path; -use tracing::{debug, info}; +use std::sync::Arc; +use tracing::{debug, info, warn}; + +/// Read buffer per streaming body. Peak memory is `concurrency * this`. +const STREAM_BUFFER_SIZE: usize = 1024 * 1024; + +/// Default range requests in flight. +pub const DEFAULT_DOWNLOAD_MAX_CONCURRENCY: usize = 1; + +/// Default range size. +pub const DEFAULT_DOWNLOAD_CHUNK_SIZE: u64 = 8 * 1024 * 1024; + +/// Maximum range requests in flight. +pub const MAX_DOWNLOAD_MAX_CONCURRENCY: usize = 64; -/// Streaming buffer size for S3 downloads. -const STREAM_BUFFER_SIZE: usize = 8 * 1024 * 1024; +/// Minimum range size. +pub const MIN_DOWNLOAD_CHUNK_SIZE: u64 = 64 * 1024; + +/// Maximum download worker threads. +const MAX_DOWNLOAD_WORKER_THREADS: usize = 8; + +/// Attempts per range. +const RANGE_ATTEMPTS: u32 = 3; + +/// Backoff between range attempts, multiplied by the attempt number. +const RANGE_RETRY_BACKOFF: std::time::Duration = std::time::Duration::from_millis(500); /// Load additional CA certificates from `AWS_SSL_CA_DIRECTORY` into a /// [`TrustStore`], if the env var is set and points at a directory with `*.pem` @@ -197,7 +234,7 @@ fn build_custom_http_client( } /// Configuration for S3 client construction. -#[derive(Debug, Default)] +#[derive(Debug, Clone)] pub struct S3ClientConfig { /// Custom S3 endpoint URL (overrides default). pub endpoint_override: Option, @@ -214,9 +251,37 @@ pub struct S3ClientConfig { /// wire logging is disabled (the default). The tuple is /// `(log_dir, program_name)`. pub wire_log: Option<(std::path::PathBuf, String)>, + /// Range requests in flight per download; `1` means one unranged + /// `GetObject`. Clamped to `1..=`[`MAX_DOWNLOAD_MAX_CONCURRENCY`]. + pub download_max_concurrency: usize, + /// Bytes per range request. Floored at [`MIN_DOWNLOAD_CHUNK_SIZE`]. + pub download_chunk_size: u64, +} + +impl Default for S3ClientConfig { + fn default() -> Self { + Self { + endpoint_override: None, + use_fips: false, + proxy_uri: None, + wire_log: None, + download_max_concurrency: DEFAULT_DOWNLOAD_MAX_CONCURRENCY, + download_chunk_size: DEFAULT_DOWNLOAD_CHUNK_SIZE, + } + } } impl S3ClientConfig { + /// Clamped concurrency; `0` becomes `1`. + fn effective_concurrency(&self) -> usize { + self.download_max_concurrency.clamp(1, MAX_DOWNLOAD_MAX_CONCURRENCY) + } + + /// Chunk size floored at [`MIN_DOWNLOAD_CHUNK_SIZE`]. + fn effective_chunk_size(&self) -> u64 { + self.download_chunk_size.max(MIN_DOWNLOAD_CHUNK_SIZE) + } + fn resolve_endpoint(&self, region: &str) -> Option { if let Some(endpoint) = &self.endpoint_override { debug!("Using S3 override endpoint {endpoint}"); @@ -235,6 +300,8 @@ pub struct S3Client { credentials: Credentials, inner: aws_sdk_s3::Client, runtime: tokio::runtime::Runtime, + max_concurrency: usize, + chunk_size: u64, } impl S3Client { @@ -243,11 +310,24 @@ impl S3Client { /// # Errors /// Returns an error if the tokio runtime cannot be created. pub fn new(credentials: Credentials, config: &S3ClientConfig) -> io::Result { - let runtime = tokio::runtime::Builder::new_current_thread().enable_all().build()?; + let max_concurrency = config.effective_concurrency(); + let chunk_size = config.effective_chunk_size(); + + // A current-thread runtime would serialize the ranges' TLS decryption + // and file writes. The workers live as long as the client. + let runtime = if max_concurrency > 1 { + tokio::runtime::Builder::new_multi_thread() + .worker_threads(download_worker_threads(max_concurrency)) + .thread_name("codedeploy-s3") + .enable_all() + .build()? + } else { + tokio::runtime::Builder::new_current_thread().enable_all().build()? + }; let inner = runtime.block_on(Self::build_client(&credentials, config))?; - Ok(Self { credentials, inner, runtime }) + Ok(Self { credentials, inner, runtime, max_concurrency, chunk_size }) } async fn build_client( @@ -334,9 +414,8 @@ impl S3Client { Ok(aws_sdk_s3::Client::from_conf(s3_config.build())) } - /// Download an S3 object to a local file. - /// - /// Returns the `ETag` of the downloaded object (with quotes stripped). + /// Download an S3 object to a local file and return its `ETag` (quotes + /// stripped). Uses parallel range requests unless concurrency is 1. /// /// # Errors /// Returns an error if the download or file write fails. @@ -362,42 +441,418 @@ impl S3Client { "Downloading artifact bundle from bucket '{bucket}' and key '{key}', version '{version_str}'" ); - let mut req = self.inner.get_object().bucket(bucket).key(key); - if let Some(v) = version { - req = req.version_id(v); - } + let etag = if self.max_concurrency == 1 { + self.download_whole_object(bucket, key, version, dest).await? + } else { + match self.download_ranged(bucket, key, version, dest).await { + Ok(etag) => etag, + Err(RangedDownloadError::Unsupported(reason)) => { + warn!( + "Ranged S3 download unavailable ({reason}); retrying as a single request" + ); + self.download_whole_object(bucket, key, version, dest).await? + }, + Err(RangedDownloadError::Failed(e)) => return Err(e), + } + }; + + info!("Download complete from bucket '{bucket}' and key '{key}'"); + Ok(etag) + } - let resp = req + /// One unranged `GetObject` streamed to disk. + async fn download_whole_object( + &self, + bucket: &str, + key: &str, + version: Option<&str>, + dest: &Path, + ) -> io::Result> { + let resp = self + .inner + .get_object() + .bucket(bucket) + .key(key) + .set_version_id(version.map(str::to_string)) .send() .await - .map_err(|e| io::Error::other(format_s3_get_object_error(&e, bucket, key, version)))?; + .map_err(|e| { + io::Error::other(format_s3_get_object_error(&e, bucket, key, version, None)) + })?; let etag = resp.e_tag().map(|e| e.trim_matches('"').to_string()); + let file = create_download_file(dest)?; + stream_body_at(&file, resp.body.into_async_read(), 0) + .await + .map_err(StreamError::into_io)?; + Ok(etag) + } - let body = resp.body.into_async_read(); - stream_to_file(body, dest).await?; + /// Download with parallel range requests. The first request fetches the first + /// chunk and learns the object size from `Content-Range`. + async fn download_ranged( + &self, + bucket: &str, + key: &str, + version: Option<&str>, + dest: &Path, + ) -> Result, RangedDownloadError> { + let first = ranged_get(&self.inner, bucket, key, version, None, 0, self.chunk_size) + .send() + .await + .map_err(|e| { + if range_unsupported(&s3_error_code(&e), http_status(&e)) { + RangedDownloadError::Unsupported(s3_error_code(&e)) + } else { + RangedDownloadError::Failed(io::Error::other(format_s3_get_object_error( + &e, + bucket, + key, + version, + Some(0), + ))) + } + })?; + + // The quoted ETag goes back out as If-Match; callers get it unquoted. + let raw_etag = first.e_tag().map(str::to_string); + let etag = raw_etag.as_deref().map(|e| e.trim_matches('"').to_string()); + let total = first + .content_range() + .and_then(parse_content_range) + .and_then(|(start, total)| (start == 0).then_some(total)); + + let file = create_download_file(dest)?; + let body = first.body.into_async_read(); + + let Some(total) = total else { + // Without a size there is nothing to plan, so the body must be the + // whole object or we start over unranged. + let len = stream_body_at(&file, body, 0).await.map_err(StreamError::into_io)?; + if first_response_is_complete(len, self.chunk_size) { + return Ok(etag); + } + return Err(RangedDownloadError::Unsupported( + "response carried no usable Content-Range".to_string(), + )); + }; - info!("Download complete from bucket '{bucket}' and key '{key}'"); + // We know the size, so a first body that fails mid-stream is re-fetched + // with the other ranges. A local write failure fails the download. + let head_len = + match stream_body_at(&file, tokio::io::AsyncReadExt::take(body, self.chunk_size), 0) + .await + { + Ok(len) => len, + Err(StreamError::Read(e)) => { + warn!( + "First range of s3://{bucket}/{key} failed mid-stream ({e}); re-fetching it" + ); + 0 + }, + Err(StreamError::Write(e)) => return Err(e.into()), + }; + if head_len >= total { + return Ok(etag); + } + + // Without a strong ETag or a version, nothing stops the object changing + // between ranges, so download it in one request instead. + let pin_etag = raw_etag.filter(|e| !e.starts_with("W/")); + if pin_etag.is_none() && version.is_none() { + return Err(RangedDownloadError::Unsupported( + "no strong ETag or version to pin the object".to_string(), + )); + } + + let ranges = plan_ranges(head_len, total, self.chunk_size); + info!( + "Bundle is {total} bytes: fetching {} ranges of up to {} bytes, {} at a time", + ranges.len(), + self.chunk_size, + self.max_concurrency + ); + file.set_len(total)?; + + let job = Arc::new(RangeJob { + client: self.inner.clone(), + bucket: bucket.to_string(), + key: key.to_string(), + version: version.map(str::to_string), + etag: pin_etag, + total, + file, + }); + self.fetch_ranges(&job, ranges).await?; Ok(etag) } + + /// Fetch `ranges` with at most `max_concurrency` in flight. On the first + /// error the other tasks are cancelled and awaited, so none of them is still + /// writing to the file when this returns. + async fn fetch_ranges(&self, job: &Arc, ranges: Vec<(u64, u64)>) -> io::Result<()> { + let permits = Arc::new(tokio::sync::Semaphore::new(self.max_concurrency)); + let mut tasks = tokio::task::JoinSet::new(); + for (offset, len) in ranges { + let job = Arc::clone(job); + let permits = Arc::clone(&permits); + tasks.spawn(async move { + let _permit = permits.acquire().await.map_err(io::Error::other)?; + job.fetch_with_retry(offset, len).await + }); + } + while let Some(joined) = tasks.join_next().await { + if let Err(e) = joined.map_err(io::Error::other).and_then(|r| r) { + tasks.shutdown().await; + return Err(e); + } + } + Ok(()) + } +} + +/// A ranged `GetObject` request. The SDK's response checksum check is turned +/// off because a full-object checksum header cannot be checked against one +/// range; each range's length is checked instead. +fn ranged_get( + client: &aws_sdk_s3::Client, + bucket: &str, + key: &str, + version: Option<&str>, + if_match: Option<&str>, + offset: u64, + len: u64, +) -> aws_sdk_s3::client::customize::CustomizableOperation< + aws_sdk_s3::operation::get_object::GetObjectOutput, + aws_sdk_s3::operation::get_object::GetObjectError, + aws_sdk_s3::operation::get_object::builders::GetObjectFluentBuilder, +> { + client + .get_object() + .bucket(bucket) + .key(key) + .set_version_id(version.map(str::to_string)) + .set_if_match(if_match.map(str::to_string)) + .range(range_header(offset, len)) + .customize() + .config_override(aws_sdk_s3::config::Builder::default().response_checksum_validation( + aws_sdk_s3::config::ResponseChecksumValidation::WhenRequired, + )) +} + +/// Why a ranged download did not complete. +enum RangedDownloadError { + /// The endpoint or object cannot serve ranges; fall back to one request. + Unsupported(String), + /// A real failure; the deployment fails. + Failed(io::Error), +} + +impl From for RangedDownloadError { + fn from(e: io::Error) -> Self { + Self::Failed(e) + } +} + +/// Worker threads for the download runtime: `min(cores, concurrency)`, capped +/// at [`MAX_DOWNLOAD_WORKER_THREADS`]. +fn download_worker_threads(max_concurrency: usize) -> usize { + let cores = std::thread::available_parallelism().map_or(1, std::num::NonZeroUsize::get); + cores.min(max_concurrency).clamp(1, MAX_DOWNLOAD_WORKER_THREADS) +} + +/// Create the download file with mode 0600. +fn create_download_file(dest: &Path) -> io::Result { + crate::system::create_file_secure(dest, 0o600) +} + +/// `Range` header for `len` (> 0) bytes from `offset`; HTTP ranges are inclusive. +fn range_header(offset: u64, len: u64) -> String { + debug_assert!(len > 0, "a range must cover at least one byte"); + format!("bytes={offset}-{}", offset + len - 1) +} + +/// Split `[start, total)` into `(offset, len)` chunks. +fn plan_ranges(start: u64, total: u64, chunk_size: u64) -> Vec<(u64, u64)> { + let chunk_size = chunk_size.max(1); + let mut ranges = Vec::new(); + let mut offset = start; + while offset < total { + let len = chunk_size.min(total - offset); + ranges.push((offset, len)); + offset += len; + } + ranges +} + +/// `(start, total)` from `Content-Range: bytes 0-99/1234`. `None` if malformed +/// or the total is `*`. +fn parse_content_range(header: &str) -> Option<(u64, u64)> { + let (range, total) = header.trim().strip_prefix("bytes ")?.split_once('/')?; + let (start, _) = range.split_once('-')?; + Some((start.trim().parse().ok()?, total.trim().parse().ok()?)) +} + +/// Whether a first response without `Content-Range` holds the whole object. +/// +/// It normally does (the endpoint ignored `Range` and sent everything). A body of +/// exactly the requested length could also be a truncated range response, so +/// that one case is downloaded again. +fn first_response_is_complete(body_len: u64, requested: u64) -> bool { + body_len != requested +} + +/// Whether a failed first request means the endpoint or object cannot serve +/// ranges: `416` on an empty object, or `501` from endpoints without range +/// support. The status is checked too, because error responses without an XML +/// body carry no code. +fn range_unsupported(code: &str, status: Option) -> bool { + matches!(code, "InvalidRange" | "NotImplemented") || matches!(status, Some(416 | 501)) +} + +/// Whether a failed range request means the object changed (`412`). +fn precondition_failed(code: &str, status: Option) -> bool { + code == "PreconditionFailed" || status == Some(412) +} + +/// S3 error code, or `"unknown"` when the failure carried none. +fn s3_error_code(err: &aws_sdk_s3::error::SdkError) -> String +where + E: aws_sdk_s3::error::ProvideErrorMetadata, +{ + use aws_sdk_s3::error::ProvideErrorMetadata; + err.code().unwrap_or("unknown").to_string() +} + +/// HTTP status of a failed request, when a response was received. +fn http_status( + err: &aws_sdk_s3::error::SdkError, +) -> Option { + err.raw_response().map(|r| r.status().as_u16()) +} + +/// Everything a range task needs, shared by all ranges of one download. +struct RangeJob { + client: aws_sdk_s3::Client, + bucket: String, + key: String, + version: Option, + /// Strong `ETag` as the first response sent it (quoted), sent as `If-Match`. + etag: Option, + /// Object size from the first response. + total: u64, + file: File, +} + +/// A range failure: a body that failed mid-stream can be retried; anything +/// else fails the download. +enum RangeError { + BodyFailed(io::Error), + Fatal(io::Error), +} + +impl RangeJob { + /// Fetch one range, retrying a body that fails mid-stream. The SDK already + /// retries failed requests, so those are not retried again here. Retries are + /// safe because each range is rewritten at a fixed offset. + async fn fetch_with_retry(&self, offset: u64, len: u64) -> io::Result<()> { + let mut attempt = 1; + loop { + match self.fetch(offset, len).await { + Ok(()) => return Ok(()), + Err(RangeError::BodyFailed(e)) if attempt < RANGE_ATTEMPTS => { + warn!( + "Range at offset {offset} of s3://{}/{} failed mid-stream (attempt {attempt} of {RANGE_ATTEMPTS}), retrying: {e}", + self.bucket, self.key + ); + tokio::time::sleep(RANGE_RETRY_BACKOFF * attempt).await; + attempt += 1; + }, + Err(RangeError::BodyFailed(e) | RangeError::Fatal(e)) => return Err(e), + } + } + } + + async fn fetch(&self, offset: u64, len: u64) -> Result<(), RangeError> { + let (bucket, key, version) = (&self.bucket, &self.key, self.version.as_deref()); + // versionId and If-Match pin the revision, so an object overwritten + // between ranges fails instead of producing a mixed file. + let resp = + ranged_get(&self.client, bucket, key, version, self.etag.as_deref(), offset, len) + .send() + .await + .map_err(|e| { + let mut message = + format_s3_get_object_error(&e, bucket, key, version, Some(offset)); + if precondition_failed(&s3_error_code(&e), http_status(&e)) { + message.push_str(" (the object changed during the download)"); + } + RangeError::Fatal(io::Error::other(message)) + })?; + + // The response must be the range we asked for, from the same object. + let content_range = resp.content_range().and_then(parse_content_range); + if content_range != Some((offset, self.total)) { + return Err(RangeError::Fatal(io::Error::other(format!( + "S3 range at offset {offset} of s3://{bucket}/{key} returned Content-Range {:?}, expected bytes {offset}-{}/{}", + resp.content_range(), + offset + len - 1, + self.total + )))); + } + + let body = tokio::io::AsyncReadExt::take(resp.body.into_async_read(), len); + let written = match stream_body_at(&self.file, body, offset).await { + Ok(n) => n, + Err(StreamError::Read(e)) => return Err(RangeError::BodyFailed(e)), + Err(StreamError::Write(e)) => return Err(RangeError::Fatal(e)), + }; + if written != len { + return Err(RangeError::BodyFailed(io::Error::other(format!( + "S3 range at offset {offset} of s3://{bucket}/{key} returned {written} bytes, expected {len}" + )))); + } + Ok(()) + } +} + +/// Which side of [`stream_body_at`] failed. +#[derive(Debug)] +enum StreamError { + /// Reading the response body (network). + Read(io::Error), + /// Writing the file (local disk). + Write(io::Error), } -async fn stream_to_file( +impl StreamError { + fn into_io(self) -> io::Error { + match self { + Self::Read(e) | Self::Write(e) => e, + } + } +} + +/// Stream `reader` into `file` from `offset` and return the byte count. Uses +/// positioned writes, so ranges can share one file handle. +async fn stream_body_at( + file: &File, mut reader: impl tokio::io::AsyncRead + Unpin, - dest: &Path, -) -> io::Result<()> { - // Downloaded bundles are created 0600 so unprivileged users cannot - // read the archive while the agent is extracting it. - let mut file = crate::system::create_file_secure(dest, 0o600)?; + offset: u64, +) -> Result { let mut buf = vec![0u8; STREAM_BUFFER_SIZE]; + let mut written = 0u64; loop { - let n = tokio::io::AsyncReadExt::read(&mut reader, &mut buf).await?; + let n = tokio::io::AsyncReadExt::read(&mut reader, &mut buf) + .await + .map_err(StreamError::Read)?; if n == 0 { - break; + return Ok(written); } - file.write_all(&buf[..n])?; + crate::system::write_all_at(file, &buf[..n], offset + written) + .map_err(StreamError::Write)?; + written += n as u64; } - file.flush() } /// Format an S3 `GetObject` failure naming the bucket/key and surfacing the S3 @@ -408,11 +863,14 @@ async fn stream_to_file( /// code/message from the error metadata; falls back to the full source-error /// chain for non-service failures (dispatch/timeout/TLS) that carry no response /// metadata. +/// +/// `range_offset` is the offset of the failing range, if any. fn format_s3_get_object_error( err: &aws_sdk_s3::error::SdkError, bucket: &str, key: &str, version: Option<&str>, + range_offset: Option, ) -> String where E: aws_sdk_s3::error::ProvideErrorMetadata + std::error::Error + Send + Sync + 'static, @@ -420,10 +878,11 @@ where { use aws_sdk_s3::error::ProvideErrorMetadata; - let location = match version { - Some(v) => format!("s3://{bucket}/{key} (version {v})"), - None => format!("s3://{bucket}/{key}"), - }; + let location = format!( + "s3://{bucket}/{key}{}{}", + version.map_or_else(String::new, |v| format!(" (version {v})")), + range_offset.map_or_else(String::new, |o| format!(" (range at offset {o})")), + ); // S3 error code and service message, when response metadata is present. let code = err.code().map(str::to_string); @@ -558,29 +1017,258 @@ mod tests { } #[tokio::test] - async fn stream_to_file_writes_content() { + async fn stream_body_at_writes_content() { let dir = tempfile::TempDir::new().unwrap(); let dest = dir.path().join("out.bin"); let data = b"hello world"; - let reader = &data[..]; - stream_to_file(reader, &dest).await.unwrap(); + let file = create_download_file(&dest).unwrap(); + let written = stream_body_at(&file, &data[..], 0).await.unwrap(); + assert_eq!(written, data.len() as u64); + drop(file); assert_eq!(std::fs::read(&dest).unwrap(), data); } #[tokio::test] - async fn stream_to_file_empty_body() { + async fn stream_body_at_empty_body() { let dir = tempfile::TempDir::new().unwrap(); let dest = dir.path().join("empty.bin"); let reader: &[u8] = &[]; - stream_to_file(reader, &dest).await.unwrap(); + let file = create_download_file(&dest).unwrap(); + assert_eq!(stream_body_at(&file, reader, 0).await.unwrap(), 0); + drop(file); assert_eq!(std::fs::read(&dest).unwrap().len(), 0); } #[tokio::test] - async fn stream_to_file_bad_path() { - let reader: &[u8] = b"data"; - let result = stream_to_file(reader, Path::new("/nonexistent/dir/file")).await; - assert!(result.is_err()); + async fn stream_body_at_honors_offset() { + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("offset.bin"); + let file = create_download_file(&dest).unwrap(); + file.set_len(6).unwrap(); + stream_body_at(&file, &b"world"[..], 1).await.unwrap(); + stream_body_at(&file, &b"h"[..], 0).await.unwrap(); + drop(file); + assert_eq!(std::fs::read(&dest).unwrap(), b"hworld"); + } + + #[test] + fn create_download_file_bad_path() { + assert!(create_download_file(Path::new("/nonexistent/dir/file")).is_err()); + } + + #[test] + fn range_header_is_inclusive_end() { + assert_eq!(range_header(0, 8 * 1024 * 1024), "bytes=0-8388607"); + assert_eq!(range_header(8_388_608, 100), "bytes=8388608-8388707"); + } + + #[test] + fn plan_ranges_splits_remainder() { + // 20 bytes total, first 8 already on disk, 8-byte chunks. + assert_eq!(plan_ranges(8, 20, 8), vec![(8, 8), (16, 4)]); + } + + #[test] + fn plan_ranges_exact_multiple() { + assert_eq!(plan_ranges(10, 30, 10), vec![(10, 10), (20, 10)]); + } + + #[test] + fn plan_ranges_nothing_left() { + assert!(plan_ranges(20, 20, 8).is_empty()); + assert!(plan_ranges(25, 20, 8).is_empty()); + } + + #[test] + fn plan_ranges_covers_whole_object_without_gaps() { + let ranges = plan_ranges(0, 1000, 128); + let mut expected_offset = 0; + for (offset, len) in &ranges { + assert_eq!(*offset, expected_offset, "gap or overlap in {ranges:?}"); + expected_offset += len; + } + assert_eq!(expected_offset, 1000, "ranges must cover the object exactly"); + } + + #[test] + fn plan_ranges_zero_chunk_size_does_not_hang() { + // Defensive: a 0 chunk size would otherwise loop forever. + assert_eq!(plan_ranges(0, 3, 0), vec![(0, 1), (1, 1), (2, 1)]); + } + + #[test] + fn parse_content_range_reads_start_and_total() { + assert_eq!(parse_content_range("bytes 0-8388607/12345678"), Some((0, 12_345_678))); + assert_eq!(parse_content_range("bytes 65536-131071/200000"), Some((65_536, 200_000))); + } + + #[test] + fn parse_content_range_rejects_unknown_and_malformed() { + assert_eq!(parse_content_range("bytes 0-99/*"), None); + assert_eq!(parse_content_range("bytes 0-99"), None); + assert_eq!(parse_content_range(""), None); + assert_eq!(parse_content_range("garbage/notanumber"), None); + } + + #[test] + fn first_response_completeness() { + // Shorter than requested → the object is smaller than one chunk. + assert!(first_response_is_complete(100, 8 * 1024 * 1024)); + // Longer than requested → `Range` was ignored and this is the whole object. + assert!(first_response_is_complete(32 * 1024 * 1024, 8 * 1024 * 1024)); + // Exactly the requested length → ambiguous, so treated as incomplete. + assert!(!first_response_is_complete(8 * 1024 * 1024, 8 * 1024 * 1024)); + } + + #[test] + fn precondition_failed_by_code_or_status() { + assert!(precondition_failed("PreconditionFailed", None)); + assert!(precondition_failed("unknown", Some(412))); + assert!(!precondition_failed("AccessDenied", Some(403))); + } + + /// A `GetObject` service error with S3 response metadata. The raw response + /// type is `()`, since the helpers only need it to be `Debug + Send + Sync`. + fn service_error( + code: &str, + message: &str, + ) -> aws_sdk_s3::error::SdkError { + let metadata = aws_smithy_types::error::ErrorMetadata::builder() + .code(code) + .message(message) + .build(); + aws_sdk_s3::error::SdkError::service_error( + aws_sdk_s3::operation::get_object::GetObjectError::generic(metadata), + (), + ) + } + + /// A failure with no response metadata, such as a connection or timeout error. + fn transport_error() + -> aws_sdk_s3::error::SdkError { + aws_sdk_s3::error::SdkError::timeout_error(Box::new(io::Error::other("connect timed out"))) + } + + #[test] + fn s3_error_code_reads_response_metadata() { + assert_eq!(s3_error_code(&service_error("AccessDenied", "denied")), "AccessDenied"); + } + + #[test] + fn s3_error_code_without_metadata_is_unknown() { + assert_eq!(s3_error_code(&transport_error()), "unknown"); + } + + #[test] + fn range_unsupported_by_code_or_status() { + // 416 on an empty object and 501 from endpoints without range support, + // with or without an XML error body. + assert!(range_unsupported("InvalidRange", Some(416))); + assert!(range_unsupported("NotImplemented", Some(501))); + assert!(range_unsupported("unknown", Some(416))); + assert!(range_unsupported("unknown", Some(501))); + } + + #[test] + fn range_unsupported_does_not_swallow_real_failures() { + for (code, status) in [ + ("AccessDenied", Some(403)), + ("NoSuchKey", Some(404)), + ("SlowDown", Some(503)), + ("unknown", None), + ] { + assert!(!range_unsupported(code, status), "{code} must not fall back"); + } + } + + #[test] + fn format_error_names_bucket_key_and_s3_code() { + let msg = format_s3_get_object_error( + &service_error("AccessDenied", "Access Denied"), + "my-bucket", + "path/to/bundle.zip", + None, + None, + ); + assert!(msg.contains("s3://my-bucket/path/to/bundle.zip"), "{msg}"); + assert!(msg.contains("AccessDenied"), "{msg}"); + assert!(msg.contains("Access Denied"), "{msg}"); + } + + #[test] + fn format_error_names_version_and_range_offset() { + let msg = format_s3_get_object_error( + &service_error("PreconditionFailed", "At least one precondition failed"), + "my-bucket", + "bundle.zip", + Some("v7"), + Some(8_388_608), + ); + assert!(msg.contains("version v7"), "{msg}"); + assert!(msg.contains("range at offset 8388608"), "{msg}"); + } + + #[test] + fn format_error_without_metadata_falls_back_to_the_error_chain() { + // No S3 code to report, so the underlying cause must still be visible + // rather than the SDK's opaque top-level "dispatch failure". + let msg = format_s3_get_object_error(&transport_error(), "bucket", "key", None, Some(1024)); + assert!(msg.contains("s3://bucket/key"), "{msg}"); + assert!(msg.contains("range at offset 1024"), "{msg}"); + assert!(msg.to_lowercase().contains("timeout"), "expected the cause, got: {msg}"); + } + + #[test] + fn effective_concurrency_is_clamped() { + let low = S3ClientConfig { download_max_concurrency: 0, ..Default::default() }; + assert_eq!(low.effective_concurrency(), 1, "0 must degrade to serial, not divide by zero"); + let high = S3ClientConfig { download_max_concurrency: 10_000, ..Default::default() }; + assert_eq!(high.effective_concurrency(), MAX_DOWNLOAD_MAX_CONCURRENCY); + assert_eq!(S3ClientConfig::default().effective_concurrency(), 1); + } + + #[test] + fn effective_chunk_size_is_floored() { + let tiny = S3ClientConfig { download_chunk_size: 1, ..Default::default() }; + assert_eq!(tiny.effective_chunk_size(), MIN_DOWNLOAD_CHUNK_SIZE); + assert_eq!(S3ClientConfig::default().effective_chunk_size(), 8 * 1024 * 1024); + } + + #[test] + fn download_worker_threads_bounded() { + assert_eq!(download_worker_threads(1), 1); + assert!(download_worker_threads(64) <= MAX_DOWNLOAD_WORKER_THREADS); + assert!(download_worker_threads(64) >= 1); + // Never more threads than ranges we are willing to have in flight. + assert!(download_worker_threads(2) <= 2); + } + + #[test] + fn serial_client_uses_single_threaded_runtime() { + // Concurrency 1 must not spin up worker threads for the daemon's lifetime. + let config = S3ClientConfig { download_max_concurrency: 1, ..Default::default() }; + let client = S3Client::new(test_credentials(), &config).unwrap(); + assert_eq!(client.max_concurrency, 1); + assert_eq!( + client.runtime.handle().runtime_flavor(), + tokio::runtime::RuntimeFlavor::CurrentThread + ); + } + + #[test] + fn parallel_client_carries_clamped_settings() { + let config = S3ClientConfig { + download_max_concurrency: 10_000, + download_chunk_size: 1, + ..Default::default() + }; + let client = S3Client::new(test_credentials(), &config).unwrap(); + assert_eq!(client.max_concurrency, MAX_DOWNLOAD_MAX_CONCURRENCY); + assert_eq!(client.chunk_size, MIN_DOWNLOAD_CHUNK_SIZE); + assert_eq!( + client.runtime.handle().runtime_flavor(), + tokio::runtime::RuntimeFlavor::MultiThread + ); } #[test] diff --git a/src/aws_clients/throttle_gate.rs b/src/aws_clients/throttle_gate.rs index 5cd30176..28a6228e 100644 --- a/src/aws_clients/throttle_gate.rs +++ b/src/aws_clients/throttle_gate.rs @@ -164,7 +164,7 @@ mod tests { let gate = ThrottleGate::new(); let start = Instant::now(); gate.wait_if_throttled(); - assert!(start.elapsed() < Duration::from_millis(50)); + assert!(start.elapsed() < Duration::from_millis(500)); } #[test] @@ -179,7 +179,16 @@ mod tests { gate.wait_if_throttled(); // Wait is 100ms + up to 25ms jitter assert!(start.elapsed() >= Duration::from_millis(90)); - assert!(start.elapsed() < Duration::from_millis(200)); + assert!(start.elapsed() < Duration::from_millis(750)); + } + + #[test] + fn jitter_is_at_most_quarter() { + let base = Duration::from_millis(100); + for _ in 0..1000 { + assert!(ThrottleGate::jitter(base) < Duration::from_millis(25)); + } + assert_eq!(ThrottleGate::jitter(Duration::from_millis(3)), Duration::ZERO); } #[test] diff --git a/src/command_poller/host_command_poller.rs b/src/command_poller/host_command_poller.rs index 28488ffb..b2615e79 100644 --- a/src/command_poller/host_command_poller.rs +++ b/src/command_poller/host_command_poller.rs @@ -647,7 +647,7 @@ mod tests { let pool = CommandThreadPool::new(16); let start = Instant::now(); pool.wait_for_completion(Duration::from_secs(5)); - assert!(start.elapsed() < Duration::from_millis(200)); + assert!(start.elapsed() < Duration::from_secs(1)); } #[test] diff --git a/src/config/mod.rs b/src/config/mod.rs index 64271593..72bb8b30 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -36,6 +36,14 @@ const FIPS_ENABLED_REGIONS: &[&str] = &[ "us-gov-east-1", ]; +/// S3 download defaults and bounds, defined once in the S3 client. +pub use crate::aws_clients::s3_client::{ + DEFAULT_DOWNLOAD_CHUNK_SIZE as DEFAULT_S3_DOWNLOAD_CHUNK_SIZE, + DEFAULT_DOWNLOAD_MAX_CONCURRENCY as DEFAULT_S3_DOWNLOAD_MAX_CONCURRENCY, + MAX_DOWNLOAD_MAX_CONCURRENCY as MAX_S3_DOWNLOAD_MAX_CONCURRENCY, + MIN_DOWNLOAD_CHUNK_SIZE as MIN_S3_DOWNLOAD_CHUNK_SIZE, +}; + /// Maximum allowed value for `kill_agent_max_wait_time_seconds` (24 hours). /// /// A misconfigured value (e.g. `999999999`) would prevent the agent from @@ -56,7 +64,7 @@ where use serde::de::Error; #[derive(Deserialize)] - #[serde(untagged)] + #[serde(untagged, expecting = "a byte count or a size string like \"8MiB\"")] enum ByteSizeValue { Int(u64), Str(String), @@ -72,6 +80,14 @@ where } } +/// Deserialize `s3_download_chunk_size`; an empty value means the default. +fn deserialize_chunk_size<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + Ok(deserialize_byte_size_option(deserializer)?.unwrap_or(DEFAULT_S3_DOWNLOAD_CHUNK_SIZE)) +} + /// Parse a human-readable byte size string into bytes. /// /// Accepts integer values with optional suffix. Fractional values (e.g. "1.5GB") @@ -312,6 +328,17 @@ pub struct AgentConfig { /// Custom S3 endpoint override. `None`. pub s3_endpoint_override: Option, + /// Parallel range requests per S3 bundle download. Default: `1`, which means + /// a single `GetObject`. Clamped to + /// `1..=`[`MAX_S3_DOWNLOAD_MAX_CONCURRENCY`]. + pub s3_download_max_concurrency: usize, + + /// Bytes per S3 range request. Default: 8 MiB. Objects this size or smaller + /// take one request. Accepts bytes or strings like `"16MB"`. Floored at + /// [`MIN_S3_DOWNLOAD_CHUNK_SIZE`]. + #[serde(deserialize_with = "deserialize_chunk_size")] + pub s3_download_chunk_size: u64, + /// Disable `IMDSv1` fallback. Default: `false`. /// When `true`, only `IMDSv2` (token-based) is used for metadata requests. pub disable_imds_v1: bool, @@ -388,6 +415,8 @@ impl Default for AgentConfig { ongoing_deployment_tracking: "ongoing-deployment".to_string(), deploy_control_endpoint: None, s3_endpoint_override: None, + s3_download_max_concurrency: DEFAULT_S3_DOWNLOAD_MAX_CONCURRENCY, + s3_download_chunk_size: DEFAULT_S3_DOWNLOAD_CHUNK_SIZE, disable_imds_v1: false, enable_command_port: false, enable_archive_reuse: true, @@ -481,6 +510,25 @@ impl AgentConfig { ); self.kill_agent_max_wait_time_seconds = MAX_KILL_AGENT_WAIT_SECONDS; } + // Clamp the download settings rather than fail startup. + let concurrency = + self.s3_download_max_concurrency.clamp(1, MAX_S3_DOWNLOAD_MAX_CONCURRENCY); + if concurrency != self.s3_download_max_concurrency { + tracing::warn!( + configured = self.s3_download_max_concurrency, + using = concurrency, + "s3_download_max_concurrency out of range" + ); + self.s3_download_max_concurrency = concurrency; + } + if self.s3_download_chunk_size < MIN_S3_DOWNLOAD_CHUNK_SIZE { + tracing::warn!( + configured = self.s3_download_chunk_size, + using = MIN_S3_DOWNLOAD_CHUNK_SIZE, + "s3_download_chunk_size below minimum" + ); + self.s3_download_chunk_size = MIN_S3_DOWNLOAD_CHUNK_SIZE; + } } /// Validate post-parse fields that should fail loud rather than be clamped. @@ -1285,6 +1333,8 @@ disable_imds_v1: true enable_command_port: true disable_core_dumps: false log_aws_wire: true +s3_download_max_concurrency: 20 +s3_download_chunk_size: "16MiB" "#; let config = AgentConfig::from_yaml(yaml, Path::new("test.yml")).unwrap(); assert_eq!(config.program_name, "my-agent"); @@ -1309,6 +1359,87 @@ log_aws_wire: true assert!(config.enable_command_port); assert!(!config.disable_core_dumps); assert!(config.log_aws_wire); + assert_eq!(config.s3_download_max_concurrency, 20); + assert_eq!(config.s3_download_chunk_size, 16 * 1024 * 1024); + } + + #[test] + fn s3_download_defaults() { + let config = AgentConfig::default(); + assert_eq!(config.s3_download_max_concurrency, 1); + assert_eq!(config.s3_download_chunk_size, 8 * 1024 * 1024); + } + + #[test] + fn s3_download_settings_parse_symbol_keys() { + let config = AgentConfig::from_yaml( + ":s3_download_max_concurrency: 4\n:s3_download_chunk_size: 32MB\n", + Path::new("test.yml"), + ) + .unwrap(); + assert_eq!(config.s3_download_max_concurrency, 4); + assert_eq!(config.s3_download_chunk_size, 32_000_000); + assert!( + config.unknown_keys.is_empty(), + "download keys must not land in the unknown_keys catch-all: {:?}", + config.unknown_keys + ); + } + + #[test] + fn s3_download_chunk_size_accepts_raw_bytes() { + let config = + AgentConfig::from_yaml("s3_download_chunk_size: 1048576\n", Path::new("test.yml")) + .unwrap(); + assert_eq!(config.s3_download_chunk_size, 1_048_576); + } + + #[test] + fn s3_download_chunk_size_empty_value_uses_default() { + let config = + AgentConfig::from_yaml(":s3_download_chunk_size:\n", Path::new("test.yml")).unwrap(); + assert_eq!(config.s3_download_chunk_size, DEFAULT_S3_DOWNLOAD_CHUNK_SIZE); + } + + #[test] + fn s3_download_chunk_size_rejects_bad_suffix() { + assert!( + AgentConfig::from_yaml("s3_download_chunk_size: 5ZB\n", Path::new("test.yml")).is_err() + ); + } + + #[test] + fn normalize_clamps_s3_download_concurrency_high() { + let mut config = + AgentConfig { s3_download_max_concurrency: 10_000, ..AgentConfig::default() }; + config.normalize(); + assert_eq!(config.s3_download_max_concurrency, MAX_S3_DOWNLOAD_MAX_CONCURRENCY); + } + + #[test] + fn normalize_treats_zero_s3_download_concurrency_as_serial() { + let mut config = AgentConfig { s3_download_max_concurrency: 0, ..AgentConfig::default() }; + config.normalize(); + assert_eq!(config.s3_download_max_concurrency, 1); + } + + #[test] + fn normalize_floors_s3_download_chunk_size() { + let mut config = AgentConfig { s3_download_chunk_size: 512, ..AgentConfig::default() }; + config.normalize(); + assert_eq!(config.s3_download_chunk_size, MIN_S3_DOWNLOAD_CHUNK_SIZE); + } + + #[test] + fn normalize_leaves_valid_s3_download_settings_alone() { + let mut config = AgentConfig { + s3_download_max_concurrency: 16, + s3_download_chunk_size: 16 * 1024 * 1024, + ..AgentConfig::default() + }; + config.normalize(); + assert_eq!(config.s3_download_max_concurrency, 16); + assert_eq!(config.s3_download_chunk_size, 16 * 1024 * 1024); } #[test] diff --git a/src/daemon/worker.rs b/src/daemon/worker.rs index 55da9679..d8b6c3af 100644 --- a/src/daemon/worker.rs +++ b/src/daemon/worker.rs @@ -174,6 +174,8 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { wire_log: config .log_aws_wire .then(|| (config.log_dir.clone(), config.program_name.clone())), + download_max_concurrency: config.s3_download_max_concurrency, + download_chunk_size: config.s3_download_chunk_size, ..Default::default() }; let s3_client = match crate::aws_clients::s3_client::S3Client::new(s3_credentials, &s3_config) { @@ -183,17 +185,22 @@ pub fn run(shutdown: &ShutdownFlag, config: &AgentConfig) { return; }, }; - // S3Client is not Clone (tokio Runtime is not Clone), so we create a - // separate client for the update command. Both use the same credentials - // and config. - let update_s3_client = - match crate::aws_clients::s3_client::S3Client::new(credentials.clone(), &s3_config) { - Ok(c) => c, - Err(e) => { - error!("Worker failed to create update S3 client: {e}"); - return; - }, - }; + // The update command only fetches the install script, so its client + // downloads serially and starts no worker threads. + let update_s3_config = crate::aws_clients::s3_client::S3ClientConfig { + download_max_concurrency: 1, + ..s3_config.clone() + }; + let update_s3_client = match crate::aws_clients::s3_client::S3Client::new( + credentials.clone(), + &update_s3_config, + ) { + Ok(c) => c, + Err(e) => { + error!("Worker failed to create update S3 client: {e}"); + return; + }, + }; let region = credentials.region.clone(); let dispatcher = CommandDispatcher::new( archives, @@ -329,14 +336,139 @@ pub fn bind_lifetime_to_parent() { } } -/// No-op on non-Linux: `PR_SET_PDEATHSIG` is Linux-specific. -#[cfg(not(target_os = "linux"))] +/// macOS counterpart of the Linux `PR_SET_PDEATHSIG` binding. macOS has no +/// parent-death signal, and launchd has no control group that holds a stop open +/// until every process exits: once the master is gone launchd considers the job +/// stopped, and a restart starts a second master and worker next to a worker +/// that is still running. +/// +/// Watch the master's pid with kqueue (`EVFILT_PROC` / `NOTE_EXIT`) on a +/// background thread and exit the moment it fires, however the master died. +/// The window between `getppid()` and registering the watch is closed the same +/// way as on Linux: `ESRCH` from the registration, or a parent that is no +/// longer the one we registered, means the master is already gone. +#[cfg(target_os = "macos")] +pub fn bind_lifetime_to_parent() { + use nix::errno::Errno; + use nix::unistd::{Pid, getppid}; + + let parent = getppid(); + if parent == Pid::from_raw(1) { + info!("Master already exited before worker startup; exiting to avoid orphan"); + process::exit(0); + } + let kq = match watch_process_exit(parent) { + Ok(kq) => kq, + Err(Errno::ESRCH) => { + info!("Master already exited before worker startup; exiting to avoid orphan"); + process::exit(0); + }, + Err(e) => { + error!("Worker failed to watch master pid {parent}; master death may orphan it: {e}"); + return; + }, + }; + if getppid() != parent { + info!("Master already exited before worker startup; exiting to avoid orphan"); + process::exit(0); + } + let spawned = std::thread::Builder::new().name("master-watch".into()).spawn(move || { + match wait_for_process_exit(&kq) { + Ok(()) => { + error!("Master (pid {parent}) exited; worker exiting to avoid running orphaned"); + // Give the non-blocking log writer a moment to flush that line; + // process::exit runs no destructors. + std::thread::sleep(std::time::Duration::from_millis(200)); + process::exit(1); + }, + Err(e) => { + error!("Watch on master pid {parent} failed; master death may orphan it: {e}"); + }, + } + }); + if let Err(e) = spawned { + error!("Worker failed to start the master watch thread; master death may orphan it: {e}"); + } +} + +/// Register a one-shot kqueue watch for `pid`'s exit. Returns `ESRCH` when the +/// process no longer exists. +#[cfg(target_os = "macos")] +fn watch_process_exit(pid: nix::unistd::Pid) -> nix::Result { + use nix::sys::event::{EventFilter, EventFlag, FilterFlag, KEvent, Kqueue}; + + let ident = usize::try_from(pid.as_raw()).map_err(|_| nix::errno::Errno::EINVAL)?; + let kq = Kqueue::new()?; + let watch = KEvent::new( + ident, + EventFilter::EVFILT_PROC, + EventFlag::EV_ADD | EventFlag::EV_ONESHOT, + FilterFlag::NOTE_EXIT, + 0, + 0, + ); + kq.kevent(&[watch], &mut [], None)?; + Ok(kq) +} + +/// Block until the watch registered by [`watch_process_exit`] fires. +#[cfg(target_os = "macos")] +fn wait_for_process_exit(kq: &nix::sys::event::Kqueue) -> nix::Result<()> { + use nix::errno::Errno; + use nix::sys::event::{EventFilter, EventFlag, FilterFlag, KEvent}; + + let mut fired = [KEvent::new( + 0, + EventFilter::EVFILT_PROC, + EventFlag::empty(), + FilterFlag::empty(), + 0, + 0, + )]; + loop { + match kq.kevent(&[], &mut fired, None) { + Ok(0) | Err(Errno::EINTR) => {}, + Ok(_) => return Ok(()), + Err(e) => return Err(e), + } + } +} + +/// No-op on other targets (Windows): neither mechanism above exists there. +#[cfg(not(any(target_os = "linux", target_os = "macos")))] pub fn bind_lifetime_to_parent() {} #[cfg(test)] mod tests { use super::*; + /// The kqueue watch the macOS worker puts on its master fires when the + /// watched process exits, so a worker cannot outlive its master. + #[cfg(target_os = "macos")] + #[test] + fn process_exit_watch_fires_when_watched_process_dies() { + let mut child = process::Command::new("sleep").arg("60").spawn().expect("spawn sleep"); + let pid = nix::unistd::Pid::from_raw(i32::try_from(child.id()).expect("pid fits i32")); + let kq = watch_process_exit(pid).expect("register exit watch"); + + child.kill().expect("kill sleep"); + + assert_eq!(wait_for_process_exit(&kq), Ok(())); + child.wait().expect("reap sleep"); + } + + /// A master that is already gone is reported as `ESRCH`, so the worker + /// exits instead of waiting on a watch that can never fire. + #[cfg(target_os = "macos")] + #[test] + fn process_exit_watch_reports_esrch_for_a_gone_process() { + let mut child = process::Command::new("true").spawn().expect("spawn true"); + let pid = nix::unistd::Pid::from_raw(i32::try_from(child.id()).expect("pid fits i32")); + child.wait().expect("reap true"); + + assert_eq!(watch_process_exit(pid).err(), Some(nix::errno::Errno::ESRCH)); + } + /// The worker must reset the umask to 0o022 so no-stored-mode customer files /// land at 0644. #[cfg(unix)] diff --git a/src/host_command/command_dispatcher.rs b/src/host_command/command_dispatcher.rs index e2a39698..cdfc0aa6 100644 --- a/src/host_command/command_dispatcher.rs +++ b/src/host_command/command_dispatcher.rs @@ -389,6 +389,9 @@ mod tests { assert!(result.is_ok()); } + // Split by platform: on macOS UpdateAgentCommand refuses before the S3 check + // (UNSUPPORTED_PLATFORM_MESSAGE), so the S3-error assertion only holds elsewhere. + #[cfg(not(target_os = "macos"))] #[test] fn update_deployment_agent_returns_error_without_s3() { let dir = tempfile::TempDir::new().unwrap(); @@ -405,6 +408,24 @@ mod tests { assert!(result.unwrap_err().to_string().contains("S3 client not configured")); } + // On macOS self-update is refused before the S3 check (see update_agent.rs). + #[cfg(target_os = "macos")] + #[test] + fn update_deployment_agent_refuses_on_macos() { + let dir = tempfile::TempDir::new().unwrap(); + let dispatcher = CommandDispatcher::new( + test_archives(&dir), + None, + HashMap::new(), + "us-east-1", + None, + Arc::new(AgentConfig::default()), + ); + let result = dispatcher.execute_command("UpdateDeploymentAgent", &test_spec()); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("not supported on macOS")); + } + #[test] fn update_deployment_agent_never_noop() { let dir = tempfile::TempDir::new().unwrap(); diff --git a/src/host_command/commands/hook.rs b/src/host_command/commands/hook.rs index 1df99e94..5cfaef62 100644 --- a/src/host_command/commands/hook.rs +++ b/src/host_command/commands/hook.rs @@ -6,7 +6,7 @@ use crate::deployment_specification::types::DeploymentSpec; use crate::host_command::DeploymentArchives; use crate::lifecycle_event::{HookEnvPolicy, LifecycleEventExecutor, ScriptError}; -use crate::logging::{DeploymentLogger, LogConfig}; +use crate::logging::{LogConfig, SharedDeploymentLog}; use std::collections::HashMap; use std::sync::Arc; use tracing::{info, warn}; @@ -18,11 +18,12 @@ pub type HookMapping = HashMap>; pub struct HookCommand { archives: Arc, hook_mapping: HookMapping, - /// When set, per-deployment lifecycle/script log lines are mirrored to + /// When set, every lifecycle/script log line is also written to /// `/deployment-logs/-deployments.log` (the /// documented `CodeDeploy` deployment log). `None` disables it - /// (`enable_deployments_log: false`, and the default in tests). - deployment_log_config: Option, + /// (`enable_deployments_log: false`, and the default in tests). One handle + /// is shared by all deployments. + deployment_log: Option, /// Opt-in hook-env hardening, from `strip_loader_env_in_hooks` / /// `restrict_hook_env_to_allowlist`. Default (all-`false`) = full inheritance. env_policy: HookEnvPolicy, @@ -42,7 +43,7 @@ impl HookCommand { Self { archives, hook_mapping, - deployment_log_config: None, + deployment_log: None, env_policy: HookEnvPolicy::default(), reject_path_traversal: false, restrict_log_permissions: false, @@ -80,7 +81,7 @@ impl HookCommand { /// `LogConfig`'s `root_dir`/`program_name` determine the log location. #[must_use] pub fn with_deployment_log_config(mut self, config: LogConfig) -> Self { - self.deployment_log_config = Some(config); + self.deployment_log = Some(SharedDeploymentLog::new(config)); self } @@ -122,56 +123,27 @@ impl HookCommand { "Executing hook command" ); - // Open the per-deployment log up front (best-effort) so the + // Open the deployment log up front (best-effort) so the // `deployment-logs/` directory and `…-deployments.log` file exist for // the deployment even before the first line is written. A failure here // must not fail the deployment — it's a diagnostic side-channel. - let mut deployment_log = - self.deployment_log_config - .as_ref() - .and_then(|cfg| match DeploymentLogger::new(cfg) { - Ok(logger) => Some(logger), - Err(e) => { - warn!("Could not open deployment log: {e}"); - None - }, - }); + if let Some(log) = &self.deployment_log + && let Err(e) = log.ensure_open() + { + warn!("Could not open deployment log: {e}"); + } + // Lines reach the deployment log as each one is written to + // `scripts.log`, so a failing event needs no extra writes here and + // the deployment log gets no failure-summary line. let mut all_logs = Vec::new(); for event_name in events { let executor = self.create_executor(event_name, spec)?; - // Run the event; on script failure still flush whatever was logged - // to the deployment log before propagating the error. - let result = executor.execute(); - if let Some(logger) = deployment_log.as_mut() { - match &result { - // Success: the returned lines already include the - // "LifecycleEvent - {event}" header, so just mirror them. - Ok(logs) => Self::mirror_to_deployment_log(logger, logs), - // Failure: `execute()` returns no lines, so the header is - // not in `result`. Write it before the failure summary so - // the deployment log keeps context for the event that failed. - Err(e) => { - let _ = logger.log(&format!("LifecycleEvent - {event_name}")); - let _ = logger - .log(&format!("{command_name}/{event_name} failed: {}", e.message)); - }, - } - } - let logs = result?; - all_logs.extend(logs); + all_logs.extend(executor.execute()?); } Ok(all_logs) } - /// Mirror a lifecycle event's collected log lines to the per-deployment log. - /// Best-effort: write failures are ignored (the log is diagnostic only). - fn mirror_to_deployment_log(logger: &mut DeploymentLogger, lines: &[String]) { - for line in lines { - let _ = logger.log(line); - } - } - /// Check if a command has any lifecycle events mapped. /// /// Pure lookup on `hook_mapping` — does not parse the `AppSpec`. Any @@ -248,6 +220,9 @@ impl HookCommand { .with_env_policy(self.env_policy) .with_reject_path_traversal(self.reject_path_traversal) .with_restrict_log_permissions(self.restrict_log_permissions) + .with_deployment_log( + self.deployment_log.as_ref().map(|log| log.for_deployment(&spec.deployment_id)), + ) }) } } @@ -463,43 +438,105 @@ mod tests { } } - /// When the deployment log is enabled, executing a hook creates - /// `deployment-logs/-deployments.log` and writes the event's - /// log lines to it. + /// Write an `AfterInstall` hook running `script_body` into the archive of + /// `deployment_id`, and return the `HookCommand` spec for it. #[cfg(unix)] - #[test] - fn deployment_log_created_and_written_when_enabled() { + fn stage_after_install( + archives: &DeploymentArchives, + deployment_id: &str, + script_body: &str, + ) -> DeploymentSpec { use std::os::unix::fs::PermissionsExt; - let dir = TempDir::new().unwrap(); - let archives = test_archives(&dir); - // root_dir for the deployment log = the executor's deploy dir parent; - // point LogConfig.root_dir at the deployments root so we can assert. - let root = dir.path().join("deployments"); - let cmd = HookCommand::new(archives.clone(), test_mapping()) - .with_deployment_log_config(log_config_for(&root)); - - // A real AfterInstall hook that succeeds, so execute() returns log lines. - let archive_dir = archives.archive_dir("dg-1", "d-123"); + let archive_dir = archives.archive_dir("dg-1", deployment_id); let scripts = archive_dir.join("scripts"); fs::create_dir_all(&scripts).unwrap(); fs::write( archive_dir.join("appspec.yml"), - "version: 0.0\nos: linux\nhooks:\n AfterInstall:\n - location: scripts/ok.sh\n timeout: 10\n", + "version: 0.0\nos: linux\nhooks:\n AfterInstall:\n - location: scripts/run.sh\n timeout: 30\n", ) .unwrap(); - fs::write(scripts.join("ok.sh"), "#!/bin/sh\necho hi\n").unwrap(); - fs::set_permissions(scripts.join("ok.sh"), fs::Permissions::from_mode(0o755)).unwrap(); + fs::write(scripts.join("run.sh"), format!("#!/bin/sh\n{script_body}")).unwrap(); + fs::set_permissions(scripts.join("run.sh"), fs::Permissions::from_mode(0o755)).unwrap(); + DeploymentSpec { deployment_id: deployment_id.into(), ..test_spec() } + } - cmd.execute("AfterInstall", &test_spec()).unwrap(); + /// Lines of the deployment log under `root`, with the + /// `[YYYY-MM-DD HH:MM:SS.mmm] ` prefix checked and removed. + #[cfg(unix)] + fn deployment_log_messages(root: &std::path::Path) -> Vec { + let body = + fs::read_to_string(root.join("deployment-logs/codedeploy-agent-deployments.log")) + .unwrap(); + body.lines() + .map(|line| { + let (ts, msg) = line + .strip_prefix('[') + .and_then(|l| l.split_once("] ")) + .unwrap_or_else(|| panic!("line lacks a [timestamp] prefix: {line:?}")); + assert!( + chrono::NaiveDateTime::parse_from_str(ts, "%Y-%m-%d %H:%M:%S%.3f").is_ok(), + "bad timestamp {ts:?} in {line:?}" + ); + msg.to_string() + }) + .collect() + } + + /// Each line is written as + /// `[] []`, once, with no second + /// timestamp and no blank line, and stdout/stderr are included. + #[cfg(unix)] + #[test] + fn deployment_log_uses_one_record_per_script_line() { + let dir = TempDir::new().unwrap(); + let archives = test_archives(&dir); + let root = dir.path().join("deployments"); + let cmd = HookCommand::new(archives.clone(), test_mapping()) + .with_deployment_log_config(log_config_for(&root)); + let spec = stage_after_install(&archives, "d-123", "echo hi\necho oops >&2\n"); + + cmd.execute("AfterInstall", &spec).unwrap(); + + let mut messages = deployment_log_messages(&root); + // stdout and stderr are read by separate tasks, so their relative + // order is not deterministic; sort the output lines. + messages[2..].sort(); + assert_eq!( + messages, + vec![ + "[d-123]LifecycleEvent - AfterInstall", + "[d-123]Script - scripts/run.sh", + "[d-123][stderr]oops", + "[d-123][stdout]hi", + ] + ); + } - let log_path = root.join("deployment-logs/codedeploy-agent-deployments.log"); - assert!(log_path.exists(), "deployment log must be created at {}", log_path.display()); - let body = fs::read_to_string(&log_path).unwrap(); - assert!( - body.contains("LifecycleEvent - AfterInstall"), - "deployment log must contain the event's lines, got: {body:?}" + /// The deployment log must hold all script output, not the 2048-byte + /// diagnostic tail that is sent to the service. + #[cfg(unix)] + #[test] + fn deployment_log_keeps_output_beyond_diagnostic_tail() { + let dir = TempDir::new().unwrap(); + let archives = test_archives(&dir); + let root = dir.path().join("deployments"); + let cmd = HookCommand::new(archives.clone(), test_mapping()) + .with_deployment_log_config(log_config_for(&root)); + // 200 lines of ~40 bytes is well past the 2048-byte tail. + let spec = stage_after_install( + &archives, + "d-123", + "i=0; while [ $i -lt 200 ]; do echo \"line $i padded to be longer\"; i=$((i+1)); done\n", ); + + cmd.execute("AfterInstall", &spec).unwrap(); + + let messages = deployment_log_messages(&root); + assert_eq!(messages.len(), 202, "header + script line + 200 output lines"); + assert_eq!(messages[0], "[d-123]LifecycleEvent - AfterInstall"); + assert_eq!(messages[2], "[d-123][stdout]line 0 padded to be longer"); + assert_eq!(messages[201], "[d-123][stdout]line 199 padded to be longer"); } /// When the deployment log is NOT configured (the default, i.e. @@ -532,49 +569,66 @@ mod tests { ); } - /// A failing script with the deployment log enabled must still record a - /// failure line in the deployment log and then propagate the error. - /// Exercises the `Err(e) => logger.log(...)` arm in `execute`. + /// A failing script's output must be in the deployment log, and no + /// failure-summary line is added: the deployment log holds only script + /// log lines. #[cfg(unix)] #[test] - fn deployment_log_records_failure_line() { - use std::os::unix::fs::PermissionsExt; - + fn deployment_log_keeps_failing_script_output_without_summary_line() { let dir = TempDir::new().unwrap(); let archives = test_archives(&dir); let root = dir.path().join("deployments"); let cmd = HookCommand::new(archives.clone(), test_mapping()) .with_deployment_log_config(log_config_for(&root)); + let spec = stage_after_install(&archives, "d-123", "echo before-failure\nexit 1\n"); - let archive_dir = archives.archive_dir("dg-1", "d-123"); - let scripts = archive_dir.join("scripts"); - fs::create_dir_all(&scripts).unwrap(); - fs::write( - archive_dir.join("appspec.yml"), - "version: 0.0\nos: linux\nhooks:\n AfterInstall:\n - location: scripts/fail.sh\n timeout: 10\n", - ) - .unwrap(); - fs::write(scripts.join("fail.sh"), "#!/bin/sh\nexit 1\n").unwrap(); - fs::set_permissions(scripts.join("fail.sh"), fs::Permissions::from_mode(0o755)).unwrap(); - - let result = cmd.execute("AfterInstall", &test_spec()); + let result = cmd.execute("AfterInstall", &spec); assert!(result.is_err(), "failing script must propagate an error"); - let log_path = root.join("deployment-logs/codedeploy-agent-deployments.log"); - assert!(log_path.exists(), "deployment log must exist even on failure"); - let body = fs::read_to_string(&log_path).unwrap(); - assert!( - body.contains("AfterInstall/AfterInstall failed:"), - "deployment log must record the failure, got: {body:?}" - ); - // The event header must precede the failure summary so the log keeps - // context for which event failed. - assert!( - body.contains("LifecycleEvent - AfterInstall"), - "deployment log must record the event header on failure, got: {body:?}" + assert_eq!( + deployment_log_messages(&root), + vec![ + "[d-123]LifecycleEvent - AfterInstall", + "[d-123]Script - scripts/run.sh", + "[d-123][stdout]before-failure", + ] ); } + /// Concurrent deployments share one deployment log handle: every record stays whole and is tagged with + /// its own deployment ID. + #[cfg(unix)] + #[test] + fn concurrent_deployments_share_one_deployment_log() { + let dir = TempDir::new().unwrap(); + let archives = test_archives(&dir); + let root = dir.path().join("deployments"); + let cmd = HookCommand::new(archives.clone(), test_mapping()) + .with_deployment_log_config(log_config_for(&root)); + let script = "i=0; while [ $i -lt 50 ]; do echo \"out $i\"; i=$((i+1)); done\n"; + let specs: Vec<_> = ["d-aaa", "d-bbb"] + .iter() + .map(|id| stage_after_install(&archives, id, script)) + .collect(); + + std::thread::scope(|scope| { + for spec in &specs { + let cmd = &cmd; + scope.spawn(move || cmd.execute("AfterInstall", spec).unwrap()); + } + }); + + let messages = deployment_log_messages(&root); + for id in ["d-aaa", "d-bbb"] { + let mine: Vec<_> = + messages.iter().filter(|m| m.starts_with(&format!("[{id}]"))).collect(); + assert_eq!(mine.len(), 52, "{id}: header + script line + 50 output lines"); + assert_eq!(*mine[0], format!("[{id}]LifecycleEvent - AfterInstall")); + assert_eq!(*mine[51], format!("[{id}][stdout]out 49")); + } + assert_eq!(messages.len(), 104, "no stray or split records: {messages:?}"); + } + /// If the deployment log can't be opened (its dir can't be created), the /// deployment still succeeds — the log is a best-effort side-channel. /// Exercises the `Err(e) => warn!(...)` arm of the logger open. diff --git a/src/host_command/commands/update_agent.rs b/src/host_command/commands/update_agent.rs index c4256bd5..ce803284 100644 --- a/src/host_command/commands/update_agent.rs +++ b/src/host_command/commands/update_agent.rs @@ -26,6 +26,13 @@ const SHELL: &str = "/bin/sh"; /// Package-type arg: `auto` tells the script to detect the package manager. const PACKAGE_TYPE_ARG: &str = "auto"; +/// Why self-update is refused on macOS. The bucket install script has no +/// `pkg` package type yet; until it does, running it on macOS would +/// only fail inside the script after the download. +#[cfg(target_os = "macos")] +pub(crate) const UNSUPPORTED_PLATFORM_MESSAGE: &str = "Agent self-update is not supported on macOS yet. \ + Install the latest .pkg from the regional aws-codedeploy- bucket instead."; + pub struct UpdateAgentCommand { region: String, s3_client: Option, @@ -70,6 +77,8 @@ impl UpdateAgentCommand { pub fn execute(&self) -> io::Result> { info!("UpdateDeploymentAgent command received"); + Self::check_platform()?; + let s3 = self .s3_client .as_ref() @@ -77,7 +86,7 @@ impl UpdateAgentCommand { let bucket = s3_bucket_name(&self.region); - // Download to a temp file (created 0600 by stream_to_file, auto-cleaned on drop). + // Download to a temp file (created 0600 by the S3 client, auto-cleaned on drop). let tmp_file = NamedTempFile::new()?; let tmp_path = tmp_file.path(); s3.download_to_file(&bucket, INSTALL_SCRIPT_KEY, None, tmp_path)?; @@ -90,6 +99,25 @@ impl UpdateAgentCommand { } } +impl UpdateAgentCommand { + /// Refuse self-update on platforms the bucket install script cannot + /// install on. Called before any credential, IMDS or S3 use so the + /// refusal is immediate and needs no network access. + /// + /// # Errors + /// `ErrorKind::Unsupported` on macOS. `Ok` elsewhere. + pub fn check_platform() -> io::Result<()> { + #[cfg(target_os = "macos")] + { + Err(io::Error::new(io::ErrorKind::Unsupported, UNSUPPORTED_PLATFORM_MESSAGE)) + } + #[cfg(not(target_os = "macos"))] + { + Ok(()) + } + } +} + /// Construct the S3 bucket name for agent packages. fn s3_bucket_name(region: &str) -> String { format!("aws-codedeploy-{region}") @@ -136,12 +164,13 @@ fn execute_script(script_path: &Path, region: &str) -> io::Result { cmd.output() } -/// Strip `LD_*` loader vars so they can't inject a shared library into the -/// root-run install script or its sub-processes. Mirrors `Script::execute_async`. +/// Strip the dynamic-loader vars (`LD_*` on Linux, `DYLD_*` on macOS) so they +/// can't inject a shared library into the root-run install script or its +/// sub-processes. Mirrors `Script::execute_async`. /// Call after any `.env()` mutation so the removal is unconditional. fn strip_dynamic_loader_vars(cmd: &mut Command) { #[cfg(unix)] - for var in ["LD_PRELOAD", "LD_LIBRARY_PATH", "LD_AUDIT"] { + for var in crate::system::DYNAMIC_LOADER_ENV_VARS { cmd.env_remove(var); } #[cfg(not(unix))] @@ -259,6 +288,24 @@ mod tests { assert!(debug.contains("us-east-1")); } + #[cfg(target_os = "macos")] + #[test] + fn check_platform_refuses_macos_before_any_network_use() { + let err = UpdateAgentCommand::check_platform().unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::Unsupported); + assert!(err.to_string().contains("not supported on macOS")); + // execute() must return the same refusal even with no S3 client. + let cmd = UpdateAgentCommand::new("us-east-1".into(), None); + assert_eq!(cmd.execute().unwrap_err().kind(), io::ErrorKind::Unsupported); + } + + #[cfg(not(target_os = "macos"))] + #[test] + fn check_platform_allows_non_macos() { + assert!(UpdateAgentCommand::check_platform().is_ok()); + } + + #[cfg(not(target_os = "macos"))] #[test] fn execute_without_s3_client_errors() { let cmd = UpdateAgentCommand::new("us-east-1".into(), None); @@ -355,6 +402,7 @@ mod tests { #[cfg(unix)] #[test] fn strip_dynamic_loader_vars_removes_loader_env_from_child() { + use crate::system::DYNAMIC_LOADER_ENV_VARS; use std::os::unix::fs::OpenOptionsExt; let tmp = tempfile::tempdir().unwrap(); let script = tmp.path().join("install"); @@ -366,30 +414,37 @@ mod tests { .mode(0o600) .open(&script) .unwrap(); - // Report the loader vars the script sees; all must be empty. - writeln!( - f, - "#!/bin/sh\nprintf '[%s][%s][%s]' \ - \"$LD_PRELOAD\" \"$LD_LIBRARY_PATH\" \"$LD_AUDIT\" > \"{}\"", - marker.display() - ) - .unwrap(); + // Print every platform loader var; all must be empty. + let refs: Vec = DYNAMIC_LOADER_ENV_VARS.iter().map(|v| format!("[${v}]")).collect(); + writeln!(f, "#!/bin/sh\nprintf '%s' \"{}\" > \"{}\"", refs.join(""), marker.display()) + .unwrap(); drop(f); // Seed the loader vars directly on the Command (simulating inheritance // from the agent env) without mutating the test process's own env — // the crate forbids unsafe, so std::env::set_var is unavailable. let mut cmd = Command::new(SHELL); - cmd.arg(&script) - .env("LD_PRELOAD", "/tmp/evil.so") - .env("LD_LIBRARY_PATH", "/tmp/evil") - .env("LD_AUDIT", "/tmp/audit.so"); + cmd.arg(&script); + for var in DYNAMIC_LOADER_ENV_VARS { + cmd.env(var, "/tmp/evil"); + } strip_dynamic_loader_vars(&mut cmd); + // `env_remove` shows as `None` in `get_envs()`. This is the check that + // holds on macOS, where SIP scrubs DYLD_* before /bin/sh could see them. + for var in DYNAMIC_LOADER_ENV_VARS { + let entry = cmd.get_envs().find(|(k, _)| *k == std::ffi::OsStr::new(var)); + assert!( + matches!(entry, Some((_, None))), + "{var} must be marked removed on the Command, got {entry:?}" + ); + } + let output = cmd.output().unwrap(); assert!(output.status.success()); - assert_eq!(std::fs::read_to_string(&marker).unwrap(), "[][][]"); + let expected = "[]".repeat(DYNAMIC_LOADER_ENV_VARS.len()); + assert_eq!(std::fs::read_to_string(&marker).unwrap(), expected); } #[cfg(unix)] diff --git a/src/installer/builder.rs b/src/installer/builder.rs index 8565838d..cb6cc538 100644 --- a/src/installer/builder.rs +++ b/src/installer/builder.rs @@ -1,9 +1,9 @@ //! Install command builder — generates copy, permission, and cleanup commands from `AppSpec`. use crate::application_specification::Permission; +#[cfg(target_os = "linux")] +use crate::installer::commands::{ChangeAclCommand, ChangeContextCommand}; #[cfg(unix)] -use crate::installer::commands::{ - ChangeAclCommand, ChangeContextCommand, ChangeModeCommand, ChangeOwnerCommand, -}; +use crate::installer::commands::{ChangeModeCommand, ChangeOwnerCommand}; use crate::installer::commands::{CopyCommand, MakeDirectoryCommand, RemoveCommand}; use crate::installer::{InstallerError, Result}; use nu_path; @@ -18,6 +18,8 @@ pub struct CommandBuilder { copy_targets: HashMap, mkdir_targets: HashSet, permission_targets: HashSet, + // SELinux is Linux-only; macOS is unix but not linux. + #[cfg(target_os = "linux")] reject_unconfined_selinux: bool, reject_unsafe_permissions: bool, reject_symlink_permission_targets: bool, @@ -32,9 +34,9 @@ pub enum Command { Chmod(ChangeModeCommand), #[cfg(unix)] Chown(ChangeOwnerCommand), - #[cfg(unix)] + #[cfg(target_os = "linux")] Acl(ChangeAclCommand), - #[cfg(unix)] + #[cfg(target_os = "linux")] Context(ChangeContextCommand), } @@ -50,9 +52,9 @@ impl Command { Command::Chmod(cmd) => cmd.execute(cleanup), #[cfg(unix)] Command::Chown(cmd) => cmd.execute(cleanup), - #[cfg(unix)] + #[cfg(target_os = "linux")] Command::Acl(cmd) => cmd.execute(cleanup), - #[cfg(unix)] + #[cfg(target_os = "linux")] Command::Context(cmd) => cmd.execute(cleanup), } } @@ -68,9 +70,9 @@ impl Command { Command::Chmod(cmd) => cmd.to_h(), #[cfg(unix)] Command::Chown(cmd) => cmd.to_h(), - #[cfg(unix)] + #[cfg(target_os = "linux")] Command::Acl(cmd) => cmd.to_h(), - #[cfg(unix)] + #[cfg(target_os = "linux")] Command::Context(cmd) => cmd.to_h(), } } @@ -94,11 +96,16 @@ impl CommandBuilder { reject_unsafe_permissions: bool, reject_symlink_permission_targets: bool, ) -> Self { + // Same signature on every platform; unused off Linux. + #[cfg(not(target_os = "linux"))] + let _ = reject_unconfined_selinux; Self { commands: Vec::new(), copy_targets: HashMap::new(), mkdir_targets: HashSet::new(), permission_targets: HashSet::new(), + // SELinux is Linux-only; macOS is unix but not linux. + #[cfg(target_os = "linux")] reject_unconfined_selinux, reject_unsafe_permissions, reject_symlink_permission_targets, @@ -166,6 +173,46 @@ impl CommandBuilder { /// # Errors /// Returns an error if there's a duplicate permission target. + /// `acls` (setfacl) and `context` (semanage) exist on Linux only; + /// reject them on other Unix before any command is built. Called on entry + /// to `Installer::process_permission` (so it fires even when the entry + /// matches nothing) and from `set_permissions`. + pub fn check_permission_fields_supported(object: &Path, permission: &Permission) -> Result<()> { + #[cfg(target_os = "linux")] + { + let _ = (object, permission); + Ok(()) + } + #[cfg(not(target_os = "linux"))] + { + let platform = if cfg!(target_os = "macos") { + "macOS" + } else { + std::env::consts::OS + }; + let field = if permission.acls().is_some() { + "acls" + } else if permission.context().is_some() { + "context" + } else { + return Ok(()); + }; + Err(InstallerError::PermissionFieldNotSupported { + object: object.to_path_buf(), + field, + platform, + }) + } + } + + /// Queue the mode, ACL, `SELinux` context and owner changes `permission` + /// asks for on `object`. + /// + /// # Errors + /// + /// Returns [`InstallerError::DuplicatePermission`] if `object` already has + /// permissions queued, and [`InstallerError::PermissionFieldNotSupported`] + /// if `permission` uses a field this platform cannot apply. pub fn set_permissions(&mut self, object: &Path, permission: &Permission) -> Result<()> { debug!("Setting permissions on {}", object.display()); @@ -181,6 +228,8 @@ impl CommandBuilder { #[cfg(unix)] { + Self::check_permission_fields_supported(&object, permission)?; + if let Some(mode) = permission.mode() { self.commands.push(Command::Chmod(ChangeModeCommand::new( object.clone(), @@ -190,21 +239,24 @@ impl CommandBuilder { ))); } - if let Some(acls) = permission.acls() { - self.commands.push(Command::Acl(ChangeAclCommand::new( - object.clone(), - acls.clone(), - self.reject_symlink_permission_targets, - ))); - } + #[cfg(target_os = "linux")] + { + if let Some(acls) = permission.acls() { + self.commands.push(Command::Acl(ChangeAclCommand::new( + object.clone(), + acls.clone(), + self.reject_symlink_permission_targets, + ))); + } - if let Some(context) = permission.context() { - self.commands.push(Command::Context(ChangeContextCommand::new( - object.clone(), - context.clone(), - self.reject_unconfined_selinux, - self.reject_symlink_permission_targets, - ))); + if let Some(context) = permission.context() { + self.commands.push(Command::Context(ChangeContextCommand::new( + object.clone(), + context.clone(), + self.reject_unconfined_selinux, + self.reject_symlink_permission_targets, + ))); + } } if permission.owner().is_some() || permission.group().is_some() { @@ -502,7 +554,7 @@ mod tests { } #[test] - #[cfg(unix)] + #[cfg(target_os = "linux")] fn command_execute_acl() { use crate::application_specification::Acl; @@ -518,7 +570,7 @@ mod tests { } #[test] - #[cfg(unix)] + #[cfg(target_os = "linux")] fn command_execute_context() { use crate::application_specification::SeLinuxContext; @@ -715,7 +767,7 @@ mod tests { } #[test] - #[cfg(unix)] + #[cfg(target_os = "linux")] fn set_permissions_with_acl() { use crate::application_specification::{Acl, ObjectType, Permission}; @@ -747,8 +799,103 @@ mod tests { fs::remove_file(&dst).ok(); } + /// On a non-Linux Unix host `acls` has no setfacl to call, so the + /// builder must reject it before any command is queued. #[test] - #[cfg(unix)] + #[cfg(all(unix, not(target_os = "linux")))] + fn set_permissions_rejects_acls_on_non_linux_unix() { + use crate::application_specification::{Acl, ObjectType, Permission}; + + let mut builder = CommandBuilder::new(); + let dst = std::env::temp_dir().join("test_setperm_acl_nonlinux.txt"); + fs::write(&dst, "test").unwrap(); + + let acl = Acl::parse(&["user:testuser:rwx".to_string()]).unwrap(); + let perm = Permission::new_for_test( + "**".to_string(), + vec![ObjectType::File], + vec![], + None, + None, + // `mode` set: the rejection must come before the chmod is queued. + Some(crate::application_specification::Mode::from_octal("0644").unwrap()), + Some(acl), + None, + ); + + let err = builder.set_permissions(&dst, &perm).unwrap_err(); + assert!( + matches!(err, InstallerError::PermissionFieldNotSupported { field: "acls", .. }), + "expected PermissionFieldNotSupported for acls, got {err:?}" + ); + assert!(builder.commands().is_empty(), "no command may be queued after rejection"); + + fs::remove_file(&dst).ok(); + } + + /// The rejection is independent of whether the permission entry + /// matches any object; `process_permission` calls this on entry. + #[test] + #[cfg(all(unix, not(target_os = "linux")))] + fn check_permission_fields_rejects_context_without_a_match() { + use crate::application_specification::{ObjectType, Permission, SeLinuxContext}; + + let ctx = SeLinuxContext::new(None, "httpd_sys_content_t".to_string(), None); + let perm = Permission::new_for_test( + "/nonexistent/dir".to_string(), + vec![ObjectType::Directory], + vec![], + None, + None, + None, + None, + Some(ctx), + ); + + let err = + CommandBuilder::check_permission_fields_supported(Path::new("/nonexistent/dir"), &perm) + .unwrap_err(); + assert!( + matches!(err, InstallerError::PermissionFieldNotSupported { field: "context", .. }), + "expected PermissionFieldNotSupported for context, got {err:?}" + ); + } + + /// `context` (SELinux) is rejected the same way on non-Linux Unix. + #[test] + #[cfg(all(unix, not(target_os = "linux")))] + fn set_permissions_rejects_context_on_non_linux_unix() { + use crate::application_specification::{ObjectType, Permission, SeLinuxContext}; + + let mut builder = CommandBuilder::new(); + let dst = std::env::temp_dir().join("test_setperm_ctx_nonlinux.txt"); + fs::write(&dst, "test").unwrap(); + + let ctx = SeLinuxContext::new(None, "httpd_sys_content_t".to_string(), None); + let perm = Permission::new_for_test( + "**".to_string(), + vec![ObjectType::File], + vec![], + None, + None, + // `mode` set: the rejection must come before the chmod is queued. + Some(crate::application_specification::Mode::from_octal("0644").unwrap()), + None, + Some(ctx), + ); + + let err = builder.set_permissions(&dst, &perm).unwrap_err(); + assert!( + matches!(err, InstallerError::PermissionFieldNotSupported { field: "context", .. }), + "expected PermissionFieldNotSupported for context, got {err:?}" + ); + assert!(builder.commands().is_empty()); + + fs::remove_file(&dst).ok(); + } + + #[test] + #[cfg(target_os = "linux")] fn set_permissions_with_context() { use crate::application_specification::{ObjectType, Permission, SeLinuxContext}; @@ -855,7 +1002,7 @@ mod tests { } #[test] - #[cfg(unix)] + #[cfg(target_os = "linux")] fn command_to_h_acl() { use crate::application_specification::Acl; let file = std::env::temp_dir().join("test_acl.txt"); @@ -870,7 +1017,7 @@ mod tests { } #[test] - #[cfg(unix)] + #[cfg(target_os = "linux")] fn command_to_h_context() { use crate::application_specification::SeLinuxContext; let file = std::env::temp_dir().join("test_ctx.txt"); diff --git a/src/installer/commands/change_mode_command.rs b/src/installer/commands/change_mode_command.rs index 859e71b7..e84ae4aa 100644 --- a/src/installer/commands/change_mode_command.rs +++ b/src/installer/commands/change_mode_command.rs @@ -26,7 +26,7 @@ impl ChangeModeCommand { /// # Errors /// Returns an error if the command execution fails. pub fn execute(&self, _cleanup_file: &mut dyn Write) -> Result<()> { - use nix::sys::stat::{Mode, fchmod}; + use nix::sys::stat::fchmod; use std::os::fd::AsRawFd; use std::os::unix::fs::OpenOptionsExt; @@ -40,7 +40,7 @@ impl ChangeModeCommand { }); } - let mode_bits = Mode::from_bits_truncate(mode); + let mode_bits = crate::system::file_ops::mode_from_bits(mode); // By default the chmod follows symlinks (backwards-compatible behavior), // which a path-based `chmod` gives. diff --git a/src/installer/commands/copy_command.rs b/src/installer/commands/copy_command.rs index c868fc50..2247b382 100644 --- a/src/installer/commands/copy_command.rs +++ b/src/installer/commands/copy_command.rs @@ -48,7 +48,7 @@ impl CopyCommand { } else { #[cfg(unix)] { - use nix::sys::stat::{Mode, fchmod}; + use nix::sys::stat::fchmod; use nix::unistd::fchown; use std::os::fd::AsRawFd; use std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt}; @@ -89,7 +89,7 @@ impl CopyCommand { // Preserve permissions on the fd, after fchown so SUID/SGID bits // survive. Uses fd-based src_meta so owner, mode, and timestamps // share one race-free source. - let mode = Mode::from_bits_truncate(src_meta.permissions().mode()); + let mode = crate::system::file_ops::mode_from_bits(src_meta.permissions().mode()); fchmod(dest.as_raw_fd(), mode) .map_err(|e| std::io::Error::from_raw_os_error(e as i32))?; diff --git a/src/installer/commands/mod.rs b/src/installer/commands/mod.rs index 4bdbdfb9..d92c6b65 100644 --- a/src/installer/commands/mod.rs +++ b/src/installer/commands/mod.rs @@ -1,7 +1,7 @@ //! Installer command types — copy, mkdir, chmod, chown, setfacl, semanage. -#[cfg(unix)] +#[cfg(target_os = "linux")] mod change_acl_command; -#[cfg(unix)] +#[cfg(target_os = "linux")] mod change_context_command; #[cfg(unix)] mod change_mode_command; @@ -10,12 +10,12 @@ mod change_owner_command; mod copy_command; mod make_directory_command; mod remove_command; -#[cfg(unix)] +#[cfg(target_os = "linux")] mod remove_context_command; -#[cfg(unix)] +#[cfg(target_os = "linux")] pub use change_acl_command::ChangeAclCommand; -#[cfg(unix)] +#[cfg(target_os = "linux")] pub use change_context_command::ChangeContextCommand; #[cfg(unix)] pub use change_mode_command::ChangeModeCommand; @@ -24,5 +24,5 @@ pub use change_owner_command::ChangeOwnerCommand; pub use copy_command::CopyCommand; pub use make_directory_command::MakeDirectoryCommand; pub use remove_command::RemoveCommand; -#[cfg(unix)] +#[cfg(target_os = "linux")] pub use remove_context_command::RemoveContextCommand; diff --git a/src/installer/core.rs b/src/installer/core.rs index fad90083..5585af20 100644 --- a/src/installer/core.rs +++ b/src/installer/core.rs @@ -1,5 +1,5 @@ //! Installer core — executes install commands and manages cleanup files. -#[cfg(unix)] +#[cfg(target_os = "linux")] use super::commands::RemoveContextCommand; use super::{ builder::{Command, CommandBuilder}, @@ -270,6 +270,11 @@ impl Installer { ) -> Result<()> { let object = Path::new(permission.object()); + // On non-Linux Unix, `acls`/`context` are rejected here, before + // matching, so an entry that matches no object is still refused. + #[cfg(unix)] + CommandBuilder::check_permission_fields_supported(object, permission)?; + // Handle directory permissions if !builder.copying_file(object) { if builder.making_directory(object) || object.is_dir() { @@ -481,7 +486,7 @@ impl Installer { } if line.starts_with("semanage\0") { - #[cfg(unix)] + #[cfg(target_os = "linux")] { let object = line.strip_prefix("semanage\0").unwrap(); // Validate the SELinux object path the same way as a removal diff --git a/src/installer/error.rs b/src/installer/error.rs index 7ac17fdd..43dbb1d1 100644 --- a/src/installer/error.rs +++ b/src/installer/error.rs @@ -48,6 +48,14 @@ pub enum InstallerError { object: PathBuf, }, SelinuxRoleNotSupported, + /// An `AppSpec` `permissions` field that requires a Linux-only tool + /// (`acls` → setfacl, `context` → semanage/restorecon) was used on a + /// non-Linux Unix host (macOS). Raised before any command is built. + PermissionFieldNotSupported { + object: PathBuf, + field: &'static str, + platform: &'static str, + }, UnconfinedSelinuxRejected { type_: String, }, @@ -174,6 +182,14 @@ impl fmt::Display for InstallerError { ) }, + Self::PermissionFieldNotSupported { object, field, platform } => { + write!( + f, + "The deployment failed because the application specification file specifies '{field}' for {}, but '{field}' is not supported on {platform}. Remove '{field}' from the permissions entry in the AppSpec file, and then try again.", + object.display() + ) + }, + Self::UnconfinedSelinuxRejected { type_ } => { write!( f, @@ -332,6 +348,19 @@ mod tests { assert!(err.to_string().contains("roles are not supported")); } + #[test] + fn permission_field_not_supported_display() { + let err = InstallerError::PermissionFieldNotSupported { + object: PathBuf::from("/var/www/app"), + field: "context", + platform: "macOS", + }; + let msg = err.to_string(); + assert!(msg.contains("'context'")); + assert!(msg.contains("/var/www/app")); + assert!(msg.contains("not supported on macOS")); + } + #[test] fn unconfined_selinux_rejected_display() { let err = InstallerError::UnconfinedSelinuxRejected { type_: "unconfined_t".to_string() }; diff --git a/src/lifecycle_event/executor.rs b/src/lifecycle_event/executor.rs index b9262ad4..c0403659 100644 --- a/src/lifecycle_event/executor.rs +++ b/src/lifecycle_event/executor.rs @@ -3,11 +3,9 @@ //! Selects the correct deployment directory, parses the appspec, and executes //! each script for a given lifecycle event in order. //! -//! ## Not yet implemented -//! -//! - **Deployment log**: Writes to a secondary `DeploymentLog` when -//! `enable_deployments_log` config is set. `DeploymentLogger` exists in `logging/` -//! but is not wired into the executor yet. Needs config system integration. +//! Every `scripts.log` line is also written to the deployment log when one is +//! attached with [`LifecycleEventExecutor::with_deployment_log`] (set when +//! `enable_deployments_log` is on). use super::LifecycleEventType; use super::deployment_selector::select_deployment_dir; @@ -16,6 +14,7 @@ use super::script::{HookEnvPolicy, Script}; use super::script_run_log::ScriptRunLog; use crate::application_specification::AppSpec; use crate::deployment_specification::types::{DeploymentSpec, RevisionLocation, RevisionSource}; +use crate::logging::DeploymentLogSink; use crate::paths::APPSPEC_PATH_SEPARATORS; use crate::system::file_ops::ensure_executable; use std::collections::HashMap; @@ -43,6 +42,9 @@ pub struct LifecycleEventExecutor { /// `restrict_agent_dir_permissions`. Default `false` (world-readable /// 0755/0644); `true` = hardened 0750/0640. restrict_log_permissions: bool, + /// Deployment log writer for this deployment; `None` when + /// `enable_deployments_log` is off. + deployment_log: Option, } impl LifecycleEventExecutor { @@ -94,9 +96,18 @@ impl LifecycleEventExecutor { env_policy: HookEnvPolicy::default(), reject_path_traversal: false, restrict_log_permissions: false, + deployment_log: None, }) } + /// Write every `scripts.log` line to the deployment log too. `None` + /// (the default) disables it. + #[must_use] + pub fn with_deployment_log(mut self, sink: Option) -> Self { + self.deployment_log = sink; + self + } + /// Set the opt-in hook-environment hardening policy. Defaults to /// [`HookEnvPolicy::default`] (full env inheritance). #[must_use] @@ -175,7 +186,8 @@ impl LifecycleEventExecutor { let log_path = self.current_deployment_root_dir.join("logs/scripts.log"); let log = Arc::new(Mutex::new( ScriptRunLog::open_with_policy(&log_path, self.restrict_log_permissions) - .unwrap_or_else(|_| ScriptRunLog::in_memory()), + .unwrap_or_else(|_| ScriptRunLog::in_memory()) + .with_deployment_log(self.deployment_log.clone()), )); log.lock().unwrap().write_line("", &format!("LifecycleEvent - {event_name}")); diff --git a/src/lifecycle_event/script.rs b/src/lifecycle_event/script.rs index bb0a94dd..8e3d41fa 100644 --- a/src/lifecycle_event/script.rs +++ b/src/lifecycle_event/script.rs @@ -32,7 +32,8 @@ const SIGKILL_GRACE: Duration = Duration::from_secs(5); /// profile suppression). #[derive(Debug, Clone, Copy, Default)] pub struct HookEnvPolicy { - /// Strip `LD_PRELOAD`/`LD_LIBRARY_PATH`/`LD_AUDIT` from the hook env. + /// Strip the dynamic-loader env vars (`LD_*` on Linux, `DYLD_*` on + /// macOS; see `system::DYNAMIC_LOADER_ENV_VARS`) from the hook env. pub strip_loader_vars: bool, /// Clear the inherited env and rebuild from shell basics + deployment vars. pub restrict_to_allowlist: bool, @@ -166,7 +167,7 @@ impl Script { // Strip loader vars last, so an AppSpec `environment:` entry cannot re-add them. #[cfg(unix)] if self.env_policy.strip_loader_vars { - for var in ["LD_PRELOAD", "LD_LIBRARY_PATH", "LD_AUDIT"] { + for var in crate::system::DYNAMIC_LOADER_ENV_VARS { cmd.env_remove(var); } } @@ -309,12 +310,12 @@ pub(super) fn build_command( match (runas, sudo) { (Some(user), true) => { let mut cmd = Command::new("sudo"); - cmd.args(["su", user, "-c", &script_path.display().to_string()]); + cmd.args(["su", user, "-c", &su_command_arg(script_path)]); cmd }, (Some(user), false) => { let mut cmd = Command::new("su"); - cmd.args([user, "-c", &script_path.display().to_string()]); + cmd.args([user, "-c", &su_command_arg(script_path)]); cmd }, (None, true) => { @@ -326,6 +327,24 @@ pub(super) fn build_command( } } +/// The argument `su -c` hands to the target user's shell. A plain path is +/// passed as-is (hooks may read `BASH_EXECUTION_STRING` and expect it to +/// equal the bare path); a path with a space or shell metacharacter is +/// single-quoted so the shell runs that file instead of parsing the path. +#[cfg(unix)] +fn su_command_arg(script_path: &Path) -> String { + let path = script_path.display().to_string(); + let plain = path.bytes().all(|b| { + b.is_ascii_alphanumeric() + || matches!(b, b'/' | b'.' | b'_' | b'-' | b'+' | b'@' | b':' | b'=') + }); + if plain { + path + } else { + format!("'{}'", path.replace('\'', "'\\''")) + } +} + /// Build the command for script execution (Windows). /// /// `runas`/`sudo` are not supported on Windows. The AppSpec validator rejects @@ -391,10 +410,12 @@ async fn stream_to_log( let mut buf: Vec = Vec::with_capacity(MAX_LOG_LINE_BYTES); let mut byte = [0u8; 1]; - let flush = |log: &Arc>, buf: &mut Vec| { - if buf.is_empty() { - return; - } + // `write_line` records `buf` as one line. A blank output line is logged + // as a bare `[stdout]` record. A newline that + // ends a line already flushed at the chunk cap is not a blank line, so + // `chunk_flushed` suppresses that record. At EOF an empty buffer is not a + // line either. + let write_line = |log: &Arc>, buf: &mut Vec| { let line = String::from_utf8_lossy(buf); if let Ok(mut log) = log.lock() { log.write_line(prefix, &line); @@ -402,25 +423,32 @@ async fn stream_to_log( buf.clear(); }; + let mut chunk_flushed = false; loop { match reader.read(&mut byte).await { // EOF (Ok(0)) or a read error: stop draining this stream. Ok(0) | Err(_) => break, Ok(_) => { if byte[0] == b'\n' { - flush(log, &mut buf); + if !(buf.is_empty() && chunk_flushed) { + write_line(log, &mut buf); + } + chunk_flushed = false; } else { buf.push(byte[0]); if buf.len() >= MAX_LOG_LINE_BYTES { // Newline-free run hit the chunk cap — flush so rotation // is evaluated and memory stays bounded. - flush(log, &mut buf); + write_line(log, &mut buf); + chunk_flushed = true; } } }, } } - flush(log, &mut buf); // trailing partial line (no final newline) + if !buf.is_empty() { + write_line(log, &mut buf); // trailing partial line (no final newline) + } } /// Log process exit diagnostics at debug level. @@ -454,9 +482,22 @@ mod tests { use std::time::Duration; use tempfile::{NamedTempFile, TempDir}; - fn sample_log() -> Arc> { - let file = NamedTempFile::new().unwrap(); - Arc::new(Mutex::new(ScriptRunLog::open(file.path()).unwrap())) + // Own the parent dir: ScriptRunLog::open chmods it, which is EPERM on + // macOS's root-owned $TMPDIR. + /// `scripts.log` records with the `YYYY-MM-DD HH:MM:SS ` prefix removed. + fn logged_records(dir: &TempDir) -> Vec { + std::fs::read_to_string(dir.path().join("scripts.log")) + .unwrap() + .lines() + .map(|l| l.get(20..).unwrap_or_default().to_string()) + .collect() + } + + fn sample_log() -> (TempDir, Arc>) { + let dir = TempDir::new().unwrap(); + let log = + Arc::new(Mutex::new(ScriptRunLog::open(&dir.path().join("scripts.log")).unwrap())); + (dir, log) } #[tokio::test] @@ -467,8 +508,8 @@ mod tests { // 64 MiB rotation cap. Asserted on the FILE (the in-memory diagnostics // buffer is byte-capped and only keeps the tail, so it can't show the // chunk count). - let file = NamedTempFile::new().unwrap(); - let path = file.path().to_path_buf(); + let dir = TempDir::new().unwrap(); + let path = dir.path().join("scripts.log"); let log = Arc::new(Mutex::new(ScriptRunLog::open(&path).unwrap())); let blob = vec![b'A'; MAX_LOG_LINE_BYTES * 3 + 100]; // ~192 KiB, no '\n' stream_to_log(Some(std::io::Cursor::new(blob)), &log, "[stdout]").await; @@ -493,7 +534,7 @@ mod tests { #[tokio::test] async fn stream_to_log_preserves_normal_lines() { - let log = sample_log(); + let (_dir, log) = sample_log(); let input = b"first line\nsecond line\n".to_vec(); stream_to_log(Some(std::io::Cursor::new(input)), &log, "[stdout]").await; let entries = log.lock().unwrap().entries(); @@ -502,8 +543,80 @@ mod tests { assert!(entries[1].contains("second line")); } + #[tokio::test] + async fn stream_to_log_keeps_blank_lines() { + let (_dir, log) = sample_log(); + let input = b"before\n\nafter\n".to_vec(); + + stream_to_log(Some(std::io::Cursor::new(input)), &log, "[stdout]").await; + + let entries = log.lock().unwrap().entries(); + assert_eq!(entries.len(), 3, "blank line must be its own record: {entries:?}"); + assert!(entries[1].ends_with(" [stdout]\n"), "got {:?}", entries[1]); + } + + #[tokio::test] + async fn stream_to_log_adds_no_blank_record_after_line_at_exact_chunk_cap() { + let (dir, log) = sample_log(); + let mut input = vec![b'x'; MAX_LOG_LINE_BYTES]; + input.extend_from_slice(b"\nnext\n"); + + stream_to_log(Some(std::io::Cursor::new(input)), &log, "[stdout]").await; + + let records = logged_records(&dir); + assert_eq!(records.len(), 2, "cap-length line + next, no phantom blank record"); + assert_eq!(records[1], "[stdout]next"); + } + + #[tokio::test] + async fn stream_to_log_keeps_blank_line_after_line_at_exact_chunk_cap() { + let (dir, log) = sample_log(); + let mut input = vec![b'x'; MAX_LOG_LINE_BYTES]; + input.extend_from_slice(b"\n\nnext\n"); + + stream_to_log(Some(std::io::Cursor::new(input)), &log, "[stdout]").await; + + let records = logged_records(&dir); + assert_eq!(records.len(), 3, "cap-length line, real blank line, next"); + assert_eq!(records[1], "[stdout]"); + } + + #[tokio::test] + async fn stream_to_log_writes_nothing_for_empty_output() { + let (_dir, log) = sample_log(); + + stream_to_log(Some(std::io::Cursor::new(Vec::new())), &log, "[stdout]").await; + + assert!(log.lock().unwrap().entries().is_empty()); + } + // --- build_command --- + #[cfg(unix)] + #[test] + fn su_command_arg_passes_plain_paths_unchanged() { + assert_eq!( + su_command_arg(std::path::Path::new( + "/opt/codedeploy-agent/deployment-root/d-1/deployment-archive/scripts/start.sh" + )), + "/opt/codedeploy-agent/deployment-root/d-1/deployment-archive/scripts/start.sh" + ); + } + + #[cfg(unix)] + #[test] + fn su_command_arg_quotes_paths_with_shell_characters() { + assert_eq!( + su_command_arg(std::path::Path::new("/tmp/my app/run.sh")), + "'/tmp/my app/run.sh'" + ); + assert_eq!(su_command_arg(std::path::Path::new("/tmp/a;id/run.sh")), "'/tmp/a;id/run.sh'"); + assert_eq!( + su_command_arg(std::path::Path::new("/tmp/it's/run.sh")), + "'/tmp/it'\\''s/run.sh'" + ); + } + #[cfg(unix)] #[test] fn build_command_no_runas_no_sudo() { @@ -629,13 +742,9 @@ mod tests { let mut env = HashMap::new(); env.insert("KEY".to_string(), "VAL".to_string()); - let script = Script::new( - "/test/script.sh".into(), - Some("deploy".to_string()), - true, - &env, - sample_log(), - ); + let (_dir, log) = sample_log(); + let script = + Script::new("/test/script.sh".into(), Some("deploy".to_string()), true, &env, log); let debug = format!("{script:?}"); assert!(debug.contains("script.sh")); @@ -645,12 +754,13 @@ mod tests { #[test] fn new_no_runas() { + let (_dir, log) = sample_log(); let script = Script::new( "/test/script.sh".into(), None, false, &HashMap::::new(), - sample_log(), + log, ); let debug = format!("{script:?}"); @@ -662,12 +772,13 @@ mod tests { #[test] fn execute_nonexistent_script() { + let (_dir, log) = sample_log(); let script = Script::new( "/nonexistent/script.sh".into(), None, false, &HashMap::::new(), - sample_log(), + log, ); let result = script.execute(Duration::from_secs(10)); @@ -813,6 +924,44 @@ mod tests { ); } + /// Checked on the `Command` (`get_envs()`), not in a child: on macOS SIP + /// scrubs `DYLD_*` before `/bin/sh` runs, so a child-side check proves nothing. + #[cfg(unix)] + #[test] + fn strip_loader_vars_marks_platform_loader_vars_removed_on_command() { + use crate::system::DYNAMIC_LOADER_ENV_VARS; + + let dir = TempDir::new().unwrap(); + let log = Arc::new(Mutex::new(ScriptRunLog::open(&dir.path().join("log")).unwrap())); + let mut env_vars = HashMap::::new(); + for var in DYNAMIC_LOADER_ENV_VARS { + env_vars.insert((*var).to_string(), "/tmp/evil".to_string()); + } + let script = Script::with_env_policy( + dir.path().join("noop.sh"), + None, + false, + &env_vars, + HookEnvPolicy { + strip_loader_vars: true, + restrict_to_allowlist: false, + disable_powershell_profile: false, + }, + log, + ); + + let mut cmd = Command::new("true"); + script.apply_env_policy(&mut cmd); + + for var in DYNAMIC_LOADER_ENV_VARS { + let entry = cmd.as_std().get_envs().find(|(k, _)| *k == std::ffi::OsStr::new(var)); + assert!( + matches!(entry, Some((_, None))), + "{var} must be marked removed on the Command, got {entry:?}" + ); + } + } + /// The three AWS credential-override names must never reach the hook, even /// under the default full-inheritance policy. #[cfg(unix)] diff --git a/src/lifecycle_event/script_run_log.rs b/src/lifecycle_event/script_run_log.rs index f45923a3..76d3db09 100644 --- a/src/lifecycle_event/script_run_log.rs +++ b/src/lifecycle_event/script_run_log.rs @@ -11,6 +11,7 @@ //! from filling the disk. use super::bounded_fifo_vec::BoundedFifoVec; +use crate::logging::DeploymentLogSink; use chrono::Local; use std::fs::{self, File, OpenOptions}; use std::io::Write; @@ -43,6 +44,8 @@ pub struct ScriptRunLog { buffer: BoundedFifoVec, /// Mode policy for the file (and rotation reopens); see [`log_file_mode`]. restrict_permissions: bool, + /// When set, every line is also written to the shared deployment log. + deployment_log: Option, } impl ScriptRunLog { @@ -73,6 +76,7 @@ impl ScriptRunLog { file: Some(file), buffer: BoundedFifoVec::new(), restrict_permissions, + deployment_log: None, }) } @@ -85,9 +89,18 @@ impl ScriptRunLog { file: None, buffer: BoundedFifoVec::new(), restrict_permissions: false, + deployment_log: None, } } + /// Also write every line to the deployment log + /// (`deployment-logs/-deployments.log`). `None` disables it. + #[must_use] + pub fn with_deployment_log(mut self, sink: Option) -> Self { + self.deployment_log = sink; + self + } + /// Write a single line to both the file and the bounded buffer. /// /// `prefix` is `"[stdout]"`, `"[stderr]"`, or `""` for headers. @@ -98,8 +111,16 @@ impl ScriptRunLog { /// ANSI escape sequences are stripped from `line` before formatting so /// hostile script output cannot forge log entries via cursor positioning, /// screen clearing, or color codes. + /// + /// With a deployment log attached, the line is written there at the same + /// time, as `[]`, under one lock so both + /// files see lines in the same order. The deployment log gets the same + /// ANSI-stripped line. pub fn write_line(&mut self, prefix: &str, line: &str) { let line = strip_ansi_escapes::strip_str(line); + if let Some(sink) = &self.deployment_log { + sink.log(&format!("{prefix}{line}")); + } let ts = Local::now().format("%Y-%m-%d %H:%M:%S"); let formatted = format!("{ts} {prefix}{line}\n"); @@ -313,6 +334,27 @@ mod tests { assert_eq!(entries.len(), 2); } + #[test] + fn write_line_sends_ansi_stripped_line_to_deployment_log() { + let dir = tempfile::tempdir().unwrap(); + let config = crate::logging::LogConfig { + root_dir: dir.path().to_path_buf(), + ..crate::logging::LogConfig::default() + }; + let shared = crate::logging::SharedDeploymentLog::new(config); + shared.ensure_open().unwrap(); + let mut log = + ScriptRunLog::in_memory().with_deployment_log(Some(shared.for_deployment("d-1"))); + + log.write_line("[stdout]", "\x1b[31mred\x1b[0m"); + + assert!(log.entries()[0].ends_with(" [stdout]red\n")); + let body = + fs::read_to_string(dir.path().join("deployment-logs/codedeploy-agent-deployments.log")) + .unwrap(); + assert!(body.ends_with("] [d-1][stdout]red\n"), "got {body:?}"); + } + #[test] fn rotated_path_appends_index() { use std::path::PathBuf; diff --git a/src/logging/deployment_logger.rs b/src/logging/deployment_logger.rs index 2137abec..68f73bc8 100644 --- a/src/logging/deployment_logger.rs +++ b/src/logging/deployment_logger.rs @@ -4,6 +4,7 @@ use std::io::{self, Write}; #[cfg(unix)] use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex, PoisonError}; use chrono::Local; @@ -62,7 +63,8 @@ impl DeploymentLogger { pub fn log(&mut self, message: &str) -> io::Result<()> { self.rotate_if_needed()?; let ts = Local::now().format("%Y-%m-%d %H:%M:%S%.3f"); - writeln!(self.file, "[{ts}] {message}") + // One `write_all` per record so concurrent appenders cannot split it. + self.file.write_all(format!("[{ts}] {message}\n").as_bytes()) } fn rotate_if_needed(&mut self) -> io::Result<()> { @@ -93,6 +95,78 @@ impl DeploymentLogger { } } +/// Process-wide handle to the deployment log, shared by every deployment. +/// +/// All concurrent deployments append through one handle and one rotation. A +/// handle per hook command would let one deployment rotate the file while +/// another keeps writing to the renamed file. +/// +/// The file is opened by [`Self::ensure_open`]; a failed open is retried on +/// the next call, so a transient failure does not disable the log for the +/// life of the process. +#[derive(Debug, Clone)] +pub struct SharedDeploymentLog { + config: LogConfig, + logger: Arc>>, +} + +impl SharedDeploymentLog { + #[must_use] + pub fn new(config: LogConfig) -> Self { + Self { config, logger: Arc::new(Mutex::new(None)) } + } + + /// Open the log file if it is not open yet. + /// + /// # Errors + /// + /// Returns an error if the log directory or file cannot be created. + pub fn ensure_open(&self) -> io::Result<()> { + let mut guard = self.logger.lock().unwrap_or_else(PoisonError::into_inner); + if guard.is_none() { + *guard = Some(DeploymentLogger::new(&self.config)?); + } + Ok(()) + } + + /// A writer that tags every record with `deployment_id`. + #[must_use] + pub fn for_deployment(&self, deployment_id: &str) -> DeploymentLogSink { + DeploymentLogSink { log: self.clone(), deployment_id: deployment_id.to_string() } + } + + fn log(&self, message: &str) { + let mut guard = self.logger.lock().unwrap_or_else(PoisonError::into_inner); + if let Some(logger) = guard.as_mut() { + // Best-effort: the deployment log is diagnostic only. + let _ = logger.log(message); + } + } +} + +/// Writes one deployment's script-log records to the shared deployment log. +#[derive(Debug, Clone)] +pub struct DeploymentLogSink { + log: SharedDeploymentLog, + deployment_id: String, +} + +impl DeploymentLogSink { + /// Write `message` as `[]`. + /// + /// Called for every `scripts.log` line. Does nothing if the log is not open. + pub fn log(&self, message: &str) { + self.log + .log(&format!("[{}]{}", self.deployment_id, strip_ascii_whitespace(message))); + } +} + +/// Trims NUL, `\t`, `\n`, `\v`, `\f`, `\r` and space. +/// Narrower than `str::trim`, which also removes Unicode whitespace. +fn strip_ascii_whitespace(s: &str) -> &str { + s.trim_matches(|c| matches!(c, '\0' | '\t' | '\n' | '\x0b' | '\x0c' | '\r' | ' ')) +} + /// Open (or create) the deployment log file with the policy mode. /// Unix: 0644 default, 0640 restricted. The mode is force-set /// on every open so a file a tightened agent left 0640 heals to 0644 on the @@ -219,6 +293,64 @@ mod tests { assert!(lines[0].contains(']')); } + #[test] + fn writes_one_line_per_message_without_blank_lines() { + let dir = tempfile::tempdir().unwrap(); + let mut logger = DeploymentLogger::new(&test_config(dir.path())).unwrap(); + + logger.log("one").unwrap(); + logger.log("two").unwrap(); + + let content = fs::read_to_string(&logger.path).unwrap(); + assert_eq!(content.matches('\n').count(), 2, "exactly one newline per record: {content:?}"); + assert!(!content.contains("\n\n")); + } + + #[test] + fn strip_ascii_whitespace_leaves_unicode_whitespace() { + assert_eq!(strip_ascii_whitespace(" \t[stdout]hi\r\n\0"), "[stdout]hi"); + // Unicode whitespace is not stripped. + assert_eq!(strip_ascii_whitespace("\u{a0}x\u{a0}"), "\u{a0}x\u{a0}"); + } + + #[test] + fn sink_prefixes_deployment_id_and_strips_message() { + let dir = tempfile::tempdir().unwrap(); + let shared = SharedDeploymentLog::new(test_config(dir.path())); + shared.ensure_open().unwrap(); + + shared.for_deployment("d-1").log("[stdout]hello \n"); + + let path = dir.path().join("deployment-logs/test-agent-deployments.log"); + let content = fs::read_to_string(path).unwrap(); + let (_, message) = content.trim_end().split_once("] ").unwrap(); + assert_eq!(message, "[d-1][stdout]hello"); + } + + #[test] + fn sink_is_a_no_op_until_opened() { + let dir = tempfile::tempdir().unwrap(); + let shared = SharedDeploymentLog::new(test_config(dir.path())); + + shared.for_deployment("d-1").log("dropped"); + + assert!(!dir.path().join("deployment-logs").exists()); + } + + #[test] + fn ensure_open_retries_after_a_failure() { + let dir = tempfile::tempdir().unwrap(); + // A regular file where `deployment-logs/`'s parent should be. + let blocker = dir.path().join("root"); + fs::write(&blocker, "x").unwrap(); + let shared = SharedDeploymentLog::new(test_config(&blocker)); + assert!(shared.ensure_open().is_err()); + + fs::remove_file(&blocker).unwrap(); + + assert!(shared.ensure_open().is_ok(), "a later call must open the log"); + } + #[test] fn rotated_path_appends_index() { let base = PathBuf::from("/var/log/test.log"); diff --git a/src/logging/mod.rs b/src/logging/mod.rs index 74eb579b..a4a59a0a 100644 --- a/src/logging/mod.rs +++ b/src/logging/mod.rs @@ -3,7 +3,7 @@ mod agent_logger; mod deployment_logger; pub use agent_logger::LogGuard; -pub use deployment_logger::DeploymentLogger; +pub use deployment_logger::{DeploymentLogSink, DeploymentLogger, SharedDeploymentLog}; use std::path::PathBuf; diff --git a/src/main.rs b/src/main.rs index cbe703a9..9e1124a0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -603,6 +603,10 @@ fn run(command: &Command, config_file: Option<&Path>) { eprintln!("Failed to register signal handlers: {e}"); process::exit(1); } + // Ensure this worker dies with the master, however the master + // dies (e.g. by SIGKILL, which runs no graceful shutdown), so it + // can't be orphaned: on Linux it would hold the systemd service + // cgroup open, on macOS it would run next to a restarted agent. worker::bind_lifetime_to_parent(); worker::run(&shutdown, &config); }, @@ -682,6 +686,13 @@ const VALID_FILE_EXISTS_BEHAVIORS: &[&str] = &["DISALLOW", "OVERWRITE", "RETAIN" fn run_update(config: &AgentConfig) { use codedeploy_agent::host_command::commands::UpdateAgentCommand; + // Refuse unsupported platforms (macOS) before touching + // credentials, IMDS or S3 — same shape as the Windows branch above. + if let Err(e) = UpdateAgentCommand::check_platform() { + eprintln!("{e}"); + process::exit(1); + } + // Resolve credentials + region exactly as the worker / S3 deploy path do: // on-premises config region first, then AWS_REGION env / IMDS. let mut credentials = match Credentials::load(&config.on_premises_config_file) { @@ -710,6 +721,9 @@ fn run_update(config: &AgentConfig) { wire_log: config .log_aws_wire .then(|| (config.log_dir.clone(), config.program_name.clone())), + // Only fetches the small install script. + download_max_concurrency: 1, + download_chunk_size: config.s3_download_chunk_size, ..Default::default() }; let s3_client = match S3Client::new(credentials, &s3_config) { @@ -1087,6 +1101,8 @@ fn run_deploy_local_s3( wire_log: config .log_aws_wire .then(|| (config.log_dir.clone(), config.program_name.clone())), + download_max_concurrency: config.s3_download_max_concurrency, + download_chunk_size: config.s3_download_chunk_size, ..Default::default() }; let s3_client = match S3Client::new(credentials, &s3_config) { diff --git a/src/runtime/file_based_deployment_tracker.rs b/src/runtime/file_based_deployment_tracker.rs index 941c711f..09e15c57 100644 --- a/src/runtime/file_based_deployment_tracker.rs +++ b/src/runtime/file_based_deployment_tracker.rs @@ -610,7 +610,8 @@ mod edge_case_tests { ); } - #[cfg(unix)] + // APFS (macOS) rejects non-UTF-8 filenames with EILSEQ; the invalid-name path is only reachable on Linux filesystems. + #[cfg(target_os = "linux")] #[test] fn get_active_deployment_invalid_filename() { use std::ffi::OsStr; diff --git a/src/system/env_ops.rs b/src/system/env_ops.rs index 0c211b2e..b09559bb 100644 --- a/src/system/env_ops.rs +++ b/src/system/env_ops.rs @@ -1,4 +1,21 @@ //! Testable environment variable access. + +/// Dynamic-loader variables that can inject shared libraries into a child +/// process; stripped from the self-update script and (opt-in) from hooks. +/// glibc reads `LD_*`, dyld reads `DYLD_*` and ignores `LD_*`. +#[cfg(target_os = "macos")] +pub const DYNAMIC_LOADER_ENV_VARS: &[&str] = &[ + "DYLD_INSERT_LIBRARIES", + "DYLD_LIBRARY_PATH", + "DYLD_FRAMEWORK_PATH", + "DYLD_FALLBACK_LIBRARY_PATH", + "DYLD_FALLBACK_FRAMEWORK_PATH", + "DYLD_VERSIONED_LIBRARY_PATH", + "DYLD_VERSIONED_FRAMEWORK_PATH", +]; +#[cfg(all(unix, not(target_os = "macos")))] +pub const DYNAMIC_LOADER_ENV_VARS: &[&str] = &["LD_PRELOAD", "LD_LIBRARY_PATH", "LD_AUDIT"]; + /// Environment variable access abstraction. /// /// Production code uses `SystemEnvOps` which reads real env vars. @@ -51,6 +68,24 @@ mod mock { mod tests { use super::*; + #[cfg(target_os = "macos")] + #[test] + fn loader_vars_are_dyld_names_on_macos() { + assert!(DYNAMIC_LOADER_ENV_VARS.iter().all(|v| v.starts_with("DYLD_"))); + assert!(DYNAMIC_LOADER_ENV_VARS.contains(&"DYLD_INSERT_LIBRARIES")); + assert!(DYNAMIC_LOADER_ENV_VARS.contains(&"DYLD_FALLBACK_LIBRARY_PATH")); + assert!(DYNAMIC_LOADER_ENV_VARS.contains(&"DYLD_FALLBACK_FRAMEWORK_PATH")); + assert!(DYNAMIC_LOADER_ENV_VARS.contains(&"DYLD_VERSIONED_LIBRARY_PATH")); + assert!(DYNAMIC_LOADER_ENV_VARS.contains(&"DYLD_VERSIONED_FRAMEWORK_PATH")); + } + + #[cfg(all(unix, not(target_os = "macos")))] + #[test] + fn loader_vars_are_ld_names_on_linux() { + assert!(DYNAMIC_LOADER_ENV_VARS.iter().all(|v| v.starts_with("LD_"))); + assert!(DYNAMIC_LOADER_ENV_VARS.contains(&"LD_PRELOAD")); + } + #[test] fn system_env_reads_path() { let ops = SystemEnvOps; diff --git a/src/system/file_ops.rs b/src/system/file_ops.rs index 89039cfd..fe94492d 100644 --- a/src/system/file_ops.rs +++ b/src/system/file_ops.rs @@ -18,6 +18,15 @@ pub trait PlatformFileOperations: Send + Sync { fn write_with_retry(&self, path: &Path, content: &str) -> io::Result<()>; } +/// Build a `Mode` from `u32` permission bits. `mode_t` is `u32` on Linux but +/// `u16` on macOS, so mask to the permission bits (`0o7777`) before the cast. +#[cfg(unix)] +#[must_use] +#[allow(clippy::unnecessary_cast, clippy::cast_possible_truncation)] // width of mode_t is platform-specific +pub fn mode_from_bits(bits: u32) -> nix::sys::stat::Mode { + nix::sys::stat::Mode::from_bits_truncate((bits & 0o7777) as nix::libc::mode_t) +} + /// If the file has no execute bits, add them. Mirrors /// `script_executable?` check + `make_executable` fallback. /// @@ -208,6 +217,40 @@ pub fn copy_dir_recursive(src: &Path, dest: &Path) -> io::Result<()> { Ok(()) } +/// Write all of `buf` to `file` at `offset`, so several threads can write +/// different regions of one handle. Uses `pwrite` on Unix and `seek_write` on +/// Windows; `seek_write` also moves the cursor, which is harmless because every +/// write passes its own offset. +/// +/// # Errors +/// Returns an error if the write fails or writes zero bytes. +pub fn write_all_at(file: &std::fs::File, buf: &[u8], offset: u64) -> io::Result<()> { + let mut buf = buf; + let mut offset = offset; + while !buf.is_empty() { + #[cfg(unix)] + let written = std::os::unix::fs::FileExt::write_at(file, buf, offset); + #[cfg(windows)] + let written = std::os::windows::fs::FileExt::seek_write(file, buf, offset); + + match written { + Ok(0) => { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + "positioned write wrote zero bytes", + )); + }, + Ok(n) => { + buf = &buf[n..]; + offset += n as u64; + }, + Err(e) if e.kind() == io::ErrorKind::Interrupted => {}, + Err(e) => return Err(e), + } + } + Ok(()) +} + #[cfg(test)] #[derive(Debug)] pub struct MockFileOperations { @@ -262,6 +305,14 @@ mod safe_relative_path_tests { #[cfg(test)] mod tests { use super::*; + + #[cfg(unix)] + #[test] + fn mode_from_bits_keeps_permission_bits_and_drops_file_type() { + let mode = mode_from_bits(0o100_644); + assert_eq!(mode, nix::sys::stat::Mode::from_bits_truncate(0o644)); + assert_eq!(mode_from_bits(0o4755), nix::sys::stat::Mode::from_bits_truncate(0o4755)); + } use std::path::PathBuf; use tempfile::TempDir; @@ -427,4 +478,79 @@ mod tests { let mode = std::fs::metadata(dest.join("scripts")).unwrap().permissions().mode() & 0o777; assert_eq!(mode, 0o751, "expected 0751 preserved, got {mode:#o}"); } + + #[test] + fn write_all_at_writes_at_offset() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("out.bin"); + let file = std::fs::File::create(&path).unwrap(); + file.set_len(8).unwrap(); + write_all_at(&file, b"BBBB", 4).unwrap(); + write_all_at(&file, b"AAAA", 0).unwrap(); + drop(file); + assert_eq!(std::fs::read(&path).unwrap(), b"AAAABBBB"); + } + + #[test] + fn write_all_at_extends_file_and_zero_fills_gap() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("sparse.bin"); + let file = std::fs::File::create(&path).unwrap(); + // Writing past the end extends the file; the skipped region reads as zeros. + write_all_at(&file, b"Z", 3).unwrap(); + drop(file); + assert_eq!(std::fs::read(&path).unwrap(), b"\0\0\0Z"); + } + + #[test] + fn write_all_at_empty_buffer_is_noop() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("empty.bin"); + let file = std::fs::File::create(&path).unwrap(); + write_all_at(&file, b"", 0).unwrap(); + drop(file); + assert_eq!(std::fs::read(&path).unwrap().len(), 0); + } + + #[test] + fn write_all_at_read_only_handle_errors() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("ro.bin"); + std::fs::write(&path, b"data").unwrap(); + let file = std::fs::File::open(&path).unwrap(); + assert!(write_all_at(&file, b"x", 0).is_err()); + } + + #[test] + fn write_all_at_concurrent_ranges_do_not_interleave() { + use std::sync::Arc; + // Threads writing distinct regions of one handle produce the exact file. + let dir = TempDir::new().unwrap(); + let path = dir.path().join("parallel.bin"); + let chunk = 4096; + let threads = 8; + let file = Arc::new(std::fs::File::create(&path).unwrap()); + file.set_len((chunk * threads) as u64).unwrap(); + + let handles: Vec<_> = (0..threads) + .map(|i| { + let file = Arc::clone(&file); + std::thread::spawn(move || { + let buf = vec![b'a' + u8::try_from(i).unwrap(); chunk]; + write_all_at(&file, &buf, (i * chunk) as u64).unwrap(); + }) + }) + .collect(); + for h in handles { + h.join().unwrap(); + } + drop(file); + + let written = std::fs::read(&path).unwrap(); + assert_eq!(written.len(), chunk * threads); + for i in 0..threads { + let expected = vec![b'a' + u8::try_from(i).unwrap(); chunk]; + assert_eq!(&written[i * chunk..(i + 1) * chunk], &expected[..], "region {i} corrupted"); + } + } } diff --git a/src/system/mod.rs b/src/system/mod.rs index 1709e697..3495e969 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -1,31 +1,37 @@ //! System abstraction layer — file ops, process ops, `SELinux`, env vars. +//! +//! `linux_ops` (setfacl) and `selinux_ops` (semanage/restorecon) shell out to +//! Linux-only tools, so they are gated on `target_os = "linux"`, not `unix`; +//! macOS takes the rejection path in `installer::builder` instead. pub mod env_ops; pub mod file_ops; -#[cfg(unix)] +#[cfg(target_os = "linux")] pub mod linux_ops; pub mod process_ops; pub mod secure_files; -#[cfg(unix)] +#[cfg(target_os = "linux")] pub mod selinux_ops; pub mod version_file; -pub use env_ops::{EnvOps, SystemEnvOps}; -pub use file_ops::{PlatformFileOperations, SystemFileOperations, ensure_executable}; #[cfg(unix)] +pub use env_ops::DYNAMIC_LOADER_ENV_VARS; +pub use env_ops::{EnvOps, SystemEnvOps}; +pub use file_ops::{PlatformFileOperations, SystemFileOperations, ensure_executable, write_all_at}; +#[cfg(target_os = "linux")] pub use linux_ops::{LinuxOps, SystemLinuxOps}; pub use process_ops::kill_process_group; pub use secure_files::{ agent_file_mode, create_deployment_dir, create_dir_secure, create_dir_world_readable, create_file_secure, write_file_secure, }; -#[cfg(unix)] +#[cfg(target_os = "linux")] pub use selinux_ops::{SeLinuxOps, SystemSeLinuxOps}; #[cfg(test)] pub use env_ops::MockEnvOps; #[cfg(test)] pub use file_ops::MockFileOperations; -#[cfg(all(test, unix))] +#[cfg(all(test, target_os = "linux"))] pub use linux_ops::MockLinuxOps; -#[cfg(all(test, unix))] +#[cfg(all(test, target_os = "linux"))] pub use selinux_ops::MockSeLinuxOps; diff --git a/tests/s3_parallel_download.rs b/tests/s3_parallel_download.rs new file mode 100644 index 00000000..fbb63551 --- /dev/null +++ b/tests/s3_parallel_download.rs @@ -0,0 +1,643 @@ +//! Tests for the parallel S3 downloader against a fake S3 endpoint that records +//! the requests it receives (count, ranges, peak in flight). Each response is +//! held briefly so concurrent requests really overlap. + +use std::collections::HashSet; +use std::io::{BufRead, BufReader, Write}; +use std::net::{TcpListener, TcpStream}; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex}; +use std::time::Duration; + +use codedeploy_agent::aws_clients::credentials::{CredentialMode, Credentials}; +use codedeploy_agent::aws_clients::{S3Client, S3ClientConfig}; + +/// Position-dependent bytes, so a range at the wrong offset fails comparison. +fn object_bytes(len: usize) -> Vec { + (0..len).map(|i| u8::try_from(i % 251).unwrap()).collect() +} + +/// How the fake endpoint should behave. +#[derive(Clone, Copy, PartialEq)] +enum ServerMode { + /// Honor `Range` like S3 does. + Ranges, + /// Ignore `Range` and always return the whole object with `200`. + IgnoreRanges, + /// Answer any `Range` with `501 NotImplemented`. + RejectRanges, + /// Answer any `If-Match` with `412`, as if the object was replaced. + ObjectChanged, + /// Send only half of each range's body the first time it is requested. + TruncateFirstAttempt, + /// Send only half of every range's body. + TruncateAlways, + /// Honor the first range, then answer later ranges with the whole object. + IgnoreLaterRanges, + /// Like `RejectRanges`, but the error has no body. + BareRejectRanges, + /// Like `ObjectChanged`, but the error has no body. + BareObjectChanged, + /// Honor `Range` but send no `ETag`. + NoEtag, + /// Honor `Range` and add a wrong full-object checksum header to every 206. + BadChecksumHeader, +} + +/// What the fake endpoint saw. +#[derive(Default)] +struct Observations { + requests: usize, + unranged_requests: usize, + ranges: HashSet<(u64, u64)>, + /// Every range request, including repeats, so retries are countable. + range_request_log: Vec<(u64, u64)>, + peak_in_flight: usize, + if_match_values: HashSet, + /// Ranged requests that asked the SDK to validate response checksums. + ranged_checksum_mode_requests: usize, + /// Ranges already truncated once. + truncated: HashSet<(u64, u64)>, +} + +struct FakeS3 { + uri: String, + observations: Arc>, +} + +impl FakeS3 { + fn start(body: Vec, etag: &'static str, mode: ServerMode) -> Self { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind fake S3"); + let uri = format!("http://{}", listener.local_addr().unwrap()); + let observations = Arc::new(Mutex::new(Observations::default())); + + let body = Arc::new(body); + let obs = Arc::clone(&observations); + let in_flight = Arc::new(AtomicUsize::new(0)); + + // Detached; the listener lives until the test process exits. + std::thread::spawn(move || { + for stream in listener.incoming() { + let Ok(stream) = stream else { continue }; + let body = Arc::clone(&body); + let obs = Arc::clone(&obs); + let in_flight = Arc::clone(&in_flight); + std::thread::spawn(move || { + handle_connection(stream, &body, etag, mode, &obs, &in_flight); + }); + } + }); + + Self { uri, observations } + } + + fn observations(&self) -> std::sync::MutexGuard<'_, Observations> { + self.observations.lock().unwrap() + } +} + +fn handle_connection( + mut stream: TcpStream, + body: &[u8], + etag: &str, + mode: ServerMode, + obs: &Mutex, + in_flight: &AtomicUsize, +) { + stream.set_read_timeout(Some(Duration::from_secs(10))).ok(); + let Some(request) = read_request(&mut stream) else { + return; + }; + + let concurrent = in_flight.fetch_add(1, Ordering::SeqCst) + 1; + { + let mut obs = obs.lock().unwrap(); + obs.requests += 1; + obs.peak_in_flight = obs.peak_in_flight.max(concurrent); + if let Some(if_match) = &request.if_match { + obs.if_match_values.insert(if_match.clone()); + } + match request.range { + Some((start, end)) => { + obs.ranged_checksum_mode_requests += usize::from(request.checksum_mode); + obs.ranges.insert((start, end)); + obs.range_request_log.push((start, end)); + }, + None => obs.unranged_requests += 1, + } + } + + // Truncate each range once, so its retry succeeds. + let truncate = match (mode, request.range) { + (ServerMode::TruncateFirstAttempt, Some(range)) => { + obs.lock().unwrap().truncated.insert(range) + }, + (ServerMode::TruncateAlways, Some(_)) => true, + _ => false, + }; + + // Hold the response briefly so overlapping requests actually overlap. Leave + // the in-flight count before answering: once the client has the response it + // may start its next request before this thread runs again. + std::thread::sleep(Duration::from_millis(50)); + in_flight.fetch_sub(1, Ordering::SeqCst); + let _ = write_response(&mut stream, body, etag, mode, &request, truncate); +} + +struct Request { + range: Option<(u64, u64)>, + if_match: Option, + checksum_mode: bool, +} + +fn read_request(stream: &mut TcpStream) -> Option { + let mut reader = BufReader::new(stream.try_clone().ok()?); + let mut line = String::new(); + reader.read_line(&mut line).ok()?; + if line.trim().is_empty() { + return None; + } + + let mut range = None; + let mut if_match = None; + let mut checksum_mode = false; + loop { + let mut header = String::new(); + if reader.read_line(&mut header).ok()? == 0 || header.trim().is_empty() { + break; + } + let Some((name, value)) = header.split_once(':') else { + continue; + }; + let value = value.trim().to_string(); + match name.trim().to_ascii_lowercase().as_str() { + "range" => range = parse_range_header(&value), + "if-match" => if_match = Some(value), + "x-amz-checksum-mode" => checksum_mode = value.eq_ignore_ascii_case("enabled"), + _ => {}, + } + } + Some(Request { range, if_match, checksum_mode }) +} + +/// `bytes=0-262143` → `(0, 262143)`. +fn parse_range_header(value: &str) -> Option<(u64, u64)> { + let spec = value.trim().strip_prefix("bytes=")?; + let (start, end) = spec.split_once('-')?; + Some((start.trim().parse().ok()?, end.trim().parse().ok()?)) +} + +fn write_response( + stream: &mut TcpStream, + body: &[u8], + etag: &str, + mode: ServerMode, + request: &Request, + truncate: bool, +) -> std::io::Result<()> { + let total = body.len() as u64; + + let etag = (mode != ServerMode::NoEtag).then_some(etag); + let checksum = mode == ServerMode::BadChecksumHeader; + + if mode == ServerMode::ObjectChanged && request.if_match.is_some() { + return write_error(stream, 412, "Precondition Failed", Some("PreconditionFailed")); + } + if mode == ServerMode::BareObjectChanged && request.if_match.is_some() { + return write_error(stream, 412, "Precondition Failed", None); + } + if mode == ServerMode::BareRejectRanges && request.range.is_some() { + return write_error(stream, 501, "Not Implemented", None); + } + + match (mode, request.range) { + (ServerMode::RejectRanges, Some(_)) => { + write_error(stream, 501, "Not Implemented", Some("NotImplemented")) + }, + (ServerMode::IgnoreLaterRanges, Some((start, _))) if start > 0 => { + write_body(stream, 200, "OK", etag, body, None, false, false) + }, + (ServerMode::IgnoreRanges, _) | (_, None) => { + write_body(stream, 200, "OK", etag, body, None, false, false) + }, + (_, Some((start, end))) => { + // S3 answers any range on a zero-byte object, or a range that starts + // past the end, with 416/InvalidRange. + if start >= total { + return write_error( + stream, + 416, + "Requested Range Not Satisfiable", + Some("InvalidRange"), + ); + } + let last = end.min(total - 1); + let slice = &body[usize::try_from(start).unwrap()..=usize::try_from(last).unwrap()]; + let content_range = format!("bytes {start}-{last}/{total}"); + write_body( + stream, + 206, + "Partial Content", + etag, + slice, + Some(&content_range), + truncate, + checksum, + ) + }, + } +} + +/// `truncate` sends half the promised body and closes the connection. +/// `bad_checksum` adds a CRC32 header that does not match the body. +#[allow(clippy::too_many_arguments)] +fn write_body( + stream: &mut TcpStream, + status: u16, + reason: &str, + etag: Option<&str>, + body: &[u8], + content_range: Option<&str>, + truncate: bool, + bad_checksum: bool, +) -> std::io::Result<()> { + let mut head = format!( + "HTTP/1.1 {status} {reason}\r\nAccept-Ranges: bytes\r\n\ + Content-Length: {}\r\nConnection: close\r\n", + body.len() + ); + if let Some(etag) = etag { + head.push_str(&format!("ETag: \"{etag}\"\r\n")); + } + if bad_checksum { + head.push_str("x-amz-checksum-crc32: AAAAAA==\r\n"); + } + if let Some(cr) = content_range { + head.push_str(&format!("Content-Range: {cr}\r\n")); + } + head.push_str("\r\n"); + stream.write_all(head.as_bytes())?; + if truncate { + stream.write_all(&body[..body.len() / 2])?; + return stream.flush(); + } + stream.write_all(body)?; + stream.flush() +} + +/// An S3 XML error, or an empty body when `code` is `None`. +fn write_error( + stream: &mut TcpStream, + status: u16, + reason: &str, + code: Option<&str>, +) -> std::io::Result<()> { + let body = code.map_or_else(String::new, |code| { + format!( + "{code}\ + {reason}fake" + ) + }); + let head = format!( + "HTTP/1.1 {status} {reason}\r\nContent-Type: application/xml\r\n\ + Content-Length: {}\r\nConnection: close\r\n\r\n", + body.len() + ); + stream.write_all(head.as_bytes())?; + stream.write_all(body.as_bytes())?; + stream.flush() +} + +fn test_credentials() -> Credentials { + Credentials { + region: "us-east-1".into(), + host_identifier: "i-parallel-download".into(), + mode: CredentialMode::IamUser { + access_key_id: "AKIAIOSFODNN7EXAMPLE".into(), + secret_access_key: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY".into(), + }, + } +} + +fn client_for(server: &FakeS3, max_concurrency: usize, chunk_size: u64) -> S3Client { + let config = S3ClientConfig { + endpoint_override: Some(server.uri.clone()), + download_max_concurrency: max_concurrency, + download_chunk_size: chunk_size, + ..Default::default() + }; + S3Client::new(test_credentials(), &config).expect("build S3 client") +} + +const CHUNK: u64 = 64 * 1024; + +#[test] +fn parallel_download_reassembles_object_exactly() { + // 10 chunks: 9 ranges fan out after the first request. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 10); + let server = FakeS3::start(body.clone(), "bundleetag", ServerMode::Ranges); + let client = client_for(&server, 6, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("bundle.zip"); + let etag = client.download_to_file("bucket", "bundle.zip", None, &dest).expect("download"); + + assert_eq!( + etag.as_deref(), + Some("bundleetag"), + "ETag must be reported with quotes stripped" + ); + assert_eq!(std::fs::read(&dest).unwrap(), body, "downloaded bytes must match the object"); + + let obs = server.observations(); + assert_eq!(obs.ranges.len(), 10, "expected one request per chunk, saw {:?}", obs.ranges); + assert!( + obs.peak_in_flight > 1, + "downloads were serialized: peak in-flight requests was {}", + obs.peak_in_flight + ); + assert!( + obs.peak_in_flight <= 6, + "concurrency cap exceeded: {} requests in flight, limit 6", + obs.peak_in_flight + ); + // Follow-up ranges send If-Match with the ETag exactly as served (quoted). + assert_eq!( + obs.if_match_values.iter().collect::>(), + vec!["\"bundleetag\""], + "expected every follow-up range to send If-Match: \"bundleetag\", saw {:?}", + obs.if_match_values + ); +} + +#[test] +fn parallel_download_respects_concurrency_limit() { + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 8); + let server = FakeS3::start(body.clone(), "cap", ServerMode::Ranges); + let client = client_for(&server, 2, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("bundle.tgz"); + client.download_to_file("bucket", "bundle.tgz", None, &dest).expect("download"); + + assert_eq!(std::fs::read(&dest).unwrap(), body); + let obs = server.observations(); + assert!( + obs.peak_in_flight <= 2, + "max_concurrency=2 must never exceed 2 in flight, saw {}", + obs.peak_in_flight + ); +} + +#[test] +fn small_object_costs_a_single_request() { + // Smaller than one chunk: one request, no separate size probe. + let body = object_bytes(1024); + let server = FakeS3::start(body.clone(), "small", ServerMode::Ranges); + let client = client_for(&server, 10, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("small.zip"); + client.download_to_file("bucket", "small.zip", None, &dest).expect("download"); + + assert_eq!(std::fs::read(&dest).unwrap(), body); + assert_eq!(server.observations().requests, 1, "a small object must not cost extra requests"); +} + +#[test] +fn concurrency_one_uses_a_single_unranged_request() { + // Concurrency 1: one unranged GetObject. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 4); + let server = FakeS3::start(body.clone(), "serial", ServerMode::Ranges); + let client = client_for(&server, 1, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("serial.zip"); + client.download_to_file("bucket", "serial.zip", None, &dest).expect("download"); + + assert_eq!(std::fs::read(&dest).unwrap(), body); + let obs = server.observations(); + assert_eq!(obs.requests, 1, "serial mode must issue exactly one request"); + assert_eq!(obs.unranged_requests, 1, "serial mode must not send a Range header"); +} + +#[test] +fn endpoint_that_rejects_ranges_falls_back_to_one_request() { + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 4); + let server = FakeS3::start(body.clone(), "norange", ServerMode::RejectRanges); + let client = client_for(&server, 8, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("fallback.zip"); + let etag = client + .download_to_file("bucket", "fallback.zip", None, &dest) + .expect("fallback download"); + + assert_eq!(etag.as_deref(), Some("norange")); + assert_eq!( + std::fs::read(&dest).unwrap(), + body, + "fallback must still produce the whole object" + ); + assert!( + server.observations().unranged_requests >= 1, + "expected an unranged retry after the endpoint refused ranges" + ); +} + +#[test] +fn endpoint_that_ignores_ranges_still_downloads_whole_object() { + // A 200 with the full body must not be truncated to the first chunk. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 3); + let server = FakeS3::start(body.clone(), "ignored", ServerMode::IgnoreRanges); + let client = client_for(&server, 8, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("ignored.zip"); + client.download_to_file("bucket", "ignored.zip", None, &dest).expect("download"); + + assert_eq!(std::fs::read(&dest).unwrap(), body); +} + +#[test] +fn empty_object_downloads_via_fallback() { + // S3 answers any range on an empty object with 416, so this uses the fallback. + let server = FakeS3::start(Vec::new(), "empty", ServerMode::Ranges); + let client = client_for(&server, 8, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("empty.zip"); + client.download_to_file("bucket", "empty.zip", None, &dest).expect("download"); + + assert_eq!(std::fs::read(&dest).unwrap().len(), 0); +} + +#[test] +fn object_replaced_mid_download_fails_loudly() { + // If-Match must fail the download rather than mix two revisions. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 4); + let server = FakeS3::start(body, "stale", ServerMode::ObjectChanged); + let client = client_for(&server, 4, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("changed.zip"); + let err = client + .download_to_file("bucket", "changed.zip", None, &dest) + .expect_err("a replaced object must not produce a spliced bundle"); + + let message = err.to_string(); + assert!( + message.contains("changed during the download"), + "error must name the cause, got: {message}" + ); + // PreconditionFailed is not retried: at most one request per chunk. + let obs = server.observations(); + assert!( + obs.requests <= 4, + "PreconditionFailed must not be retried, saw {} requests", + obs.requests + ); +} + +#[test] +fn range_body_that_dies_mid_stream_is_retried() { + // A body cut off mid-stream is retried (the SDK does not retry this). + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 4); + let server = FakeS3::start(body.clone(), "flaky", ServerMode::TruncateFirstAttempt); + let client = client_for(&server, 4, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("flaky.zip"); + client + .download_to_file("bucket", "flaky.zip", None, &dest) + .expect("retry must recover"); + + assert_eq!( + std::fs::read(&dest).unwrap(), + body, + "a retried range must overwrite its own offset exactly, leaving no stale half-chunk" + ); + + // Every range was truncated once, so each was requested at least twice. + let obs = server.observations(); + for range in &obs.ranges { + let attempts = obs.range_request_log.iter().filter(|r| *r == range).count(); + assert!(attempts >= 2, "range {range:?} was not retried (only {attempts} attempt(s))"); + } +} + +#[test] +fn versioned_download_reassembles_object() { + // Ranged downloads must keep threading versionId through every request. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 5); + let server = FakeS3::start(body.clone(), "versioned", ServerMode::Ranges); + let client = client_for(&server, 5, CHUNK); + + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join("versioned.zip"); + client + .download_to_file("bucket", "versioned.zip", Some("abc123version"), &dest) + .expect("download"); + + assert_eq!(std::fs::read(&dest).unwrap(), body); +} + +fn download(server: &FakeS3, max_concurrency: usize, name: &str) -> (std::io::Result<()>, Vec) { + let client = client_for(server, max_concurrency, CHUNK); + let dir = tempfile::TempDir::new().unwrap(); + let dest = dir.path().join(name); + let result = client.download_to_file("bucket", name, None, &dest).map(|_| ()); + let bytes = std::fs::read(&dest).unwrap_or_default(); + (result, bytes) +} + +#[test] +fn later_range_answered_with_whole_object_fails_without_retrying() { + // A proxy that drops Range on later requests must not overwrite other ranges. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 4); + let server = FakeS3::start(body, "later", ServerMode::IgnoreLaterRanges); + let (result, _) = download(&server, 4, "later.zip"); + + let message = result.expect_err("a 200 for a range must fail").to_string(); + assert!(message.contains("Content-Range"), "{message}"); + assert!(server.observations().requests <= 4, "must not retry a wrong response"); +} + +#[test] +fn bodiless_501_falls_back_to_one_request() { + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 3); + let server = FakeS3::start(body.clone(), "bare", ServerMode::BareRejectRanges); + let (result, bytes) = download(&server, 4, "bare.zip"); + + result.expect("a 501 with no body must fall back"); + assert_eq!(bytes, body); + assert_eq!(server.observations().unranged_requests, 1); +} + +#[test] +fn bodiless_412_fails_without_retrying() { + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 4); + let server = FakeS3::start(body, "bare412", ServerMode::BareObjectChanged); + let (result, _) = download(&server, 4, "bare412.zip"); + + let message = result.expect_err("a replaced object must fail").to_string(); + assert!(message.contains("changed during the download"), "{message}"); + assert!(server.observations().requests <= 4, "a 412 must not be retried"); +} + +#[test] +fn missing_etag_falls_back_to_one_request() { + // Nothing would detect the object changing between ranges. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 3); + let server = FakeS3::start(body.clone(), "unused", ServerMode::NoEtag); + let (result, bytes) = download(&server, 4, "noetag.zip"); + + result.expect("download"); + assert_eq!(bytes, body); + let obs = server.observations(); + assert_eq!(obs.ranges.len(), 1, "only the first range should be requested"); + assert_eq!(obs.unranged_requests, 1); +} + +#[test] +fn full_object_checksum_header_on_a_range_is_ignored() { + // A full-object checksum cannot be checked against one range. + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 3); + let server = FakeS3::start(body.clone(), "crc", ServerMode::BadChecksumHeader); + let (result, bytes) = download(&server, 4, "crc.zip"); + + result.expect("ranged downloads must not validate full-object checksums"); + assert_eq!(bytes, body); + assert_eq!( + server.observations().ranged_checksum_mode_requests, + 0, + "ranged requests must turn off SDK response checksum validation" + ); +} + +#[test] +fn whole_object_of_exactly_one_chunk_is_downloaded_again() { + // No Content-Range and exactly one chunk could be a truncated range. + let body = object_bytes(usize::try_from(CHUNK).unwrap()); + let server = FakeS3::start(body.clone(), "exact", ServerMode::IgnoreRanges); + let (result, bytes) = download(&server, 4, "exact.zip"); + + result.expect("download"); + assert_eq!(bytes, body); + assert_eq!(server.observations().requests, 2); +} + +#[test] +fn range_that_keeps_failing_gives_up_after_three_attempts() { + let body = object_bytes(usize::try_from(CHUNK).unwrap() * 3); + let server = FakeS3::start(body, "broken", ServerMode::TruncateAlways); + let (result, _) = download(&server, 4, "broken.zip"); + + result.expect_err("a range that never completes must fail the download"); + let obs = server.observations(); + for range in &obs.ranges { + let attempts = obs.range_request_log.iter().filter(|r| *r == range).count(); + // The first range is also requested once by the size probe. + assert!(attempts <= 4, "range {range:?} requested {attempts} times"); + } +} diff --git a/tests/security/intake/appspec_validation.rs b/tests/security/intake/appspec_validation.rs index 36c61a04..47299782 100644 --- a/tests/security/intake/appspec_validation.rs +++ b/tests/security/intake/appspec_validation.rs @@ -86,7 +86,8 @@ fn mode_suid_bits_are_detectable() { // --------------------------------------------------------------------------- /// Parser accepts unconfined types; install-time rejection happens in `ChangeContextCommand`. -#[cfg(unix)] +// ChangeContextCommand is cfg(target_os = "linux") (semanage is Linux-only). +#[cfg(target_os = "linux")] #[test] fn appspec_rejects_unconfined_selinux_context() { use codedeploy_agent::installer::InstallerError; diff --git a/tests/security/intake/installer_permissions.rs b/tests/security/intake/installer_permissions.rs index 1b807c4c..f5183ae8 100644 --- a/tests/security/intake/installer_permissions.rs +++ b/tests/security/intake/installer_permissions.rs @@ -205,7 +205,9 @@ fn privilege_separation_is_documented() { // --------------------------------------------------------------------------- /// `ChangeContextCommand` rejects unconfined types when the toggle is on. +// setfacl/semanage are Linux-only; the commands are cfg(target_os = "linux"). #[test] +#[cfg(target_os = "linux")] fn change_context_rejects_unconfined() { use codedeploy_agent::application_specification::AppSpec; use codedeploy_agent::installer::InstallerError; @@ -253,7 +255,9 @@ fn change_context_rejects_unconfined() { /// Backwards-compatible default: unconfined types reach `semanage` when the /// toggle is off. /// Accepts `Ok` (semanage present and ran) or any non-`UnconfinedSelinuxRejected` error. +// setfacl/semanage are Linux-only; the commands are cfg(target_os = "linux"). #[test] +#[cfg(target_os = "linux")] fn unconfined_selinux_allowed_when_rejection_disabled() { use codedeploy_agent::application_specification::AppSpec; use codedeploy_agent::installer::InstallerError; @@ -470,7 +474,9 @@ fn change_mode_rejects_symlink_target() { /// A symlinked permission target must be rejected by ChangeContextCommand /// before the `canonicalize`/`semanage` path (which resolves the link). +// setfacl/semanage are Linux-only; the commands are cfg(target_os = "linux"). #[test] +#[cfg(target_os = "linux")] fn change_context_rejects_symlink_target() { use codedeploy_agent::application_specification::AppSpec; use codedeploy_agent::installer::InstallerError; @@ -500,7 +506,9 @@ fn change_context_rejects_symlink_target() { /// A symlinked permission target must be rejected by ChangeAclCommand rather /// than followed (setfacl redirected onto the link target). +// setfacl/semanage are Linux-only; the commands are cfg(target_os = "linux"). #[test] +#[cfg(target_os = "linux")] fn change_acl_rejects_symlink_target() { use codedeploy_agent::application_specification::AppSpec; use codedeploy_agent::installer::InstallerError; @@ -596,6 +604,8 @@ proptest! { // --------------------------------------------------------------------------- // Every blocklist member is rejected across randomised target paths. +// setfacl/semanage are Linux-only; ChangeContextCommand is cfg(target_os = "linux"). +#[cfg(target_os = "linux")] proptest! { #![proptest_config(ProptestConfig::with_cases(100))] diff --git a/tests/security/intake/process_isolation.rs b/tests/security/intake/process_isolation.rs index aec17b2a..ae1540bf 100644 --- a/tests/security/intake/process_isolation.rs +++ b/tests/security/intake/process_isolation.rs @@ -183,8 +183,9 @@ fn infinite_loop_script_terminated_at_timeout() { assert!(elapsed >= Duration::from_secs(1), "Timeout fired too early: {elapsed:?}"); } -// Validates: A script trapping SIGTERM (trap '' TERM; sleep infinity) is eventually killed +// Validates: A script trapping SIGTERM (trap '' TERM; sleep 3600) is eventually killed // via SIGKILL escalation after a grace period, preventing unkillable zombie processes. +// `sleep infinity` is GNU-only; BSD sleep (macOS) rejects it. #[cfg(unix)] #[test] fn sigterm_trapping_script_killed_by_sigkill() { @@ -197,7 +198,7 @@ fn sigterm_trapping_script_killed_by_sigkill() { std::fs::write( &script_path, - format!("#!/bin/sh\ntrap '' TERM\necho $$ > {}\nsleep infinity\n", pid_file.display()), + format!("#!/bin/sh\ntrap '' TERM\necho $$ > {}\nsleep 3600\n", pid_file.display()), ) .expect("write script"); std::fs::set_permissions(&script_path, std::fs::Permissions::from_mode(0o755)) @@ -358,16 +359,33 @@ fn script_path_does_not_contain_dangerous_directories() { let log = Arc::new(Mutex::new(ScriptRunLog::open(&dir.path().join("log")).expect("create log"))); - let script = Script::with_env_policy( - script_path, - None, - false, - &HashMap::::new(), - HARDENED, - log, - ); + let make_script = || { + Script::with_env_policy( + script_path.clone(), + None, + false, + &HashMap::::new(), + HARDENED, + log.clone(), + ) + }; - let exit_code = script.execute(Duration::from_secs(5)).expect("execute script"); + // Retry exec on ETXTBSY: when tests run in parallel, a child forked by + // another test thread can briefly inherit this just-written script's + // write descriptor, making exec fail with "Text file busy". The window + // is microseconds wide; a bounded retry removes the flake without + // masking real failures. + let mut attempts = 0; + let exit_code = loop { + match make_script().execute(Duration::from_secs(5)) { + Ok(code) => break code, + Err(e) if attempts < 5 && e.contains("Text file busy") => { + attempts += 1; + std::thread::sleep(Duration::from_millis(50)); + }, + Err(e) => panic!("execute script: {e:?}"), + } + }; assert_eq!(exit_code, 0); let path_value = std::fs::read_to_string(&output_file) @@ -394,44 +412,47 @@ fn script_path_does_not_contain_dangerous_directories() { } } -// Validates: with the opt-in strip_loader_vars policy, LD_PRELOAD, LD_LIBRARY_PATH -// and LD_AUDIT are absent from the lifecycle script environment, preventing shared -// library injection attacks — even when supplied via the AppSpec env map. -#[cfg(unix)] +// Validates: with the opt-in strip_loader_vars policy, the platform's dynamic +// loader variables (LD_PRELOAD/LD_LIBRARY_PATH/LD_AUDIT on Linux, +// DYLD_INSERT_LIBRARIES/DYLD_LIBRARY_PATH/DYLD_FRAMEWORK_PATH on macOS) are +// absent from the lifecycle script environment, preventing shared library +// injection attacks — even when supplied via the AppSpec env map. +// +// Linux-only: on macOS SIP scrubs DYLD_* before /bin/sh runs, so this check +// would pass regardless. `script::tests::strip_loader_vars_marks_platform_ +// loader_vars_removed_on_command` covers every Unix via `Command::get_envs()`. +#[cfg(target_os = "linux")] #[test] fn ld_preload_and_ld_library_path_not_in_script_env() { + use codedeploy_agent::system::DYNAMIC_LOADER_ENV_VARS; use std::os::unix::fs::PermissionsExt; let dir = tempfile::TempDir::new().expect("create tempdir"); let script_path = dir.path().join("check_ld.sh"); let output_file = dir.path().join("ld_output.txt"); - std::fs::write( - &script_path, - format!( - "#!/bin/sh\n\ - echo \"LD_PRELOAD=${{LD_PRELOAD:-UNSET}}\" > {out}\n\ - echo \"LD_LIBRARY_PATH=${{LD_LIBRARY_PATH:-UNSET}}\" >> {out}\n\ - echo \"LD_AUDIT=${{LD_AUDIT:-UNSET}}\" >> {out}\n", + // One `NAME=${NAME:-UNSET}` line per platform loader var. + let mut body = String::from("#!/bin/sh\n"); + for (i, var) in DYNAMIC_LOADER_ENV_VARS.iter().enumerate() { + let redirect = if i == 0 { ">" } else { ">>" }; + body.push_str(&format!( + "echo \"{var}=${{{var}:-UNSET}}\" {redirect} {out}\n", out = output_file.display() - ), - ) - .expect("write script"); + )); + } + std::fs::write(&script_path, body).expect("write script"); std::fs::set_permissions(&script_path, std::fs::Permissions::from_mode(0o755)) .expect("set permissions"); let log = Arc::new(Mutex::new(ScriptRunLog::open(&dir.path().join("log")).expect("create log"))); - // Pass LD_PRELOAD/LD_LIBRARY_PATH/LD_AUDIT via the AppSpec environment-variable - // HashMap — these must be stripped even when supplied directly by the customer - // bundle, not only when inherited from the agent's own environment. Stripping - // must therefore happen after the `env_vars` entries are applied to the - // command, otherwise those entries silently re-add the variables. + // Supply the loader vars via the AppSpec env map: they must be stripped + // even when the bundle sets them, not only when inherited from the agent. let mut env_vars = HashMap::::new(); - env_vars.insert("LD_PRELOAD".to_string(), "/tmp/evil.so".to_string()); - env_vars.insert("LD_LIBRARY_PATH".to_string(), "/tmp/evil-libs".to_string()); - env_vars.insert("LD_AUDIT".to_string(), "/tmp/evil-audit.so".to_string()); + for var in DYNAMIC_LOADER_ENV_VARS { + env_vars.insert((*var).to_string(), "/tmp/evil".to_string()); + } let script = Script::with_env_policy(script_path, None, false, &env_vars, HARDENED, log); @@ -440,18 +461,12 @@ fn ld_preload_and_ld_library_path_not_in_script_env() { let output = std::fs::read_to_string(&output_file).expect("read output file"); - assert!( - output.contains("LD_PRELOAD=UNSET"), - "LD_PRELOAD must be stripped even when supplied via AppSpec env_vars. Got: {output}" - ); - assert!( - output.contains("LD_LIBRARY_PATH=UNSET"), - "LD_LIBRARY_PATH must be stripped even when supplied via AppSpec env_vars. Got: {output}" - ); - assert!( - output.contains("LD_AUDIT=UNSET"), - "LD_AUDIT must be stripped even when supplied via AppSpec env_vars. Got: {output}" - ); + for var in DYNAMIC_LOADER_ENV_VARS { + assert!( + output.contains(&format!("{var}=UNSET")), + "{var} must be stripped even when supplied via AppSpec env_vars. Got: {output}" + ); + } } // Validates: IFS (Internal Field Separator) in the script environment is the default