SamConfig.get_all() mutates shared document, leaking one command's config values into another's
- Dominant language
- Python
- Stars
- 6.7k
- Forks
- 1.2k
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 52
Description
### Description
`SamConfig.get_all()` in `samcli/lib/config/samconfig.py` mutates the in-memory `self.document` in place when merging `[default.global.parameters]` with a command-specific section. Because `self.document` is cached (only re-read from disk if empty — see `_read()`), repeated calls to `get_all()` on the same `SamConfig` instance corrupt the shared `global` section with a previous command's specific parameter values.
### Root cause
```python
# samcli/lib/config/samconfig.py:89-95
config_content = self.document.get(env, {})
params = config_content.get(self.to_key(cmd_names), {}).get(section, {})
if DEFAULT_GLOBAL_CMDNAME in config_content:
global_params = config_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {})
global_params.update(params.copy()) # <-- mutates the live dict inside self.document
params = global_params.copy()
return params
```
`global_params` is a live reference into `self.document["default"]["global"]["parameters"]`, not a copy. `.update()` permanently merges the current command's parameters into it. The `.copy()` calls only protect the *returned* dict — the corruption of `self.document` persists for the lifetime of the object.
### Steps to reproduce
`samconfig.toml`:
```toml
version = 0.1
[default.global.parameters]
stack_name = "global-stack"
[default.deploy.parameters]
stack_name = "deploy-only-stack"
region = "us-east-1"
```
```python
from samcli.lib.config.samconfig import SamConfig
sc = SamConfig("")
print(sc.get_all(['deploy'], 'parameters'))
# {'stack_name': 'deploy-only-stack', 'region': 'us-east-1'} -- correct
print(sc.get_all(['build'], 'parameters'))
# {'stack_name': 'deploy-only-stack', 'region': 'us-east-1'} -- WRONG
```
`build` has no `[default.build.parameters]` section, so it should resolve to just `{'stack_name': 'global-stack'}` (inherited from `[default.global.parameters]` only). Instead it silently inherits `deploy`'s `stack_name` and even picks up `region`, which was never defined globally at all.
I verified this directly against `samcli.lib.config.samconfig.SamConfig` — not simulated.
### Why this doesn't usually surface via the plain CLI
`cli_config_file.ConfigProvider.__call__` constructs a fresh `SamConfig` per process invocation and calls `get_all()` exactly once, so a single `sam ` run normally discards the corrupted object before it matters. The bug becomes externally observable whenever a `SamConfig` instance is reused across more than one `get_all()` call for different `cmd_names` — a valid, documented use of the public API (it takes `cmd_names` per call for exactly this purpose), e.g. by embedders/toolkits (AWS Toolkit for VS Code/Cloud9) or any code path that resolves config for multiple commands against one loaded samconfig.
### Suggested fix
```python
global_params = dict(config_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {}))
global_params.update(params)
params = global_params
```
### Expected result
`get_all()` calls for different commands against the same `SamConfig` instance should not affect each other's resolved parameters.
### Actual result
A command-specific parameter value from one `get_all()` call leaks into the resolved parameters of a subsequent `get_all()` call for a different command, if that command lacks its own value for the same key.
### Environment
- `aws-sam-cli` version: 1.165.0 (current `develop` branch, commit at time of testing)
- OS: macOS
Contributor guide
Research direction
Start in samcli/lib/config/samconfig.py, reading get_all() and _read() to understand how the cached document is used. Reproduce the two sequential get_all() calls from the issue, then ensure each command resolves only its own parameters plus the unchanged global values; verify the shared document is not modified between calls.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- cli
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100