[Microbiosis]

Adds mcp-oauth-console: sign in to MCP servers that require OAuth.

This branch also answers the review on #77 — the reply lists the Must fix items.

What it does

Runs an OAuth sign-in for a server, keeps the token inside the Node process, writes the Authorization header into the MiniMax Code MCP configuration, and can refresh the token before it expires.

The page shows two independent facts per server: what the server demands (probed from the endpoint) and what this machine holds (the local credential). A server can require OAuth and still be satisfied by a local token, and one can need no OAuth and still have no token — a single status field hides one of the two.

Notes for the reviewer

• Device code (RFC 8628) is the recommended way in. No redirect, so nothing depends on the browser reaching the Mini App. Authorization code + PKCE is the fallback: from MiniMax Code 3.1.2 the Host authenticates every request to the loopback port, so the callback page answers 401 and the authorization code has to be copied back from the address bar.
• A 401 alone is not OAuth. Only a WWW-Authenticate: Bearer resource_metadata="…" challenge (RFC 9728) counts, so the page distinguishes requires OAuth / requires a Bearer credential / requires a credential, scheme undeclared / not required / undetermined.
• The probe matches the declared transport: a real initialize call for streamable-http, a GET for sse.
• Background refresh is an explicit switch, off by default. A write to mcp.json is surgical, atomic, backed up, and bound to the path the user confirmed.

Checks

npm run check: plugins/microbiosis/mcp-oauth-console: OK, repository: OK. CI green on 10c5af9.

Files read and written, network hosts, spawned processes, state, credentials, Agent turns and the tested environment are in the package README: README.md.

Package tests: node --test "tests/**/*.test.mjs" from the package directory.

[MyPrototypeWhat]

Reviewed at f543139.

Thanks for this. The dual status model, device-code preference when the Host owns the loopback port, keeping tokens off the page, and the surgical Authorization patch are all solid. npm run check reports the package OK on this head, and GitHub Actions is green. A few things need to change before merge.

Must fix

1. miniapp/node/errors.mjs is imported but not in the package. Both the entry and the domain service import it (server.mjs:16 → describeServiceError / isKnownServiceError; oauth-service.mjs:21 → serviceError), and the client comments point at it too (app.js:36). The file is not among the 15 paths in this PR (confirmed with git ls-tree on f543139 and a direct import() of oauth-service.mjs, which fails with Cannot find module '.../errors.mjs'). scripts/validate.mjs only checks manifests, so it still prints OK. As shipped, start() cannot load. Please add the missing module (catalog of codes → HTTP status + user-facing message, plus serviceError / isKnownServiceError), or fold that catalog into an existing Node file and drop the dead imports.

2. Credential paths must not follow .. in the server name. loadCredential / saveCredential build the path as join(credentialDir, \${serverName}.json`) (oauth-service.mjs:570, :593). path.joinresolves.., so a name like ../..//xlands outside this plugin'scontext.dataDir(and can touch another plugin's directory under the same parent). The authorize / device / apply routes takeserverName from the request body (server.mjs:212, :227, :262), including names that are not yet in the Host MCP config, so this is reachable from the page's "add server" flow. Please restrict names the same way other packages do (for example ^[A-Za-z0-9._-]+$, reject ./.., and/or resolve the path and refuse anything that is not still under credentialDir`).

3. Host MCP config write: accepted, with conditions. We accept writing the Host MCP config. The README discloses which file is edited and how, and the backup / atomic / serialized write approach matches repo docs/security.md ("Writing outside dataDir"). Before merge, please meet these conditions:

   a. Only write the file the user confirmed, and realpath it. Discovery currently tries MINIMAX_MCP_CONFIG, parents of context.dataDir, cwd, the Node executable's directory, then the default Host location (resolveMcpConfigPath, oauth-service.mjs:154-176). The env var and default-location fallback are not a published contract, and paths are not realpath'd, so an unrelated mcp.json under cwd or next to the executable can win. Write only the resolved default file or one the user confirms in the UI, realpath'd before read and write.

   b. Hold the lock across the whole read-modify-write. The read / modify / verify in patchMcpAuthorization around :987 is not fully under the write lock, so another process (or a second apply) editing this file at the same time can lose entries. Take the lock before reading, re-read inside it, and release only after the write is verified.

   c. Background refresh must be opt-in or confirmed each time. Every 60s, refreshExpiring (server.mjs:300-309) can write credentials and patch the Host MCP config with no second confirm; the first apply asks the user, the timer does not. Make background refresh an explicit opt-in, or ask for confirmation on each write.

   d. Write credential files with mode 0o600. Tokens are stored in cleartext under the plugin's credential dir (save path around oauth-service.mjs:593). Set the mode explicitly on create and keep it on rewrite, so a looser umask cannot leave the grant world-readable. This applies on macOS/Linux; on Windows the mode is effectively ignored and nothing extra is needed.

   e. Back up mcp.json before every write, including background refresh writes. The plugin already backs up before writing (backupConfig into context.dataDir, oauth-service.mjs:509-511, README :73-74); please keep that. refreshServer (:919) goes through the same patchMcpAuthorization path, so refresh writes are backed up today too (:1004, :1045). Two gaps remain: backupConfig (:190-199) swallows errors and returns false, and the write still goes ahead, so a failed backup is silent; and every write overwrites the single mcp.json.bak, so a quiet refresh replaces the backup taken before the user's last confirmed apply. Please abort the write if the backup fails, and keep the pre-apply backup from being clobbered by refresh writes (for example a separate or timestamped backup).

