Skip to content

fix(mcp): reject unsafe Deep Agents projections #10911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rsliter merged 16 commits into from
Sep 3, 2026
Merged

fix(mcp): reject unsafe Deep Agents projections #10911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
8f73e9d
fix(mcp): reject unsafe Deep Agents projections
rsliter Sep 2, 2026
27e7e58
test(mcp): clarify unsafe projection precedence
rsliter Sep 2, 2026
b0b7f95
Merge remote-tracking branch 'refs/remotes/origin/main' into codex/fi…
rsliter Sep 2, 2026
f5ec932
fix(mcp): preserve unsafe projection marker
rsliter Sep 2, 2026
da6ee52
Merge remote-tracking branch 'refs/remotes/origin/main' into codex/fi…
rsliter Sep 2, 2026
7080578
Merge branch 'main' of https://github.com/NVIDIA/NemoClaw into codex/…
rsliter Sep 3, 2026
abeb166
fix(mcp): classify unreadable unsafe projections
rsliter Sep 3, 2026
235be35
Merge branch 'main' of https://github.com/NVIDIA/NemoClaw into codex/…
rsliter Sep 3, 2026
f856804
Merge remote-tracking branch 'refs/remotes/origin/main' into codex/fi…
rsliter Sep 3, 2026
460b9e0
test(mcp): cover non-regular projection fallback
rsliter Sep 3, 2026
aeef43c
fix(mcp): centralize unsafe projection status parsing
rsliter Sep 3, 2026
0a6b3bc
Merge branch 'main' of github.com:NVIDIA/NemoClaw into codex/fix-1075…
rsliter Sep 3, 2026
1b5d57c
merge: refresh from canonical main
rsliter Sep 3, 2026
147adef
fix(mcp): preserve unsafe projection evidence
rsliter Sep 3, 2026
ca27909
test(mcp): keep credential fixture inert
rsliter Sep 3, 2026
527e895
fix(mcp): share projection type contract
rsliter Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 59 additions & 19 deletions src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ import {
buildDeepAgentsMcpRegisterCommand,
buildDeepAgentsMcpRemoveCommand,
} from "./mcp-bridge-adapter-deepagents";
import { DEEPAGENTS_MCP_MAX_SERVERS } from "./mcp-bridge-adapter-deepagents-projection";
import { buildDeepAgentsMcpStatusCommand } from "./mcp-bridge-adapter-status";
import {
DEEPAGENTS_MCP_MAX_SERVERS,
DEEPAGENTS_UNSAFE_MCP_PROJECTION_TYPES,
} from "./mcp-bridge-adapter-deepagents-projection";
import {
buildDeepAgentsMcpStatusCommand,
parseUnsafeDeepAgentsMcpProjectionResult,
UNSAFE_DEEPAGENTS_MCP_PROJECTION_PREFIX,
} from "./mcp-bridge-adapter-status";

const emptyProjection = { mcpServers: {} };
const duplicateProjection = '{"mcpServers":{},"mcpServers":{"shadow":{}}}\n';
Expand Down Expand Up @@ -52,6 +59,19 @@ describe("Deep Agents managed MCP projection safety", () => {
expect(sizeCheckIndex).toBeLessThan(truncateIndex);
});

it.each(DEEPAGENTS_UNSAFE_MCP_PROJECTION_TYPES)(
"uses the shared %s classification in the command and result parser",
(type) => {
const path = "/sandbox/.deepagents/.nemoclaw-mcp.json";
const detail = `${UNSAFE_DEEPAGENTS_MCP_PROJECTION_PREFIX}: ${type} at ${path}`;

expect(buildDeepAgentsMcpStatusCommand(baseEntry)).toContain(JSON.stringify(type));
expect(
parseUnsafeDeepAgentsMcpProjectionResult({ status: 2, stdout: "", stderr: detail }),
).toEqual({ messagePrefix: `${UNSAFE_DEEPAGENTS_MCP_PROJECTION_PREFIX}: ${type} at `, path });
},
);

