Repository navigation
🚷 fix: Reject Client-Supplied Subagent Configuration - #13660
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
Pull request overview
This PR closes a security/control gap where authenticated clients could enable ephemeral subagent behavior via request payloads when the selected model spec did not define subagents. It enforces that ephemeral subagent configuration is sourced exclusively from server-side model specs, while persistent/configured agents continue to be loaded from the database as before.
Changes:
- Removed request-controlled
subagentsfallback logic from ephemeral agent loading paths (loadEphemeralAgent,loadAddedAgent). - Deleted the now-unused request sanitizer (
packages/api/src/agents/subagents.ts). - Removed
subagentsfrom the sharedTEphemeralAgenttype and updated backend/frontend tests to assert request payloads are ignored.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-provider/src/types.ts | Removes subagents from TEphemeralAgent so client/request types can’t quietly legitimize request-driven subagent config. |
| packages/api/src/agents/subagents.ts | Deletes unused request sanitizer after removing all call sites. |
| packages/api/src/agents/load.ts | Ensures ephemeral agent subagents is applied only when present in modelSpec.subagents (no request fallback). |
| packages/api/src/agents/added.ts | Applies model-spec subagents only; removes request-driven path for added ephemeral agents. |
| packages/api/src/agents/tests/load.spec.ts | Updates/strengthens coverage to verify request-provided subagents are ignored across relevant ephemeral paths. |
| client/src/utils/tests/applyModelSpecEphemeralAgent.test.ts | Tightens client test to assert subagents is not even present on the ephemeral agent state object. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
I removed the request-controlled subagents fallback introduced in #13598 so ephemeral subagent configuration can only come from server-side model specs, while configured agents continue to source theirs from the database. Previously, an authenticated user could send
ephemeralAgent: { subagents: { enabled: true } }in a chat request and activate subagent self-spawn on any ephemeral conversation whose model spec did not define subagents, bypassing the intended server-side control.sanitizeRequestSubagentsfallback fromloadEphemeralAgentandloadAddedAgentsosubagentsis assigned exclusively frommodelSpec.subagents.packages/api/src/agents/subagents.tssince the sanitizer has no remaining consumers.subagentsfield fromTEphemeralAgent— no client code reads or writes it, and dropping it prevents future code from quietly trusting the request field again.Change Type
Testing
cd packages/api && npx jest src/agents/__tests__/load.spec.ts— 23 passed, including the new ignore-request-subagents tests against a real MongoMemoryServer instance.cd client && npx jest src/utils/__tests__/applyModelSpecEphemeralAgent.test.ts— 15 passed.npx tsc --noEmitinpackages/api— clean.sort-imports --checkon all changed files — clean.npm run build— all 5 workspaces build successfully.Test Configuration:
Checklist