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."
}
45 changes: 45 additions & 0 deletions awscli/customizations/configure/writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ class ConfigFileWriter:
r'(?P<value>.*)$'
) # fmt: skip

def _validate_no_newlines_or_carriage_returns(
self,
value,
label='value',
msg_override=None,
):
if isinstance(value, str) and ('\n' in value or '\r' in value):
err_msg = msg_override if msg_override is not None else (
f"Invalid {label}: newline "
f"characters and carriage returns are not allowed: {value!r}"
)
raise ValueError(err_msg)

def update_config(self, new_values, config_filename):
"""Update config file with new values.

Expand Down Expand Up @@ -52,6 +65,38 @@ def update_config(self, new_values, config_filename):

"""
section_name = new_values.pop('__section__', 'default')
self._validate_no_newlines_or_carriage_returns(
section_name,
'section name'
)
for k, v in new_values.items():
self._validate_no_newlines_or_carriage_returns(k, 'key')
if not isinstance(v, dict):
# Override error msg to prevent
# leaking sensitive config values to stderr.
self._validate_no_newlines_or_carriage_returns(
v,
'value',
msg_override=(
f"Invalid value for key {k}: "
f"newline characters and carriage "
f"returns are not allowed."
)
)
else:
for sk, sv in v.items():
# Override error msg to prevent
# leaking sensitive config values to stderr.
self._validate_no_newlines_or_carriage_returns(sk, 'key')
self._validate_no_newlines_or_carriage_returns(
sv,
'value',
msg_override=(
f"Invalid value for key {k}: "
f"newline characters and carriage "
f"returns are not allowed."
)
)
if not os.path.isfile(config_filename):
self._create_file(config_filename)
self._write_new_section(section_name, new_values, config_filename)
Expand Down
56 changes: 56 additions & 0 deletions tests/functional/configure/test_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,62 @@ 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)
# To avoid leaking sensitive values,
# values should not appear in stderr.
self.assertNotIn("us-east-1\nus-west-2", 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)
# To avoid leaking sensitive values,
# values should not appear in stderr.
self.assertNotIn("us-east-1\rus-west-2", 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)
# To avoid leaking sensitive values,
# values should not appear in stderr.
self.assertNotIn("s3v4\nfoo", 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