feat(credentials): use the AWS default credential chain when no static keys are configured - #3486
Conversation
AI Session Checks —
|
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | secrets-detection |
- |
✅ sast-scan
| Status | Policy | Messages |
|---|---|---|
| ✅ Passed | owasp-top10-2025 |
- |
| ✅ Passed | sast |
- |
| ✅ Passed | cwe-top25 |
- |
| ✅ Passed | cwe-top26-40-cusp |
- |
⚠️ iac-scan — 1 failing
| Status | Policy | Messages |
|---|---|---|
iac-misconfiguration |
Base64 High Entropy String in "deployment/chainloop/values.yaml" (error) |
security-context — 1 file, 1 past fix
These files have a recorded security-fix history. They are pointers to what past fixes established, not findings in this diff, and they never fail the check.
pkg/credentials/aws/secretmanager.go — 1 past fix, peak medium
186888fFixes an access-control flaw where AWS S3 and Secrets Manager clients could authenticate with ambient environment/default-chain credentials instead of the explicit static keys Chainloop was configured to use. (medium, CWE-284)
When explicit AWS access keys are configured for a backend or credentials manager, those constructors must authenticate only with those static credentials; ambient environment/default-chain credentials must not influence AWS client identity.
↳ Check: When explicit AWS access keys are configured for a backend or credentials manager, those constructors must authenticate only with those static credentials; ambient environment/default-chain credentials must not influence AWS client identity. The same invariant holds at 1 other entry point. Past fixes here removed the dangerous construct rather than guarding it, so a surviving use of config.LoadDefaultConfig( is what to look for.
View security context ↗ · Security context documentation ↗
⏭️ 2 scans not applied
| Scan | Reason |
|---|---|
vulnerability-scan |
no manifest/lockfile changed |
github-actions-scan |
no workflow files changed |
PR validation — ⚠️ 1 failing
| Status | Policy | Material | Messages |
|---|---|---|---|
pr-min-approvals |
pr-info |
PR/MR #3486 has 0 approving reviews, 1 required. | |
| ✅ Passed | pr-description-required |
pr-info |
- |
| ✅ Passed | pr-user-story-linked |
pr-info |
- |
Powered by Chainloop and Chainloop Trace
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/credentials/api/credentials/v1/config.proto">
<violation number="1" location="pkg/credentials/api/credentials/v1/config.proto:41">
P2: Removing `required` from `creds` changes the accepted config shape for every consumer of this descriptor. `config.pb.go` shares this proto between control-plane and CAS, so a chart/config that omits creds (the new keyless path) will cause any component still running the pre-PR binary to fail boot validation with a `creds is required` error during a staggered rollout. Confirm control-plane, CAS, and the chart ship together, or note the version-skew constraint in the release notes/chart.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| Creds creds = 1 [(buf.validate.field).required = true]; | ||
| // Optional. When omitted, credentials are resolved through the AWS SDK's default | ||
| // credential chain (EKS Pod Identity, IRSA, instance profile, environment). | ||
| Creds creds = 1; |
There was a problem hiding this comment.
P2: Removing required from creds changes the accepted config shape for every consumer of this descriptor. config.pb.go shares this proto between control-plane and CAS, so a chart/config that omits creds (the new keyless path) will cause any component still running the pre-PR binary to fail boot validation with a creds is required error during a staggered rollout. Confirm control-plane, CAS, and the chart ship together, or note the version-skew constraint in the release notes/chart.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/credentials/api/credentials/v1/config.proto, line 41:
<comment>Removing `required` from `creds` changes the accepted config shape for every consumer of this descriptor. `config.pb.go` shares this proto between control-plane and CAS, so a chart/config that omits creds (the new keyless path) will cause any component still running the pre-PR binary to fail boot validation with a `creds is required` error during a staggered rollout. Confirm control-plane, CAS, and the chart ship together, or note the version-skew constraint in the release notes/chart.</comment>
<file context>
@@ -36,7 +36,9 @@ message Credentials {
- Creds creds = 1 [(buf.validate.field).required = true];
+ // Optional. When omitted, credentials are resolved through the AWS SDK's default
+ // credential chain (EKS Pod Identity, IRSA, instance profile, environment).
+ Creds creds = 1;
string region = 2 [(buf.validate.field).string.min_len = 1];
</file context>
…c keys are configured The AWS Secrets Manager backend required a static access key and secret and deliberately bypassed the SDK's default credential chain. That rules out the keyless options EKS offers (Pod Identity, IRSA), which is what most operators running on EKS want: no long-lived key stored anywhere. Static keys become optional, both or neither. When given, they are used exactly as before and ambient credentials are never consulted. When omitted, the region is loaded through config.LoadDefaultConfig. The chart renders the creds block only when keys are set. Signed-off-by: Khris Richardson <khris.richardson@gmail.com>
48f20ae to
a2c3193
Compare
…S config and IMDS, and name every keyless mode in the chart docs Review follow-ups on the AWS default credential chain change: - The keyless test cases load the default credential chain, which would read the developer's ~/.aws files and AWS_PROFILE and could reach for IMDS. They now run with empty config and credentials files, no profile, and AWS_EC2_METADATA_DISABLED=true, following pkg/blobmanager/s3accesspoint's tests. - The chart docs name every mode the default chain covers: IRSA, EKS Pod Identity and instance roles. Signed-off-by: Khris Richardson <khris.richardson@gmail.com>
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="deployment/chainloop/values.yaml">
<violation number="1" location="deployment/chainloop/values.yaml:73">
P2: This Helm chart change leaves `Chart.yaml` at version `1.447.0`, so the modified chart is packaged under the old version. Bump the chart patch version before merging.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| ## @extra secretsBackend.awsSecretManager.accessKey AWS Access KEY ID | ||
| ## @extra secretsBackend.awsSecretManager.secretKey AWS Secret Key | ||
| ## @extra secretsBackend.awsSecretManager.accessKey AWS Access KEY ID. Omit both for the default chain (IRSA, Pod Identity, instance role) |
There was a problem hiding this comment.
P2: This Helm chart change leaves Chart.yaml at version 1.447.0, so the modified chart is packaged under the old version. Bump the chart patch version before merging.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At deployment/chainloop/values.yaml, line 73:
<comment>This Helm chart change leaves `Chart.yaml` at version `1.447.0`, so the modified chart is packaged under the old version. Bump the chart patch version before merging.</comment>
<file context>
@@ -70,8 +70,8 @@ secretsBackend:
- ## @extra secretsBackend.awsSecretManager.accessKey AWS Access KEY ID. Omit both keys to use the default credential chain (IRSA)
- ## @extra secretsBackend.awsSecretManager.secretKey AWS Secret Key. Omit both keys to use the default credential chain
+ ## @extra secretsBackend.awsSecretManager.accessKey AWS Access KEY ID. Omit both for the default chain (IRSA, Pod Identity, instance role)
+ ## @extra secretsBackend.awsSecretManager.secretKey AWS Secret Key. Omit both for the default chain (IRSA, Pod Identity, instance role)
## @extra secretsBackend.awsSecretManager.region AWS Secrets Manager Region
</file context>
1 participant
Part of #3488.
The AWS Secrets Manager backend required a static access key and secret, and deliberately bypassed the SDK's default
credential chain. That rules out the keyless options EKS offers (Pod Identity, IRSA), which is what most operators on
EKS want: no long-lived key stored anywhere.
Static keys become optional, both or neither:
This keeps the guarantee fix(aws): do not load creds from env vars #2509 established.
TestLoadConfigasserts that explicit keys win over credentials presentin the environment.
config.LoadDefaultConfig, which resolves Pod Identity, IRSA's webidentity token, or an instance role.
Why the fallback is on omission rather than behind a separate flag: #2509's concern was ambient credentials overriding keys an operator had configured. That cannot happen here. The default chain is reached only when no key is configured at all, and today that configuration fails validation and never starts, so no working deployment changes behavior.
The chart renders the
credsblock only when keys are set.Upgrade note: config without
credsis rejected by an older binary (credswas required). The chart ships thecontrol plane and CAS together, so a chart upgrade moves both at once. A deployment that omits
credsmust runcontrol-plane and CAS images from this release or later.