4. Outbound fetches must share one URL guard, including private networks. assertTargetUrl (oauth-service.mjs:316-333) runs on the MCP endpoint at authorize / device start, but the resource_metadata URL from WWW-Authenticate (:377) and the issuer / .well-known / token / device / registration endpoints taken from that metadata are fetched without the same check. The allowlist also lets RFC1918 and IPv4-mapped forms through (10/8, 192.168/16, 172.16/12, [::ffff:127.0.0.1]). Warm-up and /api/discover probe server.url from the Host config without assertTargetUrl either (server.mjs:186, :319). A hostile MCP can point metadata at an internal address (SSRF). Please run every fetch target through one checker that rejects private / link-local / metadata addresses (and, if you ever follow redirects, re-check after each hop).

5. Callback page renders the error param as raw HTML. handleCallback takes error straight from the callback URL and concatenates it into the result message (`授权被拒绝：${error}`, oauth-service.mjs:847); renderCallbackPage then inserts that message unescaped into <p>${detail}</p> (server.mjs:416). Anyone who can get the user to open a crafted callback URL (or a hostile authorization server redirecting with a chosen error) can inject markup and script into a page served from the plugin's origin. Please HTML-escape detail (and title) before interpolating it (&, <, >, ", ').

Optional

1. PKCE complete path treats a missing state as success. In completeAuthorization (oauth-service.mjs:798), if (state && state !== session.state) only fails when a state is present and wrong; a pasted callback URL with no state still exchanges the code. Prefer requiring an exact match whenever the session stored one.

2. JSON key lookup is first-match substring of the key spelling. findMemberObjectSpan / findMemberStringSpan (oauth-service.mjs:111-142) use indexOf(JSON.stringify(key)), so a server name that appears as a key elsewhere in the file (or a nested "Authorization" outside the intended entry) can patch the wrong span — confirmed by running it. Scoping the search to mcpServers first, or parsing and re-serializing only that entry while accepting format churn, would be safer.

3. Creating a brand-new server entry rewrites the whole file (:990-1007), which conflicts with the README claim that other bytes stay unchanged.

4. Apply confirm is page-only. The confirm step lives in the page (app.js:838-881); POST /api/apply has no confirm token. Anything that can call that route can write the Host MCP config without the dialog.

5. Browser open on non-Windows / non-macOS. openInBrowser (oauth-service.mjs:706-719) uses explorer.exe or open. On Linux neither is reliable; the page already shows the URL to copy, so a short README / UI note for "open failed — use the link" is enough.

6. Manifest / catalog copy still describes only authorization-code + PKCE. .minimax-plugin/plugin.json:6 and the root README table row omit the device-code path that this head recommends. Aligning those one-liners with the package README would avoid underselling the preferred flow.

7. GRANT_TTL_MS is defined but unused. Either wire it or drop it so the lifetime story matches the code.

8. No plugin tests. There is no tests/ tree. Even a few unit cases around assertTargetUrl, safe server-name → credential path, and the Authorization patch (replace / insert headers / create entry) would have caught item 2 above and would help the next review.

[MyPrototypeWhat]

Update: please disregard condition d (0o600 on credential files); it's no longer required. Conditions a, b, c and e still apply. The other Must fix items still apply.

[Microbiosis]

MyPrototypeWhat requested changes on f543139.

All five Must fix items and the eight Optional ones are addressed on 55be487 (rebased onto upstream/main 799b772). npm run check: mcp-oauth-console: OK, repository: OK. CI green.

