Repository navigation
Connector tools only for the Bots the owner enables; shared Bots never get them - #346
Conversation
…hem on for (#335) Connector tools (mcp__github__*, mcp__composio__*) were mounted for every Bot and Session, including a Bot shared with a friend. They are now a per-Bot switch, default off, and a Bot that is shared with anyone never gets them: a guard refuses the calls and the Agent's tool list drops them. Upgrade: every Bot starts off. The Composio session address is no longer kept in state.json; the id is stored and the address is asked for at each mount.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82425181b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const botOfAgent = (agent) => { | ||
| let id = agent?.id ?? agent?.session?.id | ||
| let header = agent?.session?.header | ||
| for (let depth = 0; id !== undefined && depth < MAX_DEPTH; depth += 1) { |
There was a problem hiding this comment.
When a Bot creates eight nested sub-agents, this loop reaches its limit immediately before checking the root Bot session, so botOfAgent() returns undefined. Both the tool-list path and the guard then treat that Agent as an owner-only plain session, leaving globally mounted connector tools visible and executable even when the originating Bot is shared or has the connector disabled. An unresolved parent chain must remain restricted rather than falling back to the plain-session policy.
Useful? React with 👍 / 👎.
| // A Bot's sharing changed (a grant was added or removed): apply it to the Agents it already has. | ||
| const refreshConnectorTools = botId => eachAgent(agent => { if (botOfAgent(agent)?.id === botId) hideConnectorTools(agent) }) |
There was a problem hiding this comment.
This refresh path is invoked for connector switches and grants, but room participant changes also change sharedWith() and never call it. If a friend, another person, or another owner's Bot joins after an Agent is created, that Agent continues advertising connector tools even though calls are refused; more importantly, when that participant leaves, a still-enabled connector remains hidden and unusable until an unrelated tool change, Agent recreation, or restart. The room add/remove and inbound-room transition paths need to refresh affected Bots' masks.
Useful? React with 👍 / 👎.
|
Maintenance review at tip 8242518 (posted from the author's account, so it is not an independent PASS and records none). Verdict: needs change before the one-click approval (risk tier T3). I read
CI is green at this tip. Needs a fix commit for item 1, a fresh independent review at the new tip, then the maintainer's approval. |
…ned; follow room sharing in the tool list (#335) Docs: docs/subsystems/core-host.md updated.
Fixes #335
What changed
Connector tools (
mcp__github__*,mcp__composio__*) were mounted for every Bot and Session, including a Bot shared with a friend. Now:bot.connectors = { github?, composio? }; absent means off. Switches: Settings → Connectors → Choose Bots (a switch per local Bot; the row says "On for N of M Bots") and a Connectors card in each Bot's settings. zh strings added.sharedWith()(reach.js) is the one definition: a grant, or a current room with a friend's Bot, another person, another owner's Bot or a room code. It is read on every call. A grant also clears the Bot's switches, and turning one on for a shared Bot is refused naming who it is shared with.ctx.tools.guardrefuses every connector call from a Bot Session (own chat, group Session, earlier part; sub-agents follow their parent), andrestrict({deny})removes the tools from that Agent's list (on create, onresolveAgent, on a switch/grant change, ontools/changewhen a connector mounts later). The DSH Agent and plain DSH Sessions (owner-only) are untouched.bot-connectorsisSHARED_BLOCKED.state.json. Only{ id, apps }is kept. Composio's reference listsmcp.urlas required in the create andGET /tool_router/session/{id}answers (x-api-key), so the mount asks for the address by id + key, accepts only https on the API host, and keeps it in memory. Composio's docs show the address always paired with the key header and call the exported url+headers config a secret, but never say the address alone cannot authenticate, so it is not stored. An older record'surlis rewritten away at start; the in-memory address is redacted from connector errors.Docs: core-host (rule, upgrade, session address), core-client, team, README en/zh. Changelog fragment:
changelog.d/335-connector-bots.security.md.Verification at the tip
Tip SHA: 8242518
Ran:
ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify(Node 24) → exit 0: core 889/889, social 230/230, tree 56/56, timeline 80/80,.github/scripts86/86, interop 37/37, live-e2e 16/16; arch-docs OK;ds-bot-patches checkOK;build-client --checkOK. New:connector-access.test.js(10: default off, toggled-on local Bot gets only its connector, shared Bot via grant/room gets none even with the flag forced on, sub-agents, late-mounting connector, disconnect, duplicate),connector-bots.test.js(6), connector session tests (+4, 6 updated), notices test (+1), authz and install order rows. The new test files fail on currentmain(the module does not exist there).Not run: the
dev/bench.shsmoke test (needs a DSH checkout at../deepseek-harness, not on this box); live Composio/GitHub calls (no key; the session-address flow is tested against a stand-in for the documented API); a visual check of the new rows in a browser.Reviewer notes: T3 expected (
reach.js,authz.js, host tool path).core/andpatches/ds-bot/are generated together; newcorefiles are stored whole inpatches/ds-bot/files/.