fix(agent-server): block request-body injection into agent constructor options - #1939
Open
ulivz wants to merge 2 commits into
Open
fix(agent-server): block request-body injection into agent constructor options#1939ulivz wants to merge 2 commits into
ulivz wants to merge 2 commits into
Conversation
…r options Session creation accepted an unfiltered `agentOptions` object that was spread last into the Agent constructor, letting any caller override server configuration including `mcpServers` (spawns local processes via StdioClientTransport), `aioSandbox` (downgrades sandboxed deployments to host execution or redirects to an attacker MCP server), and model endpoints. Combined with the server binding all interfaces and having no authentication, this is unauthenticated remote command execution. The same injection existed through `runtimeSettings` when no server-side `transform` was configured, and through `sessionInfo.metadata.agentOptions` in agent-server-next. Changes: - Introduce `sanitizeSessionAgentOptions` (allowlist: only `agentMode`) and `filterDeclaredRuntimeSettings` (passes only schema-declared keys with matching primitive types) in `@tarko/shared-utils`. - Apply both filters at session creation in agent-server and agent-server-next. - Apply `filterDeclaredRuntimeSettings` at the runtime-settings update endpoint. - Default server bind to 127.0.0.1 (`resolveServerHost`), with explicit opt-in via `server.host` config / `--host` CLI flag. Log the real bind address. - Propagate `host` through agent-cli commands (start/serve/run) and interface types. BREAKING CHANGE: The server now listens on 127.0.0.1 by default instead of all interfaces. Deployments that rely on network access must set `server.host` to `0.0.0.0` or the desired address in their config or pass `--host 0.0.0.0`.
✅ Deploy Preview for agent-tars-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for tarko ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| continue; | ||
| } | ||
|
|
||
| filtered[key] = value; |
…property write - config-builder.test.ts inline snapshots updated to include `host: "127.0.0.1"` (the CI failure from the previous commit) - Use `Object.create(null)` for the filtered-settings accumulator to silence CodeQL js/remote-property-injection (keys are already allowlisted through schema lookup + FORBIDDEN_KEYS guard, but the null-prototype object makes the guarantee structural)
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.
Problem
POST /api/v1/sessions/createaccepts an unfilteredagentOptionsobject thatis spread last into the Agent constructor parameters, overriding every
server-side default. An attacker supplying
mcpServersentries with acommandfield triggers
StdioClientTransport, which spawns an arbitrary local process.The same primitive allows sandbox downgrade (
aioSandbox) and credential/modelendpoint redirection.
Three injection channels existed:
agentOptionsin the session-creation request body (both packages)runtimeSettingswhen no server-sidetransformis configured (spread intoconstructor options verbatim)
sessionInfo.metadata.agentOptionsin agent-server-next (persisted viaPOST /sessions/update, reapplied on every session init)Amplifiers: the server binds all interfaces by default, has no authentication,
the CSRF token endpoint is unauthenticated and origin-unchecked, and CORS
explicitly allows requests with no
Originheader.Fix
Input boundary (primary):
sanitizeSessionAgentOptions: strict allowlist — onlyagentModepassesthrough. Prototype-pollution keys (
__proto__,constructor,prototype)rejected. Values must be plain JSON data with bounded depth.
filterDeclaredRuntimeSettings: passes only keys declared in the server'sruntime-settings JSON schema, and only when the value matches the declared
primitive type (
boolean/string/number). No schema configured = nothingpasses. Object-typed values (like
mcpServers) are structurally impossible.runtime-settings update endpoint.
Default bind address (defense in depth):
127.0.0.1by default, matching the conventionestablished by
mcp-http-serverin fix(mcp-http-server): default host to 127.0.0.1, not all interfaces #1918.server.hostconfig key or--hostCLI flag.Breaking change
The server previously listened on all interfaces (
0.0.0.0). Deploymentsrelying on network access must now set
server.hostto0.0.0.0(or thedesired address) in their config, or pass
--host 0.0.0.0to the CLI.Verification
schema-based filtering, and host resolution defaults.
rejects dangerous keys and returns them in the error response.
Not in this change
agent-server-next security hooks never being registered — tracked separately.