search

flathub-infra / flatpak-external-data-checker

A tool for checking if the external data used in Flatpak manifests is still up to date
GNU General Public License v2.0
116 stars 34 forks source link

Don't run git against dubious-ownership repo #396

Closed wjt closed 8 months ago

wjt commented 8 months ago

(Initially I just reverted 7d6496d2b71f1b970cee3a5574d23af1155ca6ce but I think this is better.)

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

coveralls commented 8 months ago

Coverage Status

coverage: 91.761% (-0.04%) from 91.801% when pulling ac5a2cab2753eb844b6d53c023d31a5ec4dbdaab on revert-386-git-safe-directory-for-manifest-in-subdirectory into 83e538e6bb99db09ed690735659bd8e139fdbc86 on master.

wjt commented 8 months ago

I really think this is the wrong approach and the container should just be run with a UID matching the owner of the checkout… but perhaps that is one for another day.

wjt commented 8 months ago

I realised I didn't actually test this branch.

wjt commented 8 months ago

I tested this and it seems to find the root of the git checkout correctly!