Repository navigation
fix(secret): inherit the component's default identity for stores - #2746
Conversation
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe secret CLI now passes its computed default identity to store-backed authentication resolution. A regression test verifies identity inheritance. Documentation records the fallback behavior, compatibility rules, and affected commands. ChangesSecret identity inheritance
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: 🔵 Low · up to This PR changes secret-store identity inheritance so identity-less stores use the component’s effective identity instead of the default credential chain. The implementation is localized, but the accompanying fix document still uses imprecise wording about explicit-identity precedence and mock-store behavior, so the PR is mergeable with owner follow-up to correct the documentation. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…ntity The `atmos secret` CLI (set/get/init/validate) resolved a store-backed secret through injectSecretStoreAuthResolver, which called SetAuthContextResolver(resolver) and passed an empty identity to every store. An identity-less store therefore kept no identity and fell back to the AWS default credential chain -> EC2 IMDS, failing off-EC2. Call SetAuthContextResolverWithDefaultIdentity(resolver, defaultIdentity), reusing the defaultIdentity the function already computes for SecretsAuth. This matches the terraform paths (cmd/terraform/utils.go, internal/exec/terraform_execute_helpers.go) and the documented rule that a store-backed secret with no explicit identity inherits the component's effective identity. Stores with an explicit identity, and an explicit --identity, are unaffected (defaultIdentityForStore only fills empty-identity stores). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fa0ee3c to
41c35e8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Erik Osterman (Cloud Posse) (osterman)
added
the
patch
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2746 +/- ##
==========================================
- Coverage 82.98% 82.97% -0.01%
==========================================
Files 1881 1881
Lines 183067 183068 +1
==========================================
- Hits 151925 151908 -17
- Misses 23317 23332 +15
- Partials 7825 7828 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Head branch was pushed to by a user without write access
2eb9787
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/fixes/2026-07-13-secret-cli-inherit-default-identity.md`:
- Line 19: Update the documentation describing injectSecretStoreAuthResolver and
SetAuthContextResolver to state that no default identity is provided to
identity-less stores, rather than saying an empty identity is passed to every
store. Revise the compatibility guidance to say explicit identities take
precedence regardless of store implementation or test double, and remove the
mock/concrete-type explanation.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b457d79c-51bb-4c75-8eed-0e6fe8d97f53
📒 Files selected for processing (3)
cmd/secret/shared.gocmd/secret/shared_loaders_test.godocs/fixes/2026-07-13-secret-cli-inherit-default-identity.md
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/secret/shared_loaders_test.go
- cmd/secret/shared.go
…rit-default-identity
… split The store split (NewSSMStore/SSMStoreOptions moved from pkg/store to pkg/store/providers on main) broke this test after merging main into the branch. Point it at the new providers package. Test-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2eb9787 to
3475437
Compare
Address CodeRabbit review: SetAuthContextResolver supplies no default identity to identity-less stores (it does not overwrite explicit ones), and the _ResolverOnly test asserts the store receives an empty identity name - the prior 'receives its own identity' wording was inaccurate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rit-default-identity
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.226.0-rc.7. |
2 participants
what
atmos secret(set/get/init/validate) now inherits the component's effective identity for store-backed secrets whose store declares no explicitidentity:, instead of falling back to the default AWS credential chain (→ EC2 IMDS, which fails off-EC2).why
injectSecretStoreAuthResolver(cmd/secret/shared.go) calledatmosConfig.Stores.SetAuthContextResolver(resolver), which passes an empty identity to every store, so an identity-less store fell back to the AWS default chain → EC2 IMDS and failed off-EC2 (e.g.no EC2 IMDS role found ... dial tcp 169.254.169.254:80: connect: host is down).cmd/terraform/utils.go,internal/exec/terraform_execute_helpers.go) already callSetAuthContextResolverWithDefaultIdentity; the secret CLI even computed the sameDefaultIdentity(intoSecretsAuth) but never applied it to the stores.website/docs/cli/configuration/secrets.mdx: "When omitted and the secret is resolved within a component scope, the component's effective identity is inherited."identity, and an explicit--identity, are unaffected (defaultIdentityForStoreonly fills empty-identity stores).atmos terraformandatmos secret listbehavior is unchanged.Suggested label:
patch(user-visible bug fix, no new surface; no blog/roadmap required).references
docs/fixes/2026-07-13-secret-cli-inherit-default-identity.md