From b0b906155e10cdc70d2c28bcddc8471ae6f7866d Mon Sep 17 00:00:00 2001 From: Darrell van Swinderen Date: Tue, 22 Sep 2026 23:24:56 +0200 Subject: [PATCH] fix(security): workflow permissions and two e2e regexes Closes #26. **Both workflows without a `permissions` block now read and nothing else.** `ci.yml` only typechecks and `publisher-image.yml` pushes to Docker Hub with its own credentials, so neither needs the GitHub token to write anything, and without the block it gets the repository's default. `release.yml`, `pages.yml` and `dev-sync.yml` already declared theirs. **The e2e fake publishers compare the header whole.** Both parsed it with `/^Bearer\s+(.+)$/`, the same quadratic shape the real publisher just lost. In a test the exact header GoodBit sends is the thing being checked, so an equality says more than a parse did. The em dashes in the two workflow comments go too. `compress.spec.ts` and `publishGoodBits.spec.ts`: 10 passed. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/ci.yml | 7 ++++++- .github/workflows/publisher-image.yml | 10 ++++++++-- tests/e2e/compress.spec.ts | 5 +++-- tests/e2e/publishGoodBits.spec.ts | 5 +++-- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a8e073a9..b92f350f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,6 @@ name: CI -# Kept deliberately cheap. Typecheck only, on the branches that matter — the +# Kept deliberately cheap. Typecheck only, on the branches that matter: the # end-to-end suite needs a desktop session, a GPU and ffmpeg, so it runs before # a release on a real machine instead (see `npm run check:pre-release`). on: @@ -16,6 +16,11 @@ concurrency: group: ci-${{ github.ref }} cancel-in-progress: true +# Reads the code and nothing else. Without this the token gets the +# repository's default, which can be write, for a job that only typechecks. +permissions: + contents: read + jobs: check: runs-on: windows-latest diff --git a/.github/workflows/publisher-image.yml b/.github/workflows/publisher-image.yml index fb46af5d..870ec97c 100644 --- a/.github/workflows/publisher-image.yml +++ b/.github/workflows/publisher-image.yml @@ -7,7 +7,7 @@ name: Publisher image # # Only when the publisher itself changes. It used to run on every release tag, # which rebuilt an identical image for a release that had touched nothing in -# `publisher/` — the app and the publisher are versioned and shipped +# `publisher/`: the app and the publisher are versioned and shipped # separately, and there is no reason for one to drag the other along. on: push: @@ -17,6 +17,12 @@ on: - '.github/workflows/publisher-image.yml' workflow_dispatch: +# The image goes to Docker Hub with its own credentials, so the GitHub token +# only ever reads the checkout. The layer cache does not need a scope here: +# it authenticates with the runner's own token, not this one. +permissions: + contents: read + jobs: image: runs-on: ubuntu-latest @@ -76,7 +82,7 @@ jobs: ${{ secrets.PUBLISHER_IMAGE }}:${{ steps.meta.outputs.version }} ${{ secrets.PUBLISHER_IMAGE }}:latest # Layers are cached between runs, so a change to one source file does - # not reinstall the dependencies again — which matters most for + # not reinstall the dependencies again, which matters most for # arm64, where every command runs under emulation. Scoped by name so # it cannot collide with another workflow's cache in this repository. cache-from: type=gha,scope=publisher diff --git a/tests/e2e/compress.spec.ts b/tests/e2e/compress.spec.ts index 77b57792..b611f88e 100644 --- a/tests/e2e/compress.spec.ts +++ b/tests/e2e/compress.spec.ts @@ -81,8 +81,9 @@ async function startFakePublisher(): Promise { // The real publisher refuses every write without this, so the fake one does // too: otherwise the test would pass whether or not the app sent it. const gate: express.RequestHandler = (req, res, next) => { - const given = /^Bearer\s+(.+)$/i.exec(req.header('authorization') ?? '')?.[1]; - if (given !== TOKEN) { + // Compared whole rather than parsed: this is a test's fake publisher, + // and the exact header GoodBit sends is the thing being checked. + if (req.header('authorization') !== `Bearer ${TOKEN}`) { res.status(401).json({ message: 'Wrong or missing publish token.' }); return; } diff --git a/tests/e2e/publishGoodBits.spec.ts b/tests/e2e/publishGoodBits.spec.ts index 2d7d54c8..623bf8e6 100644 --- a/tests/e2e/publishGoodBits.spec.ts +++ b/tests/e2e/publishGoodBits.spec.ts @@ -47,8 +47,9 @@ async function startFakePublisher(): Promise { const TOKEN = 'a-test-publish-token'; const gate: express.RequestHandler = (req, res, next) => { - const given = /^Bearer\s+(.+)$/i.exec(req.header('authorization') ?? '')?.[1]; - if (given !== TOKEN) { + // Compared whole rather than parsed: this is a test's fake publisher, + // and the exact header GoodBit sends is the thing being checked. + if (req.header('authorization') !== `Bearer ${TOKEN}`) { res.status(401).json({ message: 'Wrong or missing publish token.' }); return; }