Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.0.0"
version = "2.0.1"
description = "AWS CodeDeploy agent responsible for deploying software on an individual EC2/On-prem instance"
license = "Apache-2.0"
authors = ["Amazon Web Services"]
Expand Down
2 changes: 1 addition & 1 deletion crates/codedeploy-commands/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "codedeploy-commands"
version = "2.0.0"
version = "2.0.1"
edition = "2024"
rust-version = "1.91"
publish = false
Expand Down
212 changes: 207 additions & 5 deletions src/installer/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use super::{
error::{InstallerError, Result},
};
use crate::application_specification::{AppSpec, FileExistsBehavior};
use crate::paths::APPSPEC_PATH_SEPARATORS;
use crate::system::write_file_secure;
use serde_json::json;
use std::fs;
Expand Down Expand Up @@ -194,11 +195,9 @@ impl Installer {
file_mapping: &crate::application_specification::FileMapping,
spec: &AppSpec,
) -> Result<()> {
// Strip leading '/' so that an absolute-looking `source` stays relative to
// the archive dir: a leading slash must not discard the base path (as
// Rust's PathBuf::join would).
let source_relative =
file_mapping.source().strip_prefix('/').unwrap_or(file_mapping.source());
// A leading separator means "the entire revision", not a filesystem root.
// Strip every one of them so `join` cannot discard the archive dir.
let source_relative = file_mapping.source().trim_start_matches(APPSPEC_PATH_SEPARATORS);
let source = self.deployment_archive_dir.join(source_relative);
debug!("Processing file mapping from source: {}", source.display());

Expand Down Expand Up @@ -813,6 +812,209 @@ files:
assert!(dest_dir.path().join("index.html").exists());
}

/// A bare separator copies every file in the archive, `appspec.yml` included.
#[test]
fn install_bare_slash_source_copies_whole_revision() {
let archive_dir = TempDir::new().unwrap();
let instructions_dir = TempDir::new().unwrap();
let dest_dir = TempDir::new().unwrap();

fs::write(archive_dir.path().join("appspec.yml"), "version: 0.0\n").unwrap();
fs::write(archive_dir.path().join("my-file.txt"), "one").unwrap();
fs::create_dir(archive_dir.path().join("my-folder")).unwrap();
fs::write(archive_dir.path().join("my-folder/my-file-2.txt"), "two").unwrap();

let installer = Installer::new(
archive_dir.path().to_path_buf(),
instructions_dir.path().to_path_buf(),
FileExistsBehavior::Overwrite,
);

let appspec_yaml = format!(
r"
version: 0.0
os: linux
files:
- source: /
destination: {}
",
dest_dir.path().display()
);
let spec = AppSpec::parse(&appspec_yaml).unwrap();
installer.install("test-group", &spec).unwrap();

assert!(dest_dir.path().join("appspec.yml").exists());
assert!(dest_dir.path().join("my-file.txt").exists());
assert!(dest_dir.path().join("my-folder/my-file-2.txt").exists());
}

/// Repeated leading separators must be fully stripped.
#[test]
fn install_repeated_leading_slash_source_resolves_inside_archive() {
let archive_dir = TempDir::new().unwrap();
let instructions_dir = TempDir::new().unwrap();
let dest_dir = TempDir::new().unwrap();

fs::write(archive_dir.path().join("index.html"), "<html></html>").unwrap();

let installer = Installer::new(
archive_dir.path().to_path_buf(),
instructions_dir.path().to_path_buf(),
FileExistsBehavior::Overwrite,
);

let appspec_yaml = format!(
r"
version: 0.0
os: linux
files:
- source: //index.html
destination: {}
",
dest_dir.path().display()
);
let spec = AppSpec::parse(&appspec_yaml).unwrap();
installer.install("test-group", &spec).unwrap();

assert!(dest_dir.path().join("index.html").exists());
}

/// On Unix `\` is a filename character, not a separator. The lint assumes it
/// is a separator on every platform, which is what this test disproves.
#[cfg(unix)]
#[allow(clippy::join_absolute_paths)]
#[test]
fn install_backslash_source_is_a_filename_on_unix() {
let archive_dir = TempDir::new().unwrap();
let instructions_dir = TempDir::new().unwrap();
let dest_dir = TempDir::new().unwrap();

fs::write(archive_dir.path().join(r"\index.html"), "<html></html>").unwrap();

let installer = Installer::new(
archive_dir.path().to_path_buf(),
instructions_dir.path().to_path_buf(),
FileExistsBehavior::Overwrite,
);

let appspec_yaml = format!(
r"
version: 0.0
os: linux
files:
- source: \index.html
destination: {}
",
dest_dir.path().display()
);
let spec = AppSpec::parse(&appspec_yaml).unwrap();
installer.install("test-group", &spec).unwrap();

assert!(dest_dir.path().join(r"\index.html").exists());
}

#[cfg(windows)]
#[test]
fn install_leading_backslash_source_resolves_inside_archive() {
let archive_dir = TempDir::new().unwrap();
let instructions_dir = TempDir::new().unwrap();
let dest_dir = TempDir::new().unwrap();

fs::write(archive_dir.path().join("index.html"), "<html></html>").unwrap();

let installer = Installer::new(
archive_dir.path().to_path_buf(),
instructions_dir.path().to_path_buf(),
FileExistsBehavior::Overwrite,
);

let appspec_yaml = format!(
r"
version: 0.0
os: windows
files:
- source: \index.html
destination: {}
",
dest_dir.path().display()
);
let spec = AppSpec::parse(&appspec_yaml).unwrap();
installer.install("test-group", &spec).unwrap();

assert!(dest_dir.path().join("index.html").exists());
}

/// A bare `\` copies every file in the archive, `appspec.yml` included.
#[cfg(windows)]
#[test]
fn install_bare_backslash_source_copies_whole_revision() {
let archive_dir = TempDir::new().unwrap();
let instructions_dir = TempDir::new().unwrap();
let dest_dir = TempDir::new().unwrap();

fs::write(archive_dir.path().join("appspec.yml"), "version: 0.0\n").unwrap();
fs::write(archive_dir.path().join("my-file.txt"), "one").unwrap();
fs::create_dir(archive_dir.path().join("my-folder")).unwrap();
fs::write(archive_dir.path().join(r"my-folder\my-file-2.txt"), "two").unwrap();

let installer = Installer::new(
archive_dir.path().to_path_buf(),
instructions_dir.path().to_path_buf(),
FileExistsBehavior::Overwrite,
);

let appspec_yaml = format!(
r"
version: 0.0
os: windows
files:
- source: \
destination: {}
",
dest_dir.path().display()
);
let spec = AppSpec::parse(&appspec_yaml).unwrap();
installer.install("test-group", &spec).unwrap();

assert!(dest_dir.path().join("appspec.yml").exists());
assert!(dest_dir.path().join("my-file.txt").exists());
assert!(dest_dir.path().join(r"my-folder\my-file-2.txt").exists());
}

/// A leading separator followed by further components still resolves inside
/// the archive, not at the drive root.
#[cfg(windows)]
#[test]
fn install_leading_backslash_subdirectory_resolves_inside_archive() {
let archive_dir = TempDir::new().unwrap();
let instructions_dir = TempDir::new().unwrap();
let dest_dir = TempDir::new().unwrap();

fs::create_dir(archive_dir.path().join("my-folder")).unwrap();
fs::write(archive_dir.path().join(r"my-folder\my-file.txt"), "one").unwrap();

let installer = Installer::new(
archive_dir.path().to_path_buf(),
instructions_dir.path().to_path_buf(),
FileExistsBehavior::Overwrite,
);

let appspec_yaml = format!(
r"
version: 0.0
os: windows
files:
- source: \my-folder
destination: {}
",
dest_dir.path().display()
);
let spec = AppSpec::parse(&appspec_yaml).unwrap();
installer.install("test-group", &spec).unwrap();

assert!(dest_dir.path().join("my-file.txt").exists());
}

