Skip to content

fix(security): patch OS packages in the published image and scope workflow permissions - #4592

Open
gantoine wants to merge 3 commits into
masterfrom
claude/harden-image-and-workflow-permissions
Open

gantoine wants to merge 3 commits into
masterfrom
claude/harden-image-and-workflow-permissions

Conversation

@gantoine

@gantoine gantoine commented Sep 17, 2026

Copy link
Copy Markdown
Member

Description
Explain the changes or enhancements you are proposing with this pull request.

Clears the security findings raised against rommapp/romm (the image) and romm (the workflows). Two independent changes.

1. OS packages in the published image

Every base image is pinned by digest, so the image ships whatever package set upstream had when it last rebuilt. Alpine backports security fixes into the 3.23 repo well ahead of those rebuilds, so the pin alone leaves known-vulnerable packages in the image even on a fresh build. apk upgrade --no-cache in production-stage takes them.

The same call is needed in python-alias, because production-stage copies /usr/lib out of that stage over its own. Without it, stale libs from the Python base overwrite the freshly patched ones (verified: liblzma.so.5.8.3 lands in the final image instead of 5.8.4).

ALPINE_SHA256 is also refreshed (3.23.4 → 3.23.5); it was the only stale digest of the five, and it feeds the builder stages.

What a rebuild now ships, measured in the built full-image:

Package Finding Before After
curl / libcurl double free, High 8.17.0-r1 8.22.0-r0
openssl (libcrypto3/libssl3) memory corruption → crash or RCE, High 3.5.6-r0 3.5.8-r0
libexpat DoS via algorithmic complexity, High 2.7.5-r0 2.8.4-r0
util-linux (libuuid) DoS, High 2.41.4-r0 2.41.6-r1
libpng DoS or RCE via heap overflow, Medium 1.6.57-r0 1.6.58-r1
nghttp2 DoS via assertion failure, Medium 1.68.0-r0 1.69.0-r0
musl libc incorrect control flow, Medium 1.2.5-r23, already the 3.23 head; the rebuild is what ships it

nginx itself stays at 1.29.8-r1 (Alpine's own repo does not carry a newer one), so the mod_zip module compiled from those sources still matches. nginx -t with both dynamic modules loaded passes in the built image.

An apk upgrade off a fixed base digest has no changing input, and release builds import a persistent registry cache — so the layer would be replayed from a previous release and reship its package set. build.yml therefore passes no-cache-filters: python-runtime,production-stage on both build-push steps. The Python-side upgrade lives in its own python-runtime stage rather than in python-alias, because python-alias is the base for backend-build and sigil-build and busting it would drag uv sync and the sigil cmake build along too. Preview builds in test-build.yml are left alone: they write a separate preview-<arch> cache that release builds never read.

2. GitHub Actions permissions

permissions: read-all grants the token read on every scope rather than the one a job needs; it is replaced with contents: read in the eight workflows that used it. On top of that:

  • typecheck, pytest and migrations carried job-level checks: write + pull-requests: write, but no step in them posts a check or a comment. Removed.
  • trunk-check keeps checks: write, which trunk-action uses for annotations.
  • update-hltb-api-url had contents: write + pull-requests: write at the workflow level; those moved down to the single job that opens the PR.

build.yml, test-build.yml and copilot-setup-steps.yml were already least-privilege and are untouched.

Out of scope

  • @vue/server-renderer XSS — scoped to marketing-site, not this repo. This repo's lockfile resolves vue/@vue/server-renderer to 3.5.29 and npm audit reports 0 vulnerabilities, so there is nothing to bump here.
  • GitHub organization IP allow list — an org setting, not a repository change.

Why both stages need the upgrade:

graph TD
  subgraph py["python-runtime"]
    P0["python:3.14-alpine3.23<br/>pinned digest"] --> P1["apk upgrade &lpar;added&rpar;"]
  end
  subgraph prod["production-stage"]
    N0["nginx:1.29.8-alpine3.23<br/>pinned digest"] --> N1["apk upgrade &lpar;added&rpar;<br/>then apk add"]
    N1 --> N2["COPY /usr/lib from python-runtime"]
  end
  P1 -. "overwrites what apk upgrade just patched,<br/>so this side has to be patched too" .-> N2
  N2 --> OUT["slim-image / full-image"]
Loading

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

No unit tests: the change is a Dockerfile build step and workflow metadata. Verified instead by building docker/Dockerfile --target full-image end to end on both linux/amd64 and linux/arm64, then in the built image: listing the installed package versions (table above), loading ngx_http_zip_module.so and ngx_http_js_module.so via nginx -t, and smoke-running python3 (backend imports reach its own DB-credentials check), RAHasher, valkey-server, 7zz and bsdtar. The cache filter was checked by rebuilding with --no-cache-filter python-runtime,production-stage and confirming only those two stages re-ran while backend-build, sigil-build, frontend-build, rahasher-build, nginx-build and emulator-stage stayed CACHED. trunk fmt && trunk check is clean.

Screenshots (if applicable)

No user-visible UI change.


AI assistance disclosure: this PR was written end to end with AI assistance (Claude Code) - the investigation, the code changes, the commits and this description. Every version number and behavioural claim above comes from actually running the builds and inspecting the resulting images, not from the model's recollection.

🤖 Generated with Claude Code

gantoine and others added 2 commits September 17, 2026 13:08
… carry

The base images are pinned by digest, so the image ships whatever packages
were current when upstream last rebuilt them. Alpine backports fixes to its
3.23 repo well ahead of that, leaving known-vulnerable curl, openssl,
libexpat, util-linux, libpng and nghttp2 in the published image.

`apk upgrade` in production-stage takes those patches, and the same call in
python-alias keeps the /usr/lib copy from putting stale libs back. The Alpine
digest is also refreshed to 3.23.5, for the builder stages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`read-all` grants the token read on every scope, not just the one a job
needs, so replace it with `contents: read`. The job-level `checks: write` and
`pull-requests: write` in typecheck, pytest and migrations were writes no
step makes; trunk-check keeps `checks: write` for its annotations, and
update-hltb-api-url moves its write scopes down to the job that opens the PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 17, 2026 17:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 3/5

The PR should not merge until published builds reliably invalidate the OS-package upgrade layers and obtain current security fixes.

Summary

This PR narrows GitHub Actions token permissions and updates the published container’s Alpine package set.

  • Replaces broad workflow-level read permissions with contents: read.
  • Removes unused write permissions while retaining job-specific permissions for annotations and pull-request creation.
  • Refreshes the Alpine builder digest and upgrades packages in both stages that contribute runtime libraries.
  • The upgrade layers currently remain reusable from the publishing cache, preventing reliable uptake of future security fixes.

Reviews (1) · Last reviewed commit: "chore(ci): scope workflow permissions to..."

Comment thread docker/Dockerfile
Release builds import a persistent registry cache, and an `apk upgrade` off a
fixed base digest has no changing input, so the layer would be reused from a
previous release and reship its package set.

`no-cache-filters` forces both upgrade stages to run on every release. The
Python-side upgrade moves into its own `python-runtime` stage so that busting
it does not also rebuild the venv and the sigil bindings.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gantoine gantoine added the on-hold Pending further research or blocked by another issue label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

on-hold Pending further research or blocked by another issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants