Skip to content

ci: declare contents:read on test_old_cpu workflow#93

Merged
SwayamInSync merged 1 commit into
numpy:mainfrom
arpitjain099:chore/test-old-cpu-permissions
May 14, 2026
Merged

ci: declare contents:read on test_old_cpu workflow#93
SwayamInSync merged 1 commit into
numpy:mainfrom
arpitjain099:chore/test-old-cpu-permissions

Conversation

@arpitjain099
Copy link
Copy Markdown
Contributor

The Test on Older CPUs (x86_64-v2) workflow runs an Intel SDE-emulated test matrix against Sandy Bridge and Haswell baselines. It only checks out, downloads SDE, installs system deps, and runs the test suite under emulation. No GitHub API write, no comment-on-PR step.

This patch pins it to permissions: contents: read at workflow scope, matching the per-job block in build_wheels.yml (id-token: write for trusted publishing) and the workflow-level shape used by big_endian.yml, build_docs.yml, and typecheck.yml.

With explicit scope:

  • the workflow token can't be widened by a future change to the repo default
  • the SLSA / OpenSSF Scorecard Token-Permissions check passes for this file

No behavioural change.

The test_old_cpu matrix runs Intel SDE-emulated tests on Sandy Bridge
and Haswell baselines. No GitHub API write. contents:read is the floor.

Style matches the per-job permissions block in build_wheels.yml
(id-token:write for trusted publishing) and the workflow-level shape
used by big_endian.yml, build_docs.yml, typecheck.yml.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@SwayamInSync
Copy link
Copy Markdown
Member

This workflow is just for testing, so we anyways don't use API tokens here.
Pinging @ngoldbaum for input on how to proceed.

@seberg
Copy link
Copy Markdown
Member

seberg commented May 14, 2026

My understanding is that this makes sense, although we should maybe set the defaults to be more restrictive to avoid just this type of thing.
(These are per org/repo settings, but the default is more permissions.)

@SwayamInSync
Copy link
Copy Markdown
Member

My understanding is that this makes sense, although we should maybe set the defaults to be more restrictive to avoid just this type of thing. (These are per org/repo settings, but the default is more permissions.)

So @seberg should we take this patch or broadly fix the default ones?

@seberg
Copy link
Copy Markdown
Member

seberg commented May 14, 2026

OK, I changed it now, it might be that other workflows fail (but I suspect they are OK). I.e the default is now:

Workflows have read permissions in the repository for the contents and packages scopes only.

I suspect that means you can do either... Merging is OK as it's explicit and other jobs still have it, but I am not sure there would be any difference now.

@arpitjain099
Copy link
Copy Markdown
Contributor Author

Thanks @seberg, tightening the org default to read-only is the more durable fix.

With the default restricted this PR is mostly defense-in-depth: the file still declares intent in-tree, which keeps the SLSA / Scorecard Token-Permissions check passing per-file and survives a future default change, but functionally the workflow gets the same scopes either way.

Happy either way. Merge if you'd like the explicit declaration on this file, or close as covered-by-org-default. All checks are green on the branch.

@SwayamInSync
Copy link
Copy Markdown
Member

Thanks @seberg and @arpitjain099
Merging this in!!

@SwayamInSync SwayamInSync merged commit fc52921 into numpy:main May 14, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants