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.1"
version = "2.1.0"
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
5 changes: 5 additions & 0 deletions conf/codedeployagent.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@
:max_revisions: 5
:enable_deployments_log: true

# Reuse a previous deployment's on-host archive when the deployment spec names
# one, instead of downloading identical bytes again (restart deployments).
# Any reuse failure falls back to a normal download.
:enable_archive_reuse: true

# Security
:use_fips_mode: false
:enable_auth_policy: false
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.1"
version = "2.1.0"
edition = "2024"
rust-version = "1.91"
publish = false
Expand Down
12 changes: 10 additions & 2 deletions hooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@ set -euo pipefail
# Ensure cargo is on PATH (rustup installs to ~/.cargo/bin)
export PATH="$HOME/.cargo/bin:$PATH"

# Remember what the author staged, before any auto-fix runs.
staged_files=$(git diff --cached --name-only --diff-filter=ACM)

# Auto-fix formatting and clippy lints before commit
cargo fmt --all
cargo clippy --fix --allow-dirty --allow-staged 2>/dev/null || true

# Re-stage any files that were auto-fixed
git diff --name-only | xargs -r git add
# Re-stage only the files that were already staged. `git diff --name-only` would
# list every modified file in the tree, so anything the author deliberately left
# unstaged -- unrelated edits, or formatting churn in files this commit does not
# touch -- was silently swept into the commit.
if [ -n "$staged_files" ]; then
printf '%s\n' "$staged_files" | xargs -r git add --
fi
32 changes: 31 additions & 1 deletion src/command_poller/command_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ impl<T: DeploymentTracker, C: CommandServiceClient> CommandProcessor<T, C> {
elapsed_ms = elapsed.as_millis().try_into().unwrap_or(u64::MAX),
"Command succeeded"
);
self.report_completion(command, "Succeeded", &diagnostics::success(""));
let note = self.completion_note(&command.command_name, spec);
self.report_completion(command, "Succeeded", &diagnostics::success(&note));
},
Err(e) => {
error!(
Expand All @@ -257,6 +258,34 @@ impl<T: DeploymentTracker, C: CommandServiceClient> CommandProcessor<T, C> {
result.map(|_| ())
}

/// Detail the service can parse off a successful completion. Empty for commands that have none,
/// which keeps their diagnostics byte for byte what they were.
///
/// Read back from the `.bundle-source` marker `DownloadBundle` has just written, rather than
/// returned up through the dispatcher: the marker is already the single record of where the
/// bundle came from, so reading it cannot disagree with what the host actually did. A missing
/// marker yields no note -- writing it is best-effort, and a metric is not worth failing a
/// deployment over.
fn completion_note(&self, command_name: &str, spec: &DeploymentSpec) -> String {
if command_name != "DownloadBundle" {
return String::new();
}

let marker = self
.dispatcher
.archives()
.deployment_root_dir(&spec.deployment_group_id, &spec.deployment_id)
.join(crate::host_command::BUNDLE_SOURCE_FILE);

match std::fs::read_to_string(&marker) {
Ok(source) => crate::host_command::bundle_source_note(source.trim()),
Err(e) => {
debug!(path = %marker.display(), "No bundle source marker to report: {e}");
String::new()
},
}
}

fn report_completion(&self, command: &HostCommand, status: &str, payload: &str) {
debug!("Calling PutHostCommandComplete: \"{status}\"");
if let Err(e) =
Expand Down Expand Up @@ -341,6 +370,7 @@ mod tests {
bundle_type: "tar".into(),
},
all_possible_lifecycle_events: None,
reuse_archive_from_deployment_id: None,
}
}

Expand Down
24 changes: 24 additions & 0 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,12 @@ pub struct AgentConfig {
/// Enable the local command port for debugging. Default: `false`.
pub enable_command_port: bool,

/// Allow reusing a previous deployment's on-host archive when the deployment
/// spec carries `ReuseArchiveFromDeploymentId` (bounce/restart deployments).
/// Default: `true`. Any reuse failure falls back to a normal download, so
/// turning this off costs only the optimisation.
pub enable_archive_reuse: bool,

/// Capture Amazon S3 HTTP wire logs to `<program_name>.aws_wire.log` in
/// `log_dir`. Default: `false`.
///
Expand Down Expand Up @@ -384,6 +390,7 @@ impl Default for AgentConfig {
s3_endpoint_override: None,
disable_imds_v1: false,
enable_command_port: false,
enable_archive_reuse: true,
log_aws_wire: false,
disable_core_dumps: true,
hardening: HardeningConfig::default(),
Expand Down Expand Up @@ -902,6 +909,23 @@ mod tests {
assert!(!AgentConfig::default().enable_command_port);
}

#[test]
fn default_enable_archive_reuse_is_true() {
assert!(AgentConfig::default().enable_archive_reuse);
}

#[test]
fn enable_archive_reuse_parses_from_yaml() {
let yaml = "enable_archive_reuse: false\n";
let config = AgentConfig::from_yaml(yaml, Path::new("test.yml")).unwrap();
assert!(!config.enable_archive_reuse);

// Ruby `:key:` form must also parse.
let yaml = ":enable_archive_reuse: false\n";
let config = AgentConfig::from_yaml(yaml, Path::new("test.yml")).unwrap();
assert!(!config.enable_archive_reuse);
}

#[test]
fn default_reject_symlinks_in_bundle_is_false() {
assert!(!AgentConfig::default().hardening.reject_symlinks_in_bundle);
Expand Down
148 changes: 148 additions & 0 deletions src/deployment_specification/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,23 @@ pub(super) fn build(
let deployment_id = extract_deployment_id(data["DeploymentId"].as_str().unwrap());
let deployment_group_id = data["DeploymentGroupId"].as_str().unwrap().to_string();

// Both values become directory names under the deployment root
// (DeploymentArchives::deployment_root_dir joins them), and `Path::join` treats
// `..` as an ordinary parent component. Reject anything that is not a single
// safe component here, at the one place every consumer flows through, rather
// than at each of the three call sites that build paths from them. Fails
// closed: a spec carrying an unsafe value is rejected, not redirected.
for (field, value) in [
("DeploymentId", &deployment_id),
("DeploymentGroupId", &deployment_group_id),
] {
if !crate::system::file_ops::is_safe_path_component(value) {
return Err(DeploymentSpecError::ParseError(format!(
"{field} is not a valid path component: {value:?}"
)));
}
}

let deployment_creator = data
.get("DeploymentCreator")
.and_then(|v| v.as_str())
Expand All @@ -84,6 +101,17 @@ pub(super) fn build(
.unwrap_or(DEFAULT_APP_SPEC_PATH)
.to_string();

// The AppSpec path is joined to the unpacked archive directory by DownloadBundle, Install and
// the lifecycle-event executor, and `Path::join` treats `..` as an ordinary parent component and
// an absolute path as a replacement for the whole join. Validate it here, at the same choke
// point as the two IDs above, so every consumer inherits the guarantee rather than each having
// to repeat the check. Nested paths stay legal; climbing out does not.
if !crate::system::file_ops::is_safe_relative_path(&app_spec_path) {
return Err(DeploymentSpecError::ParseError(format!(
"AppSpecFilename is not a safe revision-relative path: {app_spec_path:?}"
)));
}

let file_exists_behavior = data
.get("AgentActionOverrides")
.and_then(|overrides| overrides.get("AgentOverrides"))
Expand All @@ -98,6 +126,15 @@ pub(super) fn build(
.collect::<Vec<String>>()
});

// Resolved through extract_deployment_id for parity with DeploymentId, so an
// ARN-form value normalises to the short form. Format validation happens at
// the point of use (see host_command::archive_reuse).
let reuse_archive_from_deployment_id = data
.get("ReuseArchiveFromDeploymentId")
.and_then(|v| v.as_str())
.filter(|s| !s.is_empty())
.map(extract_deployment_id);

Ok(DeploymentSpec {
deployment_id,
deployment_group_id,
Expand All @@ -110,6 +147,7 @@ pub(super) fn build(
revision_source,
revision,
all_possible_lifecycle_events,
reuse_archive_from_deployment_id,
})
}

Expand All @@ -119,6 +157,116 @@ mod tests {
use crate::deployment_specification::types::{RevisionLocation, RevisionSource};
use serde_json::json;

fn minimal_revision() -> (RevisionSource, RevisionLocation) {
(
RevisionSource::S3,
RevisionLocation::S3 {
bucket: "bucket".to_string(),
key: "key".to_string(),
bundle_type: "tar".to_string(),
version: None,
etag: None,
},
)
}

#[test]
fn rejects_traversal_in_deployment_group_id() {
let data = json!({
"DeploymentId": "d-12345678",
"DeploymentGroupId": "../../../../tmp/evil",
"DeploymentGroupName": "MyGroup",
"ApplicationName": "MyApp"
});
let (src, rev) = minimal_revision();
let err = build(&data, src, rev).unwrap_err();
assert!(
err.to_string().contains("DeploymentGroupId is not a valid path component"),
"got: {err}"
);
}

/// The `AppSpec` path is joined to the unpacked archive by `DownloadBundle`, `Install` and the
/// lifecycle-event executor, and Install parses whatever it names -- so a climbing path would be
/// read and parsed as an `AppSpec`, not merely probed for existence.
#[test]
fn rejects_traversal_in_app_spec_filename() {
for bad in [
"../../etc/passwd",
"/etc/shadow",
"configs/../../appspec.yml",
] {
let data = json!({
"DeploymentId": "d-12345678",
"DeploymentGroupId": "dg-12345678",
"DeploymentGroupName": "MyGroup",
"ApplicationName": "MyApp",
"AppSpecFilename": bad
});
let (src, rev) = minimal_revision();
let err = build(&data, src, rev).unwrap_err();
assert!(
err.to_string().contains("AppSpecFilename is not a safe revision-relative path"),
"must reject {bad:?}, got: {err}"
);
}
}

#[test]
fn accepts_a_nested_app_spec_filename() {
let data = json!({
"DeploymentId": "d-12345678",
"DeploymentGroupId": "dg-12345678",
"DeploymentGroupName": "MyGroup",
"ApplicationName": "MyApp",
"AppSpecFilename": "configs/appspec.yml"
});
let (src, rev) = minimal_revision();
let spec = build(&data, src, rev).unwrap();
assert_eq!(spec.app_spec_path, "configs/appspec.yml");
}

#[test]
fn rejects_traversal_in_deployment_id() {
let data = json!({
"DeploymentId": "..",
"DeploymentGroupId": "dg-12345678",
"DeploymentGroupName": "MyGroup",
"ApplicationName": "MyApp"
});
let (src, rev) = minimal_revision();
let err = build(&data, src, rev).unwrap_err();
assert!(
err.to_string().contains("DeploymentId is not a valid path component"),
"got: {err}"
);
}

#[test]
fn rejects_separator_in_deployment_id() {
let data = json!({
"DeploymentId": "d-A/../../etc",
"DeploymentGroupId": "dg-12345678",
"DeploymentGroupName": "MyGroup",
"ApplicationName": "MyApp"
});
let (src, rev) = minimal_revision();
assert!(build(&data, src, rev).is_err());
}

#[test]
fn accepts_uuid_shaped_deployment_group_id() {
let data = json!({
"DeploymentId": "d-12345678",
"DeploymentGroupId": "f47ac10b-58cc-4372-a567-0e02b2c3d479",
"DeploymentGroupName": "MyGroup",
"ApplicationName": "MyApp"
});
let (src, rev) = minimal_revision();
let spec = build(&data, src, rev).unwrap();
assert_eq!(spec.deployment_group_id, "f47ac10b-58cc-4372-a567-0e02b2c3d479");
}

#[test]
fn build_minimal() {
let data = json!({
Expand Down
1 change: 1 addition & 0 deletions src/deployment_specification/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub struct DeploymentSpec {
pub revision_source: RevisionSource,
pub revision: RevisionLocation,
pub all_possible_lifecycle_events: Option<Vec<String>>,
pub reuse_archive_from_deployment_id: Option<String>,
}

#[derive(Debug, Clone)]
Expand Down
Loading
Loading