Skip to content

Add explicit permissions blocks so the default token can drop to read - #53

Closed
mmcky wants to merge 1 commit into
mainfrom
token-permissions-meta-347
Closed

Add explicit permissions blocks so the default token can drop to read#53
mmcky wants to merge 1 commit into
mainfrom
token-permissions-meta-347

Conversation

@mmcky

@mmcky mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Preparation for QuantEcon/meta#347 item 4 (default workflow token permissions → read). This repo currently has no permissions: block in any workflow, so three workflows rely on the repo-level write default; flipping the setting without these blocks would break publishing.

The blocks transcribe what the sibling lecture repos already carry: ci.yml and collab.yml get contents: read + pull-requests: write (what nwtgck/actions-netlify needs for the preview comment — the same block as lecture-jax/ci.yml), and publish.yml gets contents: write (the peaceiris/actions-gh-pages push) + actions: read (the dawidd6/action-download-artifact cache download — the same scopes lecture-jax/publish.yml grants them). linkcheck.yml and cache.yml write nothing with GITHUB_TOKEN and need no block. The notebooks sync uses QUANTECON_SERVICES_PAT and is unaffected.

Once a job carries a permissions: block, unlisted scopes drop to none regardless of the repo default — so this merges safely before or after the settings flip, and it is what makes the flip safe to do.

No conflict with #52 — different lines in the same files; whichever merges second picks both changes up cleanly.

🤖 Generated with Claude Code

Transcribes what the sibling repos already carry: ci and collab get
contents:read + pull-requests:write (the Netlify preview comment),
publish gets contents:write (peaceiris gh-pages push) + actions:read
(cache artifact download). linkcheck and cache write nothing with
GITHUB_TOKEN and need no block.

A job with a permissions block drops unlisted scopes to none, so this
is safe to merge before or after the repo-level flip -- and it is what
makes the flip safe. Part of QuantEcon/meta#347 item 4.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 5, 2026 02:36
@netlify

netlify Bot commented Aug 5, 2026

Copy link
Copy Markdown

Deploy Preview for timely-seahorse-68815c ready!

Name Link
🔨 Latest commit 071db14
🔍 Latest deploy log https://app.netlify.com/projects/timely-seahorse-68815c/deploys/6a72a1c4ee752100089de447
😎 Deploy Preview https://deploy-preview-53--timely-seahorse-68815c.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR prepares the repository for switching the default GITHUB_TOKEN permissions to read by adding explicit permissions: blocks to workflows that rely on elevated scopes (publishing to gh-pages and posting Netlify preview comments).

Changes:

  • Add workflow-level permissions: to publish.yml to allow gh-pages deployment and cross-workflow artifact download.
  • Add job-level permissions: to ci.yml and collab.yml intended to support Netlify PR preview comments with reduced default token permissions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/publish.yml Adds explicit workflow permissions for gh-pages publishing and artifact download.
.github/workflows/collab.yml Adds job-level permissions for PR preview/comment workflows (but needs actions: read for artifact download).
.github/workflows/ci.yml Adds job-level permissions for PR preview/comment workflows (but needs actions: read for artifact download).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/ci.yml
Comment on lines +6 to +8
permissions:
contents: read
pull-requests: write

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed — this PR was superseded by #54, whose consolidated block for ci.yml includes actions: read.

Comment on lines +6 to +8
permissions:
contents: write # peaceiris/actions-gh-pages pushes the built site to gh-pages
actions: read # dawidd6/action-download-artifact reads the cache.yml build artifact

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair catch, and now addressed in #55linkcheck.yml gets the same contents: read + actions: read block. For the record the download works today without it (public-repo Actions reads accept a token without the scope — the sibling repos' ci workflows rely on the same leniency), so #55 is consistency hardening rather than a live fix.

Comment on lines +6 to +8
permissions:
contents: read
pull-requests: write

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed — this PR was superseded by #54, whose consolidated block for collab.yml includes actions: read.

mmcky added a commit that referenced this pull request Aug 5, 2026
…#54)

Lands #51, #52, #53 and #50 as one change, and salvages the -n flag from #25.

- runs-on: disk=large -> volume=80gb, plus spot=false, on all four GPU
  workflows. Now matches lecture-jax and lecture-python.myst byte for byte,
  allowing for collab.yml's different image.
- Explicit permissions blocks on ci.yml, collab.yml and publish.yml so the
  default workflow token can drop to read. ci.yml and collab.yml also gain
  actions: read, which #53 declared only on publish.yml.
- dawidd6/action-download-artifact -> @v21 at all four call sites; collab.yml
  was still on v3 while the rest were on v9.
- -n added to the ci.yml PDF build so it matches publish.yml, closing a gap
  where a nitpick-class error passed the PR check and failed at publish time.

#51, #52 and #53 conflicted pairwise over the same region of ci.yml and
collab.yml, so they are resolved once here rather than through two sequential
rebases. Part of the rollout tracked in QuantEcon/meta#330, with the
permissions work from QuantEcon/meta#347.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mmcky

mmcky commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Landed via #54, with one addition.

This PR declared actions: read on publish.yml (with a comment noting that dawidd6/action-download-artifact needs it to read the cache.yml artifact) but not on ci.yml or collab.yml, which run the same action. #54 adds it to all three.

Worth recording that the omission was not actually breaking. lecture-jax's ci.yml runs dawidd6/action-download-artifact@v21 with contents: read and pull-requests: write alone and its recent runs are all green, because public-repo tokens can read Actions data without the explicit scope. So this was an internal inconsistency rather than a 403 in waiting. Declaring it is strictly additive and removes the reliance on that behaviour.

Merged in #54 rather than here because this PR, #51 and #52 conflicted pairwise over the same region of ci.yml and collab.yml.

@mmcky mmcky closed this Aug 5, 2026
mmcky added a commit that referenced this pull request Aug 6, 2026
#55)

Completes the hardening #54 landed for ci, collab and publish: the
dawidd6 cache download works today via public-repo API leniency, but
granting actions: read explicitly matches the documented requirement
and the blocks the other workflows now carry. Raised by Copilot on
the superseded #53.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

2 participants