Fix insecure file permissions when writing credentials in CodeArtifact login.py

Author: George Zefi

.changes/next-release/bugfix-CodeArtifact-90120.json

@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "CodeArtifact",
+  "description": "Tighten file permissions when writing credentials in CodeArtifact login"
+}

awscli/customizations/codeartifact/login.py

@@ -183,7 +183,17 @@ def _update_netrc_entry(self, hostname, new_entry, netrc_path):
             if new_contents == contents:
                 new_contents = self._append_netrc_entry(new_contents, new_entry)
 
-            with open(netrc_path, 'w') as f:
+            fd = os.open(netrc_path,
+                         os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+           
+            try:
+                os.chmod(fd, 0o600) # Ensure secure perms on pre-existing files
+            except OSError as e:
+                uni_print('Unable to set file permissions '
+                          'for %s: %s%s' % (netrc_path, e, os.linesep),
+                          sys.stderr)
+
+            with os.fdopen(fd, 'w') as f:
                 f.write(new_contents)
 
     def _create_netrc_file(self, netrc_path, new_entry):
@@ -631,7 +641,17 @@ def login(self, dry_run=False):
             sys.stdout.write(pypi_rc_str)
             sys.stdout.write(os.linesep)
         else:
-            with open(self.pypi_rc_path, 'w+') as fp:
+            fd = os.open(self.pypi_rc_path,
+                         os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+          
+            try:
+                os.chmod(fd, 0o600) # Ensure secure perms on pre-existing files
+            except OSError as e:
+                uni_print('Unable to set file permissions '
+                          'for %s: %s%s' % (self.pypi_rc_path, e, os.linesep), 
+                          sys.stderr)
+            
+            with os.fdopen(fd, 'w') as fp:
                 fp.write(pypi_rc_str)
 
             self._write_success_message('twine')

tests/unit/customizations/codeartifact/test_adapter_login.py

@@ -435,6 +435,38 @@ def test_login_dry_run(self):
         self.subprocess_utils.check_output.assert_not_called()
         self.assertFalse(os.path.exists(self.test_netrc_path))
 
+    @mock.patch('awscli.customizations.codeartifact.login.is_macos', False)
+    def test_login_adjusts_permissions_on_preexisting_file(self):
+        existing_content = (
+            f'machine {self.hostname} login token password old-token\n'
+        )
+        with open(self.test_netrc_path, 'w') as f:
+            f.write(existing_content)
+        os.chmod(self.test_netrc_path, 0o644)
+        self.test_subject.login()
+        file_mode = os.stat(self.test_netrc_path).st_mode
+        self.assertEqual(stat.S_IMODE(file_mode), 0o600)
+
+    @mock.patch('awscli.customizations.codeartifact.login.is_macos', False)
+    @mock.patch('sys.stderr')
+    @mock.patch('os.chmod', side_effect=OSError(
+        errno.EPERM, 'Operation not permitted'
+    ))
+    def test_login_writes_error_when_chmod_fails(
+        self, mock_chmod, mock_stderr
+    ):
+        existing_content = (
+            f'machine {self.hostname} login token password old-token\n'
+        )
+        with open(self.test_netrc_path, 'w') as f:
+            f.write(existing_content)
+        self.test_subject.login()
+        mock_stderr.write.assert_any_call(
+            'Unable to set file permissions for %s: '
+            '[Errno 1] Operation not permitted%s'
+            % (self.test_netrc_path, os.linesep)
+        )
+
 
 class TestNuGetLogin(unittest.TestCase):
     _NUGET_INDEX_URL_FMT = NuGetLogin._NUGET_INDEX_URL_FMT
@@ -1246,6 +1278,42 @@ def test_login_existing_pypi_rc_with_codeartifact_not_clobbered(self):
             password='JgCXIr5xGG'
         )
 
+    def test_login_sets_secure_permissions_on_new_file(self):
+        self.assertFalse(os.path.exists(self.test_pypi_rc_path))
+        self.test_subject.login()
+        file_mode = os.stat(self.test_pypi_rc_path).st_mode
+        self.assertEqual(stat.S_IMODE(file_mode), 0o600)
+
+    def test_login_adjusts_permissions_on_preexisting_file(self):
+        existing_pypi_rc = '''\
+[distutils]
+index-servers=
+    pypi
+
+[pypi]
+repository: http://www.python.org/pypi/
+username: test
+password: JgCXIr5xGG
+'''
+        with open(self.test_pypi_rc_path, 'w') as f:
+            f.write(existing_pypi_rc)
+        os.chmod(self.test_pypi_rc_path, 0o644)
+        self.test_subject.login()
+        file_mode = os.stat(self.test_pypi_rc_path).st_mode
+        self.assertEqual(stat.S_IMODE(file_mode), 0o600)
+
+    @mock.patch('sys.stderr')
+    @mock.patch('os.chmod', side_effect=OSError(
+        errno.EPERM, 'Operation not permitted'
+    ))
+    def test_login_writes_error_when_chmod_fails(self, mock_chmod, mock_stderr):
+        self.test_subject.login()
+        mock_stderr.write.assert_any_call(
+            'Unable to set file permissions for %s: '
+            '[Errno 1] Operation not permitted%s'
+            % (self.test_pypi_rc_path, os.linesep)
+        )
+
     def test_login_existing_invalid_pypi_rc_error(self):
         # This is an invalid pypirc as the list of servers are expected under
         # an 'index-servers' option instead of 'servers'.