Update nuget.org feed with MonoGame develop builds

[isadorasophia]

Intent

Hello! I am following up from the .NET 8 reddit thread. Reading this article, it seems that develop builds of MonoGame are available from the GitHub feed.

My current project setup is developing from a Linux, Mac, Windows and even SteamDeck. This is really easy to do with nuget.org feed, since all I really need to do is build the project and it will automatically pull the nuget packages.

However, the GitHub feed requires me to manually authenticate to the feed, which makes it slightly trickier to maintain in different machines and across the team.

The intent is to support pre-release packages in nuget.org, so people could still opt-in if they target a -develop build or use the latest release version without further authentication.

Motivation

The motivation is to keep the workflow with MonoGame quick and easy to anyone onboarding to the project. I do understand the motivation behind the GitHub feed, so this is why I created the issue, in case anyone sees value in this approach as well.

Users that want to use develop builds would manually target a -develop version for the package, still consumed from the nuget.org feed. No GitHub authentication would be required.

[tomspilman]

@harry-cpp @mrhelmut What do you think?

We really shouldn't push new builds to Nuget after every PR. As i don't think there is a way to take down old releases like we do on our own feed.

Maybe we should push a new development build weekly? Monthly? Any ideas?

[mrhelmut]

I agree. I think we should try to publish *-develop packages to the public feed, and we should indeed try to not do that at every PR.
Weekly sounds cool.

Ideally I think it would be nice to also have a Github Action that we can trigger manually to publish a develop package (in case there is an urgent hotfix to push to the feed).

[tomspilman]

Weekly sounds cool.

So that is 52 releases a year in general for -develop packages across our 15 different packages on nuget.

@harry-cpp how do we setup this automation of weekly -develop versions of all the packages as well as a manual trigger to publish them?

[Ragath]

We really shouldn't push new builds to Nuget after every PR. As i don't think there is a way to take down old releases like we do on our own feed.

While you can't completely remove a version on nuget.org, you can un-list it. It hides the version from anyone looking to install/add it to a project, while still allowing projects already referencing it to perform a nuget restore.

[mrhelmut]

True, and they can be marked as obsolete or unsecure, which would make nuget restore to display a warning.

Though we should check if pushing this many packages has any other impact, such as hosting costs or nuget performances.

[Ragath]

True, and they can be marked as obsolete or unsecure, which would make nuget restore to display a warning.

Though we should check if pushing this many packages has any other impact, such as hosting costs or nuget performances.

There are no hosting costs, has no impact on nuget restore and the average downloads/version count is irrelevant for pre-release versions.

[AristurtleDev]

To add to this because I would like to see it as well, if you publish weekly *-develop packages to NuGet, they will not be shown to users by default in Visual Studio's NuGet Package Manager. They would have to click the "include prerelease versions" checkbox to see them.

This means that the 52 weekly develop releases a year would not clog the list up, users would have to opt-in to see them.

[mrhelmut]

