Skip to content

fix(auth): block MCP token from app-role gates - #2451

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-mcp-token-authorization-bug
Jul 2, 2026
Merged

fix(auth): block MCP token from app-role gates#2451
JSONbored merged 1 commit into
mainfrom
codex/fix-mcp-token-authorization-bug

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Close a high-severity authorization gap where the shared GITTENSORY_MCP_TOKEN static identity could flip the deployment-wide global agent kill-switch via the new operator route.

Description

  • Update requireAppRole to explicitly reject the mcp static identity so the shared MCP token cannot satisfy operator-only app-role gates (src/api/routes.ts).
  • Add a regression test that asserts a request using GITTENSORY_MCP_TOKEN is forbidden for both GET /v1/app/kill-switch and POST /v1/app/kill-switch, does not call setGlobalAgentFrozen, and leaves the DB state unchanged (test/unit/routes-kill-switch.test.ts).
  • Regenerated the OpenAPI snapshot via npm run ui:openapi as part of the change workflow (artifact created at apps/gittensory-ui/public/openapi.json).

Testing

  • Ran the targeted unit suite with npx vitest run test/unit/routes-kill-switch.test.ts and all tests in that file passed (11/11).
  • Ran npm run typecheck and it completed with no type errors.
  • Ran npm run test:coverage -- --run test/unit/routes-kill-switch.test.ts where the test file ran and passed, but the global coverage check failed as expected because the full test corpus was not executed in this targeted run; a full unsharded coverage run was attempted but did not complete in this environment.
  • Attempts to run the full local gate (npm run test:ci) and npm audit --audit-level=moderate were blocked by external network/tooling issues (actionlint setup / npm audit endpoint) in this environment.

Codex Task

@loopover-orb

loopover-orb Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-02 05:23:07 UTC

2 files · 1 AI reviewer · no blockers · readiness 70/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The change closes the specific auth gap by making `requireAppRole` reject the MCP static identity before non-session identities can pass app-role gates. The added kill-switch regression covers both read and write paths and verifies the write side effect does not run, which matches the described failure mode. The visible diff is small, coherent, and safe enough to proceed with no blocking defects.

Nits — 3 non-blocking
  • nit: `src/api/routes.ts:4951` hard-codes the `"mcp"` actor check inline; a shared helper or named constant would make future app-role exclusions less error-prone if more static identities are added.
  • `src/api/routes.ts:4951`: consider extracting the MCP exclusion into a named predicate such as `isMcpStaticIdentity(identity)` so other app-role gates cannot accidentally reimplement this differently later.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ⚠️ 1 scoped overlap Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:XS; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 554 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 554 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 65 PR(s), 554 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (PR #2459, PR #2461)
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


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

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 2, 2026
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.88%. Comparing base (51d283e) to head (97d83c7).
⚠️ Report is 23 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2451   +/-   ##
=======================================
  Coverage   95.88%   95.88%           
=======================================
  Files         224      224           
  Lines       25142    25144    +2     
  Branches     9142     9143    +1     
=======================================
+ Hits        24108    24110    +2     
  Misses        421      421           
  Partials      613      613           
Files with missing lines Coverage Δ
src/api/routes.ts 94.73% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 758deed into main Jul 2, 2026
12 checks passed
@JSONbored
JSONbored deleted the codex/fix-mcp-token-authorization-bug branch July 2, 2026 05:28
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant