[codex] add cloud gateway account management - #402
Open
jeremymcs wants to merge 48 commits into
Open
Conversation
Contributor
🔴 PR Risk Report — CRITICAL
Affected Systems
File Breakdown
|
jeremymcs
marked this pull request as ready for review
June 2, 2026 15:27
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
jeremymcs
marked this pull request as draft
June 2, 2026 15:31
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issues.
Reviewed by Cursor Bugbot for commit beb1592. Configure here.
Member
Author
|
Accepted — queued for a code change. |
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
jeremymcs
force-pushed
the
codex/cloud-mcp-gateway-accounts-usage
branch
from
June 12, 2026 03:28
0515360 to
52ebbc7
Compare
Member
Author
|
Accepted — queued for a code change. |
Member
Author
|
Started an automated PR update. Merge conflicts were detected while updating this branch from the base branch. Conflicted files:
An automatic resolution attempt is running. If it cannot complete safely, this PR will be marked for manual follow-up. |
jeremymcs
force-pushed
the
codex/cloud-mcp-gateway-accounts-usage
branch
from
July 2, 2026 04:23
900cec2 to
44cea75
Compare
Member
Author
|
Started an automated PR update. Merge conflicts were detected while updating this branch from the base branch. Conflicted files:
An automatic resolution attempt is running. If it cannot complete safely, this PR will be marked for manual follow-up. |
Member
Author
|
Re: the latest Cursor Bugbot review (commit
Verification at HEAD: |
jeremymcs
force-pushed
the
codex/cloud-mcp-gateway-accounts-usage
branch
from
July 7, 2026 21:02
44cea75 to
c260f84
Compare
Member
Author
|
Started an automated PR update. Accepted review feedback is being applied. |
jeremymcs
added a commit
that referenced
this pull request
Jul 7, 2026
jeremymcs
added a commit
that referenced
this pull request
Jul 7, 2026
jeremymcs
force-pushed
the
codex/cloud-mcp-gateway-accounts-usage
branch
from
July 7, 2026 21:39
c9770ac to
3b88520
Compare
Member
Author
|
Started an automated PR update. Merge conflicts were detected while updating this branch from the base branch. Conflicted files:
An automatic resolution attempt is running. If it cannot complete safely, this PR will be marked for manual follow-up. |
…onal sentinel - Replace `===` bearer token comparison in mcp-server http.ts with timingSafeEqual (mirrors the cloud-mcp-gateway tokenMatches helper) - Fix readLimit in usage-limits.ts: values in (0, 1) no longer silently map to unlimited via `Math.floor(x) || undefined`; they now resolve to a minimum limit of 1. Only an explicit `0` maps to unlimited. - Add test case covering the fractional readLimit behaviour Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment on lines
+145
to
+153
| try { | ||
| await connection.client.close(); | ||
| } catch { | ||
| try { | ||
| await connection.transport.close(); | ||
| } catch { | ||
| // Best-effort cleanup only; the caller is already handling the real failure. | ||
| } | ||
| } |
…io child Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment on lines
31
to
34
| @@ -23,25 +33,91 @@ | |||
| throw new Error("GSD_CLOUD_USER_TOKEN is required"); | |||
| } | |||
Comment on lines
35
to
+38
| const authStorePath = options.authStorePath ?? process.env.GSD_CLOUD_AUTH_STORE_PATH; | ||
| const usageStorePath = options.usageStorePath ?? process.env.GSD_CLOUD_USAGE_STORE_PATH; | ||
| const adminToken = options.adminToken ?? process.env.GSD_CLOUD_ADMIN_TOKEN; | ||
| const allowRegistration = options.allowRegistration ?? parseBoolean(process.env.GSD_CLOUD_ALLOW_REGISTRATION); |
Comment on lines
+5
to
+15
| const { values } = parseArgs({ | ||
| options: { | ||
| port: { type: "string" }, | ||
| host: { type: "string" }, | ||
| "auth-store": { type: "string" }, | ||
| "usage-store": { type: "string" }, | ||
| "allow-registration": { type: "boolean" }, | ||
| help: { type: "boolean", short: "h" }, | ||
| }, | ||
| allowPositionals: false, | ||
| }); |
…eanly Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (4)
packages/cloud-mcp-gateway/src/server.ts:310
- This route match is too permissive: it will also accept paths like
/admin/api/users/<id>/disabled/anythingbecause it doesn't validate the segment count. Require the exact expected shape so extra segments return 404.
userId: user.userId,
packages/cloud-mcp-gateway/src/server.ts:321
- This route match is too permissive: it will also accept paths like
/admin/api/users/<id>/pairing-codes/anythingbecause it doesn't validate the segment count. Require the exact expected shape so extra segments return 404.
if (disabled && isLastActiveAdmin(auth, user)) {
packages/cloud-mcp-gateway/src/server.ts:331
- This route match is too permissive: it will also accept paths like
/admin/api/tokens/<id>/revoke/anythingbecause it doesn't validate the segment count. Require the exact expected shape so extra segments return 404.
return sendJson(res, 201, auth.createPairingCode(user.userId));
packages/cloud-mcp-gateway/src/server.ts:374
- This route match is too permissive: it will also accept paths like
/account/api/tokens/<id>/revoke/anythingbecause it doesn't validate the segment count. Require the exact expected shape so extra segments return 404.
return sendJson(res, 201, {
…n/account routes Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
| } | ||
| }); | ||
|
|
||
| boot(); |
…d Clerk is loaded Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment on lines
+53
to
+59
| const takeValue = (): string => { | ||
| if (inlineValue !== undefined) return inlineValue; | ||
| const next = argv[i + 1]; | ||
| if (next === undefined) throw new Error(`missing value for ${flag}`); | ||
| i += 1; | ||
| return next; | ||
| }; |
…nsuming it Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment on lines
+207
to
212
| issueUserToken(userId: string, options: { label?: string } = {}): UserTokenIssue { | ||
| if (!this.users.has(userId)) throw new Error(`Unknown user: ${userId}`); | ||
| const userToken = `${USER_TOKEN_PREFIX}${randomBytes(32).toString("hex")}`; | ||
| const record = this.addUserToken(userToken, userId, options); | ||
| return { userId, tokenId: record.tokenId, userToken }; | ||
| } |
Comment on lines
+425
to
+429
| // The Clerk scripts load with the defer attribute, so they execute only | ||
| // after the document finishes parsing (just before DOMContentLoaded). | ||
| // Running boot() synchronously here would see window.Clerk undefined and | ||
| // wrongly report "Clerk is not configured", so wait for DOMContentLoaded | ||
| // when parsing is still in progress. |
…k boot comment Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment on lines
+58
to
+66
| let configId = this.toolRoutes.get(toolName); | ||
| if (!configId) { | ||
| // Routes are only populated by advertisedTools(); a call can arrive before | ||
| // that has run (or after the routes were cleared). Refresh once before | ||
| // deciding the tool is not ours so a valid forwarded tool isn't rejected. | ||
| await this.advertisedTools().catch(() => undefined); | ||
| configId = this.toolRoutes.get(toolName); | ||
| } | ||
| if (!configId) return { handled: false }; |
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (2)
packages/cloud-mcp-gateway/src/usage-limits.ts:89
- When minute limits are not enabled,
check()still returnsusage.minuteincremented to 1. This makes the returned quota status internally inconsistent (minute usage is reported despite no minute window being tracked/enforced).
usage: {
...status.usage,
minute: minute + 1,
day: status.usage.day + (billable ? 1 : 0),
month: status.usage.month + (billable ? 1 : 0),
packages/cloud-mcp-gateway/src/usage-limits.ts:103
check()unconditionally setsresetAt.minute, even whencallsPerMinuteis unlimited/undefined. This can mislead callers/UI into thinking a minute window exists. MakeresetAt.minuteconditional on minute limits being enabled.
// noteAccepted() just recorded this call, so the minute window is now
// non-empty even if it was empty pre-acceptance; surface its reset time
// (oldest call in the window + WINDOW_MS) instead of the stale undefined.
minute: (calls[0] ?? now) + WINDOW_MS,
},
Comment on lines
+74
to
+76
| if (!status.allowed) return status; | ||
| this.noteAccepted(user.userId, now); | ||
| if (billable) this.reserveBillable(user.userId, now); |
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 39 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
packages/cloud-mcp-gateway/src/mcp.ts:93
knownTooldetection callsparams.registry.listTools(params.userId)(and then.some(...)) before quota enforcement. This means a minute-throttled (or otherwise rejected) request can still trigger a full runtime tool-list scan/allocation on the hot/mcppath, which under spammy tool calls can become an avoidable CPU cost and partially defeats the stated intent of “throttle before dispatching.” Consider restructuring so the expensive runtime-tool lookup is avoided when quota rejects, e.g. by adding an O(1)registry.hasTool(userId, toolName)index, or by splittingUsageLimiterinto a minute-only precheck followed by a billable day/month reservation once tool existence is confirmed.
// A tool is "known" when it is a gateway built-in or a runtime-advertised
// tool. Built-ins short-circuit before the runtime tool-name lookup
// (listTools + per-call scan) to keep the hot path cheap.
const knownTool = BUILTIN_TOOL_NAMES.has(toolName)
|| params.registry.listTools(params.userId).some((tool) => tool.name === toolName);
Comment on lines
+432
to
+436
| if (document.readyState === "loading") { | ||
| document.addEventListener("DOMContentLoaded", boot); | ||
| } else { | ||
| boot(); | ||
| } |
…nput Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment on lines
+70
to
+83
| await new Promise<void>((resolve, reject) => { | ||
| // A listen error (e.g. EADDRINUSE) fires the server's 'error' event, not the | ||
| // listen callback, so tie it into the promise to fail startup fast instead of | ||
| // awaiting forever. | ||
| const onError = (err: Error) => reject(err); | ||
| server.once('error', onError); | ||
| server.listen(options.port, options.host, () => { | ||
| server.removeListener('error', onError); | ||
| resolve(); | ||
| }); | ||
| }); | ||
| const displayHost = options.host === '0.0.0.0' ? 'localhost' : options.host; | ||
| return { | ||
| url: `http://${formatUrlHost(displayHost)}:${options.port}/mcp`, |
Comment on lines
354
to
357
| snapshot(): AuthStoreSnapshot { | ||
| // Return shallow copies so a caller can't mutate live in-memory auth records | ||
| // (and thus what gets persisted) by editing the returned snapshot. | ||
| return { |
…s in snapshot Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Comment on lines
+344
to
+356
| function normalizeInputSchema(value: unknown): RuntimeToolInputSchema { | ||
| if (isInputSchema(value)) { | ||
| return { | ||
| ...value, | ||
| type: "object", | ||
| ...(value.properties ? { properties: normalizeProperties(value.properties) } : {}), | ||
| ...(Array.isArray(value.required) | ||
| ? { required: value.required.filter((item): item is string => typeof item === "string") } | ||
| : {}), | ||
| }; | ||
| } | ||
| return { type: "object", properties: {} }; | ||
| } |
…perties/required Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Comment on lines
+97
to
+99
| async close(): Promise<void> { | ||
| await Promise.all(Array.from(this.connections.keys()).map((id) => this.closeConnection(id))); | ||
| } |
…s on shutdown Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gsd-mcp-server --httpplus README coverage for cloud/remote setupWhy
Remote MCP clients need a safer hosted gateway surface: operators need users, tokens, runtime visibility, usage accounting, and quotas, while local runtimes need a way to expose browser and other stdio MCP tools without exposing local websocket endpoints.
Validation
pnpm install --lockfile-only --ignore-scriptspnpm install --ignore-scriptspnpm --filter @opengsd/contracts buildpnpm --filter @opengsd/rpc-client buildpnpm --filter @opengsd/mcp-server buildpnpm --filter @opengsd/cloud-mcp-gateway testpnpm --filter @opengsd/mcp-server testpnpm --filter @opengsd/daemon buildpnpm --filter @opengsd/daemon testNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is enabled.Note
High Risk
Touches authentication, token lifecycle, Clerk integration, and quota enforcement on the public
/mcppath—misconfiguration could lock out users or expose endpoints; large surface area across gateway, daemon, and mcp-server.Overview
This PR turns the Cloud MCP Gateway into an operable hosted product: admin (
/admin) and Clerk-backed account (/account) UIs, REST APIs for users/tokens/pairing/runtimes/usage, persisted auth and usage stores, and plan-based quotas that block/mcptool calls before they hit the local runtime (throttled calls logged as non-billable).The gateway MCP layer now merges runtime-advertised tools (with
runtimeId/projectAliasrouting) alongside built-in GSD tools, records per-call metrics, and enforces limits via a new usage limiter. Auth grows into a full user registry (roles, plans, revocablegsd_usr_tokens, disabled users, optionalPOST /register).On the daemon side, cloud runtimes can bridge stdio MCP servers (default
gsd-browser mcp, configurable via env) and advertise those tools upstream; LocalToolExecutor delegates unknown tool names to that bridge.gsd-mcp-servergains--httpStreamable HTTP on/mcpwith bearer auth defaults that refuse public unauthenticated binds unless loopback or--no-auth.Reviewed by Cursor Bugbot for commit beb1592. Bugbot is set up for automated code reviews on this repo. Configure here.