The sooner we implement this, the better. We should also automate merge to master releases (so far we're manually uploading to nuget.org). I'm a bit overloaded this month to work on it, though. Contributions welcome.

[AristurtleDev]

@mrhelmut I can implement this, but before working on it, I would like to propose a change. The current build system uses a monolithic cake build script. Would there be any issue in converting this to a cake frosting project for easier maintainability and updating? It's still cake build, but as a cake frosting project it's an actual csproj with proper intellesense and separation of concerns.

Once converted, I can push that PR then add the functionality for the weekly NuGet *-develop pushes

[mrhelmut]

@harry-cpp any thoughts on moving to a frosting project?

[harry-cpp]

Yea, we should switch to use Cake.Frosting, the new dependency repos are already using it and it makes working with build scripts a lot easier (there is a CAKE VS Code extension that gives autocomple to the current build.cake, but its a rather hacky thing). @AristurtleDev If you can, move uploading and downloading of artifacts into the script as well.

On a separate note when it comes to publishing directly to nuget.org, we need to be careful about security. I know GitHub has a place to put the secrets in, but I didn't really read up on how secure it is. One idea I had was making a private repo for publishing the dev packages from MonoGame GitHub org once a day, that way the secret would have an extra layer of protection.

[AristurtleDev]

On a separate note when it comes to publishing directly to nuget.org, we need to be careful about security

Secrets are entered in the repository settings and when accessed are marked out with ****** if for some reason they should be output to the github action log, so it doesnt accidentally leak it.

Also, once the secret is created for the repository, it is not visible by anyone and can only be changed by someone with admin rights in the repo. Even then they can only change it and not view it.

Additionally, secrets settings are repository bound, so if someone were to fork the MonoGame repository, it doesn't also bring the configured secrets with it.

Tl;Dr there's really no reason to make a separate repo specifically for publishing. Secret settings are secure and bound to the repo itself and do not transfer with forks.

[harry-cpp]

The problem isn't the things you mention, but the fact that if the attacker finds a way to read the secret it would be bad. ie. what if one of the dependency in the build script is compromised, and we gave the secret to the CAKE build script for it to upload the artifacts. You have to worry about these things if you put the secret in the main repo, but not if we just put it in a separate repo that downloads and uploads the artifacts.

[AristurtleDev]

The only way an attacker would be able to read the secret in bad faith like this is if

1. They submit a PR that adjusts the build script to read the secret and then transmit it outside of the Github runner environment (like smtp it to themselves or something)
2. A maintainer approves this PR and merges it in.

I would hope that maintainers would look at the PRs carefully enough to see something like this and reject it. I'm fine with whatever decision you think is best in that regards, however I would like to state again that the only way the configured repo secrets could be compromised from a GitHub action runner is if a PR was merged that changed the script to intentionally leak it.

[harry-cpp]

It's not about how it can happen, it's about preventing it as much as possible from happening.

[Apostolique]

GitHub has a page talking about this issue: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions.

[mrhelmut]

From what I read here, there's a million of ways for things to go bad. I would be on page with harry: the more we can reasonably obfuscate, the better. It'll also prevent Actions errors when cloning MonoGame (e.g. clones trying to upload their builds).

I think it's a good separation to have: the main repository runs the tests and checks the PRs; the "distribution" repository packs, and distributes it.

[AristurtleDev]

Before I start this, I want to say that regardless of what I've posted below, I'll accept any decision made on this, I just wanted to ensure that everything was understood about how the configured secrets work in a repository and maybe ease concerns on areas that didn't seem to be understood.

@mrhelmut
It'll also prevent Actions errors when cloning MonoGame (e.g. clones trying to upload their builds).

I'm not sure if I'm misunderstanding the concern or if ya'll are misunderstanding how Github Actions works, but just to state it again, if I clone (or fork) the MonoGame repo, doing so does not bring the secrets into my clone/fork. I can also just adjust the workflow Action in my clone/fork to upload the NuGets under my own API key if I wanted to. Moving the deployment to a private repo will not solve this problem. Actually nothing will solve this problem, anyone can build and upload the NuGets under their own account. But nothing about cloning or forking the repository will also clone the configured secrets, they are bound to the repository they are set in and do not transfer on cloning or forking.

@harry-cpp
I know GitHub has a place to put the secrets in, but I didn't really read up on how secure it is

I would really suggest taking time to read on how secure it is (i'll also explain below)

It's not about how it can happen, it's about preventing it as much as possible from happening.

Putting it in a private repo does not add any extra layer of protection. Sure, the secrets are now configured in a walled off repository, but it's still the GitHub Actions workflow that accesses and uses those values. All it would take is a single PR to update the deployment to leak the keys to get merged and then whether public or private repo it's not going to matter. There are ways to leak the keys in runners other than simply outputting them in the logs, so making it in a private only repo won't protect it.

The only way you're going to protect the configured repository secrets from leaking in a GitHub Action workflow is if the maintainers do not merge a PR that leaks them.

I was exploring this a bit with @Apostolique last night, and yes, there are thousands of ways to leak the keys. One interesting way that I came up with was to have a unit test that is run as part of the workflow grab the key and output it to the log. Even this required some out of the way things to happen for it to leak (I even base64 encoded the key and the log still *** the key out).

However, you have to understand, the configured secrets themselves are not accessible by any job that is run in a workflow, unless that job is explicitly given access to the key. For instance

name: test
on: push


jobs:
  example-job:
    name: "Example Job"
    runs-on: ubuntu-latest
    steps:
      - name: "Clone Repository"
        uses: actions/checkout@v3
        with:
          fetch-depth: 0
      
      - name: "Setup .NET"
        uses: actions/setup-dotnet@v3
        with: 
          dotnet-version: ${{ env.DOTNET_VERSION }}

      - name: Run Test
        run: dotnet test

If i had a job like the above, an a configured secret named SECRET_VALUE, the dotnet test command and the program that it runs cannot access the SECRET_VALUE through environment variables. It would have to first be explicitly set in the workflow like this

- name: Run Test
  env: 
    SECRET_VALUE_ENV: ${{ secrets.SECRET_VALUE }}
  run: dotnet test

Then the program that executes from dotnet test can get it from the SECRET_VALUE_ENV environment variable.

In order for the cake scripts to deploy the NuGets, you would have to explicitly give them access to the key with something like

- name: Deploy
  env: 
    NUGET_API_KEY: ${{ secrets.NUGET_API_KEY}}
  run: cd ./build && dotnet run -- --target=Deploy

At this point, the cake Deploy target can access the environment variable set so that it can use the key to deploy, and if that target also logs it to the console, it will be automatically be logged as *** by the GitHub runner.

So the only thing that will prevent a key from being leaked, again, is if the maintainers allow a PR to get merged that explicitly adds the key as an environment variable in the workflow and the workflow executes a code to leak the key.

Which brings me to a final question as I work on converting this to cake frosting; Would you like me to completely remove the Deploy target from the cake script? If you're moving to a private repo, then you can just create a workflow that directly executes the dotnet nuget push command from the cli without the need for a cake script to do it.

[mrhelmut]

I'm not sure if I'm misunderstanding the concern

No misunderstanding on how secrets work. That was mostly about the workflow still trying to run on clones even though it's likely useless yet spinning runners time (and sending emails because they'll fail). I have big doubts that many people will be willing to push their own nugets ID. To me, it's a sufficiently small audience that they likely know how to do it if they need to, without us providing a public workflow for that.

Putting it in a private repo does not add any extra layer of protection.

We could still fence that by allowing only one dedicated github account to access the private repo secret. It's about limiting the damages if any admin account gets compromised.

I wouldn't trust myself in not merging malevolent PRs, so separating concerns is always a good thing to me. Nothing will be hack-proof, but if we can do simple stuff with good gains, I think that we should. But we sure shouldn't go into anything convoluted if it's a little gain.

Would you like me to completely remove the Deploy target from the cake script?

I would be favorable to that. Though it might still be useful to have the artifacts uploaded (to the action artifact space), if anyone would like to use the artifact nuget channel (which would be a kind of nightly channel, I guess).