Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix scan dir permission errors #377

Merged
merged 3 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions b2sdk/_internal/scan/folder.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,14 @@ def _walk_relative_paths(
return # Skip if symlink already visited
visited_symlinks.add(inode_number)

for local_path in sorted(local_dir.iterdir()):
try:
dir_children = sorted(local_dir.iterdir())
except PermissionError: # `chmod -r dir` can trigger this
if reporter is not None:
reporter.local_permission_error(str(local_dir))
return

for local_path in dir_children:
name = local_path.name
relative_file_path = join_b2_path(relative_dir_path, name)

Expand All @@ -251,7 +258,14 @@ def _walk_relative_paths(
reporter.invalid_name(str(local_path), str(e))
continue

if local_path.is_dir():
try:
is_dir = local_path.is_dir()
except PermissionError: # `chmod -x dir` can trigger this
if reporter is not None:
reporter.local_permission_error(str(local_path))
continue

if is_dir:
if policies_manager.should_exclude_local_directory(str(relative_file_path)):
continue # Skip excluded directories
# Recurse into directories
Expand Down
2 changes: 2 additions & 0 deletions changelog.d/+scan_perm_errors.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix LocalFolder.all_files(..) erroring out if one of the non-excluded directories is not readable by the user running the scan.
Warning is added to ProgressReport instead as other file access errors are.
26 changes: 26 additions & 0 deletions test/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from __future__ import annotations

import os
import shutil
import sys
from glob import glob
from pathlib import Path
Expand Down Expand Up @@ -192,3 +193,28 @@ def bucket(b2api):
@pytest.fixture
def file_info():
return {'key': 'value'}


@pytest.fixture
def tmp_path_permission_cleanup(tmp_path):
"""
Ensure tmp_path is delete-able after the test.

Important for the tests that mess with filesystem permissions.
"""
yield

try:
shutil.rmtree(tmp_path)
except OSError:
tmp_path.chmod(0o700)
for root, dirs, files in tmp_path.walk(top_down=True):
for name in dirs:
(root / name).chmod(0o700)
for name in files:
(root / name).chmod(0o600)
(root / name).unlink()
for root, dirs, files in tmp_path.walk(top_down=False):
for name in dirs:
(root / name).rmdir()
tmp_path.rmdir()
89 changes: 73 additions & 16 deletions test/unit/scan/test_folder_traversal.py
Original file line number Diff line number Diff line change
Expand Up @@ -682,23 +682,85 @@ def test_excluded_no_access_check(self, tmp_path):

reporter.close()

def test_excluded_without_permissions(self, tmp_path):
@pytest.mark.skipif(
platform.system() == 'Windows',
reason="Unix-only filesystem permissions are tested",
)
def test_dir_without_exec_permission(self, tmp_path, tmp_path_permission_cleanup):
"""Test that a excluded directory/file without permissions emits warnings."""
no_perm_dir = tmp_path / "no_perm_dir"
no_perm_dir.mkdir()
(no_perm_dir / "file.txt").touch()
(no_perm_dir / "file2.txt").touch()
# chmod -x no_perm_dir
no_perm_dir.chmod(0o600)

scan_policy = ScanPoliciesManager()
reporter = ProgressReport(sys.stdout, False)

folder = LocalFolder(str(tmp_path))
local_paths = folder.all_files(reporter=reporter, policies_manager=scan_policy)
absolute_paths = [path.absolute_path for path in local_paths]
assert not absolute_paths

# Check that no access warnings are issued for the excluded directory/file
assert set(reporter.warnings) == {
f'WARNING: {tmp_path}/no_perm_dir/file.txt could not be accessed (no permissions to read?)',
f'WARNING: {tmp_path}/no_perm_dir/file2.txt could not be accessed (no permissions to read?)',
}

reporter.close()

def test_without_permissions(self, tmp_path, tmp_path_permission_cleanup):
"""Test that a excluded directory/file without permissions emits warnings."""
no_perm_dir = tmp_path / "no_perm_dir"
no_perm_dir.mkdir()
(no_perm_dir / "file.txt").touch()

included_dir = tmp_path / "included_dir"
included_dir.mkdir()
(included_dir / "no_perm_file.txt").touch()
(included_dir / "included_file.txt").touch()

# Modify directory permissions to simulate lack of access
(included_dir / "no_perm_file.txt").chmod(0o000)
no_perm_dir.chmod(0o000)

scan_policy = ScanPoliciesManager()
reporter = ProgressReport(sys.stdout, False)

folder = LocalFolder(str(tmp_path))
local_paths = folder.all_files(reporter=reporter, policies_manager=scan_policy)
absolute_paths = [path.absolute_path for path in local_paths]

# Check that only included_dir/included_file.txt was return
assert any('included_file.txt' in path for path in absolute_paths)

# Check that no access warnings are issued for the excluded directory/file
assert set(reporter.warnings) == {
f'WARNING: {tmp_path}/no_perm_dir could not be accessed (no permissions to read?)',
f'WARNING: {tmp_path}/included_dir/no_perm_file.txt could not be accessed (no permissions to read?)'
}

reporter.close()

def test_excluded_without_permissions(self, tmp_path, tmp_path_permission_cleanup):
"""Test that a excluded directory/file without permissions is not processed and no warning is issued."""
excluded_dir = tmp_path / "excluded_dir"
excluded_dir.mkdir()
(excluded_dir / "file.txt").touch()
no_perm_dir = tmp_path / "no_perm_dir"
no_perm_dir.mkdir()
(no_perm_dir / "file.txt").touch()

included_dir = tmp_path / "included_dir"
included_dir.mkdir()
(included_dir / "excluded_file.txt").touch()
(included_dir / "no_perm_file.txt").touch()
(included_dir / "included_file.txt").touch()

# Modify directory permissions to simulate lack of access
(included_dir / "excluded_file.txt").chmod(0o000)
excluded_dir.chmod(0o000)
(included_dir / "no_perm_file.txt").chmod(0o000)
no_perm_dir.chmod(0o000)

scan_policy = ScanPoliciesManager(
exclude_dir_regexes=[r"excluded_dir$"], exclude_file_regexes=[r'.*excluded_file.txt']
exclude_dir_regexes=[r"no_perm_dir$"], exclude_file_regexes=[r'.*no_perm_file.txt']
)
reporter = ProgressReport(sys.stdout, False)

Expand All @@ -707,18 +769,13 @@ def test_excluded_without_permissions(self, tmp_path):
absolute_paths = [path.absolute_path for path in local_paths]

# Restore directory permissions to clean up
(included_dir / "excluded_file.txt").chmod(0o755)
excluded_dir.chmod(0o755)
(included_dir / "no_perm_file.txt").chmod(0o755)
no_perm_dir.chmod(0o755)

# Check that only included_dir/included_file.txt was return
assert any('included_file.txt' in path for path in absolute_paths)

# Check that no access warnings are issued for the excluded directory/file
assert not any(
re.match(
r"WARNING: .*excluded_.* could not be accessed \(no permissions to read\?\)",
warning,
) for warning in reporter.warnings
), "Access warning was issued for the excluded directory/file"
assert not reporter.warnings

reporter.close()
Loading