Supersedes my review comment above (GitHub does not allow deleting a submitted review).

Must fix

1. miniapp/node/errors.mjs now ships — catalog of code → status + message, plus serviceError / isKnownServiceError / describeServiceError. tests/entry-loads.test.mjs imports the entry, the service and the catalog, so a module that is imported but not shipped fails the tests. The same file asserts every thrown code is registered and that no file throws a raw code.
2. Server names cannot leave the credential directory — assertServerName allows only ^[A-Za-z0-9._-]{1,64}$ and rejects . / ..; credentialPath resolves and proves the path is still inside. tests/credential-path.test.mjs.
3. Host config write.
   • (a) resolveMcpConfigPath uses only the data-dir chain and ~/.minimax/mcp.json; MINIMAX_MCP_CONFIG, cwd and the Node executable's directory are gone. The path is realpathed, shown in the page, and every write carries the confirmed expectedPath.
   • (b) The read-modify-verify-write runs under one lock and re-reads inside it.
   • (c) Background refresh is an explicit switch, off by default (<dataDir>/settings.json). The manual button always runs.
   • (d) Withdrawn per your follow-up — not implemented.
   • (e) A failed backup aborts the write. A confirmed write backs up to mcp.json.applied.bak, other writes to mcp.json.bak, so a refresh cannot replace the snapshot the user was shown.
4. One URL guard for every outbound request — the probe, resource_metadata, .well-known, registration, token, device and the authorization endpoint. Private, link-local and metadata ranges are refused; IPv4-mapped, IPv4-compatible and NAT64 are resolved to IPv4 first. URLs a remote server returned are always strict; the user's own endpoint may be private when the switch is on. Redirects are followed by hand and every hop is re-checked, max 5 hops. tests/redirect-guard.test.mjs.
5. The callback page escapes title and detail.

Package tests: node --test "tests/**/*.test.mjs" from the package directory.

[MyPrototypeWhat]

Reviewed at 55be487. Thanks for the thorough round. The earlier Must fix items are resolved, but the new hand-written redirect loop introduces one problem that needs fixing before merge.

Earlier points: verified

• errors.mjs added. The error catalog is there, serviceError() throws on an unregistered code, and the plugin loads (the entry-loads test passes).
• Server name allowlist and path escape. assertServerName allows only ^[A-Za-z0-9._-]{1,64}$ and rejects . and ... credentialPath resolves the path and checks it stays under the credential directory.
• mcp.json write conditions.
  • (a) resolveMcpConfigPath checks only the data-dir chain and ~/.minimax/mcp.json. The working directory and the Node executable directory are no longer searched.
  • (b) patchMcpAuthorization runs the whole read-modify-write inside withWriteLock, re-reads the file inside the lock, and refuses the write if the path differs from the one the user confirmed.
  • (c) autoRefresh defaults to false. The interval only refreshes when it is switched on, and "refresh all" passes force only for the button press.
  • (e) backupConfig no longer swallows errors, so a failed backup aborts the write. Confirmed applies keep their own backup file.
• Shared URL guard on outbound requests. Every httpJson call passes a guard (strictHop for URLs that come from a remote server, endpointHop for the endpoint the user configured), and fetchGuarded checks each hop again with at most 5 redirects. The redirect-guard and url-guard tests pass.
• Callback page escaping. Title and detail both go through escapeHtml.

Must fix

Credentials are re-sent to every redirect target. fetchGuarded follows redirects with fetch(current, { ...init, redirect: 'manual' }), so each hop gets the original init again: the same method, the same body and the same headers. A redirect from any of these requests to another origin therefore forwards:

• Authorization: Bearer <access token> from probeMcpEndpoint (the token check after apply and refresh) and from the initialize probe;
• the token request body (refresh_token, client_secret, code, code_verifier) from requestToken and the device-code poll.

fetch's own redirect handling drops Authorization on a cross-origin redirect. The manual loop removed that protection, and the URL guard does not cover it, because the guard only checks whether a host can be reached, not whether it should receive the credentials. In practice, an MCP endpoint that answers with a 302 to another host gets the user's token sent there.

Suggested fix, in fetchGuarded:

• If the next hop has a different origin from the current one, remove authorization (and any cookie) from the headers before sending.
• For 301, 302 and 303 on a POST, follow as a GET with no body (as browsers do), or simply refuse to follow redirects on token, registration and device endpoints. OAuth endpoints are not expected to redirect, so refusing is the simplest choice.
• Add a test: a request with an Authorization header that is redirected to another origin must arrive there without the header, and a POST to a token endpoint that redirects must not resend its body.

