aws / aws/aws-sam-cli

SamConfig.get_all() mutates shared document, leaking one command's config values into another's

Open Beginner friendly
#9,181 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.