#[test]
fn install_path_traversal_allowed_when_flag_off() {
let archive_dir = TempDir::new().unwrap();
Expand Down
79 changes: 76 additions & 3 deletions src/lifecycle_event/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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::paths::APPSPEC_PATH_SEPARATORS;
use crate::system::file_ops::ensure_executable;
use std::collections::HashMap;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -195,9 +196,9 @@ impl LifecycleEventExecutor {
log: &Arc<Mutex<ScriptRunLog>>,
) -> Result<(), ScriptError> {
let location = script_info.location().to_string();
// Strip leading '/' so a leading slash in the location does not discard
// the base path (unlike Rust's PathBuf::join).
let location_relative = location.strip_prefix('/').unwrap_or(&location);
// Strip leading separators so `join` cannot discard the archive dir.
// Mirrors the `files.source` strip in installer/core.rs.
let location_relative = location.trim_start_matches(APPSPEC_PATH_SEPARATORS);
let script_path = archive_dir.join(location_relative);
// `entries()` is the bounded stdout/stderr tail the stream tasks buffered;
// it becomes the diagnostic's log so the service sees the script's output.
Expand Down Expand Up @@ -824,6 +825,78 @@ hooks:
assert!(entries.iter().any(|e| e.contains("Script - /scripts/ok.sh")));
}

/// Repeated leading separators must be fully stripped.
#[cfg(unix)]
#[test]
fn execute_repeated_leading_slash_location_resolves_inside_archive() {
use std::os::unix::fs::PermissionsExt;

let dir = TempDir::new().unwrap();
let appspec = r"
version: 0.0
os: linux
hooks:
AfterInstall:
- location: //scripts/ok.sh
timeout: 10
";
setup_appspec(dir.path(), appspec);

let scripts_dir = dir.path().join("deployment-archive/scripts");
std::fs::create_dir_all(&scripts_dir).unwrap();
std::fs::write(scripts_dir.join("ok.sh"), "#!/bin/sh\necho done\n").unwrap();
std::fs::set_permissions(scripts_dir.join("ok.sh"), std::fs::Permissions::from_mode(0o755))
.unwrap();

let spec = s3_spec();
let he = LifecycleEventExecutor::new(
LifecycleEventType::AfterInstall,
&spec,
dir.path(),
None,
None,
)
.unwrap();
let entries = he.execute().unwrap();
assert!(entries.iter().any(|e| e.contains("Script - //scripts/ok.sh")));
}

/// Uses a missing script and asserts the resolved path in the error message,
/// so nothing has to be executed to check resolution.
#[cfg(windows)]
#[test]
fn execute_leading_backslash_location_resolves_inside_archive() {
let dir = TempDir::new().unwrap();
let appspec = r"
version: 0.0
os: windows
hooks:
AfterInstall:
- location: \scripts\missing.cmd
timeout: 10
";
setup_appspec(dir.path(), appspec);

let spec = s3_spec();
let he = LifecycleEventExecutor::new(
LifecycleEventType::AfterInstall,
&spec,
dir.path(),
None,
None,
)
.unwrap();
let err = he.execute().unwrap_err();

assert_eq!(err.error_code, ErrorCode::ScriptMissing);
let expected = dir.path().join(r"deployment-archive\scripts\missing.cmd");
assert!(
err.message.contains(&expected.display().to_string()),
"location must resolve inside the archive, got: {}",
err.message
);
}

#[cfg(unix)]
#[test]
fn execute_failing_script() {
Expand Down
33 changes: 33 additions & 0 deletions src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@

use std::path::PathBuf;

/// Leading separators to strip from an `AppSpec` path (`files: source`,
/// `hooks: location`) before resolving it against the deployment archive.
///
/// `\` is a separator on Windows only; on Unix it is a legal filename character.
pub const APPSPEC_PATH_SEPARATORS: &[char] = if cfg!(windows) { &['/', '\\'] } else { &['/'] };

/// Returns the Windows base directory: `%PROGRAMDATA%\Amazon\CodeDeploy`.
///
/// Read from `%PROGRAMDATA%`, falling back to `C:\ProgramData`.
Expand Down Expand Up @@ -156,6 +162,33 @@ pub fn updater_log_path() -> PathBuf {
mod tests {
use super::*;

#[cfg(unix)]
#[test]
fn appspec_separators_exclude_backslash_on_unix() {
assert_eq!(APPSPEC_PATH_SEPARATORS, &['/']);
assert_eq!(r"\app".trim_start_matches(APPSPEC_PATH_SEPARATORS), r"\app");
}

#[cfg(windows)]
#[test]
fn appspec_separators_include_backslash_on_windows() {
assert_eq!(APPSPEC_PATH_SEPARATORS, &['/', '\\']);
assert_eq!(r"\app".trim_start_matches(APPSPEC_PATH_SEPARATORS), "app");
assert_eq!(r"\".trim_start_matches(APPSPEC_PATH_SEPARATORS), "");
}

#[test]
fn appspec_separators_strip_repeated_slashes() {
assert_eq!("//app".trim_start_matches(APPSPEC_PATH_SEPARATORS), "app");
assert_eq!("/".trim_start_matches(APPSPEC_PATH_SEPARATORS), "");
}

#[test]
fn appspec_separators_leave_relative_paths_alone() {
assert_eq!("app/sub".trim_start_matches(APPSPEC_PATH_SEPARATORS), "app/sub");
assert_eq!("./app".trim_start_matches(APPSPEC_PATH_SEPARATORS), "./app");
}

#[cfg(unix)]
#[test]
fn config_file_returns_unix_path() {
Expand Down
Loading