Tighten output file permissions (#10196)

* Set owner-only (0600) permissions on streaming and S3 Select output files

* Add tests to verify output file permissions are set to 0600

* Add changelog entry for file permissions

* Use os.chmod instead of os.fchmod

* Add comment explaining permission bitmask in tests

Author: AndrewAsseily

.changes/next-release/bugfix-s3streamingoutput-59585.json

@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "s3, streaming output",
+  "description": "Output files created by S3 Select and streaming output commands are now created with owner-only permissions (0600). Existing files are also tightened to 0600 when overwritten."
+}

awscli/customizations/s3events.py

@@ -12,6 +12,8 @@
 # language governing permissions and limitations under the License.
 """Add S3 specific event streaming output arg."""
 
+import os
+
 from awscli.arguments import CustomArgument
 
 STREAM_HELP_TEXT = 'Filename where the records will be saved'
@@ -59,8 +61,7 @@ def replace_event_stream_docs(help_command, **kwargs):
             # This should never happen, but in the rare case that it does
             # we should be raising something with a helpful error message.
             raise DocSectionNotFoundError(
-                'Could not find the "output" section for the command: %s'
-                % help_command
+                f'Could not find the "output" section for the command: {help_command}'
             )
     doc.write('======\nOutput\n======\n')
     doc.write(
@@ -98,7 +99,7 @@ class S3SelectStreamOutputArgument(CustomArgument):
     _DOCUMENT_AS_REQUIRED = True
 
     def __init__(self, stream_key, session, **kwargs):
-        super(S3SelectStreamOutputArgument, self).__init__(**kwargs)
+        super().__init__(**kwargs)
         # This is the key in the response body where we can find the
         # streamed contents.
         self._stream_key = stream_key
@@ -120,7 +121,11 @@ def save_file(self, parsed, **kwargs):
         if self._stream_key not in parsed:
             return
         event_stream = parsed[self._stream_key]
-        with open(self._output_file, 'wb') as fp:
+        fd = os.open(
+            self._output_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600
+        )
+        os.chmod(self._output_file, 0o600)
+        with os.fdopen(fd, 'wb') as fp:
             for event in event_stream:
                 if 'Records' in event:
                     fp.write(event['Records']['Payload'])

awscli/customizations/streamingoutputarg.py

@@ -10,6 +10,8 @@
 # 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 os
+
 from botocore.model import Shape
 
 from awscli.arguments import BaseCLIArgument
@@ -92,7 +94,7 @@ def add_to_params(self, parameters, value):
         service_id = self._operation_model.service_model.service_id.hyphenize()
         operation_name = self._operation_model.name
         self._session.register(
-            'after-call.%s.%s' % (service_id, operation_name), self.save_file
+            f'after-call.{service_id}.{operation_name}', self.save_file
         )
 
     def save_file(self, parsed, **kwargs):
@@ -104,7 +106,11 @@ def save_file(self, parsed, **kwargs):
             return
         body = parsed[self._response_key]
         buffer_size = self._buffer_size
-        with open(self._output_file, 'wb') as fp:
+        fd = os.open(
+            self._output_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600
+        )
+        os.chmod(self._output_file, 0o600)
+        with os.fdopen(fd, 'wb') as fp:
             data = body.read(buffer_size)
             while data:
                 fp.write(data)

tests/functional/s3api/test_select_object_content.py

@@ -15,19 +15,23 @@
 import shutil
 import tempfile
 
-from awscli.testutils import BaseAWSCommandParamsTest, BaseAWSHelpOutputTest
+from awscli.testutils import (
+    BaseAWSCommandParamsTest,
+    BaseAWSHelpOutputTest,
+    skip_if_windows,
+)
 
 
 class TestGetObject(BaseAWSCommandParamsTest):
     prefix = ['s3api', 'select-object-content']
 
     def setUp(self):
-        super(TestGetObject, self).setUp()
+        super().setUp()
         self.parsed_response = {'Payload': self.create_fake_payload()}
         self._tempdir = tempfile.mkdtemp()
 
     def tearDown(self):
-        super(TestGetObject, self).tearDown()
+        super().tearDown()
         shutil.rmtree(self._tempdir)
 
     def create_fake_payload(self):
@@ -82,6 +86,28 @@ def test_can_stream_to_file(self):
             contents = f.read()
             self.assertEqual(contents, ('a,b,c,d\n' 'e,f,g,h\n'))
 
+    @skip_if_windows('chmod is not supported on Windows')
+    def test_output_file_permissions(self):
+        filename = os.path.join(self._tempdir, 'outfile_perms')
+        cmdline = self.prefix + [
+            '--bucket',
+            'mybucket',
+            '--key',
+            'mykey',
+            '--expression',
+            'SELECT * FROM S3Object',
+            '--expression-type',
+            'SQL',
+            '--input-serialization',
+            '{"CSV": {}}',
+            '--output-serialization',
+            '{"CSV": {}}',
+            filename,
+        ]
+        self.assert_params_for_cmd(cmdline, ignore_params=True)
+        # Mask file type bits to isolate permission bits (rwxrwxrwx)
+        self.assertEqual(os.stat(filename).st_mode & 0o777, 0o600)
+
     def test_errors_are_propagated(self):
         self.http_response.status_code = 400
         self.parsed_response = {

tests/functional/test_streaming_output.py

@@ -11,17 +11,23 @@
 # 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 os
+
 from awscli.compat import BytesIO
-from awscli.testutils import BaseAWSCommandParamsTest, FileCreator
+from awscli.testutils import (
+    BaseAWSCommandParamsTest,
+    FileCreator,
+    skip_if_windows,
+)
 
 
 class TestStreamingOutput(BaseAWSCommandParamsTest):
     def setUp(self):
-        super(TestStreamingOutput, self).setUp()
+        super().setUp()
         self.files = FileCreator()
 
     def tearDown(self):
-        super(TestStreamingOutput, self).tearDown()
+        super().tearDown()
         self.files.remove_all()
 
     def test_get_media_streaming_output(self):
@@ -41,3 +47,18 @@ def test_get_media_streaming_output(self):
         self.assert_params_for_cmd(cmdline % outpath, params)
         with open(outpath, 'rb') as outfile:
             self.assertEqual(outfile.read(), b'testbody')
+
+    @skip_if_windows('chmod is not supported on Windows')
+    def test_streaming_output_file_permissions(self):
+        cmdline = (
+            'kinesis-video-media get-media --stream-name test-stream '
+            '--start-selector StartSelectorType=EARLIEST %s'
+        )
+        self.parsed_response = {
+            'ContentType': 'video/webm',
+            'Payload': BytesIO(b'testbody'),
+        }
+        outpath = self.files.full_path('outfile')
+        self.assert_params_for_cmd(cmdline % outpath, ignore_params=True)
+        # Mask file type bits to isolate permission bits (rwxrwxrwx)
+        self.assertEqual(os.stat(outpath).st_mode & 0o777, 0o600)