Nothing catches the release job losing its contents: write until a release fails to publish #107

Closed
opened 2026-08-25 15:16:36 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-08-25 15:16:36 +00:00 (Migrated from github.com)

Found by running #100's own mutation. Filed rather than fixed there, because it is a different concern.

Nothing verifies the release job can publish

build.yml's release job carries the one permission it needs:

  release:
    permissions:
      # Needed to create the release. Declared explicitly rather than relying on the
      # repository default, so the token's reach is visible here.
      contents: write

Delete those lines and nothing notices. Measured:

check result with the permission deleted
actionlint (pinned digest) exit 0 — clean
CodeQL actions/missing-workflow-permissions silent — a narrower permission is not an alert
every gating status check unaffected; none of them runs this job

The job is if: startsWith(github.ref, 'refs/tags/v'), so it runs only on a tag push. A PR
cannot exercise it, and neither can a merge to main. The first thing that would notice is a
release failing to publish
, at the moment someone is trying to cut one.

Why this is worth a ticket rather than a shrug

The repository is careful about exactly this class of thing elsewhere: every action is pinned by
SHA, both linters are pinned by digest, and #100 has just added a top-level contents: read so the
token's reach is readable in the file. The release path is the one place where a silent regression
costs the most and is caught the latest.

It is also one deletion away, and the deletion looks like tidying — a top-level
permissions: contents: read now exists directly above it, so a future reader could reasonably
think the job-level block is redundant. It is not; it is an override. #100's comment says so, but a
comment is not a check.

What would actually catch it

Cheapest first:

  1. A meta-test over the workflow. Parse build.yml and assert the release job declares
    contents: write. Runs in the JVM suite, costs milliseconds, and bites the moment the line goes.
    BackupExclusionsTest is the precedent — it asserts the content of data_extraction_rules.xml
    for the same reason: a resource nothing else checked.
  2. A dry-run release on a scratch tag. Truest, but it publishes something or needs cleanup, and
    the failure mode it guards against is rare enough that the cost may not be worth it.
  3. Document and accept, naming the exposure. Honest, and the weakest.

Recommend (1). It converts "nobody would notice" into "CI notices", which is the actual gap.

Done means

  • Removing the release job's contents: write makes something red before a tag is pushed.
  • The mutation is the acceptance: delete the line, watch the new check fail, restore. A guard
    that cannot be shown to fire is the thing this ticket exists to complain about, so it would be an
    unusually poor outcome to add one.
_Found by running #100's own mutation. Filed rather than fixed there, because it is a different concern._ ### Nothing verifies the release job can publish `build.yml`'s `release` job carries the one permission it needs: ```yaml release: permissions: # Needed to create the release. Declared explicitly rather than relying on the # repository default, so the token's reach is visible here. contents: write ``` **Delete those lines and nothing notices.** Measured: | check | result with the permission deleted | |---|---| | actionlint (pinned digest) | **exit 0** — clean | | CodeQL `actions/missing-workflow-permissions` | **silent** — a *narrower* permission is not an alert | | every gating status check | unaffected; none of them runs this job | The job is `if: startsWith(github.ref, 'refs/tags/v')`, so it runs only on a tag push. A PR cannot exercise it, and neither can a merge to `main`. **The first thing that would notice is a release failing to publish**, at the moment someone is trying to cut one. ### Why this is worth a ticket rather than a shrug The repository is careful about exactly this class of thing elsewhere: every action is pinned by SHA, both linters are pinned by digest, and #100 has just added a top-level `contents: read` so the token's reach is readable in the file. The release path is the one place where a silent regression costs the most and is caught the latest. It is also **one deletion away**, and the deletion looks like tidying — a top-level `permissions: contents: read` now exists directly above it, so a future reader could reasonably think the job-level block is redundant. It is not; it is an override. #100's comment says so, but a comment is not a check. ### What would actually catch it Cheapest first: 1. **A meta-test over the workflow.** Parse `build.yml` and assert the `release` job declares `contents: write`. Runs in the JVM suite, costs milliseconds, and bites the moment the line goes. `BackupExclusionsTest` is the precedent — it asserts the content of `data_extraction_rules.xml` for the same reason: a resource nothing else checked. 2. **A dry-run release** on a scratch tag. Truest, but it publishes something or needs cleanup, and the failure mode it guards against is rare enough that the cost may not be worth it. 3. **Document and accept**, naming the exposure. Honest, and the weakest. Recommend (1). It converts "nobody would notice" into "CI notices", which is the actual gap. ### Done means - Removing the `release` job's `contents: write` makes something red **before** a tag is pushed. - **The mutation is the acceptance**: delete the line, watch the new check fail, restore. A guard that cannot be shown to fire is the thing this ticket exists to complain about, so it would be an unusually poor outcome to add one.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#107