Skip to content

[Refactor:System] Add top-level permissions#12464

Merged
williamjallen merged 5 commits intoSubmitty:mainfrom
Ash092016:ci/add-top-level-permissions
Mar 3, 2026
Merged

[Refactor:System] Add top-level permissions#12464
williamjallen merged 5 commits intoSubmitty:mainfrom
Ash092016:ci/add-top-level-permissions

Conversation

@Ash092016
Copy link
Contributor

@Ash092016 Ash092016 commented Feb 22, 2026

Why is this Change Important & Necessary?

Fixes #12463

The CI workflows do not set a top-level permissions: block. Without one, all jobs without explicit job-level permissions inherit the default GITHUB_TOKEN permissions which on push events includes write access to contents, packages, deployments, etc.

This follows the GitHub-recommended least-privilege principle for workflow tokens and reduces the blast radius if a dependency or action is compromised.

What is the New Behavior?

Added a top-level permissions block to all 11 workflow files that were missing one:

Workflow Permission Rationale
ci.yml contents: read All 17 jobs only need actions/checkout; codecov uses its own token
documentation_check.yml contents: read Only checks PR title format
bump_repo.yml contents: read PR creation uses separate SUBMITTYBOT_DEPENDENCY_TOKEN
localization_up.yml {} (none) Dispatch uses separate SUBMITTYBOT_DEPENDENCY_TOKEN
move_to_in_review.yml contents: read Project mutations use GitHub App token
notify_issues.yml {} (none) Only uses curl with Zulip secret
notify_main_fail.yml {} (none) Only uses curl with Zulip secret
sort_draft_prs.yml contents: read Project mutations use GitHub App token
first_issue_reply.yml contents: read Existing job-level write-all override remains intact
first_pr_open.yml contents: read Existing job-level pull-requests: write override remains intact
label_stale_prs.yml contents: read Existing job-level pull-requests: write override remains intact

Jobs that need elevated permissions retain their existing job-level overrides, which take precedence over the top-level default.

What steps should a reviewer take to reproduce or test the bug or new feature?

  1. Review each workflow file to confirm the permissions: block is placed correctly (between on: and env:/jobs:)
  2. Verify that all existing CI jobs still pass this is a permissions-only change, no logic is modified
  3. Confirm that jobs with existing job-level permissions: (e.g., first_issue_reply.yml, first_pr_open.yml, label_stale_prs.yml) still have their overrides intact

Automated Testing & Documentation

  • No new tests needed this is a CI configuration change only
  • All existing CI jobs should pass unchanged since no job requires GITHUB_TOKEN write access beyond what's already declared at job-level
  • No documentation update required

Other information

  • Not a breaking change top-level permissions only restricts the default; job-level overrides are unaffected
  • No migrations needed
  • Security improvement reduces token scope from full write to read-only by default across all workflows

@github-project-automation github-project-automation bot moved this to Seeking Reviewer in Submitty Development Feb 22, 2026
@codecov
Copy link

codecov bot commented Feb 23, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.66%. Comparing base (607796c) to head (a9fe63b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main   #12464   +/-   ##
=========================================
  Coverage     21.66%   21.66%           
  Complexity     9638     9638           
=========================================
  Files           268      268           
  Lines         36233    36233           
  Branches        486      486           
=========================================
  Hits           7849     7849           
  Misses        27902    27902           
  Partials        482      482
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.04% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.68% <ø> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 91.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@IDzyre
Copy link
Member

IDzyre commented Feb 24, 2026

The title check CI test is failing, ensure your title meets the guidelines at https://submitty.org/developer/getting_started/make_a_pull_request.

@Ash092016 Ash092016 changed the title [DevOps:CI] Add top-level permissions [Refactor:System] Add top-level permissions Feb 26, 2026
@Ash092016
Copy link
Contributor Author

Ash092016 commented Feb 26, 2026

Hi @IDzyre Sir, I've updated the title to [Refactor:System] Add top-level permissions and synced the branch with main. The workflows are currently awaiting approval to run.

@automateprojectmangement automateprojectmangement bot moved this from Seeking Reviewer to In Review in Submitty Development Feb 26, 2026
williamjallen
williamjallen approved these changes Feb 28, 2026
View reviewed changes
Copy link
Member

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

Changes look good to me. I also adjusted the repository settings to make the GITHUB_TOKEN read-only by default.

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Maintainer Review in Submitty Development Feb 28, 2026
IDzyre
IDzyre approved these changes Mar 2, 2026
View reviewed changes
Copy link
Member

@IDzyre IDzyre left a comment

Choose a reason for hiding this comment

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

As we don't need more than read permissions for the existing CI runs, the changes look valid to me. The CI passes, so there I see no reason to doubt the permissions for the existing workflow.

@williamjallen williamjallen merged commit 5c06175 into Submitty:main Mar 3, 2026
25 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Maintainer Review to Done in Submitty Development Mar 3, 2026
@Ash092016
Copy link
Contributor Author

Ash092016 commented Mar 3, 2026

Thank you @williamjallen @IDzyre for the review and the merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@williamjallen williamjallen williamjallen approved these changes

@IDzyre IDzyre IDzyre approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Submitty Development
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[DevOps:CI] Add top-level permissions to CI workflows for least-privilege security

3 participants

@Ash092016 @IDzyre @williamjallen