Repository navigation
fix(security): distinguish unset vs empty cloud-accounts contextvar (… - #53
Merged
Merged
Conversation
…fail-closed)
The product had a cross-tenant credential leak: a registered org that hadn't
connected an AWS account in Settings -> Cloud Accounts could still investigate
AWS because the agent silently fell back to the platform's own AWS credentials.
Root cause in the OSS resolver: current_cloud_accounts() collapsed None and {}
into the same thing (`return _current_accounts.get() or {}`), so the resolver
couldn't distinguish "OSS self-host, contextvar never set -> use ambient creds"
(correct) from "product tenant, contextvar explicitly set to {} -> fail closed"
(also needed). With this collapsed semantics, even if the product DID always
call set_current_cloud_accounts([]), the resolver still returned _base_session().
Tri-state semantics on the per-task contextvar:
- None -> OSS / self-host; resolve_session falls back to ambient host creds.
- {} -> product tenant with no Cloud Account connected; resolve_session
raises RuntimeError instead of leaking platform creds.
- {p:a} -> per-provider resolution (existing behavior; unchanged).
Changes:
- providers/aws/credentials.py:
* current_cloud_accounts() returns Optional[dict] directly (no collapsing).
* account_for_provider / get_current_cloud_account tolerate None.
* resolve_session() branches on None vs {} vs has-aws-key.
- tools/bash_tool.py: same distinction at the credential injection block.
None -> ambient (run_env=None); {} -> block any cloud binary; else -> existing.
- tests/test_tools/test_credentials.py: three new tests pinning the behavior,
plus a small fix to test_bash_uses_both_clouds_when_both_connected so it
matches on basename (shutil.which resolves tokens[0] to an absolute path).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…fail-closed)
The product had a cross-tenant credential leak: a registered org that hadn't connected an AWS account in Settings -> Cloud Accounts could still investigate AWS because the agent silently fell back to the platform's own AWS credentials.
Root cause in the OSS resolver: current_cloud_accounts() collapsed None and {} into the same thing (
return _current_accounts.get() or {}), so the resolver couldn't distinguish "OSS self-host, contextvar never set -> use ambient creds" (correct) from "product tenant, contextvar explicitly set to {} -> fail closed" (also needed). With this collapsed semantics, even if the product DID always call set_current_cloud_accounts([]), the resolver still returned _base_session().Tri-state semantics on the per-task contextvar:
raises RuntimeError instead of leaking platform creds.
Changes: