Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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}"
Comment thread
aemous marked this conversation as resolved.
Outdated
)

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