diff --git a/src/skillspector/file_output.py b/src/skillspector/file_output.py index 8e44d0a76..507bab413 100644 --- a/src/skillspector/file_output.py +++ b/src/skillspector/file_output.py @@ -19,6 +19,7 @@ import errno import os +import sys from collections.abc import Iterator from contextlib import contextmanager from pathlib import Path @@ -46,6 +47,44 @@ def require_secure_file_output() -> None: ) +def restrict_temporary_access(descriptor: int, *, output_kind: str) -> None: + """Remove inherited access before a temporary file receives output data.""" + if os.name == "posix": + # Also masks named-user/group ACL grants on POSIX ACL implementations. + os.fchmod(descriptor, 0o600) + if sys.platform != "darwin": + return + + # macOS extended ACL grants are independent of permission bits. Use the + # already-open descriptor so clearing them cannot follow a swapped path. + import ctypes + + try: + libc = ctypes.CDLL(None, use_errno=True) + init_acl = libc.acl_init + set_acl = libc.acl_set_fd_np + free_acl = libc.acl_free + except AttributeError: + # Filesystems/platforms without extended ACL support need only mode bits. + return + init_acl.argtypes = [ctypes.c_int] + init_acl.restype = ctypes.c_void_p + set_acl.argtypes = [ctypes.c_int, ctypes.c_void_p, ctypes.c_int] + set_acl.restype = ctypes.c_int + free_acl.argtypes = [ctypes.c_void_p] + free_acl.restype = ctypes.c_int + empty_acl = init_acl(0) + if not empty_acl: + raise OSError(ctypes.get_errno(), f"Could not initialize {output_kind} ACL") + try: + if set_acl(descriptor, empty_acl, 0x100) != 0: # ACL_TYPE_EXTENDED + error = ctypes.get_errno() + if error not in {errno.ENOTSUP, errno.EOPNOTSUPP}: + raise OSError(error, f"Could not clear inherited {output_kind} ACLs") + finally: + free_acl(empty_acl) + + @contextmanager def open_output_parent(path: str | Path) -> Iterator[tuple[Path, int]]: """Keep a no-follow parent descriptor open for all operations on an output.""" @@ -118,6 +157,7 @@ def write_text_no_follow(path: str | Path, text: str) -> None: ) temporary_name = candidate with os.fdopen(fd, "w", encoding="utf-8") as stream: + restrict_temporary_access(stream.fileno(), output_kind="report") stream.write(text) os.replace( temporary_name, diff --git a/src/skillspector/suppression.py b/src/skillspector/suppression.py index b2eabcaa6..698359298 100644 --- a/src/skillspector/suppression.py +++ b/src/skillspector/suppression.py @@ -72,7 +72,7 @@ import yaml -from skillspector.file_output import open_output_parent +from skillspector.file_output import open_output_parent, restrict_temporary_access from skillspector.logging_config import get_logger from skillspector.models import Finding @@ -656,40 +656,7 @@ def build_baseline_dict( def _restrict_baseline_temporary(descriptor: int) -> None: """Remove inherited access before a temporary file receives baseline data.""" - if os.name == "posix": - # Also masks named-user/group ACL grants on POSIX ACL implementations. - os.fchmod(descriptor, 0o600) - if sys.platform != "darwin": - return - - # macOS extended ACL grants are independent of permission bits. Use the - # already-open descriptor so clearing them cannot follow a swapped path. - import ctypes - - try: - libc = ctypes.CDLL(None, use_errno=True) - init_acl = libc.acl_init - set_acl = libc.acl_set_fd_np - free_acl = libc.acl_free - except AttributeError: - # Filesystems/platforms without extended ACL support need only mode bits. - return - init_acl.argtypes = [ctypes.c_int] - init_acl.restype = ctypes.c_void_p - set_acl.argtypes = [ctypes.c_int, ctypes.c_void_p, ctypes.c_int] - set_acl.restype = ctypes.c_int - free_acl.argtypes = [ctypes.c_void_p] - free_acl.restype = ctypes.c_int - empty_acl = init_acl(0) - if not empty_acl: - raise OSError(ctypes.get_errno(), "Could not initialize baseline ACL") - try: - if set_acl(descriptor, empty_acl, 0x100) != 0: # ACL_TYPE_EXTENDED - error = ctypes.get_errno() - if error not in {errno.ENOTSUP, errno.EOPNOTSUPP}: - raise OSError(error, "Could not clear inherited baseline ACLs") - finally: - free_acl(empty_acl) + restrict_temporary_access(descriptor, output_kind="baseline") def _preserve_baseline_acl(source: int, destination: int) -> None: diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index df03c596f..2cccc5aeb 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -7977,6 +7977,96 @@ def fail_replace(*args, **kwargs): assert not list(tmp_path.glob(".skillspector-output-*")) +@pytest.mark.skipif(os.name != "posix", reason="requires POSIX descriptors") +@pytest.mark.parametrize("existing", [False, True]) +def test_report_output_restricts_access_before_writing(tmp_path, monkeypatch, existing): + from skillspector import file_output + + output = tmp_path / "report.json" + if existing: + output.write_text("previous", encoding="utf-8") + original_restrict = file_output.restrict_temporary_access + target_sizes = [] + + def restrict_empty_file(descriptor, *, output_kind): + target_sizes.append(os.fstat(descriptor).st_size) + original_restrict(descriptor, output_kind=output_kind) + + monkeypatch.setattr(file_output, "restrict_temporary_access", restrict_empty_file) + file_output.write_text_no_follow(output, "private report") + + assert target_sizes == [0] + assert output.read_text(encoding="utf-8") == "private report" + assert output.stat().st_mode & 0o777 == 0o600 + assert not list(tmp_path.glob(".skillspector-output-*")) + + +@pytest.mark.skipif(os.name != "posix", reason="requires POSIX descriptors") +@pytest.mark.parametrize("existing", [False, True]) +def test_report_output_preserves_destination_when_acl_removal_fails( + tmp_path, monkeypatch, existing +): + import ctypes + import errno + + from skillspector import file_output + + output = tmp_path / "report.json" + if existing: + output.write_text("previous", encoding="utf-8") + target_sizes = [] + + def fail_set_acl(descriptor, acl, acl_type): + target_sizes.append(os.fstat(descriptor).st_size) + ctypes.set_errno(errno.EACCES) + return -1 + + monkeypatch.setattr(sys, "platform", "darwin") + monkeypatch.setattr( + ctypes, + "CDLL", + lambda *args, **kwargs: SimpleNamespace( + acl_init=lambda count: 1, acl_set_fd_np=fail_set_acl, acl_free=lambda acl: 0 + ), + ) + with pytest.raises(ValueError, match="clear inherited report ACLs"): + file_output.write_text_no_follow(output, "private report") + + assert target_sizes == [0] + if existing: + assert output.read_text(encoding="utf-8") == "previous" + else: + assert not output.exists() + assert not list(tmp_path.glob(".skillspector-output-*")) + + +@pytest.mark.skipif(sys.platform != "darwin", reason="requires macOS inherited ACLs") +@pytest.mark.parametrize("mode", [None, 0o600, 0o200]) +def test_report_output_clears_inherited_macos_acl(tmp_path, mode): + import subprocess + + from skillspector import file_output + + output = tmp_path / "report.json" + if mode is not None: + output.write_text("previous", encoding="utf-8") + output.chmod(mode) + subprocess.run( + ["/bin/chmod", "+a", "everyone allow read,file_inherit", str(tmp_path)], + check=True, + capture_output=True, + ) + + file_output.write_text_no_follow(output, "private report") + + acl_listing = subprocess.run( + ["/bin/ls", "-le", str(output)], check=True, capture_output=True, text=True + ).stdout + assert "everyone" not in acl_listing + assert output.stat().st_mode & 0o777 == 0o600 + assert output.read_text(encoding="utf-8") == "private report" + + def test_report_output_unsupported_platform_keeps_stdout(tmp_path, monkeypatch): from skillspector import file_output