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

No diff with show_diff_only #209

Open
physkets opened this issue Jul 21, 2019 · 9 comments
Open

No diff with show_diff_only #209

physkets opened this issue Jul 21, 2019 · 9 comments

Comments

@physkets
Copy link

I have set show_diff_only => 1, but upon upgrading an AUR package, it does not show the difference between the old and new. I only get an option to edit the PKGBUILD.

@diego-treitos
Copy link

I am also having this problem: I never get the diff.

Taking a look at the code... doesn't look very promising:

trizen/trizen

Lines 1363 to 1365 in b5855c6

if ($lconfig{show_diff_only} and $info->{_exists_in_cache}) {
## ok
}

I think this option should be removed or implemented.

@trizen
Copy link
Owner

trizen commented Sep 10, 2019

If show_diff_only is set to 1, the diff is shown only when there are new changes to the build files that are not already in the cache (i.e. when there is a new commit on AUR that has not already been pulled).

This can be checked at:

trizen/trizen

Lines 1053 to 1068 in b5855c6

chomp(my $old_commit_hash = `/usr/bin/git -C \Q$dir_name\E rev-parse HEAD`);
if (!$lconfig{nopull}) {
msg(":: $c{bold}Pulling AUR changes: $pkg_base");
#<<<
system('/usr/bin/git', '-C', $dir_name, 'reset', '--hard', 'HEAD', '--quiet') && return;
system('/usr/bin/git', '-C', $dir_name, 'pull', '--ff', '--quiet') && return;
#>>>
}
$info->{_exists_in_cache} = 1;
#<<<
print sanitize_output(decode_utf8(scalar `/usr/bin/git -C \Q$dir_name\E diff --no-ext-diff $old_commit_hash --color=always ':!.SRCINFO'`));

@diego-treitos
Copy link

So where is show_diff_only used to check whether to show the diff or not? I am only seeing 3 instances of show_diff_only text in the whole code: the default settings, the help and the lines that I posted above where it seems to do nothing.

@trizen
Copy link
Owner

trizen commented Sep 10, 2019

The git diff is always shown when pulling new commits from the AUR, regardless of the value of show_diff_only.

By setting show_diff_only to 1, it only prevents the displaying of the content of the build files when they already exist in the cache.

@diego-treitos
Copy link

Ohh I see. Now I understand better how it works and now I know why I am never getting the diffs.

The value of _exists_in_cache is 1 only if the repository exists in the path designated by the configuration variable clone_dir. When trizen generates the configuration file, it defaults the clone_dir variable to a path inside tmp:

trizen/trizen

Line 193 in b5855c6

$CONFIG{clone_dir} = catdir(tmpdir(), "$execname-$user");

Which means that if you reboot you will lose the track of the repositories.

@trizen Would it make sense to make trizen to default to a path inside /var/cache/ to clone directories? (i.e. /var/cache/trizen/${UID}/)

@trizen
Copy link
Owner

trizen commented Sep 10, 2019

I think /tmp is a good default place for the clone_dir.
Users are free to change it to a permanent directory, if desired.

Personally, I have the following configuration:

In trizen.conf:

clone_dir => "$ENV{HOME}/.config/trizen/sources",

In /etc/makepkg.conf:

PKGDEST=/tmp/makepkg-packages
SRCDEST=/tmp/makepkg-sources

This keeps only the AUR build files in a permanent directory, while the building of packages is being done in /tmp.

@diego-treitos
Copy link

I think that using /tmp as a default place, with such a predictable name, is a huge security problem actually. Imagine this scenario:

privuser: user with sudo privileges that uses trizen
badguy: user that wants to escalate privileges

Now badguy checks the system and he sees that the AUR package tor-browser is installed, so he executes this:

chmod 777 /tmp/trizen-privuser/
cd /tmp/trizen-privuser/
git clone https://aur.archlinux.org/tor-browser.git
chmod -R 777 tor-browser

Then badguy can do something to wait until the right moment to change some script inside the repository and when privuser updates the tor-browser package, the system is compromised.

I think that something like /var/cache/trizen/${UID}, where the permissions of the directory are checked before anything would be safer.

@trizen
Copy link
Owner

trizen commented Sep 10, 2019

If you have a bad guy in your system, then your system is already compromised.

@diego-treitos
Copy link

diego-treitos commented Sep 10, 2019

I am talking about a privilege escalation. Like in a web server, where if the web application is compromised you might be able to run some code as the web user, which usually can write to /tmp.

With this things could escalate from a compromised web application to a full compromised system.

trizen added a commit that referenced this issue Nov 9, 2020
…es`. (#209)

Previous value was `/tmp/trizen-$USER`, which could cause a "No space left on device" error when building large packages and the size of `/tmp` is small.

- Code tidy with a new version of `perltidy`.
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

3 participants