Repository navigation
feat(core): serve mcp tools over http to admin-issued tokens - #270
Merged
Merged
Conversation
added 26 commits
September 28, 2026 07:47
…ries Modules register agent tools with ctx.mcp.tool(defineMcpTool(...), factory) and action types with ctx.actions.register(defineActionType(...), factory). Consumers reach both only through MCP_KERNEL, which validates input shape at registration, checks IAM per call through AdminGuard.assertUser, returns only allow-listed output, maps exceptions to declared codes and audits every call. - RunContext, PlatformConfig.agents, trigger catalog, agent IAM resources - first tools: player.summary, wallet.activity, kyc.status, ggr.summary - first action types: add_tag, add_note, hold_withdrawal, send_to_manual_review, request_enhanced_kyc - withdrawals can be held; admins approve or reject a held withdrawal - catalog and @openora/mcp list the agent surface; ADR-0041
Conflicts resolved by keeping both sides: the new promo audit actions next to the mcp ones, the moved KYC enum schemas next to PositiveMoneyAmountSchema, the impact tool next to list-agent-tools, and dev's kycPolicy and grantLedger wallet dependencies inside the shared WalletService factory. Adapted to dev: - wallet.withdrawal.held is filed under the player with after.transactionId, like the other withdrawal lifecycle rows since the player-scoped audit change - the mcp kernel ADR is renumbered to 0042; dev took 0041
The agent hub adds behaviour and leaves existing flows alone, so the daily cap reads the auto-approved marker by the reason alone again. A held withdrawal whose reason reads "auto-approved" counts towards the cap, which fails closed.
A replace loses the race only when its first read precedes the winner's commit. The test left that to timing, so a slow runner started the second replace after the first committed and both succeeded. A third transaction now holds the player row until both replaces wait on it.
Agents config gains an mcp block (off by default) for the HTTP transport. IAM gains mcp-access (use MCP, manage own tokens) and mcp-token (oversee and revoke any token). Contracts gain the token authenticator and revocation ports, rate-limit keys and audit actions. AdminGuard.assertUser now refuses a deactivated account and filterGranted narrows a listing without denial events. The kernel descriptor's output schema is projected to the keys a tool may return, so a client validating structured output no longer rejects redacted results.
The iam module now owns the bearer tokens an admin's MCP client sends. A token is bound to its admin, stored only as a SHA-256 hash, and always expires within the configured cap; issuing or revoking one writes an audit record in the same transaction. An admin with MCP access manages their own tokens, and token oversight lists and revokes anyone's, including an emergency revoke of all. The module binds the authenticator and revocation ports the transport and identity rely on.
Deactivating an account, demoting it from admin, or revoking all of a user's sessions now revokes every active MCP token they hold, so restoring the account later does not bring the tokens back. Identity calls the revocation port after its own write.
With agents.mcp enabled, createApp mounts a stateless MCP Streamable HTTP endpoint ahead of CORS, sessions and oRPC. Each request passes the host binding, the origin check, bearer authentication, the token's minute and day limits and the admin's MCP access before the SDK sees it. Only read-class tools are listed, narrowed to the admin's grants, and the output keys a tool marks as personal are dropped unless the operator includes them. Every call still runs through the kernel, which audits it under the token. The SDK is loaded only when the endpoint is enabled.
…ient ADR-0043 records why the transport authenticates with admin-bound tokens in the iam module, drops personal fields by default and rejects the better-auth plugins and OAuth for now. ADR-0042 gains an update block for the projected output schema and the active-account check. A new guide covers enabling the endpoint, granting access and connecting Claude Code.
The kernel's input hashing is now depth-capped and an unexpected failure still writes the call's audit record, so a crafted tools/call can no longer skip the audit trail. A call naming a tool the endpoint does not expose is audited too. The kernel drops personal output keys when the run asks, so the audited hash covers what the client received, and records the client's address and agent. The transport requires a bound host while enabled, limits each client address before any token lookup, answers 503 when the rate limiter is unreachable and logs errors without their query parameters. The MCP SDK becomes an optional peer dependency.
Automatic revocation now commits in the same transaction as the change that causes it and keys on the resulting state, so a failure rolls the change back and a retry finishes it. Tokens are also revoked on a password reset or change, and when an IAM change takes away MCP access. Players never reach the token table from session revocation. Issuing a token needs an active admin owner, respects a per-admin cap and an hourly throttle, and a bulk revocation locks the table so a concurrent issue cannot survive it. Token errors carry a typed reason, the table gains a consistency check and a status index, and the seeded admin role holds the MCP permissions.
The registration helper draws a random handle, and the username filter occasionally refuses one as prohibited language, which failed unrelated E2E tests at random. A refused generated handle is now replaced with a fresh one; a handle the test chose is still sent once.
…ide its handler The canonical JSON behind every input and output hash recursed without a limit, so an input nested a few hundred thousand levels deep threw a RangeError while the kernel hashed it, outside any catch, and the call rejected without its audit record. Nesting past 64 levels now serializes as a fixed marker, and a tool call that throws anywhere outside its handler answers internal_error and still writes its one mcp.tool.failed record. The change matches the one already on the stacked transport branch, so the two merge without rework.
…dition checkPrecondition answered any well-formed run actor without the IAM check a tool call and an execution make, so a caller without the action type's grant could learn from add_tag whether a player is self-excluded or on a break, from hold_withdrawal whether a withdrawal exists, and from request_enhanced_kyc where a player's verification stands. The precondition now resolves the actor's admin and checks the action type's grant through AdminGuard first, answering forbidden, or internal_error when the check itself fails, before the payload is parsed or the precondition runs. AdminGuard records the denial as it does for every route; the kernel still writes no record of its own for a precondition.
…r boot createApp binds MCP_KERNEL after every plugin and configure(), but it returns the container, and a later register() of the token dropped the cached kernel and served the replacement, without the IAM check or the audit record the kernel enforces. The container gains registerSealed(), which binds a token for good: every later registration of it throws. createApp binds the kernel that way, so a binding made in configure() is still overridden and a rebind after boot is refused.
…rough The output allow-list picks top-level keys only, and registration checked the output schema's top level only, so a tool allowing "player" with a z.looseObject, a catchall, a record or an untyped field under it booted and returned whatever the handler put there, personal data included. Registration now runs the input walker's record, loose, catchall, untyped and recursive rules over the whole output schema, at every depth, without the bounds an input needs. The parse then strips every key a schema does not declare at each level below an allowed key.
…thdrawal hold_withdrawal writes the proposed reason into the withdrawal's review reason, and approval rewrote the reviewer and review time but kept the reason, so an approved withdrawal showed its approver next to the text it had been held for. An admin approval now clears the reason, as a rejection replaces it with its own; the hold's reason stays on the wallet.withdrawal.held audit record. A pending withdrawal carries no reason, so approving one is unchanged.
wallet.activity sums a wallet's completed deposits, withdrawals, bets and wins over the last 1 to 365 days, and a model may call it as often as its run allows. No index leads on (wallet_id, status, created_at), so the planner read every completed row of those types the wallet ever had through (wallet_id, type, status) and dropped the ones outside the window in the heap. The new index bounds the scan by the window. The generated migration is a plain CREATE INDEX, because the migrator applies each file inside a transaction. It holds a SHARE lock on wallet_transaction while it builds, which blocks every ledger write but no read, so a large deployment applies it in a quiet window.
The changeset lists breaking type changes for core but asked for a minor bump, so the release notes would file them under minor changes. Core now bumps major, as a breaking change does elsewhere; @openora/mcp only adds tools and stays minor. The summary also covers what the review changed: the sealed MCP_KERNEL binding, the output schema rule, the precondition's IAM check, the cleared hold reason on approval, and the wallet index the migration builds while it blocks ledger writes.
# Conflicts: # packages/core/src/server/mcp/__tests__/kernel.test.ts # packages/core/src/server/mcp/kernel.ts
add_note recognised a replay by a note with the same player, author and text, so an approver who approved two proposals saying "reviewed, no issues" got one note and an already_applied for the second, and a note the same admin had typed in the Backoffice suppressed a proposal's note too. The note now keeps the proposal id in a nullable proposal_id column under a partial unique index, and the executor inserts with ON CONFLICT DO NOTHING on it: nothing inserted means a replay. The unique index also serialises two executions of one proposal, so the per-proposal advisory lock and the content lookup are gone. Backoffice notes keep proposal_id null and are never matched, and the note routes still do not return it. ADR-0042's trade-off, the executor contract wording and the changeset describe the new key.
# Conflicts: # packages/testing/src/__tests__/mcp-kernel.e2e.test.ts
mp-blurify
marked this pull request as ready for review
October 7, 2026 13:49
mp-blurify
requested review from
klaudia-blazyczek-blurify,
marek-chmielowski-blurify and
zaxovaiko
as code owners
October 7, 2026 13:49
mp-blurify
requested review from
damianrzepka,
jakubfilinger-b and
okapitula
as code owners
October 7, 2026 13:49
zaxovaiko
removed request for
damianrzepka,
jakubfilinger-b,
klaudia-blazyczek-blurify,
marek-chmielowski-blurify and
okapitula
October 7, 2026 13:49
Member
|
/codex all |
![blurify-codex[bot]](https://avatars.githubusercontent.com/u/34129845?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not quite yet - the previously raised token revocation issues remain; this pass found no new security or compliance issues. One scale risk remains: bulk revocation writes each audit row serially while holding the token-table lock.
Reviewed security, compliance, conventions, performance, reliability, rollout, spec, the whole change.
… token An issue whose guard passed before a grant change committed could wait behind the change's token-table lock and insert after its revocation scan, leaving an active token for an admin without MCP access. Issuance now takes the revocation lock after the owner's row lock and reads the owner's mcp-access grant again under it, uncached, through the predicate the grant-loss revocation uses, now shared from iam/shared/admin-grants.ts. The refusal is owner_ineligible with the unauthorized_access signal, as for an inactive owner. The owner's row is locked first because identity's deactivation locks the user row and then the token table; the reverse order could deadlock. The rate-limit call stays before the table lock, so a slow limiter never holds it, and the same requests consume a slot as before. The lock-waiter test helpers take any handle with a drizzle service and gain a table-lock waiter, so the end-to-end suite can hold a grant change open and see the issue wait on it.
…rd change A password change or reset commits through better-auth before the identity module revokes the admin's MCP tokens, so a failed revocation left the old bearer usable until it expired. The token authenticator now refuses any token issued at or before the updatedAt of its admin's credential account, which better-auth stamps in the statement that writes the new hash and leaves alone on a sign-in or a second factor. The refusal carries a new credentials_changed reason on the authenticator port. The explicit revocation still runs and records the reason. A revocation that fails inside better-auth's reset hook is now logged instead of rethrown: the hook runs before better-auth revokes the account's sessions, so a throw kept every session alive, and a stolen session could mint a fresh token the new check accepts. The end-to-end suite binds a failing revocation adapter and shows that after a change and after a reset the old token is refused, the old session cannot issue another, a token issued afterwards works, and that the stamp moves on a change and a reset but not on a sign-in or a two-factor sign-in.
A caller writing one audit row per changed row repeats the chain-head read, the sequence reservation and the insert for each of them while its transaction holds the platform-wide audit chain lock. AuditWritePort gains recordManyInTransaction. Under the same advisory lock it reads the chain head once, reserves every sequence number in one statement in ascending order, computes the chain in memory with the row builder and computeHash the single path uses, and inserts the rows in statements of at most 1,000, under Postgres' bind-parameter limit. The rows match what single appends of the same entries in the same order would store, apart from ids and timestamps. An empty list writes nothing. The sealed binding, makeAuditWriter, the transport kernel fixture and the chat expiry test's real-writer wrapper implement the new method.
markRevoked wrote one audit row per revoked token while its transaction held the token-table lock and the audit chain lock, so a global revoke over many tokens kept token issuance, other revocations and every audit writer waiting for the whole loop. It now hands every revoked token's entry to recordManyInTransaction in one call. The tests read the revocation entries from that call, and the end-to-end revoke-all checks one row per revoked token, with its actor and reason, and a valid chain.
…waited An issue whose guard passed on a session that a concurrent revoke-all, forced logout, two-factor reset or email change then ended could wait on the token-table lock and insert after the revocation's scan, leaving an active token minted from a dead session. The issue route passes the caller's session id to create(), which, under the revocation lock and after the grant recheck, refuses the issue unless that session still exists, belongs to the owner and has not expired. Expiry is compared with statement_timestamp(): now() is the issuing transaction's start and can predate the expiry a revocation that committed while it waited wrote with its own now(). A missing or ended session is refused as owner_ineligible with the unauthorized_access signal. The session read is a plain select and takes no row lock, so the order stays the owner's row, then the token-table lock, then lock-free reads, and the issue never waits on the session rows a revocation holds.
… fails changePassword rethrew a failed MCP token revocation after better-auth had committed the new password and revoked the other sessions, so the admin saw a 500 for a change that had happened. The authenticator already refuses every token issued before the change, so the change path now logs the failure and answers success, through the helper the reset hook uses.
…ules ADR-0042 still said binding MCP_KERNEL after every plugin keeps it from being replaced, which the returned container disproved. It now names the sealed binding, the output-schema rule at every depth, the IAM check before a precondition, and total input hashing. It also records how a tool references a player (its id, no pseudonym format in core) and a money amount (a bounded decimal string, value checked in superRefine), and that a money-moving executor keys durably on the proposal id.
… the mcp host ADR-0043 now says that proposing over MCP waits for the hub's review queue and a proposal-create permission, that both rate limits count per token, and that every proxy or load balancer able to reach the app, not only the backoffice one, must set the Host header and refuse absolute-form targets; the guide says the same. ADR-0042's transport update no longer claims the input hashing depth cap, which the kernel change itself now records.
The consumer guide said paths in extensions.config.ts resolve against the config file. The plugin host import()s each entry from inside @openora/core, so a relative path or a bare package name resolves from core's location, and an add-on package installed only in the consumer fails at boot with a module-not-found error. The guide, the PluginEntry path doc and the consumer template now say so; the template shows an add-on package resolved in the app with createRequire. The load-pattern paragraph names corePlugins() instead of hand-written dist paths. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # packages/core/src/pam/identity/__tests__/identity-reader.service.int.test.ts # packages/core/src/wallet/drizzle/migrations/meta/0028_snapshot.json # packages/core/src/wallet/drizzle/migrations/meta/_journal.json
…ule (BF-595) An admin with no role assignment holds nothing once the permission resolver is bound, so an MCP token owner needs a real role granting mcp-access and the super-admin-only role calls need an assigned caller. Also formats two files oxfmt flagged.
A staff account holds nothing until a role is assigned, so an MCP token owner needs one, and a test about a single narrow role needs an account that holds no other assignment - the seeded moderator now carries the default admin role.
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
2 participants
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
Exposes the in-process MCP kernel from #264 to an admin's own MCP client (Claude Code and similar) over MCP Streamable HTTP. Every call is authenticated by an admin-issued bearer token stored only as a hash in the iam module, checked per tool by
AdminGuard, and audited. The endpoint is off by default. This PR is stacked on #264.Why
The kernel serves only in-process callers such as the agent hub. Admins and analysts need to query platform data from their own MCP clients, under their own grants, with the same redaction and audit as the Backoffice, and without anything appearing on the player domain. A static bearer token with a mandatory lifetime replaces an OAuth authorization server; every current MCP client supports a static header (ADR-0043).
What ships:
agents.mcpplatform config: off by default, a bound host required while enabled, per-address and per-token rate limits, a per-admin token cap and hourly issuance throttle, and a personal-field policy (dropped by default).mcp_tokentable (hash, prefix, mandatory expiry, revocation reason, last use, call count), routes for an admin's own tokens and for oversight (list, revoke, revoke all), and two permissions:mcp-access(use MCP, own tokens) andmcp-token(oversee and revoke any token).AdminGuard.assertUserrefuses a deactivated account, which also stops hub runs acting for that admin.docs/guides/mcp-transport.md.Verification: the transport was exercised end to end from Claude Code against a local platform: tool listing and calls, dropped personal fields, one audit record per call, refusals (origin, method, host, token, batch, rate limit, expiry), automatic revocation on deactivation, and Claude Code's handling of a revoked token (the header is rejected, no OAuth attempt). The review hardening that followed is covered by the E2E suite.
Alternatives considered
Risks
createIamRoutergains a required argument.IamServiceandMcpTokenServicetake a deps object.AdminGuard.assertUserrefuses deactivated accounts.outputJsonSchemaprojected toredact.allow.RunContextgains optionalclientMetaanddropPersonal.RateLimitResultgainsunavailable.AuditWritePortgains a requiredrecordManyInTransaction; a hand-written implementation or test double needs it.McpTokenAuthenticationgains thecredentials_changedreason.@modelcontextprotocol/sdk@1.29.0(an optional peer dependency) and listingagents.mcp.allowedHosts. Every proxy or load balancer that can reach the app must set theHostheader itself and reject absolute-form request targets.0001(enum, table, indexes) and0002(status index, consistency CHECK), both additive.mcp_tokenand run one at a time.devafter feat(core): add the mcp kernel with agent tool and action-type registries #264 merges, by mergingdevinto this branch, with no force push.