From 3256b77cd8ac36986e7cdb7652b9a5a112771b0f Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Thu, 9 Nov 2023 12:21:33 +0000 Subject: [PATCH 1/2] Don't run git against dubious-ownership repo Modern git refuses to operate against a repo owned by a different user. This happens in a GitHub action because the 'checkout' action checks out the repo as UID 1001 but this app's container image runs as UID 0. We attempt to detect this "dubious ownership" situation and mark the repo as safe, subverting the check. Since 7d6496d2b71f1b970cee3a5574d23af1155ca6ce we attempt to handle the case where the manifest is not in the root of the git checkout. However, using `git rev-parse --show-toplevel` in this situation does not work because git considers the repo to have dubious ownership (which is what we are trying to solve). Instead, walk the ancestors of the manifest path, looking for a `.git` file. (It need not be a directory: working copies created with `git worktree` have a text file at `.git`, whose contents describe where the main checkout is.) Kludgy but it just might work. Fixes https://github.com/flathub/flatpak-external-data-checker/issues/395 --- src/main.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main.py b/src/main.py index c0a4ce87..b3ef6d3d 100755 --- a/src/main.py +++ b/src/main.py @@ -129,12 +129,14 @@ def check_call(args): def get_manifest_git_checkout(manifest: t.Union[Path, str]) -> str: - manifest_dir = os.path.dirname(manifest) - output = subprocess.check_output( - ["git", "rev-parse", "--show-toplevel"], - cwd=manifest_dir, - ) - return output.decode("utf-8").strip() + # Can't use git rev-parse --show-toplevel because of a chicken-and-egg problem: we + # need to find the checkout directory so that we can mark it as safe so that we can + # use git against it. + for directory in Path(manifest).parents: + if os.path.exists(directory / ".git"): + return str(directory) + + raise FileNotFoundError(f"Cannot find git checkout for {manifest}") def ensure_git_safe_directory(checkout: t.Union[Path, str]): From ac5a2cab2753eb844b6d53c023d31a5ec4dbdaab Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Thu, 9 Nov 2023 12:44:48 +0000 Subject: [PATCH 2/2] Use Path type when manipulating Git checkout Previously ensure_git_safe_directory() was declared to accept either str or Path. In fact, if passed a Path, it would fail to detect the case when the directory is already marked as safe, because Path(x) in [x] is False. Instead use Path throughout. --- src/main.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/main.py b/src/main.py index b3ef6d3d..5e9b8507 100755 --- a/src/main.py +++ b/src/main.py @@ -45,7 +45,7 @@ @contextlib.contextmanager -def indir(path): +def indir(path: Path): """ >>> with indir(path): ... # code executes with 'path' as working directory @@ -128,35 +128,38 @@ def check_call(args): subprocess.check_call(args) -def get_manifest_git_checkout(manifest: t.Union[Path, str]) -> str: +def get_manifest_git_checkout(manifest: t.Union[Path, str]) -> Path: # Can't use git rev-parse --show-toplevel because of a chicken-and-egg problem: we # need to find the checkout directory so that we can mark it as safe so that we can # use git against it. for directory in Path(manifest).parents: if os.path.exists(directory / ".git"): - return str(directory) + return directory raise FileNotFoundError(f"Cannot find git checkout for {manifest}") -def ensure_git_safe_directory(checkout: t.Union[Path, str]): +def ensure_git_safe_directory(checkout: Path): uid = os.getuid() checkout_uid = os.stat(checkout).st_uid if uid == checkout_uid: return try: - safe_dirs_output = subprocess.check_output( - ["git", "config", "--get-all", "safe.directory"] + result = subprocess.run( + ["git", "config", "--get-all", "safe.directory"], + check=True, + capture_output=True, + encoding="utf-8", ) + safe_dirs = [Path(x) for x in result.stdout.splitlines()] except subprocess.CalledProcessError as err: # git config --get-all will return 1 if the key doesn't exist. # Re-raise the error for anything else. if err.returncode != 1: raise - safe_dirs_output = b"" + safe_dirs = [] - safe_dirs = safe_dirs_output.decode("utf-8").split() if checkout in safe_dirs: return