Optional

• resolveMcpConfigPath walks up to 10 parent directories from the data dir and takes the first mcp.json it finds. If none exists in the expected place, that can be an unrelated mcp.json in a parent folder (the home directory, for example). The write is bound to the path the page shows, so this is not a blocker, but stopping the walk at the MiniMax data directory would make the result predictable.

Version

1.0.0 is right for a plugin that has not been published yet. There is no earlier release, so moving to 1.1.0 for the device-code addition is not needed.

Checks run

• npx -y -p node@22 npm run check at 55be487: passes.
• Package tests (node --test "tests/**/*.test.mjs", Node 22): 20 of 20 pass.

[Microbiosis]

MyPrototypeWhat requested changes on 55be487.

Fixed on 10c5af9: the redirect loop no longer resends the original request.

• a redirect to another origin is followed without the Authorization header and without cookies, the way fetch does;
• a 301 / 302 / 303 turns the POST into a GET without a body, like a browser;
• a request whose body holds a secret — token, registration, device — is not redirected at all (redirect_not_allowed).

httpJson takes an options object now, so sensitive is a named flag rather than another positional argument.

Tests: tests/redirect-guard.test.mjs — "a cross-origin redirect does not carry the Authorization header" and "a request whose body holds a secret is never redirected"; tests/config-path.test.mjs — the search boundary. 24 of 24 pass, npm run check OK.

The Optional point is in as well: resolveMcpConfigPath stops the walk at the MiniMax data directory, so an mcp.json in a parent folder is not picked up.

[MyPrototypeWhat]

Reviewed at 10c5af9. Thanks, the redirect fix is complete and nothing else blocks merge. Approving.

Redirect fix: verified

• Credential headers on a cross-origin hop.
  • fetchGuarded compares new URL(next).origin with new URL(current).origin. That covers scheme, host and port, so an https to http downgrade on the same host also counts as cross-origin.
  • On a cross-origin hop, authorization, proxy-authorization and cookie are removed, matched case-insensitively. Once removed, they stay removed for later hops.
  • The only credential header the plugin sends is authorization (the Bearer token in the probes and the Basic client secret on token and device requests), so no custom auth header is left uncovered.
• POST after 301, 302 and 303. The request is resent as a GET with no body.
• Requests that carry secrets are never redirected.
  • registerClient, requestToken (code exchange and refresh), the device authorization request and the device-code poll all pass sensitive: true. A 3xx with a Location header fails with redirect_not_allowed before the next URL is even resolved.
  • The plugin makes no revocation or introspection calls.
  • The remaining httpJson callers are the two endpoint probes and the two metadata GETs, and none of them sends a secret in the body. The cross-origin rule above covers the Bearer header in the probes.
• Hop limit. Still at most 5 redirects, then too_many_redirects. Every hop still goes through the guard.
• redirect_not_allowed is in the error catalog, so serviceError() accepts it.

mcp.json write conditions: still met

• (a) resolveMcpConfigPath checks only the data-dir chain and ~/.minimax/mcp.json, and the write stays bound to the realpath'd file the user confirmed. This round makes the search narrower.
• (b) The read-modify-write still runs entirely inside withWriteLock. That code did not change.
• (c) autoRefresh still defaults to false, and background refresh only writes when it is switched on.
• (e) A failed backup still aborts the write, and confirmed applies keep their own backup file.

Config path search

The walk now stops at the first .minimax directory and never steps into the home directory. The home directory's own mcp.json is not checked, and if nothing is found the result is the default ~/.minimax/mcp.json. That settles the earlier optional point. config-path.test.mjs covers both the file inside the data directory and an unrelated file one level above it.

Optional

• When a POST becomes a GET, content-type is still sent with no body. It does no harm, but you could drop it along with the body.
• A test for the 303 POST-to-GET path would round out redirect-guard.test.mjs.
• If the data directory is ever outside the home directory and outside a .minimax folder, the walk can still go up to 10 levels toward the filesystem root. The write is still bound to the path the page shows, so this is fine as it is.

Tests and scope

The two new test files sit in the plugin's own tests/ folder. This round does not change .github/workflows, the root package.json scripts, or the root test/ folder.

Checks run

• npx -y -p node@22 npm run check at the repo root on 10c5af9: passes.
• Plugin tests (node --test tests/*.test.mjs, Node 22, run inside the plugin folder): 24 of 24 pass.