Skip to content

security: remove contents:write from onboard-maintainer workflow#103

Open
huangzesen wants to merge 1 commit intomainfrom
security/workflow-audit-20260501
Open

security: remove contents:write from onboard-maintainer workflow#103
huangzesen wants to merge 1 commit intomainfrom
security/workflow-audit-20260501

Conversation

@huangzesen
Copy link
Copy Markdown
Collaborator

Security Audit — Workflow Permission Fix

Audit Date: 2026-05-01
Auditor: security-engineer
Approved by: CTO

Problem

The onboard-maintainer.yml workflow declares contents: write in its top-level permissions block. This is unnecessary — all git push operations in the workflow use secrets.PAT_TOKEN (set in the checkout step), not GITHUB_TOKEN. The contents: write scope on GITHUB_TOKEN grants the workflow more power than it needs.

Risk Assessment

Risk Level Detail
PAT_TOKEN scope too broad 🔴 High Classic token with contents:write + issues:write. Only needs repos.addCollaborator
PAT_TOKEN ownership unknown 🔴 High Could be personal token — if owner leaves, token breaks or leaks
Workflow permissions excess 🟡 Medium contents: write on GITHUB_TOKEN not needed — removed in this PR

What This PR Does

  • Removes contents: write from the workflow's permissions block
  • All git/push operations continue to use secrets.PAT_TOKEN (unaffected)
  • Adds security audit comments for traceability

What This PR Does NOT Do (follow-up required)

  • PAT_TOKEN migration: classic → fine-grained (scope limited to this repo only, addCollaborator permission)
  • PAT_TOKEN ownership confirmation (personal → organization-level)
  • Add pre-grant audit step (notify maintainer before granting write access)
  • Quarterly PAT_TOKEN rotation policy

Testing

  • Verify workflow still runs after merge (issue labeled → maintainer onboarding)
  • Verify checkout step still works (PAT_TOKEN handles git push)
  • Verify issue lock/unlock/comment still works (uses GITHUB_TOKEN with issues: write)

Co-authored-by: CTO cto@scienceintelligence

SECURITY AUDIT 2026-05-01:
- Removed 'contents: write' permission (not needed for issue workflow)
- All git/push uses PAT_TOKEN from checkout step, not GITHUB_TOKEN
- TODO: Migrate PAT_TOKEN from classic to fine-grained token

Reviewed-by: security-engineer
Approved-by: CTO
@huangzesen
Copy link
Copy Markdown
Collaborator Author

🔴 P0 Security Fix — Request for Review

This PR removes from the Onboard Reviewer workflow. Without this fix, the workflow has read/write access to all repository content.

Context: CEO has set a 48h deadline (2026-05-03T22:42Z UTC). If not resolved, PAT_TOKEN will be force-deleted.

Change: 6 lines — removes one permission, preserves , adds security audit comments.

Please review and merge. Thank you.

— Security Engineer

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.

1 participant