Skip to content

[sec-check] fix: declare top-level permissions: {} in pr-e2e.yml - #511

Merged
castrojo merged 2 commits into
mainfrom
sec/fix-top-level-permissions
Sep 5, 2026
Merged

[sec-check] fix: declare top-level permissions: {} in pr-e2e.yml #511
castrojo merged 2 commits into
mainfrom
sec/fix-top-level-permissions

Conversation

@kubestellar-hive

Copy link
Copy Markdown
Contributor

Security Fix

Adds an explicit top-level permissions: {} to: pr-e2e.yml

Each workflow already declares least-privilege permissions at the job level (verified: every job has its own block). The top-level nil default means any future job added without a permissions block gets an empty token instead of inheriting the repository default (potentially write).

Fixes #506


Filed by sec-check agent (ACMM L4/L5 — hold-gated mode). Hold-gated: human review required.

— hive: agent=sec-check backend=copilot model=kimi-k3 copilot=1.0.59

Each workflow already declares least-privilege permissions at the job level;
this makes the default explicit so any future job added without a permissions
block gets a nil token instead of the repository default.

Signed-off-by: kubestellar-hive[bot] <280983584+kubestellar-hive[bot]@users.noreply.github.com>
@kubestellar-hive
kubestellar-hive Bot requested a review from a team as a code owner August 8, 2026 22:40
@kubestellar-hive kubestellar-hive Bot added hold Work is intentionally paused. security labels Aug 8, 2026

@hanthor hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verified and approving.

I checked the claim in the body rather than taking it on faith — pr-e2e.yml has exactly two jobs and both declare their own block:

  • compose (L27) → contents: read, packages: write
  • e2e (L90) → contents: read, packages: write, and it calls ./.github/workflows/run-testsuite.yml as a reusable workflow, so the called workflow inherits the caller job's permissions rather than the top-level default. Adding permissions: {} therefore changes nothing for the current jobs and correctly fails-closed for any job added later without its own block.

On the red Lint & syntax check — not caused by this PR, and not fixable here. It is the pre-existing Containerfile:13 DL3063 warning: stage name should not be a reserved word (hadolint flags context as a reserved BuildKit named-context). I reproduced it locally against main's Containerfile and confirmed the contextctx rename clears it.

That rename landed in #501, which merged to testing at 2026-08-09T02:07Z. This PR targets main, and main line 13 is still FROM scratch AS context, so the check will stay red until the testing→main promotion carries #501 across. Nothing to do on this branch — it should go green on its own once promotion runs.

@castrojo
castrojo added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 0cb47c7 Sep 5, 2026
1 check passed
@castrojo
castrojo deleted the sec/fix-top-level-permissions branch September 5, 2026 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hold Work is intentionally paused. security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[sec-check] pr-e2e.yml: missing permissions block (secrets/write-ops present)

2 participants