AUR PKGBUILD download fails if git environment variables are set #1731

shello · 2022-03-20T19:24:23Z

Affected Version

yay v11.1.2 - libalpm v13.0.1

Describe the bug

If either of the environment variables GIT_DIR and GIT_WORK_TREE are set, yay will fail when downloading PKGBUILD.

Reproduction Steps

Set and export the variables GIT_DIR and GIT_WORK_TREE.
Install or upgrade an AUR package.

Expected behavior

The install or upgrade procedure would work correctly, and install the AUR package.

Output

yay config:

filipe@desktop ~%  yay -Pg
{
	"aururl": "https://aur.archlinux.org",
	"buildDir": "/home/filipe/.cache/yay",
	"editor": "",
	"editorflags": "",
	"makepkgbin": "makepkg",
	"makepkgconf": "",
	"pacmanbin": "pacman",
	"pacmanconf": "/etc/pacman.conf",
	"redownload": "no",
	"rebuild": "no",
	"answerclean": "",
	"answerdiff": "",
	"answeredit": "",
	"answerupgrade": "",
	"gitbin": "git",
	"gpgbin": "gpg",
	"gpgflags": "",
	"mflags": "",
	"sortby": "votes",
	"searchby": "name-desc",
	"gitflags": "",
	"removemake": "ask",
	"sudobin": "sudo",
	"sudoflags": "",
	"requestsplitn": 150,
	"completionrefreshtime": 7,
	"bottomup": true,
	"sudoloop": false,
	"timeupdate": false,
	"devel": false,
	"cleanAfter": false,
	"provides": true,
	"pgpfetch": true,
	"upgrademenu": true,
	"cleanmenu": true,
	"diffmenu": true,
	"editmenu": false,
	"combinedupgrade": false,
	"useask": false,
	"batchinstall": false,
	"singlelineresults": false,
	"version": "11.1.2"
}

Content of the relevant environment variables:

filipe@desktop ~%  env | grep GIT
GIT_DIR=/home/filipe/.dotfiles
GIT_WORK_TREE=/home/filipe

Attempt to install a new AUR package as normal with the git environment variables set makes yay fail:

filipe@deskop ~%  yay -Sa pandoc-bin
:: Checking for conflicts...
:: Checking for inner conflicts...
[Aur:1]  pandoc-bin-2.17.1.1-1

:: (0/1) Downloaded PKGBUILD: pandoc-bin
 -> error fetching pandoc-bin: fatal: working tree '/home/filipe' already exists. 
	context: exit status 128

Un-setting GIT_WORK_TREE when running yay is not enough and will fail as well:

filipe@deskop ~%  env -u GIT_WORK_TREE yay -Sa pandoc-bin
:: Checking for conflicts...
:: Checking for inner conflicts...
[Aur:1]  pandoc-bin-2.17.1.1-1

:: (1/1) Downloaded PKGBUILD: pandoc-bin
  1 pandoc-bin                       (Build Files Exist)
==> Diffs to show?
==> [N]one [A]ll [Ab]ort [I]nstalled [No]tInstalled or (1 2 3, 1-3, ^4)
==> 
 -> error resetting pandoc-bin: fatal: this operation must be run in a work tree

Un-setting both GIT_WORK_TREE and GIT_DIR makes yay behave correctly:

filipe@desktop ~%  env -u GIT_WORK_TREE -u GIT_DIR yay -Sa pandoc-bin
:: Checking for conflicts...
:: Checking for inner conflicts...
[Aur:1]  pandoc-bin-2.17.1.1-1

  1 pandoc-bin                       (Build Files Exist)
==> Packages to cleanBuild?
==> [N]one [A]ll [Ab]ort [I]nstalled [No]tInstalled or (1 2 3, 1-3, ^4)
==> 
:: PKGBUILD up to date, Skipping (1/0): pandoc-bin
  1 pandoc-bin                       (Build Files Exist)
==> Diffs to show?
==> [N]one [A]ll [Ab]ort [I]nstalled [No]tInstalled or (1 2 3, 1-3, ^4)
==> 
:: (1/1) Parsing SRCINFO: pandoc-bin
==> Making package: pandoc-bin 2.17.1.1-1 (Sun 20 Mar 2022 18:11:48 WET)
==> Retrieving sources...
  -> Downloading pandoc-bin-source-2.17.1.1.tar.gz...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   130  100   130    0     0    400      0 --:--:-- --:--:-- --:--:--   401
100 7076k    0 7076k    0     0  3041k      0 --:--:--  0:00:02 --:--:-- 3555k
  -> Downloading pandoc-bin-bin-2.17.1.1.tar.gz...
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   667  100   667    0     0   2577      0 --:--:-- --:--:-- --:--:--  2585
100 15.1M  100 15.1M    0     0  2417k      0  0:00:06  0:00:06 --:--:-- 1096k
==> Validating source files with sha512sums...
    pandoc-bin-source-2.17.1.1.tar.gz ... Passed
==> Validating source_x86_64 files with sha512sums...
    pandoc-bin-bin-2.17.1.1.tar.gz ... Passed
==> Making package: pandoc-bin 2.17.1.1-1 (Sun 20 Mar 2022 18:11:58 WET)
==> Checking runtime dependencies...
==> Checking buildtime dependencies...
==> Retrieving sources...
  -> Found pandoc-bin-source-2.17.1.1.tar.gz
  -> Found pandoc-bin-bin-2.17.1.1.tar.gz
==> Validating source files with sha512sums...
    pandoc-bin-source-2.17.1.1.tar.gz ... Passed
==> Validating source_x86_64 files with sha512sums...
    pandoc-bin-bin-2.17.1.1.tar.gz ... Passed
==> Removing existing $srcdir/ directory...
==> Extracting sources...
  -> Extracting pandoc-bin-source-2.17.1.1.tar.gz with bsdtar
  -> Extracting pandoc-bin-bin-2.17.1.1.tar.gz with bsdtar
==> Sources are ready.
==> Making package: pandoc-bin 2.17.1.1-1 (Sun 20 Mar 2022 18:12:01 WET)
==> Checking runtime dependencies...
==> Checking buildtime dependencies...
==> WARNING: Using existing $srcdir/ tree
==> Entering fakeroot environment...
==> Starting package()...
==> Tidying install...
  -> Removing libtool files...
  -> Purging unwanted files...
  -> Removing static library files...
  -> Stripping unneeded symbols from binaries and libraries...
  -> Compressing man and info pages...
==> Checking for packaging issues...
==> Creating package "pandoc-bin"...
  -> Generating .PKGINFO file...
  -> Generating .BUILDINFO file...
  -> Generating .MTREE file...
  -> Compressing package...
==> Leaving fakeroot environment.
==> Finished making: pandoc-bin 2.17.1.1-1 (Sun 20 Mar 2022 18:12:04 WET)
==> Cleaning up...
[sudo] password for filipe: 
loading packages...
resolving dependencies...
looking for conflicting packages...

Packages (1) pandoc-bin-2.17.1.1-1

Total Installed Size:  70,60 MiB

:: Proceed with installation? [Y/n] 
(1/1) checking keys in keyring                                                                  [########################################################] 100%
(1/1) checking package integrity                                                                [########################################################] 100%
(1/1) loading package files                                                                     [########################################################] 100%
(1/1) checking for file conflicts                                                               [########################################################] 100%
(1/1) checking available disk space                                                             [########################################################] 100%
:: Running pre-transaction hooks...
(1/1) Taking a snapshot of root dataset
:: Processing package changes...
(1/1) installing pandoc-bin                                                                     [########################################################] 100%
Optional dependencies for pandoc-bin
    texlive-core: for pdf output
:: Running post-transaction hooks...
(1/1) Arming ConditionNeedsUpdate...

The text was updated successfully, but these errors were encountered:

grandchild · 2022-03-21T10:55:31Z

Yay could specifically unset these two env variables. Maybe it should? I cannot think of a situation where it would make sense for yay to transport these from the calling env.

On the other hand, why would you set these globally? Do you never use git for anything else except for your dotfiles?

A quick fix would of course be to write a small wrapper script that unsets GIT_DIR and GIT_WORK_TREE and then calls /usr/bin/yay.

shello · 2022-03-21T17:07:19Z

I cannot think of a situation where it would make sense for yay to transport these from the calling env.

I forgot to leave a rationale for reporting this as a bug, apologies. But yes, that's my thought process.

As yay is dealing with its own git repos, it should isolate itself from the user's git settings, including the ones that are set from the environment. The fact that the user has some strange settings should not break yay's workflow.

On the other hand, why would you set these globally? Do you never use git for anything else except for your dotfiles?

Ah yes, I missed explaining this: these environment variables are not set globally, but rather loaded and unloaded based on the working directory of the shell; I'm personally doing this with a hook in zsh, but the same is achievable with tools such as direnv.

grandchild · 2022-03-22T10:36:32Z

it should isolate itself from the user's git settings

I'm not sure that's true for all settings. I could imagine there might be some global git settings that should be respected (which?). Perhaps the reset-to-default-values should be an allowlist-approach, and not a blanket reset, with some exceptions?

these environment variables are not set globally, but rather loaded and unloaded based on the working directory of the shell

Ah, that makes more sense. 🙂

shello · 2022-03-23T15:38:50Z

I'm not sure that's true for all settings. I could imagine there might be some global git settings that should be respected (which?).

My thought process was that any user settings have the potential to disrupt yay's operation, and as the repos yay manages are not user-visible, no user-defined settings should pass.
But I admit there might be exceptions, as I don't know neither yay's internals nor git's settings well enough to say otherwise.

DrKittens · 2022-03-24T10:21:13Z

+1 would appreciate yay's use of git isolating itself from the user git settings / not inheriting any config issues the user makes

To add onto the bug report...

GIT_SSL_NO_VERIFY would be a dangerous envvar for a user to pass unknowingly into yay's process that they could hypothetically have set if using arch for work and the workplace has poor practices around git repos...

If we're looking at a whitelist of git envars for yay to accept i'd only take the following config variables used for committing and user identification as part of yay's process since it allows for editting the pkgbuild and i cant think of a way they'd be misconfigured in a breaking manner...

GIT_AUTHOR_NAME
GIT_AUTHOR_EMAIL
GIT_COMMITTER_NAME
GIT_COMMITTER_EMAIL
EMAIL

Morganamilo · 2022-03-24T10:38:19Z

GIT_SSL_NO_VERIFY would be a dangerous envvar for a user to pass unknowingly into yay's process that they could hypothetically have set if using arch for work and the workplace has poor practices around git repos...

This sounds ridiculous to me. "It would be dangerous if the user set a dangerous option".

The options named above that are suggested to be whitelisted are actually the option that yay overrides by default. Then there's other settings like pager, diff tool, gpgsign.

If the user has configured git in a way that breaks it it's on them IMO.

shello · 2022-03-25T18:51:04Z

This sounds ridiculous to me. "It would be dangerous if the user set a dangerous option".

Users might have a valid reason to, in particular and isolated cases, want to knowingly set that option, for instance, for a specific local network git repo they work with that does not have a "valid" TLS cert.
Is it a poor practice to use it? Of course, but this option is valid in a few situations.

The danger of GIT_SSL_NO_VERIFY is of compromising the one layer of integrity of the connection between yay and AUR. While for one user's project directory this might make sense, and the danger there can be limited, yay is going to install software on the system, and ultimately run code from the git repos it downloads.
Is this scenario fat-fetched? Perhaps, but it's still a simple way to opportunistically compromise a user's machine.

If the user has configured git in a way that breaks it it's on them IMO.

To me as a user, it's unexpected that yay would follow the same behaviour set for a different repo of mine, just because one or more environment variables happen to be set.
In my case, I set mine for my own repo, not for yay; I would've looked into yay's configuration if I wanted changes to affect its behaviour.

Then there's other settings like pager, diff tool, gpgsign.

I think this is a fair point, if the criteria used is that these affect the parts of git that end up exposed to the user through yay.

grandchild · 2022-03-25T19:40:56Z

I agree with @Morganamilo in principle. But I also think that there might be some exceptions (like GIT_DIR and GIT_WORK_TREE) that yay could prune from the environment. Sort of as a favor to the user, because they're not really useful to yay anyway.

But I'd suggest to keep that list as clear-cut and minimal as possible, because after all, by setting env vars you are kind of explicitly setting global git config values that should apply to all git operations, even those done "under the hood".

DrKittens · 2022-03-30T11:57:27Z

The options named above that are suggested to be whitelisted are actually the option that yay overrides by default. Then there's other settings like pager, diff tool, gpgsign.

Good to know lol

If the user has configured git in a way that breaks it it's on them IMO.

I agree in principal aswell, but it wouldn't hurt to have some logic inplace to warn them they've done a potential dumb would it?

GIT_SSL_NO_VERIFY would be a dangerous envvar for a user to pass unknowingly into yay's process that they could hypothetically have set if using arch for work and the workplace has poor practices around git repos...

This sounds ridiculous to me. "It would be dangerous if the user set a dangerous option".

It comes down to threat model. It could be an acceptable risk option if they believe it's only applicable to one intranet or local repo, but a completely unacceptable risk to apply to sourcing application / software updates through yay.

In my opinion it would be good to have yay handle that in the same manner as gpgsign / difftool etc by overriding it and exposing that override in yay.conf or pruning it out. Theres a difference between letting a user set env vars that break yay and letting the user accidentally apply a dangerous config to yay that reduces the security of yay through ignorance of an unexpected interaction imo.

Edit: and yes, for alot of AUR helper users (thanks manjaro) not thinking about the interaction between git globals and the helper tool is definitely becoming a thing

shello added Status: Triage Type: Bug labels Mar 20, 2022

Jguer added Status: Confirmed Bug has been verified and removed Status: Triage labels Mar 23, 2022

Jguer mentioned this issue Feb 21, 2023

fix(git): add git denylist for workspace variables #1926

Merged

Jguer closed this as completed in #1926 Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AUR PKGBUILD download fails if git environment variables are set #1731

AUR PKGBUILD download fails if git environment variables are set #1731

shello commented Mar 20, 2022 •

edited

Loading

grandchild commented Mar 21, 2022

shello commented Mar 21, 2022

grandchild commented Mar 22, 2022

shello commented Mar 23, 2022 •

edited

Loading

DrKittens commented Mar 24, 2022 •

edited

Loading

Morganamilo commented Mar 24, 2022

shello commented Mar 25, 2022

grandchild commented Mar 25, 2022

DrKittens commented Mar 30, 2022 •

edited

Loading

AUR PKGBUILD download fails if git environment variables are set #1731

AUR PKGBUILD download fails if git environment variables are set #1731

Comments

shello commented Mar 20, 2022 • edited Loading

Affected Version

Describe the bug

Reproduction Steps

Expected behavior

Output

grandchild commented Mar 21, 2022

shello commented Mar 21, 2022

grandchild commented Mar 22, 2022

shello commented Mar 23, 2022 • edited Loading

DrKittens commented Mar 24, 2022 • edited Loading

Morganamilo commented Mar 24, 2022

shello commented Mar 25, 2022

grandchild commented Mar 25, 2022

DrKittens commented Mar 30, 2022 • edited Loading

shello commented Mar 20, 2022 •

edited

Loading

shello commented Mar 23, 2022 •

edited

Loading

DrKittens commented Mar 24, 2022 •

edited

Loading

DrKittens commented Mar 30, 2022 •

edited

Loading