GitHub Actions break using the new (non-root) CKAN images

[kowh-ai]

Failure in Post Run actions/checkput@v4

Post job cleanup.
/usr/bin/docker exec  db704cb0994323de548223410112d5ae2aa34f6956eb596426e1b34660746d42 sh -c "cat /etc/*release | grep ^ID"
node:fs:2346
    return binding.writeFileUtf8(
                   ^

Error: EACCES: permission denied, open '/__w/_temp/_runner_file_commands/save_state_4604b84b-649d-4192-9347-a0bb87279729'
    at Object.writeFileSync (node:fs:2346:20)
    at Object.appendFileSync (node:fs:2427:6)
    at Object.issueFileCommand (/__w/_actions/actions/checkout/v4/dist/index.js:3060:8)
    at Object.saveState (/__w/_actions/actions/checkout/v4/dist/index.js:2977:31)
    at 4866 (/__w/_actions/actions/checkout/v4/dist/index.js:2402:10)
    at __nccwpck_require__ (/__w/_actions/actions/checkout/v4/dist/index.js:38194:43)
    at 2565 (/__w/_actions/actions/checkout/v4/dist/index.js:150:34)
    at __nccwpck_require__ (/__w/_actions/actions/checkout/v4/dist/index.js:38194:43)
    at 9210 (/__w/_actions/actions/checkout/v4/dist/index.js:1171:36)
    at __nccwpck_require__ (/__w/_actions/actions/checkout/v4/dist/index.js:38194:43) {
  errno: -13,
  code: 'EACCES',
  syscall: 'open',
  path: '/__w/_temp/_runner_file_commands/save_state_4604b84b-649d-4192-9347-a0bb87279729'
}

Node.js v20.13.1

PR merged for reference

GitHub Actions sets up a user named github to execute commands inside the Docker container. This user often has UID/GID 1001, but it might vary based on the container’s configuration.

As the CKAN images now use a non-root user (ckan/ckan-sys - UID/GID = 503/502) in the running container there may be problems with permissions on GitHub Actions

A temporary workaround is to add an option to the CKAN container in the workflow file to run as root

…
runs-on: ubuntu-latest
container:
image: ckan/ckan-dev:${{ ckan-version }}
options: --user root
…

[kowh-ai]

Maybe a specific GitHub Actions image is required in future rather than a base or dev image

[amercader]

Copying my comment in #80 re options:

• Stick with the changes on ckan-dev, make a proper release and document that existing workflows need to be updated to include the following, plus update the extension template in core and all extensions in the ckan org:

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index e8bb23a..73d4d51 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -24,6 +24,7 @@ jobs:
     runs-on: ubuntu-latest
     container:
       image: ckan/ckan-dev:${{ matrix.ckan-version }}
+      options: --user root
     services:
       solr:
         image: ckan/ckan-solr:${{ matrix.ckan-version }}-solr9

• Revert the ownership changes in the ckan-dev images only. I'm not sure if there are wider implications for this or if its feasible at all but perhaps is not a stretch to run images meant for testing and development as root?
• Create a dedicated Github Actions image as @kowh-ai suggests
• Something else?

[ThrawnCA]

Maybe a specific GitHub Actions image is required in future rather than a base or dev image

I'm in favour of the ckan-dev image simply reverting the change and running as root, which would make it compatible with GitHub Actions. By definition that image shouldn't be used in any situation where privilege escalation is a major concern, right? It's meant to run code that is not yet fully tested, maybe outright broken, so it should never have access to anything genuinely sensitive. Simplicity of use is more important than defence-in-depth security.

@amercader The 'test-infrastructure' setup does look interesting - but testing a plugin would still involve checking out the plugin code, right? And the GitHub checkout action is broken by the account change.

[wardi]

Let's avoid the ckan and ckan-dev images diverging even more than they already do. If we aren't running the same images for both at least they should be as similar as possible to minimize problems discovered only when moving to production. Running ckan and ckan-dev as different users is a big difference.

Reverting the ckan-dev changes would also be going against best practices https://www.docker.com/blog/understanding-the-docker-user-instruction/

Can the better release management for ckan-docker-base that @amercader has been working on be used so that extensions can point to a specific ckan-docker-base release version for running tests and when we make changes to master we don't break everything all at once?

[amercader]

Let's avoid the ckan and ckan-dev images diverging even more than they already do. If we aren't running the same images for both at least they should be as similar as possible to minimize problems discovered only when moving to production.

I'm not sure I agree, the base and dev images are virtually the same except the dev ones:

• Install the dev requirements
• Have a different entrypoint script that:
  • Installs locally mounted extensions
  • Serves CKAN via werkzeug instead of uwsgi

All of these are required to do development work or run the tests, and you want none of these in a production image, so it makes sense to keep them separate. We could create a common image that installs the dev reqs (bad IMO as it increases the risk of vulnearbilites) and run either werkzeug or uwsgi depeding on a runtime env var, but I don't think the complexity and potential risks justify the benefit of using just one image.

Running ckan and ckan-dev as different users is a big difference.

That is true, and the main thing we need to assess when choosing which way to go.

1. Keeping the images aligned in how they manage permissions is good from a security and maintainability point of view, but involves having to update hundreds (ok maybe several dozens) of workflow files in the near future.
2. Reverting the dev images to run as root is less secure. This a difference that can be clearly documented going forward.

I'm less concern about the best practice and security aspect of this in the ckan-dev image, as I agree with @ThrawnCA that it's fair to assume that these are not secure to run a production site. I'm slightly more worried about problems arising from different permissions related errors happening in one or the other but not both sets of images, which will complicate debugging.

Can the better release management for ckan-docker-base that @amercader has been working on be used so that extensions can point to a specific ckan-docker-base release version for running tests and when we make changes to master we don't break everything all at once?

A better release management is needed and definitely merging to master shouldn't change the pushed images (it doesn't now), but pinning the image in the workflow in practice would mean 90% of maintainers not upgrading the images when a new release is out and workflows stuck in old patch releases. For the ones that want to keep track we will create release tags but I'm skeptical that most maintainers will keep those up to date (including us in extensions in the ckan org)

