Add `artifacts` publish for `Github Actions` builds

[SergeyShorokhov]

It's a necessity for convenience.

[dvander]

Our artifacts are published by buildbot - we only use GitHub CI as a smoke test for PRs.

[ShootingKing-AM]

I think it would be helpful for people who want to test unmerged PRs and contribute in that particular PR-solution development/testing without having to setup development environment locally. Was lazy to add artifact-publish earlier 😸

[SergeyShorokhov]

With GitHub Actions it is enough to fork the repository and builds logs, the artifacts will be available for development not only in buildbot. Also can use https://github.com/nektos/act

It's just convenient. One does not interfere with the other.

[dvander]

For security reasons we only publish artifacts built by Buildbot, and only ones that contain reviewed changes. You're welcome to enable this functionality in your own forks though.

[ShootingKing-AM]

I think there is some sort of misunderstanding here, build artifacts appear in GitHub actions page like this, post successful build,

It is much easier to trust and test these artifacts rather than setup local dev env with multiple OS/toolkit versions.

As a very concrete example, when I proposed a fix for Issue https://github.com/alliedmodders/amxmodx/issues/988#issuecomment-918426637 . I was forced to upload binaries, and other people for testing had to download and run those binaries, which pose much more security implications than transparant multi-os/toolkit GitHub actions artifacts.

And ofcourse these GitHub artifacts are not official snapshot, hence no warrenty, no extra support.

[dvander]

I still view this as a security issue, because it's under the purview of our repository, and it's unreviewed code.

But I'll let @asherkin comment and if he's okay with it, we can do it. At a minimum though, the origin of the build would have to be clearly marked as not from us (in the .rc file and any kind of status line). Basically, the same as what you'd get if you made a local build versus a production build.

[ShootingKing-AM]

fennec fox is busy i guess.

[asherkin]

@dvander I think we could be OK here as long as it was only enabled for PRs and not regular branch builds, and clearly identified separately from a normal snapshot build as you've said. GitHub Actions do require manual approval before workflows are run for first-time contributors, we can make that required for all external contributors if desired.

As a very concrete example, when I proposed a fix for Issue #988 (comment) . I was forced to upload binaries, and other people for testing had to download and run those binaries, which pose much more security implications than transparant multi-os/toolkit GitHub actions artifacts.

@ShootingKing-AM It doesn't look like anyone actually asked for those binaries, so you were hardly forced, but I agree that it's much easier to trust binaries from the CI system - but dvander's point is that those binaries aren't actually trusted, which is an important consideration, and GitHub doesn't let us warn people about that easily.

I'd expect anyone who actually wanted to test a PR who should be testing a PR to both be capable of building it themselves and actually desire to build it themselves - in order to have debug symbols and be able to tweak it if required. So my view here is that it isn't too harmful to enable, but it'll need some work done to label the builds appropriately, and that's probably best made as a PR to the actions config and associated build files if it's something you'd like to see.

[ShootingKing-AM]

Thanks for taking your time to comment on the issue.

There is another point to note regarding amxmodx specifically that needs attention. Amxmodx is not as active as it was before, this includes both contributor and maintainer activity. It is common here to get reviews after 1week or none at all. That is why I was pressing to publish atleast pr-artifacts, so that testing can be done even by people who don’t understand how to build amxmodx. Then it will also be easier for limited no. of maintainers to accommodate those contributions.

I also understand that testers should have the capability to tweak/build themselves, but then they would have attached a fix too themselves instead of making an issue. Again due to less activity, my opinion was to decentralize activities which can be done even by a non-maintainer without compromising security. Imo, Opensource software thrives on rich network of contributors rather than a closely knit core team - which is expected to do/does everything.

I also understood dvander's point as the position of the team, but what is the core-team's security policy? Is it untrusted only because of unreviewed code or it is untrusted because team does exercise control over github build environment?

[dvander]

It's both.

Review speed is definitely a problem. Feel free to ping me if you've been waiting too long. And if we need to add new maintainers, that's a discussion we can totally have as well.