From 1823a9e0eaa60ab5cc7515ff5aa32d191c80cc13 Mon Sep 17 00:00:00 2001 From: Stefan Berger Date: Tue, 15 Jul 2025 12:04:52 -0500 Subject: [PATCH] Use a lock file to avoid exceptions due to concurrenct symlink creation We have seen exceptions being raised from _update_root_symlink() on the level of the sigstore-python library when multiple concurrent threads were creating symlinks in this function with the same symlink name (in a test environment running tests concurrently). To avoid this issue, have each thread open a lock file and create an exclusive lock on it to serialize the access to the removal and creation of the symlink. The reproducer for this issue, that should be run in 2 or more python interpreters concurrently, looks like this: from sigstore import sign while True: sign.TrustedRoot.production() Use fcntl.lockf-based locking for Linux and Mac and a different implementation on Windows. The source originally comes from a discussion on stockoverflow (link below). Resolves: https://github.com/theupdateframework/python-tuf/issues/2836 Link: https://stackoverflow.com/questions/489861/locking-a-file-in-python Signed-off-by: Stefan Berger --- tuf/ngclient/_internal/file_lock.py | 26 ++++++++++++++++++++++++++ tuf/ngclient/updater.py | 26 ++++++++++++++++++++------ 2 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 tuf/ngclient/_internal/file_lock.py diff --git a/tuf/ngclient/_internal/file_lock.py b/tuf/ngclient/_internal/file_lock.py new file mode 100644 index 0000000000..a764f44a77 --- /dev/null +++ b/tuf/ngclient/_internal/file_lock.py @@ -0,0 +1,26 @@ +# Copyright 2025, New York University and the TUF contributors +# SPDX-License-Identifier: MIT OR Apache-2.0 + +"""Platform-specific support for file locking + +""" + +import os +import sys + + +if sys.platform in ['win32']: + def create_lockfile(filename: str) -> str: + # Use the name of the file but create lockfile in %TEMP%. + fn = os.path.basename(filename) + lockfile = os.path.join(os.getenv("TEMP"), "tuf" + fn) + #try: + # with open(filename, "w+") as f: + # f.truncate(1) + # f.flush() + #except: + # pass + return lockfile +else: + def create_lockfile(filename: str) -> str: + return filename diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index a98e799ce4..fd48ae122c 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -62,8 +62,11 @@ from typing import TYPE_CHECKING, cast from urllib import parse +from filelock import FileLock + from tuf.api import exceptions from tuf.api.metadata import Root, Snapshot, TargetFile, Targets, Timestamp +from tuf.ngclient._internal.file_lock import create_lockfile from tuf.ngclient._internal.trusted_metadata_set import TrustedMetadataSet from tuf.ngclient.config import EnvelopeType, UpdaterConfig from tuf.ngclient.urllib3_fetcher import Urllib3Fetcher @@ -348,7 +351,11 @@ def _persist_file(self, filename: str, data: bytes) -> None: ) as temp_file: temp_file_name = temp_file.name temp_file.write(data) - os.replace(temp_file.name, filename) + #print(f"filename: {filename}") + + lck = os.path.join(self._dir, ".root.json.lck") + with FileLock(create_lockfile(lck)): + os.replace(temp_file.name, filename) except OSError as e: # remove tempfile if we managed to create one, # then let the exception happen @@ -362,9 +369,13 @@ def _update_root_symlink(self) -> None: linkname = os.path.join(self._dir, "root.json") version = self._trusted_set.root.version current = os.path.join("root_history", f"{version}.root.json") - with contextlib.suppress(FileNotFoundError): - os.remove(linkname) - os.symlink(current, linkname) + + #print(f"lck:{lck}") + lck = os.path.join(self._dir, ".root.json.lck") + with FileLock(create_lockfile(lck)): + with contextlib.suppress(FileNotFoundError): + os.remove(linkname) + os.symlink(current, linkname) def _load_root(self) -> None: """Load root metadata. @@ -375,6 +386,7 @@ def _load_root(self) -> None: If metadata is loaded from remote repository, store it in local cache. """ + Path(self._dir).mkdir(exist_ok=True, parents=True) # Update the root role lower_bound = self._trusted_set.root.version + 1 upper_bound = lower_bound + self.config.max_root_rotations @@ -386,8 +398,10 @@ def _load_root(self) -> None: root_path = os.path.join( self._dir, "root_history", f"{next_version}.root.json" ) - with open(root_path, "rb") as f: - self._trusted_set.update_root(f.read()) + lck = os.path.join(self._dir, ".root.json.lck") + with FileLock(create_lockfile(lck)): + with open(root_path, "rb") as f: + self._trusted_set.update_root(f.read()) continue except (OSError, exceptions.RepositoryError) as e: # this root did not exist locally or is invalid