Skip to content

fix(github): preserve install health least privilege - #1275

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-installation-health-over-request-issue
Jun 24, 2026
Merged

fix(github): preserve install health least privilege#1275
JSONbored merged 1 commit into
mainfrom
codex/fix-installation-health-over-request-issue

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent the install-health display from over-requesting pull_requests:write when the persisted missingPermissions list contains only the permission name and not the required access level, preserving least privilege.

Description

  • Change enrichInstallationHealth to only upgrade pull_requests to write when missingPermissions contains pull_requests and the stored health.permissions.pull_requests satisfies read (indicating an upgrade to write is required); otherwise keep the baseline pull_requests: read remediation. (src/github/backfill.ts)
  • Add a unit test that covers the least-privilege display case where pull_requests is absent entirely and the remediation should request read rather than write. (test/unit/backfill.test.ts)

Testing

  • Ran the focused unit tests with npx vitest run test/unit/backfill.test.ts -t "pull_requests", and the targeted tests passed.
  • Attempted coverage with npm run test:coverage -- test/unit/backfill.test.ts, but V8 coverage remapping failed with TypeError: jsTokens is not a function while generating the local coverage report.
  • A full npm run test:ci could not complete in this environment due to local dependency/registry issues (pg / ioredis type resolution and a 403 from the registry), so the complete gate was not run here.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.03%. Comparing base (654e7ca) to head (5723767).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1275   +/-   ##
=======================================
  Coverage   95.03%   95.03%           
=======================================
  Files         177      177           
  Lines       19894    19894           
  Branches     7170     7170           
=======================================
  Hits        18906    18906           
  Misses        395      395           
  Partials      593      593           
Files with missing lines Coverage Δ
src/github/backfill.ts 92.88% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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