Skip to content

fix(mcp): require live write access for staged agent actions - #859

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-mcp-action-staging-vulnerability
Jun 19, 2026
Merged

fix(mcp): require live write access for staged agent actions#859
JSONbored merged 1 commit into
mainfrom
codex/fix-mcp-action-staging-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The MCP gittensory_propose_action path could stage write-capable actions (approve/merge/close/label) based on cached control-panel maintainer scope derived from PR author_association, which can misrepresent real GitHub write permission.
  • Staged actions are executable later via the approval endpoint, so cached maintainer evidence could let a low-privileged session escalate to repository mutations when the app has installation permissions.

Description

  • Require live GitHub collaborator write/maintain/admin permission before a session can stage MCP actions by updating requireRepoManageAccess to call getRepositoryCollaboratorPermission and validate against REPO_WRITE_PERMISSIONS in src/mcp/server.ts.
  • Import getRepositoryCollaboratorPermission and add the REPO_WRITE_PERMISSIONS constant in src/mcp/server.ts to centralize the write-permission check.
  • Harden the approval decision endpoint to use the stricter requireRepoWriteAccess gate so accepting/rejecting pending actions also requires verified live write permission in src/api/routes.ts.
  • Add regression coverage by mocking getRepositoryCollaboratorPermission and asserting that cached COLLABORATOR association without live write permission cannot stage actions in test/unit/mcp-automation-state.test.ts.
  • Files changed: src/mcp/server.ts, src/api/routes.ts, test/unit/mcp-automation-state.test.ts.

Testing

  • Ran unit tests for the MCP change with npm test -- --run test/unit/mcp-automation-state.test.ts, which passed.
  • Ran related approval and BYOK route tests with npm test -- --run test/unit/routes-agent-approval.test.ts test/unit/routes-ai-byok.test.ts, which passed.
  • Performed static type checking with npm run typecheck, which completed successfully.

Codex Task

@ghost

ghost commented Jun 18, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #859 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 18, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.69%. Comparing base (5d9d73a) to head (077a7ab).

Files with missing lines Patch % Lines
src/mcp/server.ts 72.72% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #859      +/-   ##
==========================================
- Coverage   96.71%   96.69%   -0.02%     
==========================================
  Files         108      108              
  Lines       14641    14650       +9     
  Branches     5298     5301       +3     
==========================================
+ Hits        14160    14166       +6     
- Misses        102      103       +1     
- Partials      379      381       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

@ghost

ghost commented Jun 18, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 3 changed file(s) — two independent AI reviewers.

Changed files: src/api/routes.ts, src/mcp/server.ts, test/unit/mcp-automation-state.test.ts

Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the suggestions below before merging.

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
The PR tightens MCP repo‑action gating by verifying live GitHub write permissions via the collaborator API and updates the route to use the new gate, adding corresponding unit tests.

Suggestions

  • Add "push" to the REPO_WRITE_PERMISSIONS set (GitHub returns "push" for write access, not "write").
  • Confirm that a helper named requireRepoWriteAccess exists and is exported; if not, implement or import it.
  • Consider adding a test case where getRepositoryCollaboratorPermission returns "push" to ensure the new permission is accepted.

Worth double-checking

  • If "push" is not included, legitimate users with write access will be incorrectly blocked.
  • Missing or mismatched requireRepoWriteAccess implementation could cause compile/runtime errors.

Reviewer B · mistral-small-3.1-24b-instruct — recommends ✅ merge
This PR updates the MCP server to require live write access for staged agent actions, ensuring that only users with the appropriate permissions can propose actions. The changes are well-documented and include necessary updates to the API routes and tests.

Suggestions

  • Consider adding a comment in the requireRepoWriteAccess method to explain why the scope.operator check is separate from the permission check.
  • Ensure that the getRepositoryCollaboratorPermission function is thoroughly tested, especially for edge cases where permissions might not be as expected.

Worth double-checking

  • Verify that the new permission check does not introduce any performance bottlenecks, especially if the getRepositoryCollaboratorPermission function involves network calls.
  • Ensure that the tests cover all possible permission scenarios, including cases where the installation ID is null or the permission check fails.

@JSONbored JSONbored self-assigned this Jun 18, 2026
@JSONbored JSONbored changed the title Require live write permission for staged agent actions fix(mcp): require live write access for staged agent actions Jun 18, 2026
@JSONbored
JSONbored force-pushed the codex/fix-mcp-action-staging-vulnerability branch from 91ed609 to 077a7ab Compare June 18, 2026 22:21
@JSONbored
JSONbored merged commit 8bd04cb into main Jun 19, 2026
18 of 20 checks passed
@JSONbored
JSONbored deleted the codex/fix-mcp-action-staging-vulnerability branch June 19, 2026 05:48
@github-actions github-actions Bot mentioned this pull request Jun 19, 2026
12 tasks
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