From 116210d855c0bbcc751db75281ce563d23ae0d54 Mon Sep 17 00:00:00 2001 From: Joe Wakefield Date: Tue, 27 Jan 2026 11:15:12 -0500 Subject: [PATCH 1/6] feat: Add relative path resolution for nested parameter override file includes --- samcli/cli/types.py | 11 +++++++++-- tests/unit/cli/test_types.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/samcli/cli/types.py b/samcli/cli/types.py index 261c3ee70e..aeec3ba913 100644 --- a/samcli/cli/types.py +++ b/samcli/cli/types.py @@ -124,7 +124,7 @@ class CfnParameterOverridesType(click.ParamType): name = "list,object,string" - def convert(self, values, param, ctx, seen_files=None): + def convert(self, values, param, ctx, seen_files=None, parent_dir=None): """ Takes parameter overrides loaded from various supported config file formats and flattens and normalizes them into a dictionary where all keys and values are strings. @@ -143,6 +143,8 @@ def convert(self, values, param, ctx, seen_files=None): Click context for error reporting. seen_files : set List of files processed in the current execution branch, used to detect infinite recursion + parent_dir : Path + Directory of the parent file, used to resolve relative paths in nested includes Returns ------- @@ -169,6 +171,11 @@ def convert(self, values, param, ctx, seen_files=None): # If the string is a file reference (e.g., 'file://params.yaml') if value.startswith("file://"): file_path = Path(value[7:]) + # Resolve relative paths against parent directory + if not file_path.is_absolute() and parent_dir: + file_path = parent_dir / file_path + file_path = file_path.resolve() + if not file_path.is_file(): self.fail(f"{value} was not found or is a directory", param, ctx) file_manager = FILE_MANAGER_MAPPER.get(file_path.suffix, None) @@ -181,7 +188,7 @@ def convert(self, values, param, ctx, seen_files=None): seen_files.add(file_path) try: nested_values = file_manager.read(file_path) - parameters.update(self.convert(nested_values, param, ctx, seen_files)) + parameters.update(self.convert(nested_values, param, ctx, seen_files, file_path.parent)) finally: seen_files.remove(file_path) else: diff --git a/tests/unit/cli/test_types.py b/tests/unit/cli/test_types.py index b65ab3348a..59f7087076 100644 --- a/tests/unit/cli/test_types.py +++ b/tests/unit/cli/test_types.py @@ -217,6 +217,38 @@ def mock_is_file(file_path): self.assertIn("Infinite recursion detected in file references", str(exception.exception)) + @parameterized.expand( + [ + ( + {"config/default.yaml": "Base: default\nEnv: dev", "config/prod.yaml": "- file://default.yaml\n- Env: production\n- Region: us-east-1"}, + "config/prod.yaml", + {"Base": "default", "Env": "production", "Region": "us-east-1"}, + ), + ( + {"config/shared/common.yaml": "Common: shared", "config/prod.yaml": "- file://shared/common.yaml\n- Env: prod"}, + "config/prod.yaml", + {"Common": "shared", "Env": "prod"}, + ), + ] + ) + def test_relative_path_nested_includes(self, file_contents, entry_file, expected): + """Test that nested files can reference other files using relative paths""" + from pathlib import Path + import tempfile + + with tempfile.TemporaryDirectory() as temp: + temp_path = Path(temp) + + # Create all files + for file_path, content in file_contents.items(): + full_path = temp_path / file_path + full_path.parent.mkdir(parents=True, exist_ok=True) + full_path.write_text(content) + + entry_path = temp_path / entry_file + result = self.param_type.convert(f"file://{entry_path}", None, MagicMock()) + self.assertEqual(result, expected) + class TestCfnMetadataType(TestCase): def setUp(self): From e81a3779a70fe64f709216aaa7c621e23f1b7070 Mon Sep 17 00:00:00 2001 From: Joe Wakefield Date: Tue, 27 Jan 2026 11:48:29 -0500 Subject: [PATCH 2/6] feat: Add $include key for dict-based parameter file includes --- samcli/cli/types.py | 9 +++++++++ tests/unit/cli/test_types.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/samcli/cli/types.py b/samcli/cli/types.py index aeec3ba913..19359da753 100644 --- a/samcli/cli/types.py +++ b/samcli/cli/types.py @@ -212,6 +212,15 @@ def convert(self, values, param, ctx, seen_files=None, parent_dir=None): if v is None: # Unset value if previously set parameters[str(k)] = "" + elif k == "$include": + # Process includes (can be string or list) + if not isinstance(v, (str, list, CommentedSeq)): + self.fail( + f"$include must be a string or list of strings, got {type(v).__name__}", + param, + ctx, + ) + parameters.update(self.convert(v, param, ctx, seen_files, parent_dir)) elif isinstance(v, (list, CommentedSeq)): # Join list elements into comma-separated string, strip whitespace and ignore empty entries parameters[str(k)] = ",".join(str(x).strip() for x in v if x not in (None, "")) diff --git a/tests/unit/cli/test_types.py b/tests/unit/cli/test_types.py index 59f7087076..0050f4bf18 100644 --- a/tests/unit/cli/test_types.py +++ b/tests/unit/cli/test_types.py @@ -174,6 +174,18 @@ def test_successful_parsing(self, input, expected): ("file://nested.yaml",), {"A": "yaml", "B": "yaml", "Toml": "toml", "Yaml": "yaml"}, ), + ( + ("file://nested.toml",), + {"A": "toml", "B": "toml", "Toml": "toml", "Env": "production"}, + ), + ( + ("file://nested-list.toml",), + {"A": "yaml", "B": "yaml", "Toml": "toml", "Yaml": "yaml", "Env": "prod"}, + ), + ( + ("file://nested-list.yaml",), + {"A": "yaml", "B": "yaml", "Toml": "toml", "Yaml": "yaml", "Env": "prod"}, + ), ] ) def test_merge_file_parsing(self, inputs, expected): @@ -182,6 +194,9 @@ def test_merge_file_parsing(self, inputs, expected): "params.yaml": "A: yaml\nB: yaml\nYaml: yaml", "list.yaml": "- - - - - - A: a\n- List:\n - 1\n - 2\n - 3\n", "nested.yaml": "- file://params.toml\n- file://params.yaml", + "nested.toml": "'$include' = 'file://params.toml'\nEnv = 'production'", + "nested-list.toml": "'$include' = ['file://params.toml', 'file://params.yaml']\nEnv = 'prod'", + "nested-list.yaml": "$include:\n - file://params.toml\n - file://params.yaml\nEnv: prod", } def mock_read_text(file_path): @@ -249,6 +264,25 @@ def test_relative_path_nested_includes(self, file_contents, entry_file, expected result = self.param_type.convert(f"file://{entry_path}", None, MagicMock()) self.assertEqual(result, expected) + def test_include_key_invalid_type(self): + """Test that $include with invalid type fails with clear error""" + mock_files = { + "invalid.yaml": "$include: 123\nEnv: prod", + } + + def mock_read_text(file_path): + return mock_files.get(file_path.name, "") + + def mock_is_file(file_path): + return file_path.name in mock_files + + with self.assertRaises(BadParameter) as exception, patch("pathlib.Path.is_file", new=mock_is_file), patch( + "pathlib.Path.read_text", new=mock_read_text + ): + self.param_type.convert("file://invalid.yaml", None, MagicMock()) + + self.assertIn("$include must be a string or list of strings", str(exception.exception)) + class TestCfnMetadataType(TestCase): def setUp(self): From db3e606471f9d7318188ff9b8349ef13052b73c1 Mon Sep 17 00:00:00 2001 From: Joe Wakefield Date: Wed, 28 Jan 2026 13:14:00 -0500 Subject: [PATCH 3/6] black linting --- samcli/cli/types.py | 2 +- tests/unit/cli/test_types.py | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/samcli/cli/types.py b/samcli/cli/types.py index 19359da753..80a9ea3ffa 100644 --- a/samcli/cli/types.py +++ b/samcli/cli/types.py @@ -175,7 +175,7 @@ def convert(self, values, param, ctx, seen_files=None, parent_dir=None): if not file_path.is_absolute() and parent_dir: file_path = parent_dir / file_path file_path = file_path.resolve() - + if not file_path.is_file(): self.fail(f"{value} was not found or is a directory", param, ctx) file_manager = FILE_MANAGER_MAPPER.get(file_path.suffix, None) diff --git a/tests/unit/cli/test_types.py b/tests/unit/cli/test_types.py index 0050f4bf18..09d4b5a94c 100644 --- a/tests/unit/cli/test_types.py +++ b/tests/unit/cli/test_types.py @@ -235,12 +235,18 @@ def mock_is_file(file_path): @parameterized.expand( [ ( - {"config/default.yaml": "Base: default\nEnv: dev", "config/prod.yaml": "- file://default.yaml\n- Env: production\n- Region: us-east-1"}, + { + "config/default.yaml": "Base: default\nEnv: dev", + "config/prod.yaml": "- file://default.yaml\n- Env: production\n- Region: us-east-1", + }, "config/prod.yaml", {"Base": "default", "Env": "production", "Region": "us-east-1"}, ), ( - {"config/shared/common.yaml": "Common: shared", "config/prod.yaml": "- file://shared/common.yaml\n- Env: prod"}, + { + "config/shared/common.yaml": "Common: shared", + "config/prod.yaml": "- file://shared/common.yaml\n- Env: prod", + }, "config/prod.yaml", {"Common": "shared", "Env": "prod"}, ), @@ -253,13 +259,13 @@ def test_relative_path_nested_includes(self, file_contents, entry_file, expected with tempfile.TemporaryDirectory() as temp: temp_path = Path(temp) - + # Create all files for file_path, content in file_contents.items(): full_path = temp_path / file_path full_path.parent.mkdir(parents=True, exist_ok=True) full_path.write_text(content) - + entry_path = temp_path / entry_file result = self.param_type.convert(f"file://{entry_path}", None, MagicMock()) self.assertEqual(result, expected) From 64804166ed455f32c66b5ccf0d6fcd1393d37497 Mon Sep 17 00:00:00 2001 From: Joe Wakefield Date: Wed, 28 Jan 2026 14:24:39 -0500 Subject: [PATCH 4/6] rename parent_dir->current_file for clarity --- samcli/cli/types.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/samcli/cli/types.py b/samcli/cli/types.py index 80a9ea3ffa..5a910b937b 100644 --- a/samcli/cli/types.py +++ b/samcli/cli/types.py @@ -124,7 +124,7 @@ class CfnParameterOverridesType(click.ParamType): name = "list,object,string" - def convert(self, values, param, ctx, seen_files=None, parent_dir=None): + def convert(self, values, param, ctx, seen_files=None, current_file=None): """ Takes parameter overrides loaded from various supported config file formats and flattens and normalizes them into a dictionary where all keys and values are strings. @@ -143,8 +143,8 @@ def convert(self, values, param, ctx, seen_files=None, parent_dir=None): Click context for error reporting. seen_files : set List of files processed in the current execution branch, used to detect infinite recursion - parent_dir : Path - Directory of the parent file, used to resolve relative paths in nested includes + current_file : Path + Path to the file currently being processed, used to resolve relative paths in nested includes Returns ------- @@ -171,9 +171,9 @@ def convert(self, values, param, ctx, seen_files=None, parent_dir=None): # If the string is a file reference (e.g., 'file://params.yaml') if value.startswith("file://"): file_path = Path(value[7:]) - # Resolve relative paths against parent directory - if not file_path.is_absolute() and parent_dir: - file_path = parent_dir / file_path + # Resolve relative paths against current file's directory + if not file_path.is_absolute() and current_file: + file_path = current_file.parent / file_path file_path = file_path.resolve() if not file_path.is_file(): @@ -188,7 +188,7 @@ def convert(self, values, param, ctx, seen_files=None, parent_dir=None): seen_files.add(file_path) try: nested_values = file_manager.read(file_path) - parameters.update(self.convert(nested_values, param, ctx, seen_files, file_path.parent)) + parameters.update(self.convert(nested_values, param, ctx, seen_files, file_path)) finally: seen_files.remove(file_path) else: @@ -220,7 +220,7 @@ def convert(self, values, param, ctx, seen_files=None, parent_dir=None): param, ctx, ) - parameters.update(self.convert(v, param, ctx, seen_files, parent_dir)) + parameters.update(self.convert(v, param, ctx, seen_files, current_file)) elif isinstance(v, (list, CommentedSeq)): # Join list elements into comma-separated string, strip whitespace and ignore empty entries parameters[str(k)] = ",".join(str(x).strip() for x in v if x not in (None, "")) From 7a96145f9e2e18dd7c38c73ce4afbb1c9f80197e Mon Sep 17 00:00:00 2001 From: Joe Wakefield Date: Mon, 2 Feb 2026 09:02:23 -0500 Subject: [PATCH 5/6] Add $include test for infinite recursion --- tests/unit/cli/test_types.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/unit/cli/test_types.py b/tests/unit/cli/test_types.py index 09d4b5a94c..b937f5e8b3 100644 --- a/tests/unit/cli/test_types.py +++ b/tests/unit/cli/test_types.py @@ -211,6 +211,27 @@ def mock_is_file(file_path): print(result) self.assertEqual(result, expected, msg="Failed with Input = " + str(inputs)) + def test_include_infinite_recursion_protection(self): + mock_files = { + "A.yaml": "$include: file://B.yaml", + "B.yaml": "$include: file://C.yaml", + "C.yaml": "$include: file://A.yaml", + } + + def mock_read_text(file_path): + file_name = file_path.name + return mock_files.get(file_name, "") + + def mock_is_file(file_path): + return file_path.name in mock_files + + with self.assertRaises(BadParameter) as exception, patch("pathlib.Path.is_file", new=mock_is_file), patch( + "pathlib.Path.read_text", new=mock_read_text + ): + self.param_type.convert("file://A.yaml", None, MagicMock()) + + self.assertIn("Infinite recursion detected in file references", str(exception.exception)) + def test_infinite_recursion_protection(self): mock_files = { "A.yaml": "- file://B.yaml", @@ -228,7 +249,7 @@ def mock_is_file(file_path): with self.assertRaises(BadParameter) as exception, patch("pathlib.Path.is_file", new=mock_is_file), patch( "pathlib.Path.read_text", new=mock_read_text ): - self.param_type.convert(f"file://A.yaml", None, MagicMock()) + self.param_type.convert("file://A.yaml", None, MagicMock()) self.assertIn("Infinite recursion detected in file references", str(exception.exception)) From ad5362989f1c8bf20e0fe3fc9fab28ab9979cb51 Mon Sep 17 00:00:00 2001 From: Joe Wakefield Date: Mon, 2 Feb 2026 09:03:01 -0500 Subject: [PATCH 6/6] Add type hint for CfnParameterOverridesType convert() --- samcli/cli/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samcli/cli/types.py b/samcli/cli/types.py index 5a910b937b..802a1b3799 100644 --- a/samcli/cli/types.py +++ b/samcli/cli/types.py @@ -124,7 +124,7 @@ class CfnParameterOverridesType(click.ParamType): name = "list,object,string" - def convert(self, values, param, ctx, seen_files=None, current_file=None): + def convert(self, values, param, ctx, seen_files=None, current_file: Optional[Path] = None): """ Takes parameter overrides loaded from various supported config file formats and flattens and normalizes them into a dictionary where all keys and values are strings.