Change repo update hook behaviour

[git-bruh]

        [ ! -x update ] || {
            log "$PWD" "Running update hook"
            ./update
        }

I suggest that the update hook in repos should just be cat-ed (these "hooks" aren't used much, and they're mostly to deliver news like is being done in repo-community) instead of executed because if you use an unofficial repo, the maintainer can just put something like ssu rm -rf /* (Very unlikely to happen) in update and mess up a few people's systems. This is different from post-install because you can inspect those scripts manually, but if you just run kiss u then you won't be able to check the update file beforehand.

What do you think @dilyn-corner ?

[dilyn-corner]

I'm uncertain how much I like this in general, but if it is kept there are a couple things I'd want from it...

1. don't exec, just cat (as you've suggested). Easy change, it's like a post-install hook at that point.
2. only print when the file itself gets updated - no need to reprint it on every kiss u.

I'll think about whether to keep it and in the meantime probably just implement your suggested change!

[cemkeylan]

I added the same comment on the commit itself, but I will re-add here so it doesn't get lost in the commit https://github.com/kiss-community/kiss/commit/d1287cb652666624a0b8b7092c63db5c399a6935#commitcomment-47679816

I didn't see the issue tracker before adding the comment, but I also think that catting is the better option. Repositories themselves should never run code on your behalf, that is bad repository management. In my opinion, repository update hooks should solely exist so that they relay a message.

[cemkeylan]

post-install scripts on the other hand exist for the package you explicitly install. If you are installing a package from the repository, you are explicitly giving the permission to run those commands unlike update hooks.

[FriendlyNeighborhoodShane]

I think the custom update script is a valuable mechanism for some cases, even if I only use it for minor clever things in not-very-important ways.

I propose that a new post-update file should be added to the package spec, which will just be a text message like post-install. And public repo creators should be discouraged from using update unless strictly neccesary (e.g. other VCS than git). It will prevent cases of accidental mishap, and people would explicitly pay attention when it is actually there.

Preserves backwards compatibility too, even if that doesn't exactly matter much here.

[FriendlyNeighborhoodShane]

To elaborate on my comment above, I don't think this is a security issue in a meaningful sense. If we're talking malice, then there's many ways to do bad stuff if the user doesn't do their job of inspection, the build scripts are executables after all. The only problem to solve here are IMO accidents and maybe conceptual simplification.

If it's very rarely used, since the primary responsibility it currently fulfills would be shifted to post-update, I think it would be fair to put the burden of inspecting it when it is actually used on the user.

[cemkeylan]

FriendlyNeighborhoodShane writes:

I think the custom update script is a valuable mechanism for some cases, even if I only use it for minor clever things in not-very-important ways. I have conflicted views on this. For instance, I added pre-fetch and post-fetch hooks to CPT back at May 2020 (which I am planning on altering the functionality). By this, I mean user hooks and not scripts owned by repositories. In my opinion, THOSE should be used instead, simply because I only see value out of personal configurations interacting with repository updates.

I propose that a new post-update file should be added to the package spec, which will just be a text message for those situations. And public repo creators should be discouraged from using update unless neccesary. I don't think adding a new file will benefit the package manager at this point for repositories. It adds an unnecessary complexity. Naming it post-update also would cause a lot of confusion.

Perhaps the method used for post-install could also be used to achieve what you are saying, which is, run it if it is an executable, print it if it is a normal file. This limits the complexity and confusion, and also preserves backwards compatibility.

To elaborate on my comment above, I don't think this is a security issue in a meaningful sense. If we're talking malice, then there's many ways to do bad stuff if the user doesn't do their job of inspection, the build scripts are executables after all. The only problem to solve here are IMO accidents and maybe conceptual simplification. In a security point of view, I also agree that this is a non-issue. Why trust in someone's package if you think that the repository maintainer might run some malicious code? However, my personal point of discussion on this subject is completely different. When you are building a package, you are specifically requesting to run the build script, and the post-install script when installing the built package. You don't have that control when you are fetching a remote repository. You don't get to choose whether you want to run the repository update script, you don't even get to know the contents of the script until the script is finished/failed.

My idea about this whole situation is still that running an update script is bad repository management and should be left to user defined hooks. I simply can't think of a good reason why a repository maintainer would need to run a code on someone's computer. I'd also like to see how you are utilising update scripts. Maybe there are spots that I am overseeing here.

-- Best Regards, Cem Keylan

[FriendlyNeighborhoodShane]

@cemkeylan

For instance, I added pre-fetch and post-fetch hooks to CPT back at May 2020 (which I am planning on altering the functionality). By this, I mean user hooks and not scripts owned by repositories. In my opinion, THOSE should be used instead, simply because I only see value out of personal configurations interacting with repository updates.

Perhaps the method used for post-install could also be used to achieve what you are saying, which is, run it if it is an executable, print it if it is a normal file. This limits the complexity and confusion, and also preserves backwards compatibility.

Both of those sound like pretty neat alternatives to me, I personally prefer the second one more but both are reasonable. I didn't actually know post-install worked that way.

you don't even get to know the contents of the script until the script is finished/failed.

I actually didn't understand what you meant here, so I looked at the code and did some tests and realised that my understanding of update's usage was actually pretty wrong, haha. I didn't know that the hook was run after a git pull too. Because of the limited situations in which I have been using it (simple non-git repositories), I thought it was supposed to conceptually operate as an alternative to kiss running git pull, that if it exists, kiss would not run pull itself at all and it was the responsibility of the script to fetch any updates.

So to clarify, I would say that currently update theoretically serves two separate conceptual functions:

• Running code/displaying messages after a pull made by kiss

• Actually doing the update procedure itself, whether it be calling another VCS, or retrieving raw data itself, etc.

I simply can't think of a good reason why a repository maintainer would need to run a code on someone's computer.

I completely agree, for when the repository is managed by git itself it serves no purpose. The first function underlined above should be trimmed down to just displaying messages.

The second function, however, I find valuable, in cases like when the repository is hosted on a separate VCS, the script can call the correct command itself. Or if it is a git repository but has a... peculiar relationship with remote branches, instead of pulling and tripping on an error it would fetch and merge/rebase in whatever way it wants to.

So I guess what I'm proposing is... splitting the current functions of update to two different files. One with a simple messaging system shown after an update, and another with a hook to be run to receive updates instead of kiss doing its hardcoded update procedure.

Aside from the ones I gave about custom git workflows or other VCSs, I guess an interesting one I made was a joke repository that harvested firmware and kernel packages from other distros. Like, the script would retrieve the latest kernel firmware package from arch, checksum it and create a versioned kiss package. The update script was essentially acting as the repository maintainer itself, which was executed on a kiss u. It was pretty crazy stuff.

As I did say, it's not that personally important to me that this is preserved, because I can do the same thing I'm doing now in other ways too. But I think this is a useful and convenient gimmick for some cases.

[FriendlyNeighborhoodShane]

Rough patch ideas:

https://github.com/FriendlyNeighborhoodShane/kiss/commit/7b91f97d7856037a7796a1bac516fe25ec3da220 https://github.com/FriendlyNeighborhoodShane/kiss/commit/d06d78fa66bb3c169e658a9cde471958e858939b

The names of update-hook and post-update are arbitrary, of course. I'd have preferred to keep the hook as just update, but with that any users using repositories with that hook will find themselves never getting an update.

[cemkeylan]

I agree that other VCS could be supported, but I also believe that VCS support should be hardcoded to the package manager. Handling repository updates are the most important part of every package manager and I don't find utilising custom update scripts for repositories to be a good idea. It is not complex to support other types of version control systems, with approval I can patch in given VCS support.

As for harvesting firmware and kernel packages from other distributions, you can commit harvested packages to the git repository instead of using an update script to do that. Don't get me wrong I am not trying to be rude, but if you are using an update script to populate a package repository, that means that the repository doesn't have a maintainer, and it doesn't have the tiniest amount of quality control. This is absolutely okay for your personal workflow (in user-defined hooks), but this shouldn't be a thing for public repositories. Dylan always advocated that packages should be static (which I agree with) and this workflow is the opposite of static packaging as you are depending on so many external factors.

My idea for a patch would be doing the following:

• Add a pre-fetch user-defined hook that runs when you first run kiss u

• Add repository user-defined hook for each repository with the $DEST variable set to the repository location. This can be used for custom repository updates for unsupported VCS and complex workflows. A repository maintainer can advise adding a certain repository hook for their own repository.

• Add post-fetch user defined hook after all the repositories are processed.

• Add back update as a non-executable file for printing announcements and other messages. As I said, we can do this the post-install way, but my suggestion is to get rid of this functionality altogether.

-- Best Regards, Cem Keylan

[dilyn-corner]

Lots of interesting discussion happening around this.

@FriendlyNeighborhoodShane those two commits are very similar to the idea I had and what I was working on before I dropped the update idea entirely - indeed I ran into the issue you specify in that second commit. I mostly just didn't want to deal with it.

Ultimately, my thinking is this:

• The way we run updates via kiss u is a straightforward and simple way, but does not easily allow users to vet changes to the repository. Therefore, nothing outside of the minimum required to actually update the repo should be done. Specifically, no code should ever be ran by package manager that doesn't directly facilitate updating the repositories, and no code should ever be ran that the user has not had at least an opportunity to inspect. Therefore, any file kiss sees that resembles an update-hook should only ever be printed.

• If a repository updates in such a way that it results in breaking changes, the user has to be notified in some way. I feel like the huge number of filenames, patches, + and - that scroll across stdin when the repository gets updated telegraphs this fine, and the user will find out if their packages have moved soon enough, yes? But it shouldn't happen often if ever (in my mind, repository structure should be stable. Otherwise, it isn't much of a repository), so this isn't a problem for kiss to solve.

• If a repository updates in such a way that user intervention is required on a package level, then there's no need to tell every user of the repository about this update - this is part of what post-install is for, after all. Further, if I need to tell users something, bumping baselayout and adding post-install is the best way. It's a package (most) everyone has, requires no extra files in the repository, and requires no extra functionality.

Ultimately, if repository maintainers need to print things to stdin to tell users about horrible things, something horrible has happened. I feel like this adds a crutch we don't want.

I've been thinking about this issue a lot since it was raised, but I can't come up with an instance (that would happen in repo or community) where this would be useful. Arch has a similar feature (except you have to shudder go to their website) to inform users about what they broke. But user intervention is a thing we don't want to encourage. I think this is best relegated to the thing we use that already talks to users: post-install.

Tangentially, kiss supporting different VCS is interesting and something I'm looking into, but it's unclear to me how much it matters - git probably outnumbers the sum total of fossil, mercurial et al. 100:1. I only know of two users who have fossil packaged, and one of them is me!

[FriendlyNeighborhoodShane]

I agree with about all of the points made by @dilyn-corner about it. Though I'm not a repository maintainer, so I'll leave opinions on whether they need a per-repo messaging system to those who are.

Ah, but just so we're clear on this, having an update-hook from the first patch was not going to lead to situations where unexaminable code would be run. Since kiss would not be running pull on those repositories, the script to be run would always be the one that the user could have looked at before running a kiss update.

Again, I have to express, being the only person in the thread in defence of the hook concept, that literally the only reason I have used it for is meaningless wacky fun shit. I am a git-lover (though "git but meaner" Fossil is pretty neat in its own regards) and not the maintainer of anything (technically except a practically-dead package in kiss). If the kiss ecosystem is better off with this complexity removed, then that's what's to be done. If I want to do that meaningless wacky fun shit again, I'll simply copy-paste the function from my patch into a kiss extension.

[dilyn-corner]

Please don't think I was trying to misrepresent you! My comments mostly weren't directed at your update-hook idea. I was mostly just trying to explicitly state my position on the matter of update hooks, for the record. 

The ability of users to do meaningless, silly, and fun things is what makes kiss so nice - it's hackability. A fortune was recently made (in awk, ofc) that could be linked into the package manager to deliver fun messages from IRC during updates, it's all very fun. Users are allowed to do that (even encouraged!), it's just not a thing that should be up streamed. And it's important that these things get written down, so nobody gets surprised-pikachu'd when their PRs get closed as wontfix :o

Mar 8, 2021, 10:58 AM by notifications@github.com:

I agree with about all of the points made by > @dilyn-corner https://github.com/dilyn-corner> about it. Though I'm not a repository maintainer, so I'll leave opinions on whether they need a per-repo messaging system to those who do.

Ah, but just so we're clear on this, having an > update-hook> from the first patch was not going to lead to situations where unexaminable code would be run. Since kiss would not be running pull on those repositories, the script to be run would always be the one that the user could have looked at before running a > kiss update> .

Again, I have to express, being the only person in the thread in defence of the hook concept, that literally the only reason I have used it for is meaningless wacky fun shit. I am a git-lover (though "git but > meaner> " Fossil is pretty neat in its own regards) and not the maintainer of anything (technically except a practically-dead package in kiss). If the kiss ecosystem if better off with this complexity removed, then that's what's to be done. If I want to do that meaningless wacky fun shit again, I'll simply copy-paste the function from my patch into a kiss extension.

— You are receiving this because you were mentioned. Reply to this email directly, > view it on GitHub https://github.com/kiss-community/kiss/issues/4#issuecomment-792854137> , or > unsubscribe https://github.com/notifications/unsubscribe-auth/AEVM3SJDK7ABG6WZLNAPKZTTCTX4FANCNFSM4YIEAZEA> .

[FriendlyNeighborhoodShane]

Oh of course not, haha. Didn't interpret your comments that way. Just thought it's kinda important to put my stances on the issue in perspective of how much practicalities they're based on, some just self-evaluation ;P

To put things another way, my only intention here was to add nuance to the case for an update hook, not to add (significant) weight to it.