JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

CodeQL: build.yml has no top-level permissions, so its test job inherits the repository default #100

New Issue
Closed
opened 2026-08-25 13:46:44 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-08-25 13:46:44 +00:00 (Migrated from github.com)
Copy Link
Copy Source

The one open CodeQL alert on this repository. Three siblings of it are already fixed.

The alert

#1 — actions/missing-workflow-permissions, warning / security severity medium,
at .github/workflows/build.yml:23:

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an
explicit permissions block, using the following as a minimal starting point: {contents: read}

Alerts 2, 3 and 4 were the same rule against status_check.yml and are fixed — that file gained a
top-level permissions: contents: read.

What is actually missing

build.yml declares permissions in one place, and it is the right place as far as it goes:

  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

There is no top-level block, so the test job inherits the repository default instead.

workflow top-level jobs
status_check.yml contents: read —
api37-debug.yml present —
build.yml none test: inherits · release: contents: write

This is hygiene, not exposure — say so rather than overstating it

The repository default is already read (default_workflow_permissions: read,
can_approve_pull_request_reviews: false, checked via the API). So the test job holds a read token
today and nothing is over-privileged right now.

What the fix buys is protection against that default changing — a repository setting, or an
organisation policy — and silently widening a job that never needed write. That is not a
hypothetical worth dismissing: it is the exact reasoning already written into status_check.yml,
which even names this file:

…the token's reach should be readable here, and a default that widens later should not silently
widen these jobs with it. build.yml's release job makes the opposite declaration for the same
reason.

So the repo has already decided this principle and applied it in two of three workflows plus one job
of the third. build.yml's top level is the gap, and the file's own sibling argues for closing it.

Fix

Two lines at the top of build.yml, with the comment carrying the reason the way the others do:

permissions:
  contents: read

The release job's contents: write continues to override it for the one job that needs it — that
is how job-level permissions work, and it is why this does not break releases.

Done means

  • CodeQL alert #1 closes (it re-scans on push to main).
  • The release job can still create a release. This is the thing to check rather than assume:
    a top-level block that accidentally replaced rather than defaulted would break tagging, and
    tagging is not exercised on an ordinary PR. Confirm the job-level contents: write is still
    present and still wins.
  • actionlint and shellcheck stay clean at their pinned digests (#70 added actionlint; it parses
    these workflows).

Mutation: delete the release job's contents: write and confirm something notices — if
nothing does, the release path has no guard at all and that is a second finding worth its own
ticket rather than a silent pass.

_The one open CodeQL alert on this repository. Three siblings of it are already `fixed`._ ### The alert **#1** — `actions/missing-workflow-permissions`, `warning` / security severity `medium`, at `.github/workflows/build.yml:23`: > Actions job or workflow does not limit the permissions of the `GITHUB_TOKEN`. Consider setting an > explicit permissions block, using the following as a minimal starting point: `{contents: read}` Alerts 2, 3 and 4 were the same rule against `status_check.yml` and are `fixed` — that file gained a top-level `permissions: contents: read`. ### What is actually missing `build.yml` declares permissions in **one** place, and it is the right place as far as it goes: ```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 ``` There is **no top-level block**, so the `test` job inherits the repository default instead. | workflow | top-level | jobs | |---|---|---| | `status_check.yml` | `contents: read` | — | | `api37-debug.yml` | present | — | | **`build.yml`** | **none** | `test`: inherits · `release`: `contents: write` | ### This is hygiene, not exposure — say so rather than overstating it **The repository default is already `read`** (`default_workflow_permissions: read`, `can_approve_pull_request_reviews: false`, checked via the API). So the `test` job holds a read token today and nothing is over-privileged right now. What the fix buys is protection against that default **changing** — a repository setting, or an organisation policy — and silently widening a job that never needed write. That is not a hypothetical worth dismissing: it is the exact reasoning already written into `status_check.yml`, which even names this file: > …the token's reach should be readable here, and a default that widens later should not silently > widen these jobs with it. **build.yml's release job makes the opposite declaration for the same > reason.** So the repo has already decided this principle and applied it in two of three workflows plus one job of the third. `build.yml`'s top level is the gap, and the file's own sibling argues for closing it. ### Fix Two lines at the top of `build.yml`, with the comment carrying the reason the way the others do: ```yaml permissions: contents: read ``` The `release` job's `contents: write` continues to override it for the one job that needs it — that is how job-level permissions work, and it is why this does not break releases. ### Done means - CodeQL alert #1 closes (it re-scans on push to `main`). - **The release job can still create a release.** This is the thing to check rather than assume: a top-level block that accidentally *replaced* rather than defaulted would break tagging, and tagging is not exercised on an ordinary PR. Confirm the job-level `contents: write` is still present and still wins. - `actionlint` and `shellcheck` stay clean at their pinned digests (#70 added actionlint; it parses these workflows). **Mutation:** delete the `release` job's `contents: write` and confirm something notices — if nothing does, the release path has no guard at all and that is a second finding worth its own ticket rather than a silent pass.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels confirmed documentation sev:low
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#100