diff --git a/.changes/next-release/bugfix-Configuration-47190.json b/.changes/next-release/bugfix-Configuration-47190.json new file mode 100644 index 000000000000..a3cfd1350208 --- /dev/null +++ b/.changes/next-release/bugfix-Configuration-47190.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "Configuration", + "description": "Update ``configure set`` command to return an error when newline or carriage-return characters are specified in the value." +} diff --git a/awscli/customizations/configure/writer.py b/awscli/customizations/configure/writer.py index 8a173ab68673..da47cd2b53b3 100644 --- a/awscli/customizations/configure/writer.py +++ b/awscli/customizations/configure/writer.py @@ -24,6 +24,12 @@ class ConfigFileWriter: r'(?P.*)$' ) # fmt: skip + def _validate_no_newlines(self, value, label='value'): + if isinstance(value, str) and ('\n' in value or '\r' in value): + raise ValueError( + f"Invalid {label}: newline characters are not allowed: {value!r}" + ) + def update_config(self, new_values, config_filename): """Update config file with new values. @@ -52,6 +58,15 @@ def update_config(self, new_values, config_filename): """ section_name = new_values.pop('__section__', 'default') + self._validate_no_newlines(section_name, 'section name') + for k, v in new_values.items(): + self._validate_no_newlines(k, 'key') + if not isinstance(v, dict): + self._validate_no_newlines(v, 'value') + else: + for sk, sv in v.items(): + self._validate_no_newlines(sk, 'key') + self._validate_no_newlines(sv, 'value') if not os.path.isfile(config_filename): self._create_file(config_filename) self._write_new_section(section_name, new_values, config_filename) diff --git a/tests/functional/configure/test_configure.py b/tests/functional/configure/test_configure.py index b93c662e4879..c8eed2a5a0b7 100644 --- a/tests/functional/configure/test_configure.py +++ b/tests/functional/configure/test_configure.py @@ -299,6 +299,53 @@ def test_get_nested_attribute(self): ) self.assertEqual(stdout, "") + def test_set_rejects_newline_in_value(self): + _, stderr, _ = self.run_cmd( + ["configure", "set", "region", "us-east-1\nus-west-2"], + expected_rc=255, + ) + self.assertIn("newline", stderr) + + def test_set_rejects_carriage_return_in_value(self): + _, stderr, _ = self.run_cmd( + ["configure", "set", "region", "us-east-1\rus-west-2"], + expected_rc=255, + ) + self.assertIn("newline", stderr) + + def test_set_rejects_newline_in_nested_value(self): + _, stderr, _ = self.run_cmd( + ["configure", "set", "default.s3.signature_version", "s3v4\nfoo"], + expected_rc=255, + ) + self.assertIn("newline", stderr) + + def test_newline_injection_does_not_write_injected_key_to_file(self): + # Simulates: aws configure set output $'table\nregion = us-east-1' + # The injected key must not appear anywhere in the config file. + self.set_config_file_contents("[default]\n") + self.run_cmd( + ["configure", "set", "output", "table\nregion = us-east-1"], + expected_rc=255, + ) + contents = self.get_config_file_contents() + self.assertNotIn("region", contents) + + def test_newline_injection_does_not_set_injected_key_in_parsed_config(self): + # Even if the file were somehow written, the injected key must not be + # readable back via 'configure get'. + self.set_config_file_contents("[default]\n") + self.run_cmd( + ["configure", "set", "output", "table\nregion = us-east-1"], + expected_rc=255, + ) + # Re-create the driver so it re-reads the (unchanged) config file. + self.driver = create_clidriver() + stdout, _, _ = self.run_cmd( + "configure get region", expected_rc=1 + ) + self.assertEqual(stdout.strip(), "") + class TestConfigureHasArgTable(unittest.TestCase): def test_configure_command_has_arg_table(self): diff --git a/tests/unit/customizations/configure/test_writer.py b/tests/unit/customizations/configure/test_writer.py index ae6a5de49410..a3c62bd30896 100644 --- a/tests/unit/customizations/configure/test_writer.py +++ b/tests/unit/customizations/configure/test_writer.py @@ -314,3 +314,39 @@ def test_appends_newline_on_new_section(self): '[new-section]\n' 'region = us-west-2\n', ) + + def test_newline_in_value_raises(self): + with open(self.config_filename, 'w') as f: + f.write('[default]\nfoo = bar\n') + with self.assertRaises(ValueError): + self.writer.update_config({'foo': 'bad\nvalue'}, self.config_filename) + + def test_carriage_return_in_value_raises(self): + with open(self.config_filename, 'w') as f: + f.write('[default]\nfoo = bar\n') + with self.assertRaises(ValueError): + self.writer.update_config({'foo': 'bad\rvalue'}, self.config_filename) + + def test_newline_in_key_raises(self): + with open(self.config_filename, 'w') as f: + f.write('[default]\nfoo = bar\n') + with self.assertRaises(ValueError): + self.writer.update_config({'bad\nkey': 'value'}, self.config_filename) + + def test_newline_in_section_name_raises(self): + with open(self.config_filename, 'w') as f: + f.write('[default]\nfoo = bar\n') + with self.assertRaises(ValueError): + self.writer.update_config( + {'foo': 'value', '__section__': 'bad\nsection'}, + self.config_filename + ) + + def test_newline_in_nested_value_raises(self): + with open(self.config_filename, 'w') as f: + f.write('[default]\n') + with self.assertRaises(ValueError): + self.writer.update_config( + {'__section__': 'default', 's3': {'key': 'bad\nvalue'}}, + self.config_filename + )