From 31baf3b0a7b4f5f5012eb23ba938eebe298c86df Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 28 Jan 2026 15:48:20 -0500 Subject: [PATCH 1/3] Tighten history file permissions --- awscli/customizations/history/__init__.py | 32 ++++++---- awscli/customizations/history/db.py | 23 ++++--- tests/unit/customizations/history/test_db.py | 52 ++++++++++++---- .../customizations/history/test_history.py | 62 +++++++++++++++++-- 4 files changed, 132 insertions(+), 37 deletions(-) diff --git a/awscli/customizations/history/__init__.py b/awscli/customizations/history/__init__.py index a3e580827493..3c9666d322c3 100644 --- a/awscli/customizations/history/__init__.py +++ b/awscli/customizations/history/__init__.py @@ -10,24 +10,27 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. +import logging import os import sys -import logging -from botocore.history import get_global_history_recorder from botocore.exceptions import ProfileNotFound +from botocore.history import get_global_history_recorder from awscli.compat import sqlite3 from awscli.customizations.commands import BasicCommand -from awscli.customizations.history.constants import HISTORY_FILENAME_ENV_VAR -from awscli.customizations.history.constants import DEFAULT_HISTORY_FILENAME -from awscli.customizations.history.db import DatabaseConnection -from awscli.customizations.history.db import DatabaseRecordWriter -from awscli.customizations.history.db import RecordBuilder -from awscli.customizations.history.db import DatabaseHistoryHandler -from awscli.customizations.history.show import ShowCommand +from awscli.customizations.history.constants import ( + DEFAULT_HISTORY_FILENAME, + HISTORY_FILENAME_ENV_VAR, +) +from awscli.customizations.history.db import ( + DatabaseConnection, + DatabaseHistoryHandler, + DatabaseRecordWriter, + RecordBuilder, +) from awscli.customizations.history.list import ListCommand - +from awscli.customizations.history.show import ShowCommand LOG = logging.getLogger(__name__) HISTORY_RECORDER = get_global_history_recorder() @@ -49,8 +52,13 @@ def attach_history_handler(session, parsed_args, **kwargs): history_filename = os.environ.get( HISTORY_FILENAME_ENV_VAR, DEFAULT_HISTORY_FILENAME) - if not os.path.isdir(os.path.dirname(history_filename)): - os.makedirs(os.path.dirname(history_filename)) + history_dir = os.path.dirname(history_filename) + if not os.path.isdir(history_dir): + os.makedirs(history_dir) + try: + os.chmod(history_dir, 0o700) + except OSError as e: + LOG.debug('Unable to set directory permissions: %s', e) connection = DatabaseConnection(history_filename) writer = DatabaseRecordWriter(connection) diff --git a/awscli/customizations/history/db.py b/awscli/customizations/history/db.py index bdb96d1dc4bd..b6ddde4bbaa3 100644 --- a/awscli/customizations/history/db.py +++ b/awscli/customizations/history/db.py @@ -10,19 +10,17 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. -import uuid -import time -import json import datetime -import threading +import json import logging -from awscli.compat import collections_abc +import os +import threading +import time +import uuid from botocore.history import BaseHistoryHandler -from awscli.compat import sqlite3 -from awscli.compat import binary_type - +from awscli.compat import binary_type, collections_abc, sqlite3 LOG = logging.getLogger(__name__) @@ -40,6 +38,15 @@ class DatabaseConnection(object): _ENABLE_WAL = 'PRAGMA journal_mode=WAL' def __init__(self, db_filename): + # Skip file operations for in-memory databases + if db_filename != ':memory:': + if not os.path.exists(db_filename): + # Create file so we can set permissions before sqlite opens it + open(db_filename, 'a').close() + try: + os.chmod(db_filename, 0o600) + except OSError as e: + LOG.debug('Unable to set file permissions: %s', e) self._connection = sqlite3.connect( db_filename, check_same_thread=False, isolation_level=None) self._ensure_database_setup() diff --git a/tests/unit/customizations/history/test_db.py b/tests/unit/customizations/history/test_db.py index c481d06cd520..a5487290d3e4 100644 --- a/tests/unit/customizations/history/test_db.py +++ b/tests/unit/customizations/history/test_db.py @@ -10,21 +10,24 @@ # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. +import datetime +import json +import numbers import os import re -import json +import stat import threading -import datetime -import numbers from awscli.compat import queue -from awscli.customizations.history.db import DatabaseConnection -from awscli.customizations.history.db import DatabaseHistoryHandler -from awscli.customizations.history.db import DatabaseRecordWriter -from awscli.customizations.history.db import DatabaseRecordReader -from awscli.customizations.history.db import PayloadSerializer -from awscli.customizations.history.db import RecordBuilder -from awscli.testutils import mock, unittest, FileCreator +from awscli.customizations.history.db import ( + DatabaseConnection, + DatabaseHistoryHandler, + DatabaseRecordReader, + DatabaseRecordWriter, + PayloadSerializer, + RecordBuilder, +) +from awscli.testutils import FileCreator, mock, skip_if_windows, unittest from tests import CaseInsensitiveDict @@ -46,8 +49,12 @@ def tearDown(self): class TestDatabaseConnection(unittest.TestCase): + @mock.patch('awscli.customizations.history.db.os.path.exists', + return_value=True) + @mock.patch('awscli.customizations.history.db.os.chmod') @mock.patch('awscli.compat.sqlite3.connect') - def test_can_connect_to_argument_file(self, mock_connect): + def test_can_connect_to_argument_file(self, mock_connect, mock_chmod, + mock_exists): expected_location = os.path.expanduser(os.path.join( '~', 'foo', 'bar', 'baz.db')) DatabaseConnection(expected_location) @@ -82,6 +89,29 @@ def test_can_close(self, mock_connect): self.assertTrue(connection.close.called) +@skip_if_windows +class TestDatabaseConnectionPermissions: + def setup_method(self): + self.files = FileCreator() + + def teardown_method(self): + self.files.remove_all() + + def test_create_new_file_with_secure_permissions(self): + db_path = self.files.full_path('history.db') + DatabaseConnection(db_path) + file_mode = stat.S_IMODE(os.stat(db_path).st_mode) + assert file_mode == 0o600 + + def test_tighten_existing_file_permissions(self): + db_path = self.files.full_path('history.db') + open(db_path, 'a').close() + os.chmod(db_path, 0o644) + DatabaseConnection(db_path) + file_mode = stat.S_IMODE(os.stat(db_path).st_mode) + assert file_mode == 0o600 + + class TestDatabaseHistoryHandler(unittest.TestCase): UUID_PATTERN = re.compile( '^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', diff --git a/tests/unit/customizations/history/test_history.py b/tests/unit/customizations/history/test_history.py index 5e364b98e81b..2ffae56505a0 100644 --- a/tests/unit/customizations/history/test_history.py +++ b/tests/unit/customizations/history/test_history.py @@ -12,17 +12,20 @@ # language governing permissions and limitations under the License. import argparse import os +import stat -from botocore.session import Session -from botocore.history import HistoryRecorder from botocore.exceptions import ProfileNotFound +from botocore.history import HistoryRecorder +from botocore.session import Session -from awscli.testutils import unittest, mock, FileCreator from awscli.compat import StringIO -from awscli.customizations.history import attach_history_handler -from awscli.customizations.history import add_history_commands -from awscli.customizations.history import HistoryCommand +from awscli.customizations.history import ( + HistoryCommand, + add_history_commands, + attach_history_handler, +) from awscli.customizations.history.db import DatabaseHistoryHandler +from awscli.testutils import FileCreator, mock, skip_if_windows, unittest class TestAttachHistoryHandler(unittest.TestCase): @@ -149,6 +152,53 @@ def test_profile_does_not_exist(self, mock_recorder, mock_db_sqlite3, self.assertFalse(mock_recorder.add_handler.called) self.assertFalse(mock_db_sqlite3.connect.called) + @skip_if_windows + @mock.patch('awscli.customizations.history.sqlite3') + @mock.patch('awscli.customizations.history.db.sqlite3') + @mock.patch('awscli.customizations.history.HISTORY_RECORDER', + spec=HistoryRecorder) + def test_create_directory_with_secure_permissions( + self, mock_recorder, mock_db_sqlite3, mock_sqlite3): + mock_session = mock.Mock(Session) + mock_session.get_scoped_config.return_value = { + 'cli_history': 'enabled' + } + + parsed_args = argparse.Namespace() + parsed_args.command = 's3' + + directory_to_create = os.path.join(self.files.rootdir, 'secure-dir') + db_filename = os.path.join(directory_to_create, 'name.db') + with mock.patch('os.environ', {'AWS_CLI_HISTORY_FILE': db_filename}): + attach_history_handler( + session=mock_session, parsed_args=parsed_args) + dir_mode = stat.S_IMODE(os.stat(directory_to_create).st_mode) + assert dir_mode == 0o700 + + @skip_if_windows('File permissions are not supported on Windows') + @mock.patch('awscli.customizations.history.sqlite3') + @mock.patch('awscli.customizations.history.db.sqlite3') + @mock.patch('awscli.customizations.history.HISTORY_RECORDER', + spec=HistoryRecorder) + def test_tighten_existing_directory_permissions( + self, mock_recorder, mock_db_sqlite3, mock_sqlite3): + mock_session = mock.Mock(Session) + mock_session.get_scoped_config.return_value = { + 'cli_history': 'enabled' + } + + parsed_args = argparse.Namespace() + parsed_args.command = 's3' + + directory_to_create = os.path.join(self.files.rootdir, 'existing-dir') + os.makedirs(directory_to_create, mode=0o755) + db_filename = os.path.join(directory_to_create, 'name.db') + with mock.patch('os.environ', {'AWS_CLI_HISTORY_FILE': db_filename}): + attach_history_handler( + session=mock_session, parsed_args=parsed_args) + dir_mode = stat.S_IMODE(os.stat(directory_to_create).st_mode) + assert dir_mode == 0o700 + class TestAddHistoryCommand(unittest.TestCase): def test_add_history_command(self): From 4d8f43fee88347d789fcd220b0ff189bc7d06b07 Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 28 Jan 2026 15:50:00 -0500 Subject: [PATCH 2/3] Pre-commit formatting --- awscli/customizations/history/__init__.py | 17 +- awscli/customizations/history/db.py | 35 +- tests/unit/customizations/history/test_db.py | 486 +++++++++++------- .../customizations/history/test_history.py | 93 ++-- 4 files changed, 375 insertions(+), 256 deletions(-) diff --git a/awscli/customizations/history/__init__.py b/awscli/customizations/history/__init__.py index 3c9666d322c3..ed147d8d21f3 100644 --- a/awscli/customizations/history/__init__.py +++ b/awscli/customizations/history/__init__.py @@ -37,13 +37,13 @@ def register_history_mode(event_handlers): - event_handlers.register( - 'session-initialized', attach_history_handler) + event_handlers.register('session-initialized', attach_history_handler) def register_history_commands(event_handlers): event_handlers.register( - "building-command-table.main", add_history_commands) + "building-command-table.main", add_history_commands + ) def attach_history_handler(session, parsed_args, **kwargs): @@ -51,7 +51,8 @@ def attach_history_handler(session, parsed_args, **kwargs): LOG.debug('Enabling CLI history') history_filename = os.environ.get( - HISTORY_FILENAME_ENV_VAR, DEFAULT_HISTORY_FILENAME) + HISTORY_FILENAME_ENV_VAR, DEFAULT_HISTORY_FILENAME + ) history_dir = os.path.dirname(history_filename) if not os.path.isdir(history_dir): os.makedirs(history_dir) @@ -106,10 +107,12 @@ class HistoryCommand(BasicCommand): ) SUBCOMMANDS = [ {'name': 'show', 'command_class': ShowCommand}, - {'name': 'list', 'command_class': ListCommand} + {'name': 'list', 'command_class': ListCommand}, ] def _run_main(self, parsed_args, parsed_globals): if parsed_args.subcommand is None: - raise ValueError("usage: aws [options] " - "[parameters]\naws: error: too few arguments") + raise ValueError( + "usage: aws [options] " + "[parameters]\naws: error: too few arguments" + ) diff --git a/awscli/customizations/history/db.py b/awscli/customizations/history/db.py index b6ddde4bbaa3..255b54bfc322 100644 --- a/awscli/customizations/history/db.py +++ b/awscli/customizations/history/db.py @@ -25,7 +25,7 @@ LOG = logging.getLogger(__name__) -class DatabaseConnection(object): +class DatabaseConnection: _CREATE_TABLE = """ CREATE TABLE IF NOT EXISTS records ( id TEXT, @@ -48,7 +48,8 @@ def __init__(self, db_filename): except OSError as e: LOG.debug('Unable to set file permissions: %s', e) self._connection = sqlite3.connect( - db_filename, check_same_thread=False, isolation_level=None) + db_filename, check_same_thread=False, isolation_level=None + ) self._ensure_database_setup() def close(self): @@ -99,8 +100,9 @@ def _remove_non_unicode_stings(self, obj): if isinstance(obj, str): obj = self._try_decode_bytes(obj) elif isinstance(obj, dict): - obj = dict((k, self._remove_non_unicode_stings(v)) for k, v - in obj.items()) + obj = dict( + (k, self._remove_non_unicode_stings(v)) for k, v in obj.items() + ) elif isinstance(obj, (list, tuple)): obj = [self._remove_non_unicode_stings(o) for o in obj] return obj @@ -139,7 +141,7 @@ def default(self, obj): return repr(obj) -class DatabaseRecordWriter(object): +class DatabaseRecordWriter: _WRITE_RECORD = """ INSERT INTO records( id, request_id, source, event_type, timestamp, payload) @@ -159,26 +161,30 @@ def write_record(self, record): def _create_db_record(self, record): event_type = record['event_type'] - json_serialized_payload = json.dumps(record['payload'], - cls=PayloadSerializer) + json_serialized_payload = json.dumps( + record['payload'], cls=PayloadSerializer + ) db_record = ( record['command_id'], record.get('request_id'), record['source'], event_type, record['timestamp'], - json_serialized_payload + json_serialized_payload, ) return db_record -class DatabaseRecordReader(object): +class DatabaseRecordReader: _ORDERING = 'ORDER BY timestamp' - _GET_LAST_ID_RECORDS = """ + _GET_LAST_ID_RECORDS = ( + """ SELECT * FROM records WHERE id = (SELECT id FROM records WHERE timestamp = - (SELECT max(timestamp) FROM records)) %s;""" % _ORDERING + (SELECT max(timestamp) FROM records)) %s;""" + % _ORDERING + ) _GET_RECORDS_BY_ID = 'SELECT * from records where id = ? %s' % _ORDERING _GET_ALL_RECORDS = ( 'SELECT a.id AS id_a, ' @@ -225,9 +231,10 @@ def iter_all_records(self): yield row -class RecordBuilder(object): +class RecordBuilder: _REQUEST_LIFECYCLE_EVENTS = set( - ['API_CALL', 'HTTP_REQUEST', 'HTTP_RESPONSE', 'PARSED_RESPONSE']) + ['API_CALL', 'HTTP_REQUEST', 'HTTP_RESPONSE', 'PARSED_RESPONSE'] + ) _START_OF_REQUEST_LIFECYCLE_EVENT = 'API_CALL' def __init__(self): @@ -261,7 +268,7 @@ def build_record(self, event_type, payload, source): 'event_type': event_type, 'payload': payload, 'source': source, - 'timestamp': int(time.time() * 1000) + 'timestamp': int(time.time() * 1000), } request_id = self._get_request_id(event_type) if request_id: diff --git a/tests/unit/customizations/history/test_db.py b/tests/unit/customizations/history/test_db.py index a5487290d3e4..1375d942e5f2 100644 --- a/tests/unit/customizations/history/test_db.py +++ b/tests/unit/customizations/history/test_db.py @@ -31,7 +31,7 @@ from tests import CaseInsensitiveDict -class FakeDatabaseConnection(object): +class FakeDatabaseConnection: def __init__(self): self.execute = mock.MagicMock() self.closed = False @@ -49,17 +49,21 @@ def tearDown(self): class TestDatabaseConnection(unittest.TestCase): - @mock.patch('awscli.customizations.history.db.os.path.exists', - return_value=True) + @mock.patch( + 'awscli.customizations.history.db.os.path.exists', return_value=True + ) @mock.patch('awscli.customizations.history.db.os.chmod') @mock.patch('awscli.compat.sqlite3.connect') - def test_can_connect_to_argument_file(self, mock_connect, mock_chmod, - mock_exists): - expected_location = os.path.expanduser(os.path.join( - '~', 'foo', 'bar', 'baz.db')) + def test_can_connect_to_argument_file( + self, mock_connect, mock_chmod, mock_exists + ): + expected_location = os.path.expanduser( + os.path.join('~', 'foo', 'bar', 'baz.db') + ) DatabaseConnection(expected_location) mock_connect.assert_called_with( - expected_location, check_same_thread=False, isolation_level=None) + expected_location, check_same_thread=False, isolation_level=None + ) @mock.patch('awscli.compat.sqlite3.connect') def test_does_try_to_enable_wal(self, mock_connect): @@ -114,8 +118,7 @@ def test_tighten_existing_file_permissions(self): class TestDatabaseHistoryHandler(unittest.TestCase): UUID_PATTERN = re.compile( - '^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', - re.I + '^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', re.I ) def test_emit_does_write_cli_rc_record(self): @@ -124,13 +127,16 @@ def test_emit_does_write_cli_rc_record(self): handler = DatabaseHistoryHandler(writer, record_builder) handler.emit('CLI_RC', 0, 'CLI') call = writer.write_record.call_args[0][0] - self.assertEqual(call, { - 'command_id': mock.ANY, - 'event_type': 'CLI_RC', - 'payload': 0, - 'source': 'CLI', - 'timestamp': mock.ANY - }) + self.assertEqual( + call, + { + 'command_id': mock.ANY, + 'event_type': 'CLI_RC', + 'payload': 0, + 'source': 'CLI', + 'timestamp': mock.ANY, + }, + ) self.assertTrue(self.UUID_PATTERN.match(call['command_id'])) self.assertIsInstance(call['timestamp'], numbers.Number) @@ -140,13 +146,16 @@ def test_emit_does_write_cli_version_record(self): handler = DatabaseHistoryHandler(writer, record_builder) handler.emit('CLI_VERSION', 'Version Info', 'CLI') call = writer.write_record.call_args[0][0] - self.assertEqual(call, { - 'command_id': mock.ANY, - 'event_type': 'CLI_VERSION', - 'payload': 'Version Info', - 'source': 'CLI', - 'timestamp': mock.ANY - }) + self.assertEqual( + call, + { + 'command_id': mock.ANY, + 'event_type': 'CLI_VERSION', + 'payload': 'Version Info', + 'source': 'CLI', + 'timestamp': mock.ANY, + }, + ) self.assertTrue(self.UUID_PATTERN.match(call['command_id'])) self.assertIsInstance(call['timestamp'], numbers.Number) @@ -157,14 +166,17 @@ def test_emit_does_write_api_call_record(self): payload = {'foo': 'bar'} handler.emit('API_CALL', payload, 'BOTOCORE') call = writer.write_record.call_args[0][0] - self.assertEqual(call, { - 'command_id': mock.ANY, - 'request_id': mock.ANY, - 'event_type': 'API_CALL', - 'payload': payload, - 'source': 'BOTOCORE', - 'timestamp': mock.ANY - }) + self.assertEqual( + call, + { + 'command_id': mock.ANY, + 'request_id': mock.ANY, + 'event_type': 'API_CALL', + 'payload': payload, + 'source': 'BOTOCORE', + 'timestamp': mock.ANY, + }, + ) self.assertTrue(self.UUID_PATTERN.match(call['command_id'])) self.assertTrue(self.UUID_PATTERN.match(call['request_id'])) @@ -178,14 +190,17 @@ def test_emit_does_write_http_request_record(self): handler.emit('API_CALL', '', 'BOTOCORE') handler.emit('HTTP_REQUEST', payload, 'BOTOCORE') call = writer.write_record.call_args[0][0] - self.assertEqual(call, { - 'command_id': mock.ANY, - 'request_id': mock.ANY, - 'event_type': 'HTTP_REQUEST', - 'payload': payload, - 'source': 'BOTOCORE', - 'timestamp': mock.ANY - }) + self.assertEqual( + call, + { + 'command_id': mock.ANY, + 'request_id': mock.ANY, + 'event_type': 'HTTP_REQUEST', + 'payload': payload, + 'source': 'BOTOCORE', + 'timestamp': mock.ANY, + }, + ) self.assertTrue(self.UUID_PATTERN.match(call['command_id'])) self.assertTrue(self.UUID_PATTERN.match(call['request_id'])) @@ -199,14 +214,17 @@ def test_emit_does_write_http_response_record(self): handler.emit('API_CALL', '', 'BOTOCORE') handler.emit('HTTP_RESPONSE', payload, 'BOTOCORE') call = writer.write_record.call_args[0][0] - self.assertEqual(call, { - 'command_id': mock.ANY, - 'request_id': mock.ANY, - 'event_type': 'HTTP_RESPONSE', - 'payload': payload, - 'source': 'BOTOCORE', - 'timestamp': mock.ANY - }) + self.assertEqual( + call, + { + 'command_id': mock.ANY, + 'request_id': mock.ANY, + 'event_type': 'HTTP_RESPONSE', + 'payload': payload, + 'source': 'BOTOCORE', + 'timestamp': mock.ANY, + }, + ) self.assertTrue(self.UUID_PATTERN.match(call['command_id'])) self.assertTrue(self.UUID_PATTERN.match(call['request_id'])) @@ -220,14 +238,17 @@ def test_emit_does_write_parsed_response_record(self): handler.emit('API_CALL', '', 'BOTOCORE') handler.emit('PARSED_RESPONSE', payload, 'BOTOCORE') call = writer.write_record.call_args[0][0] - self.assertEqual(call, { - 'command_id': mock.ANY, - 'request_id': mock.ANY, - 'event_type': 'PARSED_RESPONSE', - 'payload': payload, - 'source': 'BOTOCORE', - 'timestamp': mock.ANY - }) + self.assertEqual( + call, + { + 'command_id': mock.ANY, + 'request_id': mock.ANY, + 'event_type': 'PARSED_RESPONSE', + 'payload': payload, + 'source': 'BOTOCORE', + 'timestamp': mock.ANY, + }, + ) self.assertTrue(self.UUID_PATTERN.match(call['command_id'])) self.assertTrue(self.UUID_PATTERN.match(call['request_id'])) @@ -237,13 +258,12 @@ def assert_contains_lines_in_order(self, lines, contents): for line in lines: self.assertIn(line, contents) beginning = contents.find(line) - contents = contents[(beginning + len(line)):] + contents = contents[(beginning + len(line)) :] class BaseDatabaseRecordWriterTester(BaseDatabaseRecordTester): UUID_PATTERN = re.compile( - '^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', - re.I + '^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', re.I ) def setUp(self): @@ -267,180 +287,233 @@ def test_can_close(self): self.assertTrue(connection.close.called) def test_can_write_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'FOO', - 'payload': 'bar', - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'FOO', + 'payload': 'bar', + 'source': 'TEST', + 'timestamp': 1234, + } + ) # Now that we have verified the order of the fields in the insert # statement we can verify that the record values are in the correct # order in the tuple. # (command_id, request_id, source, event_type, timestamp, payload) written_record = self._read_last_record() - self.assertEqual(written_record, - ('command', None, 'TEST', 'FOO', 1234, '"bar"')) + self.assertEqual( + written_record, ('command', None, 'TEST', 'FOO', 1234, '"bar"') + ) def test_commit_count_matches_write_count(self): records_to_write = 10 for _ in range(records_to_write): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'foo', - 'payload': '', - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'foo', + 'payload': '', + 'source': 'TEST', + 'timestamp': 1234, + } + ) cursor = self.db.execute('SELECT COUNT(*) FROM records') record_count = cursor.fetchone()[0] self.assertEqual(record_count, records_to_write) def test_can_write_cli_version_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'CLI_VERSION', - 'payload': ('aws-cli/1.11.184 Python/3.6.2 Darwin/15.6.0 ' - 'botocore/1.7.42'), - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'CLI_VERSION', + 'payload': ( + 'aws-cli/1.11.184 Python/3.6.2 Darwin/15.6.0 ' + 'botocore/1.7.42' + ), + 'source': 'TEST', + 'timestamp': 1234, + } + ) written_record = self._read_last_record() self.assertEqual( written_record, - ('command', None, 'TEST', 'CLI_VERSION', 1234, - '"aws-cli/1.11.184 Python/3.6.2 Darwin/15.6.0 botocore/1.7.42"') + ( + 'command', + None, + 'TEST', + 'CLI_VERSION', + 1234, + '"aws-cli/1.11.184 Python/3.6.2 Darwin/15.6.0 botocore/1.7.42"', + ), ) def test_can_write_cli_arguments_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'CLI_ARGUMENTS', - 'payload': ['s3', 'ls'], - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'CLI_ARGUMENTS', + 'payload': ['s3', 'ls'], + 'source': 'TEST', + 'timestamp': 1234, + } + ) written_record = self._read_last_record() self.assertEqual( written_record, - ('command', None, 'TEST', 'CLI_ARGUMENTS', 1234, '["s3", "ls"]') + ('command', None, 'TEST', 'CLI_ARGUMENTS', 1234, '["s3", "ls"]'), ) def test_can_write_api_call_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'API_CALL', - 'payload': { - 'service': 's3', - 'operation': 'ListBuckets', - 'params': {}, - }, - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'API_CALL', + 'payload': { + 'service': 's3', + 'operation': 'ListBuckets', + 'params': {}, + }, + 'source': 'TEST', + 'timestamp': 1234, + } + ) written_record = self._read_last_record() self.assertEqual( written_record, - ('command', None, 'TEST', 'API_CALL', 1234, json.dumps({ - 'service': 's3', - 'operation': 'ListBuckets', - 'params': {}, - })) + ( + 'command', + None, + 'TEST', + 'API_CALL', + 1234, + json.dumps( + { + 'service': 's3', + 'operation': 'ListBuckets', + 'params': {}, + } + ), + ), ) def test_can_write_http_request_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'HTTP_REQUEST', - 'payload': { - 'method': 'GET', - 'headers': CaseInsensitiveDict({}), - 'body': '...', - }, - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'HTTP_REQUEST', + 'payload': { + 'method': 'GET', + 'headers': CaseInsensitiveDict({}), + 'body': '...', + }, + 'source': 'TEST', + 'timestamp': 1234, + } + ) written_record = self._read_last_record() self.assertEqual( written_record, - ('command', None, 'TEST', 'HTTP_REQUEST', 1234, json.dumps({ - 'method': 'GET', - 'headers': {}, - 'body': '...', - })) + ( + 'command', + None, + 'TEST', + 'HTTP_REQUEST', + 1234, + json.dumps( + { + 'method': 'GET', + 'headers': {}, + 'body': '...', + } + ), + ), ) def test_can_write_http_response_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'HTTP_RESPONSE', - 'payload': { - 'streaming': False, - 'headers': {}, - 'body': '...', - 'status_code': 200, - 'request_id': '1234abcd' - }, - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'HTTP_RESPONSE', + 'payload': { + 'streaming': False, + 'headers': {}, + 'body': '...', + 'status_code': 200, + 'request_id': '1234abcd', + }, + 'source': 'TEST', + 'timestamp': 1234, + } + ) written_record = self._read_last_record() self.assertEqual( written_record, - ('command', None, 'TEST', 'HTTP_RESPONSE', 1234, json.dumps({ - 'streaming': False, - 'headers': {}, - 'body': '...', - 'status_code': 200, - 'request_id': '1234abcd' - })) + ( + 'command', + None, + 'TEST', + 'HTTP_RESPONSE', + 1234, + json.dumps( + { + 'streaming': False, + 'headers': {}, + 'body': '...', + 'status_code': 200, + 'request_id': '1234abcd', + } + ), + ), ) def test_can_write_parsed_response_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'PARSED_RESPONSE', - 'payload': {}, - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'PARSED_RESPONSE', + 'payload': {}, + 'source': 'TEST', + 'timestamp': 1234, + } + ) written_record = self._read_last_record() self.assertEqual( written_record, - ('command', None, 'TEST', 'PARSED_RESPONSE', 1234, '{}') + ('command', None, 'TEST', 'PARSED_RESPONSE', 1234, '{}'), ) def test_can_write_cli_rc_record(self): - self.writer.write_record({ - 'command_id': 'command', - 'event_type': 'CLI_RC', - 'payload': 0, - 'source': 'TEST', - 'timestamp': 1234 - }) + self.writer.write_record( + { + 'command_id': 'command', + 'event_type': 'CLI_RC', + 'payload': 0, + 'source': 'TEST', + 'timestamp': 1234, + } + ) written_record = self._read_last_record() self.assertEqual( - written_record, - ('command', None, 'TEST', 'CLI_RC', 1234, '0') + written_record, ('command', None, 'TEST', 'CLI_RC', 1234, '0') ) -class ThreadedRecordBuilder(object): +class ThreadedRecordBuilder: def __init__(self, tracker): self._read_q = queue.Queue() self._write_q = queue.Queue() self._thread = threading.Thread( - target=self._threaded_request_tracker, - args=(tracker,)) + target=self._threaded_request_tracker, args=(tracker,) + ) def _threaded_request_tracker(self, builder): while True: @@ -528,8 +601,9 @@ def test_can_close(self): self.assertTrue(self.fake_connection.closed) def test_row_factory_set(self): - self.assertEqual(self.fake_connection.row_factory, - self.reader._row_factory) + self.assertEqual( + self.fake_connection.row_factory, self.reader._row_factory + ) def test_iter_latest_records_performs_correct_query(self): expected_query = ( @@ -541,27 +615,32 @@ def test_iter_latest_records_performs_correct_query(self): [_ for _ in self.reader.iter_latest_records()] self.assertEqual( self.fake_connection.execute.call_args[0][0].strip(), - expected_query.strip()) + expected_query.strip(), + ) def test_iter_latest_records_does_iter_records(self): records_to_get = [1, 2, 3] self.fake_connection.execute.return_value.__iter__.return_value = iter( - records_to_get) + records_to_get + ) records = [r for r in self.reader.iter_latest_records()] self.assertEqual(records, records_to_get) def test_iter_records_performs_correct_query(self): - expected_query = ('SELECT * from records where id = ? ' - 'ORDER BY timestamp') + expected_query = ( + 'SELECT * from records where id = ? ' 'ORDER BY timestamp' + ) [_ for _ in self.reader.iter_records('fake_id')] self.assertEqual( self.fake_connection.execute.call_args[0][0].strip(), - expected_query.strip()) + expected_query.strip(), + ) def test_iter_records_does_iter_records(self): records_to_get = [1, 2, 3] self.fake_connection.execute.return_value.__iter__.return_value = iter( - records_to_get) + records_to_get + ) records = [r for r in self.reader.iter_records('fake_id')] self.assertEqual(records, records_to_get) @@ -572,10 +651,8 @@ def test_can_serialize_basic_types(self): 'string': 'foo', 'int': 4, 'list': [1, 2, 'bar'], - 'dict': { - 'sun': 'moon' - }, - 'float': 1.2 + 'dict': {'sun': 'moon'}, + 'float': 1.2, } string_value = json.dumps(original, cls=PayloadSerializer) reloaded = json.loads(string_value) @@ -589,9 +666,7 @@ def test_can_serialize_datetime(self): self.assertEqual(iso_now, reloaded) def test_can_serialize_case_insensitive_dict(self): - original = CaseInsensitiveDict({ - 'fOo': 'bar' - }) + original = CaseInsensitiveDict({'fOo': 'bar'}) string_value = json.dumps(original, cls=PayloadSerializer) reloaded = json.loads(string_value) self.assertEqual(original, reloaded) @@ -645,8 +720,8 @@ def test_can_serialize_non_utf_8_bytes_nested(self): 'list': ['foo', b'\xfe\xed'], 'more_nesting': { 'bytes': b'\xfe\xed', - 'tuple': ('bar', 'baz', b'\xfe\ed') - } + 'tuple': ('bar', 'baz', b'\xfe\ed'), + }, } encoded = { 'foo': 'bar', @@ -654,8 +729,8 @@ def test_can_serialize_non_utf_8_bytes_nested(self): 'list': ['foo', ''], 'more_nesting': { 'bytes': '', - 'tuple': ['bar', 'baz', ''] - } + 'tuple': ['bar', 'baz', ''], + }, } string_value = json.dumps(original, cls=PayloadSerializer) reloaded = json.loads(string_value) @@ -664,8 +739,7 @@ def test_can_serialize_non_utf_8_bytes_nested(self): class TestRecordBuilder(unittest.TestCase): UUID_PATTERN = re.compile( - '^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', - re.I + '^[0-9a-f]{8}-([0-9a-f]{4}-){3}[0-9a-f]{12}$', re.I ) def setUp(self): @@ -714,7 +788,8 @@ def test_can_get_request_id_from_api_call(self): def test_does_get_id_for_http_request_with_api_call(self): call_identifier = self._get_request_id_for_event_type('API_CALL') request_identifier = self._get_request_id_for_event_type( - 'HTTP_REQUEST') + 'HTTP_REQUEST' + ) self.assertEqual(call_identifier, request_identifier) self.assertTrue(self.UUID_PATTERN.match(call_identifier)) @@ -722,7 +797,8 @@ def test_does_get_id_for_http_request_with_api_call(self): def test_does_get_id_for_http_response_with_api_call(self): call_identifier = self._get_request_id_for_event_type('API_CALL') response_identifier = self._get_request_id_for_event_type( - 'HTTP_RESPONSE') + 'HTTP_RESPONSE' + ) self.assertEqual(call_identifier, response_identifier) self.assertTrue(self.UUID_PATTERN.match(call_identifier)) @@ -730,7 +806,8 @@ def test_does_get_id_for_http_response_with_api_call(self): def test_does_get_id_for_parsed_response_with_api_call(self): call_identifier = self._get_request_id_for_event_type('API_CALL') response_identifier = self._get_request_id_for_event_type( - 'PARSED_RESPONSE') + 'PARSED_RESPONSE' + ) self.assertEqual(call_identifier, response_identifier) self.assertTrue(self.UUID_PATTERN.match(call_identifier)) @@ -755,29 +832,38 @@ def setUp(self): def _get_multiple_request_ids(self, events): fake_payload = {'body': b''} request_ids = [ - self.builder.build_record( - event, - fake_payload.copy(), - '' - )['request_id'] + self.builder.build_record(event, fake_payload.copy(), '')[ + 'request_id' + ] for event in events ] return request_ids def test_multiple_http_lifecycle_writes_have_same_request_id(self): request_ids = self._get_multiple_request_ids( - ['API_CALL', 'HTTP_REQUEST', 'HTTP_RESPONSE', 'PARSED_RESPONSE'] - ) + ['API_CALL', 'HTTP_REQUEST', 'HTTP_RESPONSE', 'PARSED_RESPONSE'] + ) # All request_ids should match since this is one request lifecycle unique_request_ids = set(request_ids) self.assertEqual(len(unique_request_ids), 1) def test_request_id_reset_on_api_call(self): request_ids = self._get_multiple_request_ids( - ['API_CALL', 'HTTP_REQUEST', 'HTTP_RESPONSE', 'PARSED_RESPONSE', - 'API_CALL', 'HTTP_REQUEST', 'HTTP_RESPONSE', 'PARSED_RESPONSE', - 'API_CALL', 'HTTP_REQUEST', 'HTTP_RESPONSE', 'PARSED_RESPONSE'] - ) + [ + 'API_CALL', + 'HTTP_REQUEST', + 'HTTP_RESPONSE', + 'PARSED_RESPONSE', + 'API_CALL', + 'HTTP_REQUEST', + 'HTTP_RESPONSE', + 'PARSED_RESPONSE', + 'API_CALL', + 'HTTP_REQUEST', + 'HTTP_RESPONSE', + 'PARSED_RESPONSE', + ] + ) # There should be three distinct requet_ids since there are three # distinct calls that end with a parsed response. diff --git a/tests/unit/customizations/history/test_history.py b/tests/unit/customizations/history/test_history.py index 2ffae56505a0..2263b3395f6e 100644 --- a/tests/unit/customizations/history/test_history.py +++ b/tests/unit/customizations/history/test_history.py @@ -37,9 +37,12 @@ def tearDown(self): @mock.patch('awscli.customizations.history.sqlite3') @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) - def test_attach_history_handler(self, mock_recorder, mock_db_sqlite3, mock_sqlite3): + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) + def test_attach_history_handler( + self, mock_recorder, mock_db_sqlite3, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.return_value = { 'cli_history': 'enabled' @@ -51,15 +54,18 @@ def test_attach_history_handler(self, mock_recorder, mock_db_sqlite3, mock_sqlit attach_history_handler(session=mock_session, parsed_args=parsed_args) self.assertEqual(mock_recorder.add_handler.call_count, 1) self.assertIsInstance( - mock_recorder.add_handler.call_args[0][0], DatabaseHistoryHandler) + mock_recorder.add_handler.call_args[0][0], DatabaseHistoryHandler + ) self.assertTrue(mock_db_sqlite3.connect.called) @mock.patch('awscli.customizations.history.sqlite3') @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) def test_no_attach_history_handler_when_history_not_configured( - self, mock_recorder, mock_db_sqlite3, mock_sqlite3): + self, mock_recorder, mock_db_sqlite3, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.return_value = {} @@ -72,10 +78,12 @@ def test_no_attach_history_handler_when_history_not_configured( @mock.patch('awscli.customizations.history.sqlite3') @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) def test_no_attach_history_handler_when_command_is_history( - self, mock_recorder, mock_db_sqlite3, mock_sqlite3): + self, mock_recorder, mock_db_sqlite3, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.return_value = { 'cli_history': 'enabled' @@ -90,10 +98,12 @@ def test_no_attach_history_handler_when_command_is_history( @mock.patch('awscli.customizations.history.sqlite3', None) @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) def test_no_attach_history_handler_when_no_sqlite3( - self, mock_recorder, mock_sqlite3): + self, mock_recorder, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.return_value = { 'cli_history': 'enabled' @@ -104,18 +114,22 @@ def test_no_attach_history_handler_when_no_sqlite3( with mock.patch('sys.stderr', StringIO()) as mock_stderr: attach_history_handler( - session=mock_session, parsed_args=parsed_args) + session=mock_session, parsed_args=parsed_args + ) self.assertIn( - 'enabled but sqlite3 is unavailable', mock_stderr.getvalue()) + 'enabled but sqlite3 is unavailable', mock_stderr.getvalue() + ) self.assertFalse(mock_recorder.add_handler.called) self.assertFalse(mock_sqlite3.connect.called) @mock.patch('awscli.customizations.history.sqlite3') @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) - def test_create_directory_no_exists(self, mock_recorder, - mock_db_sqlite3, mock_sqlite3): + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) + def test_create_directory_no_exists( + self, mock_recorder, mock_db_sqlite3, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.return_value = { 'cli_history': 'enabled' @@ -128,7 +142,8 @@ def test_create_directory_no_exists(self, mock_recorder, db_filename = os.path.join(directory_to_create, 'name.db') with mock.patch('os.environ', {'AWS_CLI_HISTORY_FILE': db_filename}): attach_history_handler( - session=mock_session, parsed_args=parsed_args) + session=mock_session, parsed_args=parsed_args + ) self.assertEqual(mock_recorder.add_handler.call_count, 1) # Is should create any missing parent directories of the # file as well. @@ -137,13 +152,16 @@ def test_create_directory_no_exists(self, mock_recorder, @mock.patch('awscli.customizations.history.sqlite3') @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) - def test_profile_does_not_exist(self, mock_recorder, mock_db_sqlite3, - mock_sqlite3): + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) + def test_profile_does_not_exist( + self, mock_recorder, mock_db_sqlite3, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.side_effect = ProfileNotFound( - profile='no-exist') + profile='no-exist' + ) parsed_args = argparse.Namespace() parsed_args.command = 'configure' @@ -155,10 +173,12 @@ def test_profile_does_not_exist(self, mock_recorder, mock_db_sqlite3, @skip_if_windows @mock.patch('awscli.customizations.history.sqlite3') @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) def test_create_directory_with_secure_permissions( - self, mock_recorder, mock_db_sqlite3, mock_sqlite3): + self, mock_recorder, mock_db_sqlite3, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.return_value = { 'cli_history': 'enabled' @@ -171,17 +191,20 @@ def test_create_directory_with_secure_permissions( db_filename = os.path.join(directory_to_create, 'name.db') with mock.patch('os.environ', {'AWS_CLI_HISTORY_FILE': db_filename}): attach_history_handler( - session=mock_session, parsed_args=parsed_args) + session=mock_session, parsed_args=parsed_args + ) dir_mode = stat.S_IMODE(os.stat(directory_to_create).st_mode) assert dir_mode == 0o700 @skip_if_windows('File permissions are not supported on Windows') @mock.patch('awscli.customizations.history.sqlite3') @mock.patch('awscli.customizations.history.db.sqlite3') - @mock.patch('awscli.customizations.history.HISTORY_RECORDER', - spec=HistoryRecorder) + @mock.patch( + 'awscli.customizations.history.HISTORY_RECORDER', spec=HistoryRecorder + ) def test_tighten_existing_directory_permissions( - self, mock_recorder, mock_db_sqlite3, mock_sqlite3): + self, mock_recorder, mock_db_sqlite3, mock_sqlite3 + ): mock_session = mock.Mock(Session) mock_session.get_scoped_config.return_value = { 'cli_history': 'enabled' @@ -195,7 +218,8 @@ def test_tighten_existing_directory_permissions( db_filename = os.path.join(directory_to_create, 'name.db') with mock.patch('os.environ', {'AWS_CLI_HISTORY_FILE': db_filename}): attach_history_handler( - session=mock_session, parsed_args=parsed_args) + session=mock_session, parsed_args=parsed_args + ) dir_mode = stat.S_IMODE(os.stat(directory_to_create).st_mode) assert dir_mode == 0o700 @@ -204,8 +228,7 @@ class TestAddHistoryCommand(unittest.TestCase): def test_add_history_command(self): command_table = {} mock_session = mock.Mock(Session) - add_history_commands( - command_table=command_table, session=mock_session) + add_history_commands(command_table=command_table, session=mock_session) self.assertIn('history', command_table) self.assertIsInstance(command_table['history'], HistoryCommand) From 9327568aa7c6c49b17ece822ba3f0b628d4ca8fa Mon Sep 17 00:00:00 2001 From: Andrew Asseily Date: Wed, 28 Jan 2026 15:53:10 -0500 Subject: [PATCH 3/3] add changelog entry --- .changes/next-release/enhancement-clihistory-71745.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/next-release/enhancement-clihistory-71745.json diff --git a/.changes/next-release/enhancement-clihistory-71745.json b/.changes/next-release/enhancement-clihistory-71745.json new file mode 100644 index 000000000000..8bc65bcbfa57 --- /dev/null +++ b/.changes/next-release/enhancement-clihistory-71745.json @@ -0,0 +1,5 @@ +{ + "type": "enhancement", + "category": "cli-history", + "description": "Create local history files with specific permissions" +}