Add support for AWS_CONTAINER_CREDENTIALS_FULL_URI (EKS Pod Identity Agent)#449
Add support for AWS_CONTAINER_CREDENTIALS_FULL_URI (EKS Pod Identity Agent)#449LockedThread wants to merge 8 commits into
Conversation
📝 WalkthroughWalkthroughAdds support for two container metadata sources: ECS RELATIVE_URI and EKS FULL_URI; implements optional container authorization token loading (file or env) and allowlist validation for link-local agent hosts; refactors container metadata request construction to include conditional Authorization header; expands unit tests and test helpers. ChangesContainer credentials: FULL_URI, token loading, and request refactor
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant CP as CredentialsProvider
participant TS as TokenSource\n(File / Env)
participant MS as MetadataServer\n(ECS RELATIVE_URI / EKS FULL_URI)
App->>CP: from_container_credentials_provider()
alt FULL_URI path
CP->>TS: get_container_authorization_token()
TS-->>CP: token (optional)
CP->>MS: GET FULL_URI with Authorization header
else RELATIVE_URI path
CP->>MS: GET RELATIVE_URI (no auth)
end
MS-->>CP: credential JSON
CP-->>App: Credentials
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Brownian Motion (Brass)Recommendation: Refactor Summary: PR adds support for EKS Pod Identity, but complexity may outweigh benefits. Highlights
Unknowns
Next actions
Reflection questions
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
aws-creds/src/credentials.rs (2)
344-350: Add a rustdoc example forfrom_container_credentials_provider.The updated public API docs explain precedence and auth behavior, but they still lack a concrete usage example.
As per coding guidelines, "Provide documentation examples for all public APIs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aws-creds/src/credentials.rs` around lines 344 - 350, Add a rustdoc example for the public function from_container_credentials_provider that shows how to construct and use the provider: demonstrate calling from_container_credentials_provider(), await-ing/getting credentials, and illustrate the precedence order (AWS_CONTAINER_CREDENTIALS_RELATIVE_URI checked before AWS_CONTAINER_CREDENTIALS_FULL_URI) and how an Authorization header is supplied from AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE or AWS_CONTAINER_AUTHORIZATION_TOKEN; mark the example with the #[cfg(feature = "http-credentials")] note in the docs and include a short snippet that sets the relevant env vars (or token file) and retrieves credentials so users can copy/paste to verify behavior.
196-201: Avoid silently masking token-file misconfiguration.If
AWS_CONTAINER_AUTHORIZATION_TOKEN_FILEis set but unreadable at Line 197, the code silently falls back. For explicit file configuration, returning an error is usually safer and easier to debug.Proposed refactor
- fn get_container_authorization_token() -> Option<String> { + fn get_container_authorization_token() -> Result<Option<String>, CredentialsError> { if let Ok(path) = env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE") { - if let Ok(token) = std::fs::read_to_string(path) { - return Some(token.trim_end().to_string()); - } + let token = std::fs::read_to_string(path)?; + return Ok(Some(token.trim_end().to_string())); } - env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN").ok() + Ok(env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN").ok()) }- let token = get_container_authorization_token(); + let token = get_container_authorization_token()?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aws-creds/src/credentials.rs` around lines 196 - 201, The code silently falls back to AWS_CONTAINER_AUTHORIZATION_TOKEN when AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE is set but std::fs::read_to_string fails; change the logic so that if AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE is present but unreadable you return/propagate an explicit error (rather than silently falling back) — detect env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE") being Ok and, on read_to_string error, return a Result::Err (or otherwise propagate a io::Error with context) referencing the failing std::fs::read_to_string call and the AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE variable instead of proceeding to env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN").ok().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@aws-creds/src/credentials.rs`:
- Around line 669-709: The three tests
(test_get_container_authorization_token_from_file,
test_get_container_authorization_token_from_env,
test_get_container_authorization_token_file_precedence) mutate process-global
env vars without synchronizing on CONTAINER_CREDENTIALS_TEST_LOCK; update each
test to acquire the same lock at the start (e.g. let _guard_lock =
CONTAINER_CREDENTIALS_TEST_LOCK.lock().unwrap();) so they use the shared mutex
while setting/removing env vars via EnvGuard and calling
get_container_authorization_token(), preventing races in parallel test runs.
- Around line 352-365: The code currently accepts any env-provided URL in the
url variable and may forward auth_token to it; parse and validate the URL before
making the request by calling Url::parse(&full_uri) (or equivalent) and ensure
the scheme is http or https and the host is in a trusted allowlist (reject
otherwise, e.g. return Err(CredentialsError::NotContainer)). Only call
get_container_authorization_token() and attach the Authorization header to the
request created by apply_timeout(attohttpc::get(&url)) if the parsed URL passes
validation; do not attach the token for untrusted or invalid URIs.
---
Nitpick comments:
In `@aws-creds/src/credentials.rs`:
- Around line 344-350: Add a rustdoc example for the public function
from_container_credentials_provider that shows how to construct and use the
provider: demonstrate calling from_container_credentials_provider(),
await-ing/getting credentials, and illustrate the precedence order
(AWS_CONTAINER_CREDENTIALS_RELATIVE_URI checked before
AWS_CONTAINER_CREDENTIALS_FULL_URI) and how an Authorization header is supplied
from AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE or
AWS_CONTAINER_AUTHORIZATION_TOKEN; mark the example with the #[cfg(feature =
"http-credentials")] note in the docs and include a short snippet that sets the
relevant env vars (or token file) and retrieves credentials so users can
copy/paste to verify behavior.
- Around line 196-201: The code silently falls back to
AWS_CONTAINER_AUTHORIZATION_TOKEN when AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE is
set but std::fs::read_to_string fails; change the logic so that if
AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE is present but unreadable you
return/propagate an explicit error (rather than silently falling back) — detect
env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE") being Ok and, on
read_to_string error, return a Result::Err (or otherwise propagate a io::Error
with context) referencing the failing std::fs::read_to_string call and the
AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE variable instead of proceeding to
env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN").ok().
Brownian Motion (Brass)Recommendation: Refactor Summary: PR adds support for EKS Pod Identity, but complexity may outweigh benefits. Highlights
Unknowns
Next actions
Reflection questions
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
aws-creds/src/credentials.rs (1)
344-350: Add a rustdoc example for this public API.The method-level docs explain behavior well, but this public API still lacks an example snippet. Adding a short
no_runexample for RELATIVE_URI/FULL_URI usage would satisfy the project doc standard.Suggested doc addition
/// Load credentials from container metadata (ECS task role or EKS Pod Identity). /// /// Checks `AWS_CONTAINER_CREDENTIALS_RELATIVE_URI` first (ECS), then /// `AWS_CONTAINER_CREDENTIALS_FULL_URI` (EKS Pod Identity Agent, etc.). /// When using `FULL_URI`, optionally sends an `Authorization` header from /// `AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE` or `AWS_CONTAINER_AUTHORIZATION_TOKEN`. + /// + /// # Example + /// + /// ```no_run + /// use awscreds::Credentials; + /// + /// // In ECS, set AWS_CONTAINER_CREDENTIALS_RELATIVE_URI. + /// // In EKS Pod Identity, set AWS_CONTAINER_CREDENTIALS_FULL_URI + /// // (optionally with AWS_CONTAINER_AUTHORIZATION_TOKEN[_FILE]). + /// let creds = Credentials::from_container_credentials_provider().unwrap(); + /// ```As per coding guidelines, "Provide documentation examples for all public APIs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aws-creds/src/credentials.rs` around lines 344 - 350, Add a rustdoc example (annotated with no_run) to the public method Credentials::from_container_credentials_provider() demonstrating typical usage for ECS (set AWS_CONTAINER_CREDENTIALS_RELATIVE_URI) and EKS Pod Identity (set AWS_CONTAINER_CREDENTIALS_FULL_URI and optional AWS_CONTAINER_AUTHORIZATION_TOKEN or AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE), returning a Credentials instance; place the example in the doc comment for the same function (keeping the existing #[cfg(feature = "http-credentials")] guard) and ensure it follows the project's doc style (short, shows environment variables and a call to Credentials::from_container_credentials_provider() and unwrap()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@aws-creds/src/credentials.rs`:
- Around line 344-350: Add a rustdoc example (annotated with no_run) to the
public method Credentials::from_container_credentials_provider() demonstrating
typical usage for ECS (set AWS_CONTAINER_CREDENTIALS_RELATIVE_URI) and EKS Pod
Identity (set AWS_CONTAINER_CREDENTIALS_FULL_URI and optional
AWS_CONTAINER_AUTHORIZATION_TOKEN or AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE),
returning a Credentials instance; place the example in the doc comment for the
same function (keeping the existing #[cfg(feature = "http-credentials")] guard)
and ensure it follows the project's doc style (short, shows environment
variables and a call to Credentials::from_container_credentials_provider() and
unwrap()).
Patches rust-s3 to use LockedThread/rust-s3 fork which adds support for AWS_CONTAINER_CREDENTIALS_FULL_URI (EKS Pod Identity Agent). Without this, the S3 client falls through to IMDS and picks up the node group role instead of the Pod Identity role. See: durch/rust-s3#449
|
Hey @durch, can you please review this? It's a complete blocker for anyone wanting to use the recommended approach to granting AWS resources to EKS Pods. I'm glad someone is getting use out of my fork so far, and I wasn't the only one running into this issue. --- @typedrat See the comparison table for more info. |
|
Thanks for this PR. The EKS Pod Identity use case is important, and I agree this is a gap in the current container credentials provider. Before I can merge it, I need a few safety and documentation issues addressed because this is credential-resolution code:
I’m interested in merging EKS Pod Identity support, but credential-provider changes need conservative failure behavior and clear token-handling semantics. |
Brownian Motion (Brass)Recommendation: Stop Summary: PR adds support for EKS Pod Identity but introduces unnecessary complexity. Highlights
Unknowns
Next actions
Reflection questions
|
Thanks for the reply. |
@durch This bot is ridiculous. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@aws-creds/src/credentials.rs`:
- Around line 202-208: In get_container_authorization_token(), ensure you fully
trim whitespace for both the file path and the token value: call .trim() on the
AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE string before using it to read the file
(so stray whitespace in the path is removed) and call .trim() on the contents
returned from std::fs::read_to_string to remove both leading and trailing
whitespace; likewise, when returning AWS_CONTAINER_AUTHORIZATION_TOKEN from
env::var, return its .trim()ed value (if present) instead of the raw string to
keep behavior consistent with the file-path branch.
- Around line 210-230: The
validate_container_credentials_full_uri_for_auth_token function currently only
allows specific EKS/ECS addresses; update its logic so that for HTTPS
(parsed.scheme() == "https") it accepts any host, and for HTTP (parsed.scheme()
== "http") it only allows loopback and known link-local/EKS addresses: accept
Host::Ipv4 where ip.octets() equals EKS_POD_IDENTITY_AGENT_IPV4 or equals
127.0.0.1, accept Host::Ipv6 where ip.segments() equals
EKS_POD_IDENTITY_AGENT_IPV6 or equals the ::1 segments, and accept Host::Domain
where the domain equals "localhost"; otherwise return
CredentialsError::InvalidContainerCredentialsFullUri(url.to_string()) as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b5a4d93-f4b5-4762-b4f1-a89dc9e3265c
📒 Files selected for processing (2)
aws-creds/src/credentials.rsaws-creds/src/error.rs
| fn get_container_authorization_token() -> Result<Option<String>, CredentialsError> { | ||
| if let Ok(path) = env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE") { | ||
| let token = std::fs::read_to_string(path)?; | ||
| return Ok(Some(token.trim_end().to_string())); | ||
| } | ||
| Ok(env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN").ok()) | ||
| } |
There was a problem hiding this comment.
trim_end() leaves potential leading whitespace in file-sourced tokens; the env-var path is untrimmed entirely.
token.trim_end().to_string() on line 205 strips trailing whitespace (the common trailing newline) but does not strip leading whitespace. AWS CLI's equivalent reads the file and does a full strip(). If a token file has leading whitespace due to a template or editor artefact, the value passed in the Authorization header will be silently corrupted.
Additionally, the env-var path at line 207 applies no trimming at all, while the file path applies partial trimming—an inconsistency that can lead to subtle header-value mismatches.
✏️ Proposed fix
- let token = std::fs::read_to_string(path)?;
- return Ok(Some(token.trim_end().to_string()));
+ let token = std::fs::read_to_string(path)?;
+ return Ok(Some(token.trim().to_string()));
...
- Ok(env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN").ok())
+ Ok(env::var("AWS_CONTAINER_AUTHORIZATION_TOKEN").ok().map(|t| t.trim().to_string()))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@aws-creds/src/credentials.rs` around lines 202 - 208, In
get_container_authorization_token(), ensure you fully trim whitespace for both
the file path and the token value: call .trim() on the
AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE string before using it to read the file
(so stray whitespace in the path is removed) and call .trim() on the contents
returned from std::fs::read_to_string to remove both leading and trailing
whitespace; likewise, when returning AWS_CONTAINER_AUTHORIZATION_TOKEN from
env::var, return its .trim()ed value (if present) instead of the raw string to
keep behavior consistent with the file-path branch.
| /// Returns whether it is safe to send a container authorization token to `url`. | ||
| #[cfg(feature = "http-credentials")] | ||
| fn validate_container_credentials_full_uri_for_auth_token( | ||
| url: &str, | ||
| ) -> Result<(), CredentialsError> { | ||
| let parsed = Url::parse(url)?; | ||
|
|
||
| if parsed.scheme() != "http" && parsed.scheme() != "https" { | ||
| return Err(CredentialsError::InvalidContainerCredentialsFullUri( | ||
| url.to_string(), | ||
| )); | ||
| } | ||
|
|
||
| match parsed.host() { | ||
| Some(Host::Ipv4(ip)) if ip.octets() == EKS_POD_IDENTITY_AGENT_IPV4 => Ok(()), | ||
| Some(Host::Ipv6(ip)) if ip.segments() == EKS_POD_IDENTITY_AGENT_IPV6 => Ok(()), | ||
| _ => Err(CredentialsError::InvalidContainerCredentialsFullUri( | ||
| url.to_string(), | ||
| )), | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
AWS container credential provider FULL_URI allowed hosts loopback HTTPS policy Java SDK Go SDK botocore
💡 Result:
The AWS Container Credential Provider uses the AWS_CONTAINER_CREDENTIALS_FULL_URI environment variable to specify a full HTTP/HTTPS endpoint for fetching credentials in container environments like ECS and EKS. Due to security policies across AWS SDKs, this URI is restricted: For HTTP URIs: Only loopback hosts (localhost, 127.0.0.1, ::1) and specific AWS service endpoints (e.g., ECS 169.254.170.2, EKS IPv6 like fd00:ec2::23) are allowed. Non-loopback HTTP URIs are rejected. For HTTPS URIs: All hosts are permitted. This policy is enforced consistently: - Java SDK v1 (1.x): Only loopback hosts for HTTP; all HTTPS allowed. Docs explicitly state: "For absolute paths, only loopback hosts are allowed when using HTTP... All HTTPS endpoints are allowed." {1,5} - Java SDK v2 (2.x): Validates host resolves to loopback addresses for HTTP (via InetAddress check); HTTPS unrestricted. Source code confirms: if not HTTPS and not allowed host (loopback rules), throws error. {20} - Go SDK (v1 and v2): Follows same standard; supports FULL_URI with loopback/known endpoints for HTTP. {14,15} - Botocore (Python/CLI): ContainerMetadataFetcher._ALLOWED_HOSTS limits to ['169.254.170.2', 'localhost', '127.0.0.1'] for FULL_URI; cannot extend due to org-wide security policy. {2,16} This is a deliberate cross-SDK security measure to prevent arbitrary remote credential fetches, as confirmed in GitHub issues. {2,4} No configuration to override allowed hosts; use relative URI (AWS_CONTAINER_CREDENTIALS_RELATIVE_URI) with default ECS host or ensure HTTPS/loopback compliance. IPv6 loopback supported in recent versions. {13,23}
Citations:
- 1: https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/auth/ContainerCredentialsProvider.html
- 2: Allow other hosts for AWS_CONTAINER_CREDENTIALS_FULL_URI boto/botocore#2480
- 3: https://docs.aws.amazon.com/sdkref/latest/guide/feature-container-credentials.html
- 4:
AWS_CONTAINER_CREDENTIALS_FULL_URIonly accepts localhost aws/aws-sdk-java#2898 - 5: https://docs.aws.amazon.com/sdk-for-java/v1/reference/com/amazonaws/auth/ContainerCredentialsProvider.html
- 6: Can't use General HTTP Provider: AWS_CONTAINER_CREDENTIALS_FULL_URI is not HTTPS and is not within loop back CIDR: [fd00:ec2::23] aws/aws-sdk-cpp#3666
- 7: Panic when using EcsContainer and AWS_CONTAINER_CREDENTIALS_FULL_URI aws/aws-sdk-go#4572
- 8: Read
AWS_CONTAINER_CREDENTIALS_FULL_URIenv variable if set when reading a profile withcredential_source. aws/aws-sdk-go-v2#2790 - 9: Add host.docker.internal to ContainerMetadataFetcher list of approved hosts boto/botocore#2515
- 10: https://github.com/aws/aws-sdk-java-v2/blob/master/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ContainerCredentialsProvider.java
- 11: Add EKS container host in ContainerCredentialsProvider , also validate the resolved endpoint for RelativeURI aws/aws-sdk-java-v2#4695
validate_container_credentials_full_uri_for_auth_token allowlist is narrower than AWS SDK standard.
The current implementation only permits 169.254.170.23 (IPv4) and fd00:ec2::23 (IPv6), rejecting https://any-host, http://localhost, and http://127.0.0.1 when a token is present. This deviates from AWS SDK behavior across Java, Go, and Python:
- Java SDK v1/v2: "For absolute paths, only loopback hosts are allowed when using HTTP... All HTTPS endpoints are allowed."
- Go SDK: Supports loopback and known endpoints for HTTP; all hosts for HTTPS
- Botocore: Allows
['169.254.170.2', 'localhost', '127.0.0.1']for FULL_URI
This breaks:
- Custom HTTPS credential providers (IoT Greengrass, custom auth servers)
- Local testing with auth tokens via
localhost
Correct policy: HTTPS → allow any host (transport encrypted); HTTP → restrict to loopback (127.0.0.1, ::1, localhost) and known EKS/ECS link-local IPs.
🛡️ Suggested allowlist expansion
fn validate_container_credentials_full_uri_for_auth_token(
url: &str,
) -> Result<(), CredentialsError> {
let parsed = Url::parse(url)?;
if parsed.scheme() != "http" && parsed.scheme() != "https" {
return Err(CredentialsError::InvalidContainerCredentialsFullUri(
url.to_string(),
));
}
+ // All HTTPS endpoints are trusted (transport security covers the token).
+ if parsed.scheme() == "https" {
+ return Ok(());
+ }
+
+ // For HTTP, restrict to loopback and the known EKS/ECS link-local IPs.
match parsed.host() {
Some(Host::Ipv4(ip)) if ip.octets() == EKS_POD_IDENTITY_AGENT_IPV4 => Ok(()),
Some(Host::Ipv6(ip)) if ip.segments() == EKS_POD_IDENTITY_AGENT_IPV6 => Ok(()),
+ Some(Host::Ipv4(ip)) if ip.is_loopback() => Ok(()),
+ Some(Host::Ipv6(ip)) if ip.is_loopback() => Ok(()),
+ Some(Host::Domain(d)) if d == "localhost" => Ok(()),
_ => Err(CredentialsError::InvalidContainerCredentialsFullUri(
url.to_string(),
)),
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@aws-creds/src/credentials.rs` around lines 210 - 230, The
validate_container_credentials_full_uri_for_auth_token function currently only
allows specific EKS/ECS addresses; update its logic so that for HTTPS
(parsed.scheme() == "https") it accepts any host, and for HTTP (parsed.scheme()
== "http") it only allows loopback and known link-local/EKS addresses: accept
Host::Ipv4 where ip.octets() equals EKS_POD_IDENTITY_AGENT_IPV4 or equals
127.0.0.1, accept Host::Ipv6 where ip.segments() equals
EKS_POD_IDENTITY_AGENT_IPV6 or equals the ::1 segments, and accept Host::Domain
where the domain equals "localhost"; otherwise return
CredentialsError::InvalidContainerCredentialsFullUri(url.to_string()) as before.
There was a problem hiding this comment.
I wholeheartedly disagree. The PRs you reference are people asking for support to point to other URIs for the credentials address, for which in all 3 PRs AWS employees say the same answer - that's it's against an org-wide (AWS-wide) security policy to allow that.
There was a problem hiding this comment.
@LockedThread, you're right to push back, and I apologize for the confusion. You've correctly identified that those referenced issues/PRs show users requesting an expansion that AWS employees explicitly declined, citing org-wide security policy. I misread the direction of those conversations.
Looking at the actual AWS Java SDK documentation more carefully, the policy is: "For absolute paths, only loopback hosts are allowed when using HTTP, including known endpoints for ECS and EKS. All HTTPS endpoints are allowed." The Java SDK v2 enforces the same rule, rejecting endpoints where the "host should resolve to a loopback address or have the full URI be HTTPS."
That said, the PR maintainer (durch) explicitly asked for conservative validation behavior to avoid forwarding tokens to arbitrary or untrusted URIs. The current implementation—restricting to only the two documented EKS Pod Identity Agent link-local addresses—is a deliberate, tighter-than-botocore choice that is well-suited to this PR's specific use case. Since Amazon EKS sets AWS_CONTAINER_CREDENTIALS_FULL_URI and AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE environment variables pointing to those exact addresses, the current allowlist correctly covers the intended EKS Pod Identity scenario without over-broadening the attack surface.
I withdraw my suggestion to expand the allowlist. The current implementation is a valid and defensible conservative choice.
✏️ Learnings added
Learnt from: LockedThread
Repo: durch/rust-s3 PR: 449
File: aws-creds/src/credentials.rs:210-230
Timestamp: 2026-05-04T20:38:34.950Z
Learning: In `aws-creds/src/credentials.rs` (rust-s3 repo), the `validate_container_credentials_full_uri_for_auth_token` function intentionally restricts authorization token forwarding to only the two documented EKS Pod Identity Agent link-local addresses: `169.254.170.23` (IPv4) and `fd00:ec2::23` (IPv6). This is a deliberately conservative policy requested by the maintainer (`durch`) to avoid forwarding tokens to arbitrary/untrusted URIs. Do not suggest expanding this allowlist to include loopback addresses or all HTTPS endpoints.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Brownian Motion (Brass)Recommendation: Refactor Summary: PR adds support for EKS Pod Identity but introduces unnecessary complexity. Highlights
Unknowns
Next actions
Reflection questions
|
2 participants
Summary
Extends the container credentials provider to support
AWS_CONTAINER_CREDENTIALS_FULL_URIand the optional authorization token environment variables (AWS_CONTAINER_AUTHORIZATION_TOKEN,AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE). This allows applications to obtain credentials when running with AWS EKS Pod Identity Agent or other credential endpoints that use a full URI and bearer token (e.g. AWS Greengrass Token Exchange, custom sidecars).Why this is important
http://169.254.170.23/v1/credentials). The agent expects the client to set:AWS_CONTAINER_CREDENTIALS_FULL_URIto that URLAWS_CONTAINER_AUTHORIZATION_TOKEN_FILE(orAWS_CONTAINER_AUTHORIZATION_TOKEN) with the pod identity tokenFULL_URIsupport,Credentials::default()(and the rest of the chain) never considers the container provider when only these env vars are set, so pods using EKS Pod Identity getNoCredentialsand cannot call S3 or other AWS services with this crate.Prior work
This builds on the container credentials support added in Priorize ECS/EKS credentials over EC2's when available (PR #441). That PR introduced
from_container_credentials_provider()and prioritized it over EC2 instance metadata, but only forAWS_CONTAINER_CREDENTIALS_RELATIVE_URI(used by ECS task IAM roles). This change adds theFULL_URIpath and authorization token handling so both ECS-style and EKS Pod Identity–style container credentials are supported by the same provider.Changes
aws-creds/src/credentials.rsfrom_container_credentials_provider()now:AWS_CONTAINER_CREDENTIALS_RELATIVE_URIif set (unchanged; buildshttp://169.254.170.2{path}).AWS_CONTAINER_CREDENTIALS_FULL_URIif set.FULL_URI, the request optionally includes anAuthorizationheader:AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE(file path) if set.AWS_CONTAINER_AUTHORIZATION_TOKEN.get_container_authorization_token()implements the token lookup (file precedence over env).#[cfg(feature = "http-credentials")]):test_container_credentials_not_container_when_no_env– neither URI set →NotContainer.test_container_credentials_relative_uri_precedence– both set → RELATIVE_URI is used.test_container_credentials_full_uri_used_when_no_relative– only FULL_URI set → that URL is used.test_get_container_authorization_token_from_file– token read from file.test_get_container_authorization_token_from_env– token from env var.test_get_container_authorization_token_file_precedence– file overrides env.Environment variables (after this PR)
AWS_CONTAINER_CREDENTIALS_RELATIVE_URI169.254.170.2(ECS task role).AWS_CONTAINER_CREDENTIALS_FULL_URIAWS_CONTAINER_AUTHORIZATION_TOKEN_FILEAuthorizationheader.AWS_CONTAINER_AUTHORIZATION_TOKENAuthorizationheader.Behavior matches the AWS SDK / container credential provider (RELATIVE_URI precedence, FULL_URI, optional auth token).
Testing
cargo test -p aws-creds --features http-credentials– all new and existing tests pass.This change is
Summary by CodeRabbit
New Features
Improvements
Tests