diff --git a/tensorboard/manager.py b/tensorboard/manager.py index 525b91cf85..38d4145e37 100644 --- a/tensorboard/manager.py +++ b/tensorboard/manager.py @@ -239,6 +239,8 @@ def _get_info_dir(): pass else: raise + else: + os.chmod(path, 0o777) return path diff --git a/tensorboard/manager_test.py b/tensorboard/manager_test.py index 7d2fc5cfea..ada20691c2 100644 --- a/tensorboard/manager_test.py +++ b/tensorboard/manager_test.py @@ -268,6 +268,24 @@ def setUp(self): def _list_info_dir(self): return os.listdir(self.info_dir) + def assertMode(self, path, expected): + """Assert that the permission bits of a file are as expected. + + Args: + path: File to stat. + expected: `int`; a subset of 0o777. + + Raises: + AssertionError: If the permissions bits of `path` do not match + `expected`. + """ + stat_result = os.stat(path) + format_mode = lambda m: "0o%03o" % m + self.assertEqual( + format_mode(stat_result.st_mode & 0o777), + format_mode(expected), + ) + def test_fails_if_info_dir_name_is_taken_by_a_regular_file(self): os.rmdir(self.info_dir) with open(self.info_dir, "w") as outfile: @@ -276,6 +294,43 @@ def test_fails_if_info_dir_name_is_taken_by_a_regular_file(self): manager._get_info_dir() self.assertEqual(cm.exception.errno, errno.EEXIST, cm.exception) + @mock.patch("os.getpid", lambda: 76540) + def test_directory_world_accessible(self): + """Test that the TensorBoardInfo directory is world-accessible. + + Regression test for issue #2010: + + """ + if os.name == "nt": + self.skipTest("Windows does not use POSIX-style permissions.") + os.rmdir(self.info_dir) + # The default umask is typically 0o022, in which case this test is + # nontrivial. In the unlikely case that the umask is 0o000, we'll + # still be covered by the "restrictive umask" test case below. + manager.write_info_file(_make_info()) + self.assertMode(self.info_dir, 0o777) + self.assertEqual(self._list_info_dir(), ["pid-76540.info"]) + + @mock.patch("os.getpid", lambda: 76540) + def test_writing_file_with_restrictive_umask(self): + if os.name == "nt": + self.skipTest("Windows does not use POSIX-style permissions.") + os.rmdir(self.info_dir) + # Even if umask prevents owner-access, our I/O should still work. + old_umask = os.umask(0o777) + try: + # Sanity-check that, without special accommodation, this would + # create inaccessible directories... + sanity_dir = os.path.join(self.get_temp_dir(), "canary") + os.mkdir(sanity_dir) + self.assertMode(sanity_dir, 0o000) + + manager.write_info_file(_make_info()) + self.assertMode(self.info_dir, 0o777) + self.assertEqual(self._list_info_dir(), ["pid-76540.info"]) + finally: + self.assertEqual(oct(os.umask(old_umask)), oct(0o777)) + @mock.patch("os.getpid", lambda: 76540) def test_write_remove_info_file(self): info = _make_info()