Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce safe directory #762

Merged
merged 2 commits into from Apr 14, 2022
Merged

Enforce safe directory #762

merged 2 commits into from Apr 14, 2022

Conversation

thboop
Copy link
Contributor

@thboop thboop commented Apr 13, 2022

This pr fixes #760 by setting the repositories path as a safe directory when running checkout and removing that config on cleanup.

We aren't able to set it as a local config, as this setting only works for global configs.

So, using the standard we used for submodules, lets go ahead and copy the existing global config to a new location. Then we can go ahead and modify that global config (so we don't modify like a users global config if they test something on a self hosted runner real fast). Then, we can delete that global config at the end of the checkout step.

This does carry some limitations, it doesn't persist this configuration for the duration of the job mainly. So if your job pushes to git or something after checkout, that will continue to fail. We need to figure out how to address this at an ecosystem level, outside of the checkout action.

// Arrange
await setup(removeGlobalAuth_removesOverride)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed this function, changes are a result of that

src/git-auth-helper.ts Outdated Show resolved Hide resolved
@cbalioglu
Copy link

@thboop thanks a lot for taking quick action! Do you have an ETA on when you plan to land this fix?

@thboop thboop mentioned this pull request Apr 14, 2022
liketechnik added a commit to Compiler-CampusMinden/CB-Vorlesung-Bachelor that referenced this pull request Jul 5, 2022
Fixes an issue introduced with a recent git update
(https://github.blog/2022-04-12-git-security-vulnerability-announced/)
with a common workaround (actions/checkout#762,
https://stackoverflow.com/questions/71901632/fatal-error-unsafe-repository-home-repon-is-owned-by-someone-else,
actions/checkout#760),
by marking the /data directory inside the container as safe for git
during the container build.
liketechnik added a commit to Compiler-CampusMinden/CB-Vorlesung-Bachelor that referenced this pull request Jul 5, 2022
Fixes an issue introduced with a recent git update
(https://github.blog/2022-04-12-git-security-vulnerability-announced/)
with a common workaround (actions/checkout#762,
https://stackoverflow.com/questions/71901632/fatal-error-unsafe-repository-home-repon-is-owned-by-someone-else,
actions/checkout#760),
by marking the /data directory inside the container as safe for git
during the container build.
cagix added a commit to Compiler-CampusMinden/CB-Vorlesung-Bachelor that referenced this pull request Jul 18, 2022
* tooling: git safe repo directory for docker image

Fixes an issue introduced with a recent git update
(https://github.blog/2022-04-12-git-security-vulnerability-announced/)
with a common workaround (actions/checkout#762,
https://stackoverflow.com/questions/71901632/fatal-error-unsafe-repository-home-repon-is-owned-by-someone-else,
actions/checkout#760),
by marking the /data directory inside the container as safe for git
during the container build.

* tooling: point git to directory instead of disabling security features

Easier to maintain version of 7c2b552
that additionally does not fiddle with security sensitive settings.

* style(Makefile): docker git env into separate variable

* tooling: extract repo location inside container into variable

* tooling: replace missing hardcoded /data with variable

Co-authored-by: Carsten Gips <cagix@fh-bielefeld.de>

* tooling(delete-rem-tags): pass git commit info (#19)

* tooling(delete-rem-tags): pass git commit info

Passes git author information via environment variables into the docker
container, in order to ensure commits done by the script have correct
author information.

* tooling(delete-rem-tags): pass git full commit info

Pass not only author information, but committer information too, since
git seems to be *sometimes* unhappy with only author information, for
whatever reason.

* tooling: makefile formatting

Co-authored-by: Carsten Gips <cagix@fh-bielefeld.de>

* tooling: makefile formatting

Co-authored-by: Carsten Gips <cagix@fh-bielefeld.de>

* tooling: makefile formatting

Co-authored-by: Carsten Gips <cagix@fh-bielefeld.de>

Co-authored-by: Carsten Gips <cagix@fh-bielefeld.de>
clebergnu added a commit to avocado-framework/avocado that referenced this pull request Oct 25, 2022
This workflow is currently failing, giving the following error:

   fatal: unsafe repository ('/__w/avocado/avocado' is owned by someone else)
   To add an exception for this directory, call:

          git config --global --add safe.directory /__w/avocado/avocado
   Error: Process completed with exit code 128.

Let's work around this limitation and more recent git behavior and set
the safe mode on the repo directory manually.

Reference: actions/checkout#762
Signed-off-by: Cleber Rosa <crosa@redhat.com>
ondrejbudai added a commit to ondrejbudai/osbuild-composer that referenced this pull request Jan 2, 2023
Github actions have a peculiar behaviour: The "job root" directory is owned
by user with ID 1001 whereas many containers (like the Fedora one) run as root
by default. This causes git to freak out because repositories owned by
different users aren't considered safe anymore and must be explicitly allow-
listed.

This becomes a problem when we switch to Go 1.18 because it stamps build with
Git information which fails because the repository technically isn't safe.

Let's fix this by marking the repository as a safe one.

Note that the actions/checkout action has a set-safe-directory feature, but
this one is not permanent. After the action is done, the setting is removed.
See actions/checkout#762

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
ondrejbudai added a commit to osbuild/osbuild-composer that referenced this pull request Jan 6, 2023
Github actions have a peculiar behaviour: The "job root" directory is owned
by user with ID 1001 whereas many containers (like the Fedora one) run as root
by default. This causes git to freak out because repositories owned by
different users aren't considered safe anymore and must be explicitly allow-
listed.

This becomes a problem when we switch to Go 1.18 because it stamps build with
Git information which fails because the repository technically isn't safe.

Let's fix this by marking the repository as a safe one.

Note that the actions/checkout action has a set-safe-directory feature, but
this one is not permanent. After the action is done, the setting is removed.
See actions/checkout#762

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fatal: unsafe repository (REPO is owned by someone else) with ubuntu 20.04 container
5 participants