Skip to content

Set detection job to explicit empty permissions#2644

Merged
pelikhan merged 3 commits into
mainfrom
copilot/update-detection-job-permissions
Oct 28, 2025
Merged

Set detection job to explicit empty permissions#2644
pelikhan merged 3 commits into
mainfrom
copilot/update-detection-job-permissions

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 27, 2025

The detection job had permissions: read-all which granted unnecessary repository access. The job only needs to download artifacts and run threat analysis.

Changes

  • Source: Changed buildThreatDetectionJob() to use NewPermissionsEmpty() instead of NewPermissionsReadAll()
  • Added explicitEmpty field to Permissions struct to support explicit empty permissions rendering
  • Created NewPermissionsEmpty() function that renders as permissions: {}
  • Updated RenderToYAML() to output permissions: {} when explicitEmpty is true
  • Workflows: Updated detection jobs in all 50 compiled .lock.yml files to use permissions: {}
  • Tests: Updated expectations to verify permissions: {}

Result

Detection jobs now explicitly declare empty permissions using permissions: {} instead of omitting the field entirely. This makes the intent clearer - the job has no permissions granted. The job still has implicit metadata read access which is sufficient for:

  • Downloading artifacts from the agent job
  • Running threat detection analysis
  • Uploading detection logs

Before:

detection:
  needs: agent
  runs-on: ubuntu-latest
  permissions: read-all
  steps: [...]

After:

detection:
  needs: agent
  runs-on: ubuntu-latest
  permissions: {}
  steps: [...]
Original prompt

The detection job should only have "metadata: read" as permission set.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Oct 27, 2025
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Update detection job to include only 'metadata: read' permission Remove read-all permissions from detection job Oct 27, 2025
Copilot AI requested a review from pelikhan October 27, 2025 23:57
Copy link
Copy Markdown
Collaborator

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

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

@copilot set permissions: metadata: true

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 28, 2025

@copilot set permissions: metadata: true

I investigated adding permissions: metadata: read but discovered that "metadata" is not a valid permission scope in the official GitHub Actions schema. The schema validation fails with: additional properties 'metadata' not allowed.

The official GitHub Actions permissions are: actions, attestations, checks, contents, deployments, discussions, id-token, issues, models, packages, pages, pull-requests, repository-projects, security-events, and statuses.

All GitHub Actions jobs have implicit metadata read access by default, regardless of their permission configuration. This is why removing the permissions: read-all line (my current implementation) achieves minimal permissions while still allowing the detection job to read metadata.

Could you clarify what you'd like me to do? Should I:

  1. Keep the current implementation (no explicit permissions = implicit metadata read only)
  2. Add "metadata" as a custom extension despite it not being in the official schema
  3. Something else?

Copilot AI requested a review from pelikhan October 28, 2025 00:30
@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot set permissions: meta: true

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 28, 2025

@copilot set permissions: meta: true

I checked the official GitHub Actions schema and "meta" is also not a valid permission scope. The schema validation would fail with: additional properties 'meta' not allowed.

The official permission scopes are: actions, attestations, checks, contents, deployments, discussions, id-token, issues, models, packages, pages, pull-requests, repository-projects, security-events, and statuses.

The current implementation (no explicit permissions) gives the detection job only implicit metadata access, which is the minimal permission set. This is working correctly and all tests pass.

If you need a different permission configuration for the detection job, could you specify which official permission scope you'd like to use?

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot use "permissions: {}"

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Oct 28, 2025

@copilot use "permissions: {}"

Done! The detection job now explicitly uses permissions: {} instead of omitting the permissions field. Commit: c507685

All 50 workflows have been updated with the explicit empty permissions object.

Copilot AI changed the title Remove read-all permissions from detection job Set detection job to explicit empty permissions Oct 28, 2025
@pelikhan pelikhan marked this pull request as ready for review October 28, 2025 01:39
Copilot AI review requested due to automatic review settings October 28, 2025 01:39
@github-actions
Copy link
Copy Markdown
Contributor

Agentic Changeset Generator triggered by this pull request.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 reduces the permissions granted to the detection job from read-all to explicitly empty (permissions: {}), following the principle of least privilege. The detection job only requires implicit metadata read access for its operations (downloading artifacts, running analysis, uploading logs) and doesn't need the broad repository read access previously granted.

Key changes:

  • Modified buildThreatDetectionJob() to use NewPermissionsEmpty() instead of NewPermissionsReadAll()
  • Added support for explicit empty permissions rendering in the Permissions struct
  • Updated all 50 compiled workflow lock files to reflect the new permissions: {} configuration

Reviewed Changes

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

File Description
pkg/workflow/permissions.go Added explicitEmpty field and NewPermissionsEmpty() function to support rendering permissions: {}
pkg/workflow/threat_detection.go Changed detection job to use NewPermissionsEmpty() instead of NewPermissionsReadAll()
pkg/workflow/threat_detection_test.go Updated test expectations to verify permissions: {} instead of permissions: read-all
.github/workflows/*.lock.yml (50 files) Updated compiled workflow files to use permissions: {} for detection jobs

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

@pelikhan pelikhan merged commit 520355f into main Oct 28, 2025
49 of 51 checks passed
@pelikhan pelikhan deleted the copilot/update-detection-job-permissions branch October 28, 2025 03:18
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