[amercader]

@EricSoroos @pdelboca @smotornyuk @tino097 @Zharktas any thoughts on this?

[EricSoroos]

What is the github user trying to write? Would it work to run as the ckan user instead of root?

[wardi]

Let's avoid the ckan and ckan-dev images diverging even more than they already do. If we aren't running the same images for both at least they should be as similar as possible to minimize problems discovered only when moving to production.

I'm not sure I agree, the base and dev images are virtually the same except the dev ones:

* Install the dev requirements

* Have a different entrypoint script that:
  
  * Installs locally mounted extensions

This no longer true and IMHO a big improvement. Now locally mounted extensions are installed with a separate command, not every time on start up. This also makes restarting the dev environment much faster.

Running ckan and ckan-dev as different users is a big difference.

That is true, and the main thing we need to assess when choosing which way to go.

1. Keeping the images aligned in how they manage permissions is good from a security and maintainability point of view, but involves having to update hundreds (ok maybe several dozens) of workflow files in the near future.

2. Reverting the dev images to run as root is less secure. This a difference that can be clearly documented going forward.

We could publish -dev images that revert just the user change now (but leave the change on master) then announce the change as an upcoming ckan-docker-base release to give users some time to update their workflow files. That's probably better than leaving everyone's workflows broken.

[ThrawnCA]

We could publish -dev images that revert just the user change now (but leave the change on master) then announce the change as an upcoming ckan-docker-base release to give users some time to update their workflow files. That's probably better than leaving everyone's workflows broken.

I would like to point out that the current "fix" in workflow files involves running the container with --user root and therefore has all the same problems of "DEV and PROD permissions aren't the same."

Currently I don't know a way someone could use ckan-dev in a GitHub Actions workflow while preserving the non-root user.

[wardi]

For github action tests we could recommend running the ckan cli and pytest with sudo -u ckan so that we catch permission errors that would happen in dev or prod.

[ThrawnCA]

IIRC the older Alpine Linux containers don't have sudo, so there would need to be extra steps to jump through in order to maintain support for them.

[amercader]

IIRC the older Alpine Linux containers don't have sudo, so there would need to be extra steps to jump through in order to maintain support for them.

We are deprecating the Alpine Linux images and no longer working on them so I assume the new ownership changes weren't ported there. We need to do a proper announcement but you can already migrate to the Python/Debian based ones, see the tags in the README for details

[kowh-ai]

@amercader - yes correct the ownership changes were only ported to the Dockerfiles (and subsequent images) that have python:3.10-slim-bookworm as a base image. All images that use Alpine Linux as a base image were not changed as we are encouraging users to migrate to the python:3.xx-slim-bookworm images

[amercader]

For github action tests we could recommend running the ckan cli and pytest with sudo -u ckan so that we catch permission errors that would happen in dev or prod.

The Debian based Python image don't have sudo either, so we will have to install it on the image ourselves if we want to recommend this

[wardi]

If we don't have sudo how about su ckan -c 'pytest ...'?

[ThrawnCA]

If we don't have sudo how about su ckan -c 'pytest ...'?

That would behave differently if it's run as the non-root user, though (eg needing a password). So it adds even more hoops to jump through, which leaves more scope for DEV and PROD getting out of sync.

Unless you're proposing to entirely replace the USER commands with su?

[wardi]

I'm proposing only changing the commands run in github actions since they must be executed with user=root.

i.e. no changes to ckan-docker-base, prod and dev would be the same defaulting to run as a non-root user.

[EricSoroos]

As part of the github workflow, can we create/mount directories that need to be GHA writable that aren't part of the usual ckan stuff?

[amercader]

I tested using su ckan -c "" commands in here. Annoyingly there are failures caused by the pytest coverage plugin, which is part of the test command generated in the extension template:

https://github.com/ckan/ckanext-dcat/actions/runs/11935587926/job/33267263002#step:8:118

In general, I think we'll have the same issue with any process that tries to write a file in the mounted runner directory. In this case it was some utility, but tests themselves trying to write a file in their own directory will also fail, see e.g:

https://github.com/ckan/ckanext-dcat/actions/runs/11951458054/job/33315134324#step:8:416

If we keep the dev images as they are, I think we should stick with options: --user root and not complicate things more

[amercader]

As discussed in today's dev meeting:

• We'll follow ahead with the permission changes in both base and dev images
• We won't be recommending any alternative setups in GitHub Actions other than options: --user root as they are just not supported by Github and bound to break in the future
• We will do a new release listing the changes done to the images, and announcing the changes needed in workflow files:
  • Use Debian based images e.g. ckan/ckan-dev:2.10 -> ckan/ckan-dev:2.10-py3.10
  • Use the options: --user root options

The only thing left to do the release is closing #88, merging #89 and finishing #73 .