Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-Configuration-47190.json
Original file line number Diff line number Diff line change
@@ -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."
}
15 changes: 15 additions & 0 deletions awscli/customizations/configure/writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ class ConfigFileWriter:
r'(?P<value>.*)$'
) # 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.
Expand Down Expand Up @@ -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)
Expand Down
47 changes: 47 additions & 0 deletions tests/functional/configure/test_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
36 changes: 36 additions & 0 deletions tests/unit/customizations/configure/test_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Loading