Skip to content

Commit

Permalink
Fix overly permissive file permissions in luigi/lock.py
Browse files Browse the repository at this point in the history
Fixes #3303

Update file permissions in `luigi/lock.py` to be more restrictive.

* Change the file permissions of the `pid_dir` directory from `0o777` to `0o700` in the `acquire_for` function.
* Update the test cases `test_acquiring_partially_taken_lock` and `test_acquiring_lock_from_missing_process` in `test/lock_test.py` to check for the new file permissions `0o700`.
  • Loading branch information
Ali-Razmjoo committed Sep 4, 2024
1 parent 74e6e63 commit 67768d1
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion luigi/lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def acquire_for(pid_dir, num_available=1, kill_signal=None):
# Create a pid file if it does not exist
try:
os.mkdir(pid_dir)
os.chmod(pid_dir, 0o777)
os.chmod(pid_dir, 0o700)
except OSError as exc:
if exc.errno != errno.EEXIST:
raise
Expand Down
4 changes: 2 additions & 2 deletions test/lock_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def test_acquiring_partially_taken_lock(self):
self.assertTrue(acquired)

s = os.stat(self.pid_file)
self.assertEqual(s.st_mode & 0o777, 0o777)
self.assertEqual(s.st_mode & 0o700, 0o700)

def test_acquiring_lock_from_missing_process(self):
fake_pid = 99999
Expand All @@ -111,7 +111,7 @@ def test_acquiring_lock_from_missing_process(self):
self.assertTrue(acquired)

s = os.stat(self.pid_file)
self.assertEqual(s.st_mode & 0o777, 0o777)
self.assertEqual(s.st_mode & 0o700, 0o700)

@mock.patch('os.kill')
def test_take_lock_with_kill(self, kill_fn):
Expand Down

0 comments on commit 67768d1

Please sign in to comment.