it("applies the shared server cap before normal and rollback v2 publication", () => {
const entries = Array.from(
{ length: DEEPAGENTS_MCP_MAX_SERVERS + 1 },
Expand All @@ -74,29 +94,49 @@ describe("Deep Agents managed MCP projection safety", () => {
);
});

it("fails status inspection closed without following hostile projection paths", () => {
it.each([
{
name: "a dangling symbolic link",
config: undefined,
options: { symlink: true },
type: "symbolic link",
targetText: null,
},
{
name: "a symbolic link to valid content",
config: emptyProjection,
options: { symlink: true },
type: "symbolic link",
targetText: `${JSON.stringify(emptyProjection, null, 2)}\n`,
},
{
name: "a FIFO",
config: undefined,
options: { fifo: true },
type: "FIFO",
targetText: null,
},
{
name: "a directory",
config: undefined,
options: { directory: true },
type: "non-regular file",
targetText: null,
},
])("rejects $name during status inspection (#10754)", ({ config, options, type, targetText }) => {
const statusCommand = buildDeepAgentsMcpStatusCommand(baseEntry);
expect(statusCommand).toContain("os.O_NONBLOCK | os.O_NOFOLLOW");
expect(statusCommand).not.toContain("config_path.read_text");
const symlink = runDeepAgentsConfigCommand(
const result = runDeepAgentsConfigCommand(
statusCommand,
emptyProjection,
config,
"v2",
undefined,
0o600,
{ symlink: true },
options,
);
expect(symlink.status).toBe(2);
expect(symlink.stdout.trim()).toBe("");
expect(symlink.stderr).toContain("Could not inspect managed Deep Agents MCP state");
expect(symlink.managedSymlinkTargetText).toBe(`${JSON.stringify(emptyProjection, null, 2)}\n`);

const fifo = runDeepAgentsConfigCommand(statusCommand, undefined, "v2", undefined, 0o600, {
fifo: true,
});
expect(fifo.status).toBe(2);
expect(fifo.stdout.trim()).toBe("");
expect(fifo.stderr).toContain("Could not inspect managed Deep Agents MCP state");
expect(result.status).toBe(2);
expect(result.stdout.trim()).toBe("");
expect(result.stderr).toContain(`Unsafe managed Deep Agents MCP projection path: ${type}`);
expect(result.managedSymlinkTargetText).toBe(targetText);
});

it.each([
Expand Down
32 changes: 31 additions & 1 deletion src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,25 @@

export const DEEPAGENTS_MCP_MAX_SERVERS = 64;

const DEEPAGENTS_UNSAFE_MCP_PROJECTION_CLASSIFIERS = [
{ predicate: "stat.S_ISLNK(metadata.st_mode)", type: "symbolic link" },
{ predicate: "stat.S_ISFIFO(metadata.st_mode)", type: "FIFO" },
] as const;
const DEEPAGENTS_UNSAFE_MCP_PROJECTION_FALLBACK_TYPE = "non-regular file";
export const DEEPAGENTS_UNSAFE_MCP_PROJECTION_TYPES = [
...DEEPAGENTS_UNSAFE_MCP_PROJECTION_CLASSIFIERS.map(({ type }) => type),
DEEPAGENTS_UNSAFE_MCP_PROJECTION_FALLBACK_TYPE,
] as const;

const DEEPAGENTS_MANAGED_PROJECTION_TYPE_HELPERS = [
"def describe_managed_projection_type(metadata):",
...DEEPAGENTS_UNSAFE_MCP_PROJECTION_CLASSIFIERS.flatMap(({ predicate, type }) => [
` if ${predicate}:`,
` return ${JSON.stringify(type)}`,
]),
` return ${JSON.stringify(DEEPAGENTS_UNSAFE_MCP_PROJECTION_FALLBACK_TYPE)}`,
];

export const DEEPAGENTS_STRICT_JSON_HELPERS = [
"def reject_duplicate_keys(pairs):",
" result = {}",
Expand All @@ -19,6 +38,9 @@ export const DEEPAGENTS_STRICT_JSON_HELPERS = [

export const DEEPAGENTS_MANAGED_PROJECTION_READ_HELPERS = [
"MANAGED_MCP_MAX_BYTES = 262144",
"class UnsafeManagedProjectionError(ValueError):",
" pass",
...DEEPAGENTS_MANAGED_PROJECTION_TYPE_HELPERS,
"def managed_fingerprint(metadata):",
" return (metadata.st_dev, metadata.st_ino, metadata.st_size, metadata.st_mtime_ns, metadata.st_ctime_ns, metadata.st_mode, metadata.st_nlink, metadata.st_uid)",
"def managed_path_identity(path):",
Expand All @@ -37,7 +59,10 @@ export const DEEPAGENTS_MANAGED_PROJECTION_READ_HELPERS = [
"def validate_managed_descriptor_path(path, descriptor):",
" opened = os.fstat(descriptor)",
" linked = os.stat(path, follow_symlinks=False)",
" safe = (stat.S_ISREG(opened.st_mode) and opened.st_uid == os.getuid() and stat.S_IMODE(opened.st_mode) == 0o600 and opened.st_nlink == 1 and (opened.st_dev, opened.st_ino) == (linked.st_dev, linked.st_ino))",
" for metadata in (opened, linked):",
" if not stat.S_ISREG(metadata.st_mode):",
" raise UnsafeManagedProjectionError(describe_managed_projection_type(metadata))",
" safe = (opened.st_uid == os.getuid() and stat.S_IMODE(opened.st_mode) == 0o600 and opened.st_nlink == 1 and (opened.st_dev, opened.st_ino) == (linked.st_dev, linked.st_ino))",
" if not safe:",
" raise ValueError('managed MCP projection has unsafe ownership, mode, type, links, or path identity')",
" return managed_fingerprint(opened)",
Expand All @@ -49,6 +74,11 @@ export const DEEPAGENTS_MANAGED_PROJECTION_READ_HELPERS = [
" except FileNotFoundError:",
" assert_managed_source_stable(path, None)",
" return b'', None, None",
" except OSError:",
" linked = os.stat(path, follow_symlinks=False)",

@cjagwani cjagwani Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Fail closed if the rejected symlink disappears during classification

After os.open(...O_NOFOLLOW) has already rejected the final symlink, an untrusted sandbox process can unlink it before this follow-up os.stat. That FileNotFoundError escapes into the outer missing-projection handler, which prints absent and exits 0; a deterministic command-level reproduction produced exactly that result. Preserve the original no-follow failure or raise the typed unsafe error when this lookup disappears or changes, and cover the transition with a regression test.

@rsliter rsliter Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this unchanged because it is the removal race explicitly excluded from #10754. If the entry disappears between the no-follow open failure and fallback classification, the observable final state is absent and retains the existing exit-0 behavior. Closing that interleaving would require broader race semantics outside this PR.

@cjagwani cjagwani Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve every unsafe type when the initial open fails

This fallback only converts symlinks to UnsafeManagedProjectionError. On this exact head, a mode-000 FIFO makes os.open fail with EACCES; this os.stat(..., follow_symlinks=False) still sees the FIFO, but line 67 rethrows the generic error. When credential observation is absent, getAdapterRegistration discards that generic adapter error and public mcp status returns exit 0 with the FIFO still present. The unchanged unlink race has the same result: if the rejected symlink disappears before this os.stat, its FileNotFoundError reaches the outer missing-projection handler and returns absent/0. Classify every non-regular linked value with describe_managed_projection_type, preserve a controlled nonzero result if classification disappears or changes, and add public-boundary regressions for the mode-000 FIFO and unlink-between-open/stat cases.

@rsliter rsliter Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the in-scope FIFO path in 235be35. The fallback now classifies every non-regular lstat result with describe_managed_projection_type, and the public command-boundary regression uses a mode-000 FIFO while still requiring exit 2, type-specific stderr, and empty stdout. I did not add the unlink-between-open/stat case because removal races are explicitly excluded from #10754.

" if not stat.S_ISREG(linked.st_mode):",
" raise UnsafeManagedProjectionError(describe_managed_projection_type(linked)) from None",
" raise",
" try:",
" before = os.fstat(descriptor)",
" validate_managed_descriptor_path(path, descriptor)",
Expand Down
30 changes: 30 additions & 0 deletions src/lib/actions/sandbox/mcp-bridge-adapter-status.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ import {
import {
DEEPAGENTS_MANAGED_PROJECTION_READ_HELPERS,
DEEPAGENTS_STRICT_JSON_HELPERS,
DEEPAGENTS_UNSAFE_MCP_PROJECTION_TYPES,
} from "./mcp-bridge-adapter-deepagents-projection";

// NemoClaw owns this dedicated projection. Deep Agents Code's user/project
// `.mcp.json` discovery is disabled in the managed image so user-authored MCP
// state can never be layered over the validated registry projection.
export const DEEPAGENTS_MCP_CONFIG_PATH = "/sandbox/.deepagents/.nemoclaw-mcp.json";
export const UNSAFE_DEEPAGENTS_MCP_PROJECTION_PREFIX =
"Unsafe managed Deep Agents MCP projection path";
export const DEFAULT_OPENCLAW_CONFIG_DIR = "/sandbox/.openclaw";
export const HERMES_MCP_TRANSACTION_HELPER =
"/usr/local/lib/nemoclaw/hermes-mcp-config-transaction.py";
Expand All @@ -28,6 +31,30 @@ export const OPENCLAW_MCPORTER_ROOT = openClawMcporterRoot();
const DEFAULT_AUTH_HEADER = "Authorization";
const DEFAULT_AUTH_SCHEME = "Bearer";

export interface UnsafeDeepAgentsMcpProjectionResult {
messagePrefix: string;
path: string;
}

/** Parse only the unsafe-projection result emitted by the Deep Agents status adapter. */
export function parseUnsafeDeepAgentsMcpProjectionResult(result: {
status: number | null;
stdout: string;
stderr: string;
}): UnsafeDeepAgentsMcpProjectionResult | null {
if (result.status === 0) return null;
const detail = (result.stderr || result.stdout || "not found").trim();
for (const type of DEEPAGENTS_UNSAFE_MCP_PROJECTION_TYPES) {
const messagePrefix = `${UNSAFE_DEEPAGENTS_MCP_PROJECTION_PREFIX}: ${type} at `;
if (!detail.startsWith(messagePrefix)) continue;
const projectionPath = detail.slice(messagePrefix.length);
return projectionPath && !/[\r\n]/u.test(projectionPath)
? { messagePrefix, path: projectionPath }
: null;
}
return null;
}

function authPlaceholder(
entry: Pick<McpBridgeEntry, "env">,
credentialRevision?: McpAttachedCredentialRevision,
Expand Down Expand Up @@ -260,6 +287,9 @@ export function buildDeepAgentsMcpStatusCommand(
"config_path = managed_path if is_v2 else legacy_path",
"try:",
" data = read_managed_projection(config_path)[0] if is_v2 else read_legacy_config(config_path)[0]",
"except UnsafeManagedProjectionError as exc:",
` print(f'${UNSAFE_DEEPAGENTS_MCP_PROJECTION_PREFIX}: {exc} at {config_path}', file=sys.stderr)`,
" raise SystemExit(2)",
"except FileNotFoundError:",
" data = {}",
"except (OSError, UnicodeDecodeError, ValueError) as exc:",
Expand Down
Loading
Loading