From 119dff337c97740fe6c2b86d650e0cbe562ca7e1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:32:13 +0000 Subject: [PATCH 1/2] Keep container claims in a private, user-owned directory Claims fell back to a shared, world-writable TMPDIR or /tmp, where another user could pre-create the claim directory or a symlink in its place to plant or delete claims. Fall back via tempfile.gettempdir(), create the directory 0700, and only use it when it is a real directory owned by this user, tightening its permissions if they are open. Co-Authored-By: jason.han --- tig-cli/src/tig_cli/spec.py | 53 +++++++++++++++++++++---- tig-cli/tests/test_container.py | 70 +++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 8 deletions(-) diff --git a/tig-cli/src/tig_cli/spec.py b/tig-cli/src/tig_cli/spec.py index d5027df..9c6fe2c 100644 --- a/tig-cli/src/tig_cli/spec.py +++ b/tig-cli/src/tig_cli/spec.py @@ -11,6 +11,7 @@ import json import os import signal +import stat import sys from pathlib import Path from .config import ( @@ -658,20 +659,53 @@ def __init__(self) -> None: @staticmethod def directory() -> Path: - base = ( - os.environ.get("XDG_RUNTIME_DIR") - or os.environ.get("TMPDIR") - or "/tmp" - ) + base = os.environ.get("XDG_RUNTIME_DIR") + if not base: + # Imported only here: the warm path almost always has a runtime + # directory, and tempfile is comparatively slow to import. + import tempfile + + base = tempfile.gettempdir() return Path(base) / f"{CLAIM_DIR_NAME}-{os.getuid()}" + @classmethod + def _private_directory(cls, create: bool) -> Path | None: + """The claim directory, if it is safe to trust. + + The fallback base is shared and world-writable, so another user could + have made the directory first, or a symlink in its place, to plant or + delete claims. Only a real directory owned by this user and closed to + everyone else is used. + """ + directory = cls.directory() + if create: + try: + directory.mkdir(mode=0o700, parents=True) + except FileExistsError: + pass + except OSError: + return None + try: + info = os.lstat(directory) + except OSError: + return None + if not stat.S_ISDIR(info.st_mode) or info.st_uid != os.getuid(): + return None + if stat.S_IMODE(info.st_mode) & 0o077: + try: + os.chmod(directory, 0o700) + except OSError: + return None + return directory + def acquire(self, container_name: str) -> None: """Record that this process is about to use ``container_name``.""" self.release() - directory = self.directory() + directory = self._private_directory(create=True) + if directory is None: + return claim = directory / f"{container_name}.{os.getpid()}" try: - directory.mkdir(parents=True, exist_ok=True) claim.touch() except OSError: # Unclaimable state directory: reaping then just keeps containers. @@ -696,8 +730,11 @@ def claimed_containers(cls) -> set: found. """ claimed = set() + directory = cls._private_directory(create=False) + if directory is None: + return claimed try: - claims = list(cls.directory().iterdir()) + claims = list(directory.iterdir()) except OSError: return claimed diff --git a/tig-cli/tests/test_container.py b/tig-cli/tests/test_container.py index 773bf51..fffe06b 100644 --- a/tig-cli/tests/test_container.py +++ b/tig-cli/tests/test_container.py @@ -936,6 +936,76 @@ def test_releasing_the_claim_makes_the_container_reapable(home_dir): manager.release_claim() # idempotent +def test_the_claim_directory_is_private_to_the_user(home_dir): + claim = spec.Claim() + claim.acquire("tig-vicar-private") + + info = os.lstat(spec.Claim.directory()) + assert info.st_mode & 0o777 == 0o700 + assert info.st_uid == os.getuid() + assert claim.path.exists() + + +def test_an_open_claim_directory_of_the_users_own_is_closed(home_dir): + directory = spec.Claim.directory() + directory.mkdir(parents=True) + directory.chmod(0o777) + + claim = spec.Claim() + claim.acquire("tig-vicar-open") + + assert os.lstat(directory).st_mode & 0o777 == 0o700 + assert claim.path is not None + + +def test_a_symlink_in_place_of_the_claim_directory_is_not_trusted( + home_dir, tmp_path +): + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + (elsewhere / f"tig-vicar-planted.{os.getpid()}").touch() + spec.Claim.directory().symlink_to(elsewhere) + + claim = spec.Claim() + claim.acquire("tig-vicar-linked") + + assert claim.path is None + assert list(elsewhere.iterdir()) == [ + elsewhere / f"tig-vicar-planted.{os.getpid()}" + ] + assert spec.Claim.claimed_containers() == set() + + +def test_a_claim_directory_owned_by_another_user_is_not_trusted( + home_dir, monkeypatch +): + other = os.getuid() + 1 + monkeypatch.setattr(spec.os, "getuid", lambda: other) + directory = spec.Claim.directory() + directory.mkdir(mode=0o700) + (directory / f"tig-vicar-planted.{os.getpid()}").touch() + + claim = spec.Claim() + claim.acquire("tig-vicar-foreign") + + assert claim.path is None + assert spec.Claim.claimed_containers() == set() + + +def test_claims_fall_back_to_the_temporary_directory( + home_dir, tmp_path, monkeypatch +): + import tempfile + + monkeypatch.delenv("XDG_RUNTIME_DIR", raising=False) + monkeypatch.setenv("TMPDIR", str(tmp_path)) + monkeypatch.setattr(tempfile, "tempdir", None) + + assert spec.Claim.directory() == ( + tmp_path / f"{spec.CLAIM_DIR_NAME}-{os.getuid()}" + ) + + def test_reaping_tolerates_a_container_removed_concurrently(home_dir): runtime = FakeRuntime({ f"{CONTAINER_PREFIX}-a": described(), From f061f3bd59b590c827b2a71a50bf9910907bcdf2 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 11 Nov 2026 18:34:23 +0000 Subject: [PATCH 2/2] Ignore claim files not owned by the user A claim directory that was open to others may hold claims planted before it was closed; only trust regular files the user owns. Co-Authored-By: jason.han --- tig-cli/src/tig_cli/spec.py | 8 ++++++++ tig-cli/tests/test_container.py | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/tig-cli/src/tig_cli/spec.py b/tig-cli/src/tig_cli/spec.py index 9c6fe2c..e3ecfed 100644 --- a/tig-cli/src/tig_cli/spec.py +++ b/tig-cli/src/tig_cli/spec.py @@ -738,10 +738,18 @@ def claimed_containers(cls) -> set: except OSError: return claimed + uid = os.getuid() for claim in claims: name, _, pid = claim.name.rpartition(".") if not pid.isdigit(): continue + try: + info = os.lstat(claim) + except OSError: + continue + if not stat.S_ISREG(info.st_mode) or info.st_uid != uid: + # Planted while the directory was open to others. + continue try: os.kill(int(pid), 0) except ProcessLookupError: diff --git a/tig-cli/tests/test_container.py b/tig-cli/tests/test_container.py index fffe06b..0a07fc5 100644 --- a/tig-cli/tests/test_container.py +++ b/tig-cli/tests/test_container.py @@ -992,6 +992,26 @@ def test_a_claim_directory_owned_by_another_user_is_not_trusted( assert spec.Claim.claimed_containers() == set() +def test_claims_not_owned_by_the_user_are_ignored(home_dir, monkeypatch): + directory = spec.Claim.directory() + directory.mkdir(mode=0o700) + (directory / f"tig-vicar-planted.{os.getpid()}").touch() + (directory / f"tig-vicar-linked.{os.getpid()}").symlink_to(directory) + real_lstat = os.lstat + + def lstat(path): + info = real_lstat(path) + if "planted" in os.fspath(path): + return os.stat_result( + info[:4] + (os.getuid() + 1,) + info[5:] + ) + return info + + monkeypatch.setattr(spec.os, "lstat", lstat) + + assert spec.Claim.claimed_containers() == set() + + def test_claims_fall_back_to_the_temporary_directory( home_dir, tmp_path, monkeypatch ):