Skip to content

Commit cdfac99

Browse files
jchrostek-ddclaude
andcommitted
refactor: address PR review comments on delegated auth
- Pass reqwest::Client and AwsCredentials from upstream into get_delegated_api_key instead of building them internally - Extract try_delegated_auth helper in decrypt.rs to resolve credentials (including SnapStart) before calling the client - Remove duplicate get_snapstart_credentials from client.rs - Remove IntakeKeyResponse/Data/Attributes structs; use serde_json::Value - Read response body once to avoid conceptual double-read - Log body length only (not body content) in parse error for security - Downgrade info! URL log to debug! in client.rs - Move success info! log to decrypt.rs (caller) only - Add doc comments to magic constants in auth_proof.rs - Add comments explaining SnapStart credential flow and us-east-1 fallback - Add proxy comment on get_delegated_api_key explaining HTTPS_PROXY support Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 4f3f4da commit cdfac99

File tree

4 files changed

+104
-150
lines changed

4 files changed

+104
-150
lines changed

bottlecap/src/bin/bottlecap/main.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ async fn main() -> anyhow::Result<()> {
150150
let config = Arc::new(config::get_config(Path::new(&lambda_directory)));
151151

152152
let aws_config = Arc::new(aws_config);
153-
let api_key_factory = create_api_key_factory(&config, &aws_config);
153+
let shared_client = bottlecap::http::get_client(&config);
154+
let api_key_factory = create_api_key_factory(&config, &aws_config, &shared_client);
154155

155156
let r = response
156157
.await
@@ -161,6 +162,7 @@ async fn main() -> anyhow::Result<()> {
161162
Arc::clone(&aws_config),
162163
&config,
163164
&client,
165+
shared_client,
164166
&r,
165167
Arc::clone(&api_key_factory),
166168
start_time,
@@ -246,17 +248,23 @@ fn get_flush_strategy_for_mode(
246248
}
247249
}
248250

249-
fn create_api_key_factory(config: &Arc<Config>, aws_config: &Arc<AwsConfig>) -> Arc<ApiKeyFactory> {
251+
fn create_api_key_factory(
252+
config: &Arc<Config>,
253+
aws_config: &Arc<AwsConfig>,
254+
client: &reqwest::Client,
255+
) -> Arc<ApiKeyFactory> {
250256
let config = Arc::clone(config);
251257
let aws_config = Arc::clone(aws_config);
258+
let client = client.clone();
252259
let api_key_secret_reload_interval = config.api_key_secret_reload_interval;
253260

254261
Arc::new(ApiKeyFactory::new_from_resolver(
255262
Arc::new(move || {
256263
let config = Arc::clone(&config);
257264
let aws_config = Arc::clone(&aws_config);
265+
let client = client.clone();
258266

259-
Box::pin(async move { resolve_secrets(config, aws_config).await })
267+
Box::pin(async move { resolve_secrets(config, aws_config, client).await })
260268
}),
261269
api_key_secret_reload_interval,
262270
))
@@ -285,6 +293,7 @@ async fn extension_loop_active(
285293
aws_config: Arc<AwsConfig>,
286294
config: &Arc<Config>,
287295
client: &Client,
296+
shared_client: reqwest::Client,
288297
r: &RegisterResponse,
289298
api_key_factory: Arc<ApiKeyFactory>,
290299
start_time: Instant,
@@ -294,11 +303,6 @@ async fn extension_loop_active(
294303
let account_id = r.account_id.as_ref().unwrap_or(&"none".to_string()).clone();
295304
let tags_provider = setup_tag_provider(&Arc::clone(&aws_config), config, &account_id);
296305

297-
// Build one shared reqwest::Client for metrics, logs, and trace proxy flushing.
298-
// reqwest::Client is Arc-based internally, so cloning just increments a refcount
299-
// and shares the connection pool.
300-
let shared_client = bottlecap::http::get_client(config);
301-
302306
let (
303307
logs_agent_channel,
304308
logs_flusher,

bottlecap/src/secrets/decrypt.rs

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,15 @@ use tracing::{debug, error, info, warn};
1818

1919
use crate::secrets::delegated_auth;
2020

21-
pub async fn resolve_secrets(config: Arc<Config>, aws_config: Arc<AwsConfig>) -> Option<String> {
22-
if !config.dd_org_uuid.is_empty() {
23-
match delegated_auth::get_delegated_api_key(&config, &aws_config).await {
24-
Ok(api_key) => {
25-
info!("Delegated auth API key obtained successfully");
26-
return clean_api_key(Some(api_key));
27-
}
28-
Err(e) => {
29-
warn!("Delegated auth failed, falling back to other methods: {e}");
30-
}
31-
}
21+
pub async fn resolve_secrets(
22+
config: Arc<Config>,
23+
aws_config: Arc<AwsConfig>,
24+
client: Client,
25+
) -> Option<String> {
26+
if !config.dd_org_uuid.is_empty()
27+
&& let Some(api_key) = try_delegated_auth(&config, &aws_config, &client).await
28+
{
29+
return Some(api_key);
3230
}
3331

3432
let api_key_candidate = if !config.api_key_secret_arn.is_empty()
@@ -126,6 +124,58 @@ pub async fn resolve_secrets(config: Arc<Config>, aws_config: Arc<AwsConfig>) ->
126124
clean_api_key(api_key_candidate)
127125
}
128126

127+
async fn try_delegated_auth(
128+
config: &Arc<Config>,
129+
aws_config: &Arc<AwsConfig>,
130+
client: &Client,
131+
) -> Option<String> {
132+
let mut aws_credentials = AwsCredentials::from_env();
133+
134+
if aws_credentials.aws_secret_access_key.is_empty()
135+
&& aws_credentials.aws_access_key_id.is_empty()
136+
&& !aws_credentials
137+
.aws_container_credentials_full_uri
138+
.is_empty()
139+
&& !aws_credentials.aws_container_authorization_token.is_empty()
140+
{
141+
// We're in Snap Start
142+
let credentials = match get_snapstart_credentials(&aws_credentials, &client).await {
143+
Ok(credentials) => credentials,
144+
Err(err) => {
145+
error!(
146+
"Error getting Snap Start credentials for delegated auth: {}",
147+
err
148+
);
149+
return None;
150+
}
151+
};
152+
aws_credentials.aws_access_key_id = credentials["AccessKeyId"]
153+
.as_str()
154+
.unwrap_or_default()
155+
.to_string();
156+
aws_credentials.aws_secret_access_key = credentials["SecretAccessKey"]
157+
.as_str()
158+
.unwrap_or_default()
159+
.to_string();
160+
aws_credentials.aws_session_token = credentials["Token"]
161+
.as_str()
162+
.unwrap_or_default()
163+
.to_string();
164+
}
165+
166+
match delegated_auth::get_delegated_api_key(config, aws_config, client, &aws_credentials).await
167+
{
168+
Ok(api_key) => {
169+
info!("Delegated auth API key obtained successfully");
170+
clean_api_key(Some(api_key))
171+
}
172+
Err(e) => {
173+
warn!("Delegated auth failed, falling back to other methods: {e}");
174+
None
175+
}
176+
}
177+
}
178+
129179
fn clean_api_key(maybe_key: Option<String>) -> Option<String> {
130180
if let Some(key) = maybe_key {
131181
let clean_key = key.trim_end_matches('\n').replace(' ', "").clone();

bottlecap/src/secrets/delegated_auth/auth_proof.rs

Lines changed: 9 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@ use tracing::debug;
88

99
use crate::config::aws::AwsCredentials;
1010

11+
/// The STS `GetCallerIdentity` request body (form-encoded)
1112
const GET_CALLER_IDENTITY_BODY: &str = "Action=GetCallerIdentity&Version=2011-06-15";
13+
/// Content-Type for the STS request (required by `SigV4`)
1214
const CONTENT_TYPE: &str = "application/x-www-form-urlencoded; charset=utf-8";
15+
/// Datadog organization ID header included in the signed headers for backend verification
1316
const ORG_ID_HEADER: &str = "x-ddog-org-id";
17+
/// AWS service name used in `SigV4` credential scope
1418
const STS_SERVICE: &str = "sts";
1519

1620
/// Generates an authentication proof from AWS credentials.
@@ -34,6 +38,8 @@ pub fn generate_auth_proof(
3438
) -> Result<String, Box<dyn std::error::Error + Send + Sync>> {
3539
debug!("Generating delegated auth proof for region: {}", region);
3640

41+
// By the time this function is called, the caller has already resolved SnapStart
42+
// credentials from the container credentials endpoint if needed.
3743
if aws_credentials.aws_access_key_id.is_empty()
3844
|| aws_credentials.aws_secret_access_key.is_empty()
3945
{
@@ -44,11 +50,7 @@ pub fn generate_auth_proof(
4450
return Err("Missing org UUID for delegated auth".into());
4551
}
4652

47-
let sts_host = if region.is_empty() {
48-
"sts.amazonaws.com".to_string()
49-
} else {
50-
format!("sts.{region}.amazonaws.com")
51-
};
53+
let sts_host = format!("sts.{region}.amazonaws.com");
5254
let sts_url = format!("https://{sts_host}");
5355

5456
let now = Utc::now();
@@ -87,12 +89,7 @@ pub fn generate_auth_proof(
8789
);
8890

8991
let algorithm = "AWS4-HMAC-SHA256";
90-
let effective_region = if region.is_empty() {
91-
"us-east-1"
92-
} else {
93-
region
94-
};
95-
let credential_scope = format!("{date_stamp}/{effective_region}/{STS_SERVICE}/aws4_request");
92+
let credential_scope = format!("{date_stamp}/{region}/{STS_SERVICE}/aws4_request");
9693
let string_to_sign = format!(
9794
"{algorithm}\n{amz_date}\n{credential_scope}\n{}",
9895
hex::encode(Sha256::digest(canonical_request.as_bytes()))
@@ -101,7 +98,7 @@ pub fn generate_auth_proof(
10198
let signing_key = get_aws4_signature_key(
10299
&aws_credentials.aws_secret_access_key,
103100
&date_stamp,
104-
effective_region,
101+
region,
105102
STS_SERVICE,
106103
)?;
107104

@@ -292,28 +289,4 @@ mod tests {
292289
);
293290
}
294291

295-
#[test]
296-
fn test_generate_auth_proof_default_region() {
297-
let creds = AwsCredentials {
298-
aws_access_key_id: "AKIDEXAMPLE".to_string(),
299-
aws_secret_access_key: "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY".to_string(),
300-
aws_session_token: String::new(),
301-
aws_container_credentials_full_uri: String::new(),
302-
aws_container_authorization_token: String::new(),
303-
};
304-
305-
// Empty region should use global STS endpoint
306-
let result = generate_auth_proof(&creds, "", "test-org-uuid");
307-
assert!(result.is_ok());
308-
309-
let proof = result.expect("Failed to generate auth proof");
310-
let parts: Vec<&str> = proof.split('|').collect();
311-
let url = String::from_utf8(
312-
BASE64_STANDARD
313-
.decode(parts[3])
314-
.expect("Failed to decode base64 URL"),
315-
)
316-
.expect("Failed to convert URL to UTF-8");
317-
assert!(url.contains("sts.amazonaws.com"));
318-
}
319292
}

0 commit comments

Comments
 (0)