Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 16 additions & 2 deletions src/lib/cli/public-argv-translation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,20 @@ function startsWithTokens(tokens: readonly string[], prefix: readonly string[]):
return prefix.every((token, index) => tokens[index] === token);
}

function matchRegisteredSandboxRoute(tokens: readonly string[]): SandboxRoute | null {
return sandboxRoutes().find((route) => startsWithTokens(tokens, route.publicTokens)) ?? null;
}

/**
* Return the longest registered sandbox route that prefixes the public input.
*
* Diagnostics use these registered tokens so untrusted action arguments never
* reach terminal or log output (#10212).
*/
export function matchSandboxRoute(tokens: readonly string[]): string[] | null {
return matchRegisteredSandboxRoute(tokens)?.publicTokens ?? null;
}

function nativeArgv(commandId: string, args: string[], argv?: string[]): NativeArgvTranslation {
return { kind: "nativeArgv", commandId, args, argv: argv ?? [...commandId.split(":"), ...args] };
}
Expand Down Expand Up @@ -162,8 +176,8 @@ export function translatePublicSandboxArgv(
}

const inputTokens = [action, ...actionArgs];
for (const route of sandboxRoutes()) {
if (!startsWithTokens(inputTokens, route.publicTokens)) continue;
const route = matchRegisteredSandboxRoute(inputTokens);
if (route) {
const remainingArgs = inputTokens.slice(route.publicTokens.length);
return nativeArgv(route.commandId, [sandboxName, ...remainingArgs]);
}
Expand Down
68 changes: 63 additions & 5 deletions src/lib/cli/public-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
import { resolveBareConnectArgv } from "./bare-connect-routing";
import { getRegisteredOclifCommandMetadata } from "./oclif-metadata";
import {
matchSandboxRoute,
type PublicTranslationResult,
translatePublicGlobalArgv,
translatePublicSandboxArgv,
Expand Down Expand Up @@ -134,11 +135,20 @@ function isMigrationRecoveryInvocation(argv: readonly string[]): boolean {
);
}

function registeredSandboxNames(): string[] {
function sandboxRegistrationNames(): { published: string[]; pending: string[] } {
const registryApi = registry();
// Suggestions must use the same published sandbox inventory as `list` and global `status`.
return registryApi.listSandboxes().sandboxes.filter(registryApi.isPublishedSandboxRegistration)
.map((sandbox) => sandbox.name);
const sandboxes = registryApi.listSandboxes().sandboxes;
return {
// Suggestions must use the same published inventory as `list` and global `status`.
published: sandboxes.filter(registryApi.isPublishedSandboxRegistration).map(({ name }) => name),
pending: sandboxes
.filter(({ pendingRouteReservation }) => pendingRouteReservation === true)
.map(({ name }) => name),
};
}

function registeredSandboxNames(): string[] {
return sandboxRegistrationNames().published;
}

function findRegisteredSandboxName(tokens: string[]): string | null {
Expand Down Expand Up @@ -267,17 +277,65 @@ function printGlobalStatusScopeHint(sandboxName: string, args: readonly string[]
process.exit(2);
}

/**
* Report the sandbox-first grammar for a first token that names a sandbox action.
*
* `nemoclaw doctor` and `nemoclaw policy list` name an action, not a sandbox.
* Reporting a missing sandbox sends the reader to `onboard` for a sandbox they
* never asked for (#10212).
*/
function printSandboxScopeHint(action: string, remainingArgs: readonly string[]): never {
// Render the registered route, never the tokens the reader typed. An action
// argument can carry a credential, a newline that forges a diagnostic line,
// or an ESC byte that rewrites terminal output.
const route: string[] = matchSandboxRoute([action, ...remainingArgs]) ?? [action];
console.error(` '${action}' is a sandbox command. It needs a sandbox name.`);
console.error("");
console.error(` Run: ${CLI_NAME} <name> ${route.join(" ")}`);
const { published: allNames, pending: pendingNames } = sandboxRegistrationNames();
if (allNames.length > 0) {
console.error(` Registered sandboxes: ${allNames.join(", ")}`);
console.error(` Run '${CLI_NAME} list' to see all sandboxes.`);
} else if (pendingNames.length > 0) {
console.error(` Sandbox setup is still pending: ${pendingNames.join(", ")}`);
console.error(" Wait for onboarding to finish.");
console.error(` If onboarding stopped, run '${CLI_NAME} onboard --resume' to continue it.`);
} else {
console.error(` Run '${CLI_NAME} onboard' to create one.`);
}
process.exit(1);
}

/** Recover an explicit sandbox invocation before reporting an action-like name as a grammar error. */
async function recoverRequestedSandboxIfNeeded(
sandboxName: string,
action: string,
rawArgsAfterSandboxName: string[],
): Promise<void> {
if (registry().getSandbox(sandboxName) || !isKnownSandboxAction(action)) return;
if (registry().getSandbox(sandboxName)) return;
const namesSandboxAction = isKnownSandboxAction(sandboxName);
const hasExplicitSandboxAction =
rawArgsAfterSandboxName.length > 0 && isKnownSandboxAction(rawArgsAfterSandboxName[0] ?? "");

// An action-first diagnostic must stay read-only. Seeded registry recovery
// can select or start a gateway and persist entries, which is inappropriate
// for a bare action or registered multi-token route entered without a name.
// Keep recovery for an explicit action-name sandbox invocation such as
// `doctor status`, where `doctor` can be a real sandbox with a stale entry.
if (namesSandboxAction && !hasExplicitSandboxAction) {
printSandboxScopeHint(sandboxName, rawArgsAfterSandboxName);
}
if (!namesSandboxAction && !isKnownSandboxAction(action)) return;

validateName(sandboxName, "sandbox name");
await registryRecovery().recoverRegistryEntries({ requestedSandboxName: sandboxName });
if (registry().getSandbox(sandboxName)) return;

// Recovery runs first so a live sandbox named after an action stays reachable
// through the name-first grammar. A token that recovery cannot resolve is a
// scope error, not a missing sandbox.
if (namesSandboxAction) printSandboxScopeHint(sandboxName, rawArgsAfterSandboxName);

if (rawArgsAfterSandboxName.length === 0) {
const suggestion = suggestGlobalCommand(sandboxName);
if (suggestion) {
Expand Down
180 changes: 180 additions & 0 deletions test/cli/dispatch-basics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,186 @@ describe("CLI dispatch", () => {
);
});

it("reports the sandbox-first grammar without recovering a bare action (#10212)", async () => {
await withDirectPublicDispatch(
async ({ dispatchCli, exitSpy, recoverRegistryEntries, stderr }) => {
await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1");

const output = stderr.join("\n");
expect(recoverRegistryEntries).not.toHaveBeenCalled();
expect(output).toContain("'doctor' is a sandbox command. It needs a sandbox name.");
expect(output).toContain("Run: nemoclaw <name> doctor");
expect(output).toContain("Run 'nemoclaw onboard' to create one.");
expect(output).not.toContain("Sandbox 'doctor' does not exist.");
expect(exitSpy).toHaveBeenCalledWith(1);
},
);
});

it.each([
{
input: "flag argument",
argv: ["doctor", "--json"],
route: "doctor",
forbidden: /--json/,
},
{
input: "credential-bearing argument",
argv: ["exec", "--", "curl", "-H", "Authorization: Bearer nvapi-SECRET12345"],
route: "exec",
forbidden: /Authorization|nvapi-SECRET12345/,
},
{
input: "newline argument",
argv: ["exec", "x\n Sandbox alpha was destroyed."],
route: "exec",
forbidden: /Sandbox alpha was destroyed\./,
},
{
input: "terminal escape argument",
argv: ["exec", "\u001b[31mRED"],
route: "exec",
forbidden: /\u001b|RED/,
},
{
input: "long argument",
argv: ["exec", "A".repeat(4000)],
route: "exec",
forbidden: /A{80}/,
},
])(
"omits an untrusted $input from the sandbox-first grammar hint (#10212)",
async ({ argv, route, forbidden }) => {
await withDirectPublicDispatch(async ({ dispatchCli, recoverRegistryEntries, stderr }) => {
await expect(dispatchCli(argv)).rejects.toThrow("process.exit:1");

const output = stderr.join("\n");
const runLine = stderr.find((line) => line.includes("Run: nemoclaw <name>")) ?? "";
expect(recoverRegistryEntries).not.toHaveBeenCalled();
expect(runLine).toBe(` Run: nemoclaw <name> ${route}`);
expect(output).not.toMatch(forbidden);
});
},
);

it("renders the registered route for an unregistered trailing token (#10212)", async () => {
await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => {
await expect(dispatchCli(["policy", "not-a-verb"])).rejects.toThrow("process.exit:1");

const output = stderr.join("\n");
expect(output).toContain("Run: nemoclaw <name> policy");
expect(output).not.toContain("not-a-verb");
});
});

it.each([
{ argv: ["policy", "list"], action: "policy", route: "policy list" },
{ argv: ["policy-add"], action: "policy-add", route: "policy-add" },
])(
"reports the sandbox-first grammar for the registered $route route (#10212)",
async ({ argv, action, route }) => {
await withDirectPublicDispatch(async ({ dispatchCli, recoverRegistryEntries, stderr }) => {
await expect(dispatchCli(argv)).rejects.toThrow("process.exit:1");

const output = stderr.join("\n");
expect(recoverRegistryEntries).not.toHaveBeenCalled();
expect(output).toContain(`'${action}' is a sandbox command. It needs a sandbox name.`);
expect(output).toContain(`Run: nemoclaw <name> ${route}`);
expect(output).not.toContain(`Unknown command: ${action}`);
});
},
);

it("lists registered sandboxes in the sandbox-first grammar hint (#10212)", async () => {
await withDirectPublicDispatch(
async ({ dispatchCli, stderr }) => {
await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1");

const output = stderr.join("\n");
expect(output).toContain("Registered sandboxes: alpha, beta");
expect(output).not.toContain("Run 'nemoclaw onboard' to create one.");
},
{ sandboxNames: ["alpha", "beta"] },
);
});

it("reports pending setup instead of new onboarding for a bare sandbox action (#10212)", async () => {
await withDirectPublicDispatch(
async ({ dispatchCli, stderr }) => {
await expect(dispatchCli(["doctor"])).rejects.toThrow("process.exit:1");

const output = stderr.join("\n");
expect(output).toContain("Sandbox setup is still pending: alpha");
expect(output).toContain("Wait for onboarding to finish.");
expect(output).toContain(
"If onboarding stopped, run 'nemoclaw onboard --resume' to continue it.",
);
expect(output).not.toContain("Run 'nemoclaw onboard' to create one.");
},
{ sandboxNames: ["alpha"], pendingSandboxNames: ["alpha"] },
);
});

it("does not suggest an unrelated global command for a sandbox action (#10212)", async () => {
await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => {
await expect(dispatchCli(["share"])).rejects.toThrow("process.exit:1");

const output = stderr.join("\n");
expect(output).toContain("'share' is a sandbox command. It needs a sandbox name.");
expect(output).not.toContain("Did you mean: nemoclaw start?");
});
});

it("prefers the sandbox-action report over a near-match global suggestion (#10212)", async () => {
await withDirectPublicDispatch(async ({ dispatchCli, stderr }) => {
await expect(dispatchCli(["agent"])).rejects.toThrow("process.exit:1");

// `agent` names a sandbox action and `agents` is a separate global
// command. An exact action-token match is more accurate than an
// edit-distance guess, so the scope report replaces the suggestion.
const output = stderr.join("\n");
expect(output).toContain("'agent' is a sandbox command. It needs a sandbox name.");
expect(output).not.toContain("Did you mean: nemoclaw agents?");
});
});

it("keeps the name-first grammar for a sandbox literally named doctor (#10212)", async () => {
await withDirectPublicDispatch(
async ({ dispatchCli, runOclifCommandById, stderr }) => {
await dispatchCli(["doctor", "status"]);

expect(runOclifCommandById).toHaveBeenCalledWith(
"sandbox:status",
["doctor"],
expect.anything(),
);
expect(stderr.join("\n")).not.toContain("is a sandbox command");
},
{ sandboxNames: ["doctor"] },
);
});

it("recovers a live sandbox named after an action before reporting scope (#10212)", async () => {
await withDirectPublicDispatch(
async ({ dispatchCli, recoverRegistryEntries, runOclifCommandById, sandboxes, stderr }) => {
recoverRegistryEntries.mockImplementation(async () => {
sandboxes.set("doctor", { name: "doctor" });
return { sandboxes: [...sandboxes.values()], defaultSandbox: null };
});

await dispatchCli(["doctor", "status"]);

expect(recoverRegistryEntries).toHaveBeenCalledTimes(1);
expect(runOclifCommandById).toHaveBeenCalledWith(
"sandbox:status",
["doctor"],
expect.anything(),
);
expect(stderr.join("\n")).not.toContain("is a sandbox command");
},
);
});

it("does not suggest a route-only reservation in the connect command-order hint (#8801)", async () => {
await withDirectPublicDispatch(
async ({ dispatchCli, exitSpy, stderr }) => {
Expand Down
Loading