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

Jailing git commands makes bwrap complain about GNUPG related path parameters #216

Closed
Hs-Yeah opened this issue Oct 4, 2023 · 13 comments
Closed

Comments

@Hs-Yeah
Copy link

Hs-Yeah commented Oct 4, 2023

When I ran rua install command after upgraded to 0.19.8, this error popped up:

$ rua install rua
thread 'main' panicked at 'Command git fetch -q upstream failed with exit code Some(1)
Stderr: bwrap: Can't find source path /dev/null/.gnupg/pubring.kbx: Not a directory

Stdout: ', src/git_utils.rs:68:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I think this is introduced by commit eaa5910 where $HOME is set to /dev/null at line 84 of git_utils.rs.

After downgrading to 0.19.7, the error disappeared.

P.S. In my system, $GNUPGHOME is not set:

$ echo $GNUPGHOME

OS: Arch Linux x86_64
Kernel: 6.5.5-arch1-1
Shell: bash 5.1.16

@vn971
Copy link
Owner

vn971 commented Oct 4, 2023

@Hs-Yeah Hi, thanks for raising the issue! My bad (insufficient testing).

I need to understand how to fix it. I'm not 100% sure that the override you're referring is the culprit (reason of the failure). I think it has been working for a while already (setting $HOME to /dev/null as recommended in man git), but I could be wrong. Need to investigate and test properly.

If you've built rua from source and can confirm that removing this line (or the referenced commit) fixes the issue, do tell. Otherwise I'm running similar tests now.

@Hs-Yeah
Copy link
Author

Hs-Yeah commented Oct 4, 2023

I can build rua from source, I'll test removing this line and report back later.

archlinux-github pushed a commit to archlinux/aur that referenced this issue Oct 4, 2023
Temporary revert version 0.19.8 until issue #216 has been fixed
vn971/rua#216

This reverts commit 08ecca2.
@vn971
Copy link
Owner

vn971 commented Oct 4, 2023

@Hs-Yeah From my current understanding, the conflict is because the commit you reference eaa5910 introduces jailing for all git invocations, and the jailing script is this one: https://github.com/vn971/rua/blob/master/res/wrapper/security-wrapper.sh
This jail, however, uses the $HOME variable.

@Hs-Yeah
Copy link
Author

Hs-Yeah commented Oct 4, 2023

@Hs-Yeah From my current understanding, the conflict is because the commit you reference eaa5910 introduces jailing for all git invocations, and the jailing script is this one: https://github.com/vn971/rua/blob/master/res/wrapper/security-wrapper.sh This jail, however, uses the $HOME variable.

Yes, and as my $GNUPGHOME not set, $HOME is used, which set to /dev/null

@vn971
Copy link
Owner

vn971 commented Oct 4, 2023

On the other hand, the line 84 (that sets $HOME to /dev/null) is needed exclusively to avoid messing up with people's local git configurations (or rather, avoid being messed up by people's local git configurations). If, however, git is being put into a jail, then the $HOME directory is cleaned anyway. So line 84 can be removed now, which I suspect will fix the issue as well.

Will test it now.

@vn971
Copy link
Owner

vn971 commented Oct 4, 2023

Just for a background info if anyone might be interested. I've decided to start jailing git operations to avoid packages being able to execute what they want when reviewing them via rua. It is extremely unlikely that this would happen, but just to keep things clean, it's better to keep the jail constraints even in presence of .gitattributes file.

TL&DR; Might be an overkill, but cleaner this way.

@Hs-Yeah Hs-Yeah changed the title Jailing git commands makes error in version 0.19.8 Jailing git commands makes bwrap complain about GNUPG related path parameters Oct 4, 2023
vn971 added a commit that referenced this issue Oct 4, 2023
See #216,
in particular #216 (comment)
and #216 (comment)

With this commit, git invocations by `rua` should be safe
even in presence of arbitrary .gitattributes files,
and should not be disrupted by people's local git configurations.
@vn971
Copy link
Owner

vn971 commented Oct 4, 2023

This should fix the issue: #217
Releasing in a moment

@Hs-Yeah
Copy link
Author

Hs-Yeah commented Oct 4, 2023

If you've built rua from source and can confirm that removing this line (or the referenced commit) fixes the issue, do tell. Otherwise I'm running similar tests now.

Can confirm that removing this line fixes this issue. I'll try 0.19.9 and report back.

@vn971
Copy link
Owner

vn971 commented Oct 4, 2023

@Hs-Yeah Thanks! And thanks again for reporting!

@Hs-Yeah
Copy link
Author

Hs-Yeah commented Oct 4, 2023

If you've built rua from source and can confirm that removing this line (or the referenced commit) fixes the issue, do tell. Otherwise I'm running similar tests now.

Can confirm that removing this line fixes this issue. I'll try 0.19.9 and report back.

Oh, sorry for the false reporting, I ran the wrong command (rua upgrade and did not entered o to install packages, rather than rua install directly).

With the aforementioned line removed, a new error shows up:

$ rua install rua
thread 'main' panicked at 'Command git fetch -q upstream failed with exit code Some(128)
Stderr: fatal: Not a git repository (or any parent up to mount parent /home/kozi)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

Stdout: ', src/git_utils.rs:68:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@vn971
Copy link
Owner

vn971 commented Oct 4, 2023

@Hs-Yeah yes, that's why the full PR #217 is needed I think. Try the PR (or simply 0.19.9), it should work smoothly

@Hs-Yeah
Copy link
Author

Hs-Yeah commented Oct 4, 2023

@Hs-Yeah yes, that's why the full PR #217 is needed I think. Try the PR (or simply 0.19.9), it should work smoothly

I am building 0.19.9 now, will report back soon. :)

@Hs-Yeah
Copy link
Author

Hs-Yeah commented Oct 4, 2023

Can confirm 0.19.9 fix this issue. @vn971 Thank you for the quick response and quick fix!

@Hs-Yeah Hs-Yeah closed this as completed Oct 4, 2023
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

No branches or pull requests

2 participants