Skip to content

fix: SamConfig.get_all() mutates shared document, leaking one command's params into another's - #9183

Open
Adityaj0 wants to merge 1 commit into
aws:developfrom
Adityaj0:fix/samconfig-get-all-global-mutation
Open

fix: SamConfig.get_all() mutates shared document, leaking one command's params into another's#9183
Adityaj0 wants to merge 1 commit into
aws:developfrom
Adityaj0:fix/samconfig-get-all-global-mutation

Conversation

@Adityaj0

Copy link
Copy Markdown

Which issue(s) does this change fix?

Fixes #9181

Why is this change necessary?

SamConfig.get_all() merged the current command's section-specific parameters into the [global] section using a live reference into self.document rather than a copy:

global_params = config_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {})
global_params.update(params.copy())
params = global_params.copy()

global_params here is the actual dict stored inside self.document["<env>"]["global"][section]. Calling .update() on it permanently writes the current command's values into the shared global section. Since self.document is cached across calls (_read() only re-reads from disk when self.document is falsy), this corruption persists in memory for the lifetime of the SamConfig instance.

The result: if a SamConfig object is reused across multiple get_all() calls for different commands (a valid, documented use of the public API — get_all() takes cmd_names per call for exactly this purpose), a later command silently inherits an earlier command's parameter values instead of the true global default.

How does this change work?

Copy the global-section dict before merging into it, instead of mutating the dict stored in self.document:

global_params = dict(config_content.get(DEFAULT_GLOBAL_CMDNAME, {}).get(section, {}))
global_params.update(params)
params = global_params

What tests ran and what were the results?

Added a regression test, test_get_all_does_not_leak_command_params_into_global_across_calls, in tests/unit/lib/samconfig/test_samconfig.py. It fails against the pre-fix code (reproducing the exact leak: build's resolved config wrongly includes deploy's stack_name/region) and passes after the fix.

Full unit suite (tests/unit) passes: 9387 passed, 25 skipped.
mypy on the changed file: clean.

Checklist

  • Add/update tests for this change
  • make pr passes locally (unit tests + mypy on changed files; full local make pr target run via pytest tests/unit -q and scoped mypy)
  • Write a clear PR title and description

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…'s params into another's

global_params obtained in get_all() was a live reference into self.document
rather than a copy. Calling .update() on it permanently merged the current
command's section-specific values into the shared global section, and since
self.document is cached across calls (_read() only re-reads when empty),
a subsequent get_all() for a different command would silently inherit the
previous command's parameter values instead of the true global default.

Fixes aws#9181
@Adityaj0
Adityaj0 requested a review from a team as a code owner August 15, 2026 00:33
@github-actions github-actions Bot added pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant