Repository navigation
[Feat] Add inbound MCP client access management and revocation - #3657
Conversation
|
No new code issues found. See session Reviewed the three test-file updates since Current-head CI: lint, type checks, Knip, docs, upgrade compatibility and backup restore passed. Tests, both Docker builds and JavaScript/TypeScript CodeQL analysis were still running when inspected. The earlier CodeQL alert discussion remains unresolved; this test-only update does not alter the hashing paths. Repository validation suites were left to CI. No approval submitted. Reviewed 6f6267d |
|
Pushed 6f6267d7 to fix the Test failures. Reproduced all 13 failures: older injected OAuth test contexts lacked the newly required live-grant identity. Updated the three affected suites; database-backed download tests now create and clean up real Redis grants. Added a regression keeping the verified credential present while revoking its grant, proving the subsequent download is refused before object storage, plus catalog refusal on revoked/unavailable grants. Verified on the pushed head: GitHub Test now passes (8m43s), and the fresh automated review found no new code issues. Current-head lint, typecheck, knip, docs, compatibility/restore, both Docker builds and all three CodeQL analysis jobs passed. Judgement reports no rule violations with incomplete coverage. Local validation: 63/63 targeted tests, then the full API suite: 3,434/3,434 tests across 290 files. API typecheck, scoped lint, managed commit check and normal pre-push lint/typecheck/knip gates passed. This follow-up is test-only, so no new browser proof applies. PR metadata has been refreshed. CodeQL's result check remains blocked on alert-write permission, not an identified code defect. Current check 125898620951 reports the same alerts 159 / 160. Inspected SARIF: sources lead to public user-ID index hashing or hashes of 256-bit random OAuth codes/tokens—not human passwords. Targeted false-positive dismissal is the appropriate correction; the attempt returned HTTP 403: Resource not accessible by integration. Default CodeQL setup does not justify a checked-in rule-wide exclusion, and changing the key encoding or adding a slow password KDF would be inappropriate. No scanner suppression or hashing workaround was added. The CodeQL thread remains unresolved because the alerts/check are not cleared; security/code-scanning write authorization is required. No merge performed. |
Related issue
Fixes #3584, including the latest request to revoke all of a user's clients.
Why this PR exists
Operators and members need deployment-side controls for inbound MCP access. Deleting a refresh session alone left previously issued access tokens usable for up to an hour.
What changed
Upgrade impact
Deploy the new web token issuer, drain all old issuer instances, then deploy API enforcement. Legacy already-issued access JWTs contain no client/grant identity and receive 401 from the enforcing API. Existing usable refresh tokens can obtain tracked access tokens without extending the original session expiry. Clients without a usable refresh session, or unable to recover by refreshing after 401, must reconnect/re-consent. In-progress old authorization codes must repeat consent. Redis data loss cannot be backfilled; outages fail closed. Rolling back the API removes prompt revocation enforcement; rolling back the issuer can require reconnecting migrated clients.
How it was tested
REDIS_URL=redis://127.0.0.1:16379 pnpm exec dotenvx run -f .env.test -- pnpm --filter @roomote/api exec vitest run --maxWorkers=4. Reproduced all 13 initial CI failures in older API fixtures, then updated injected OAuth contexts to carry grant identity. Database-backed download fixtures now create real Redis grants. Added regressions for download access after live grant revoke and catalog refusal on revoked/unavailable grants. All 63 tests in the three directly affected suites pass. Real signed-token HTTP tests cover refusal on subsequent/mcp, model and artifact-download requests, including old-token rejection after reconnect.pnpm lint:fast, all 27check-types:fasttasks, andpnpm knippassed, including normal pre-push hooks. Docs build validation and broken-link checks passed. Full monorepo tests were not run.js/insufficient-password-hashalerts 159 and 160. SARIF traces public user IDs and generated 256-bit OAuth codes/tokens into SHA-256 Redis lookup keys, not human-password storage. Targeted false-positive dismissal was denied HTTP 403: Resource not accessible by integration. No password-hash rule exclusion, scanner-evasion source change or unnecessary slow KDF was introduced; clearing the alerts requires code-scanning write authorization.The latest fix changes test files only; new browser proof is not applicable. The initial feature proof report retains the earlier real-app screenshots and provenance. Those images establish UI states, not the CI correction or backend enforcement.
Checklist
[Fix],[Feat],[Improve],[Refactor],[Docs], or[Chore]followed by a user-facing descriptionpnpm lintandpnpm check-typespass locally — the repository's normallint:fast,check-types:fastand pre-push gates passed; the full formatting-inclusive commands were not run.pnpm changeset— an equivalent changeset was authored directly at.changeset/inbound-mcp-authorizations.md, with a major bump and explicit compatibility note.