fix: harden CLI parsing and generated artifacts
This commit is contained in:
parent
8beee8764f
commit
7491ed5a85
@ -74,16 +74,26 @@ export function metadataPathForArtifact(artifactPath: string): string {
|
||||
// readCliMetadata loads metadata for a generated CLI artifact, preferring the embedded
|
||||
// inspect command and falling back to legacy sidecar files.
|
||||
export async function readCliMetadata(artifactPath: string): Promise<CliArtifactMetadata> {
|
||||
let embeddedError: unknown;
|
||||
try {
|
||||
return await readMetadataFromCli(artifactPath);
|
||||
} catch (error) {
|
||||
embeddedError = error;
|
||||
}
|
||||
|
||||
const legacyPath = metadataPathForArtifact(artifactPath);
|
||||
try {
|
||||
const buffer = await fs.readFile(legacyPath, 'utf8');
|
||||
return JSON.parse(buffer) as CliArtifactMetadata;
|
||||
} catch (error) {
|
||||
if (isErrno(error, 'ENOENT') && embeddedError) {
|
||||
throw embeddedError;
|
||||
}
|
||||
if (!isErrno(error, 'ENOENT')) {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
return await readMetadataFromCli(artifactPath);
|
||||
throw embeddedError;
|
||||
}
|
||||
|
||||
async function readMetadataFromCli(artifactPath: string): Promise<CliArtifactMetadata> {
|
||||
|
||||
21
src/cli.ts
21
src/cli.ts
@ -239,16 +239,16 @@ export async function runCli(argv: string[]): Promise<void> {
|
||||
: null;
|
||||
const runtime = createKeepAliveRuntime(baseRuntime, { daemonClient, keepAliveServers });
|
||||
|
||||
const inference = inferCommandRouting(command, args, runtime.getDefinitions());
|
||||
if (inference.kind === 'abort') {
|
||||
process.exitCode = inference.exitCode;
|
||||
return;
|
||||
}
|
||||
const resolvedCommand = inference.command;
|
||||
const resolvedArgs = inference.args;
|
||||
|
||||
let primaryError: unknown;
|
||||
try {
|
||||
const inference = inferCommandRouting(command, args, runtime.getDefinitions());
|
||||
if (inference.kind === 'abort') {
|
||||
process.exitCode = inference.exitCode;
|
||||
return;
|
||||
}
|
||||
const resolvedCommand = inference.command;
|
||||
const resolvedArgs = inference.args;
|
||||
|
||||
if (resolvedCommand === 'list') {
|
||||
if (consumeHelpTokens(resolvedArgs)) {
|
||||
const { printListHelp } = await import('./cli/list-command.js');
|
||||
@ -308,14 +308,15 @@ export async function runCli(argv: string[]): Promise<void> {
|
||||
await importedHandleResource(runtime, resolvedArgs);
|
||||
return;
|
||||
}
|
||||
|
||||
printHelp(`Unknown command '${resolvedCommand}'.`);
|
||||
process.exit(1);
|
||||
} catch (error) {
|
||||
primaryError = error;
|
||||
throw error;
|
||||
} finally {
|
||||
await closeRuntimeAfterCommand(runtime, { suppressReplayCloseError: primaryError !== undefined });
|
||||
}
|
||||
printHelp(`Unknown command '${resolvedCommand}'.`);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
async function closeRuntimeAfterCommand(
|
||||
|
||||
@ -49,7 +49,7 @@ export function resolveEphemeralServer(spec: EphemeralServerSpec): EphemeralServ
|
||||
headers: __configInternals.ensureHttpAcceptHeader(spec.headers),
|
||||
};
|
||||
const canonical = spec.name ? undefined : canonicalKeepAliveName(command);
|
||||
const name = slugify(spec.name ?? canonical ?? inferNameFromUrl(url));
|
||||
const name = normalizeEphemeralName(spec.name ?? canonical ?? inferNameFromUrl(url));
|
||||
const lifecycle = resolveLifecycle(name, undefined, command);
|
||||
const definition: ServerDefinition = {
|
||||
name,
|
||||
@ -84,7 +84,7 @@ export function resolveEphemeralServer(spec: EphemeralServerSpec): EphemeralServ
|
||||
cwd,
|
||||
};
|
||||
const canonical = spec.name ? undefined : canonicalKeepAliveName(command);
|
||||
const name = slugify(spec.name ?? canonical ?? inferNameFromCommand(parts));
|
||||
const name = normalizeEphemeralName(spec.name ?? canonical ?? inferNameFromCommand(parts));
|
||||
const lifecycle = resolveLifecycle(name, undefined, command);
|
||||
const definition: ServerDefinition = {
|
||||
name,
|
||||
@ -206,6 +206,14 @@ function slugify(value: string): string {
|
||||
.replace(/-{2,}/g, '-');
|
||||
}
|
||||
|
||||
function normalizeEphemeralName(value: string): string {
|
||||
const name = slugify(value);
|
||||
if (!name) {
|
||||
throw new Error('Ad-hoc server name must contain at least one letter or digit.');
|
||||
}
|
||||
return name;
|
||||
}
|
||||
|
||||
export function splitCommandLine(input: string): string[] {
|
||||
const result: string[] = [];
|
||||
let current = '';
|
||||
|
||||
@ -347,6 +347,9 @@ function resolveNamedArgumentValue(
|
||||
const literal = rawValue.slice(1);
|
||||
return { value: literal, schemaValue: literal };
|
||||
}
|
||||
if (rawValue.length > 0 && rawValue.trim() === '') {
|
||||
return { value: rawValue, schemaValue: rawValue };
|
||||
}
|
||||
if (!rawValue.startsWith('@')) {
|
||||
return { value: coerceValue(rawValue, coercionMode), schemaValue: rawValue };
|
||||
}
|
||||
|
||||
@ -2,6 +2,7 @@ import { resolveConfigPath } from '../config/path-discovery.js';
|
||||
import { parseLogLevel } from '../logging.js';
|
||||
import { extractFlags } from './flag-utils.js';
|
||||
import { getActiveLogger, getActiveLogLevel, logError, setLogLevel } from './logger-context.js';
|
||||
import { parsePositiveInteger } from './timeouts.js';
|
||||
|
||||
export interface GlobalCliContext {
|
||||
readonly globalFlags: Record<string, string | undefined>;
|
||||
@ -29,8 +30,8 @@ export function buildGlobalContext(argv: string[]): GlobalCliContext | { exit: t
|
||||
|
||||
let oauthTimeoutOverride: number | undefined;
|
||||
if (globalFlags['--oauth-timeout']) {
|
||||
const parsed = Number.parseInt(globalFlags['--oauth-timeout'], 10);
|
||||
if (!Number.isFinite(parsed) || parsed <= 0) {
|
||||
const parsed = parsePositiveInteger(globalFlags['--oauth-timeout']);
|
||||
if (parsed === undefined) {
|
||||
logError("Flag '--oauth-timeout' must be a positive integer (milliseconds).");
|
||||
return { exit: true, code: 1 };
|
||||
}
|
||||
|
||||
@ -259,7 +259,7 @@ async function writeFile(targetPath: string, contents: string): Promise<void> {
|
||||
function computeImportPath(fromPath: string, typesPath: string): string {
|
||||
const fromDir = path.dirname(fromPath);
|
||||
const relative = path.relative(fromDir, typesPath).replace(/\\/g, '/');
|
||||
const withoutExt = relative.replace(/\.[^.]+$/, '');
|
||||
const withoutExt = relative.endsWith('.d.ts') ? relative.slice(0, -5) : relative.replace(/\.[^.]+$/, '');
|
||||
if (withoutExt.startsWith('.')) {
|
||||
return withoutExt;
|
||||
}
|
||||
|
||||
@ -1,3 +1,5 @@
|
||||
import { parsePositiveInteger } from '../timeouts.js';
|
||||
|
||||
export interface GeneratorCommonFlags {
|
||||
runtime?: 'node' | 'bun';
|
||||
timeout?: number;
|
||||
@ -31,8 +33,8 @@ export function extractGeneratorFlags(args: string[], options: ExtractOptions =
|
||||
if (!raw) {
|
||||
throw new Error("Flag '--timeout' requires a value.");
|
||||
}
|
||||
const parsed = Number.parseInt(raw, 10);
|
||||
if (!Number.isFinite(parsed) || parsed <= 0) {
|
||||
const parsed = parsePositiveInteger(raw);
|
||||
if (parsed === undefined) {
|
||||
throw new Error('--timeout must be a positive integer.');
|
||||
}
|
||||
result.timeout = parsed;
|
||||
|
||||
@ -101,6 +101,7 @@ export function renderTemplate({
|
||||
tool: entry.tool,
|
||||
})
|
||||
);
|
||||
assertUniqueGeneratedCommandNames(renderedTools);
|
||||
const toolHelp = renderedTools.map((entry) => ({
|
||||
name: entry.commandName,
|
||||
description: entry.tool.tool.description ?? '',
|
||||
@ -237,7 +238,7 @@ function parseArrayOption(value: string, itemType: 'string' | 'number' | 'boolea
|
||||
\t}
|
||||
\tconst values = value.split(',').map((entry) => entry.trim());
|
||||
\tif (itemType === 'number') {
|
||||
\t\treturn values.map((entry) => parseFloat(entry));
|
||||
\t\treturn values.map((entry) => parseFiniteNumber(entry));
|
||||
\t}
|
||||
\tif (itemType === 'boolean') {
|
||||
\t\treturn values.map((entry) => entry !== 'false');
|
||||
@ -245,6 +246,15 @@ function parseArrayOption(value: string, itemType: 'string' | 'number' | 'boolea
|
||||
\treturn values;
|
||||
}
|
||||
|
||||
function parseFiniteNumber(value: string): number {
|
||||
\tconst trimmed = value.trim();
|
||||
\tconst parsed = Number(trimmed);
|
||||
\tif (trimmed === '' || !Number.isFinite(parsed)) {
|
||||
\t\tthrow new Error('Expected a finite number.');
|
||||
\t}
|
||||
\treturn parsed;
|
||||
}
|
||||
|
||||
function normalizeEmbeddedServer(server: typeof embeddedServer) {
|
||||
\tconst base = { ...server } as Record<string, unknown>;
|
||||
\tif ((server.command as any).kind === 'http') {
|
||||
@ -462,7 +472,9 @@ export function renderToolCommand(
|
||||
({ option, camelCaseProp }) =>
|
||||
`{ value: cmdOpts.${camelCaseProp}, flag: ${JSON.stringify(`--${option.cliName}`)} }`
|
||||
)
|
||||
.join(', ')}].filter((entry) => entry.value === undefined).map((entry) => entry.flag);
|
||||
.join(
|
||||
', '
|
||||
)}].filter((entry) => entry.value === undefined || (typeof entry.value === 'string' && entry.value.trim() === '')).map((entry) => entry.flag);
|
||||
\t\t\tif (missingRequired.length > 0) {
|
||||
\t\t\t\tthrow new Error('Missing required option' + (missingRequired.length === 1 ? '' : 's') + ': ' + missingRequired.join(', '));
|
||||
\t\t\t}`
|
||||
@ -549,7 +561,7 @@ export const templateTestHelpers = { computeRelativeStdioCwd };
|
||||
function optionParser(option: GeneratedOption): string | undefined {
|
||||
switch (option.type) {
|
||||
case 'number':
|
||||
return '(value) => parseFloat(value)';
|
||||
return '(value) => parseFiniteNumber(value)';
|
||||
case 'boolean':
|
||||
return "(value) => value !== 'false'";
|
||||
case 'object':
|
||||
@ -570,3 +582,16 @@ function optionParser(option: GeneratedOption): string | undefined {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function assertUniqueGeneratedCommandNames(tools: Array<{ commandName: string; tool: ToolMetadata }>): void {
|
||||
const commands = new Map<string, string>();
|
||||
for (const entry of tools) {
|
||||
const previous = commands.get(entry.commandName);
|
||||
if (previous) {
|
||||
throw new Error(
|
||||
`Generated command name collision '${entry.commandName}' for tools '${previous}' and '${entry.tool.tool.name}'.`
|
||||
);
|
||||
}
|
||||
commands.set(entry.commandName, entry.tool.tool.name);
|
||||
}
|
||||
}
|
||||
|
||||
@ -50,6 +50,27 @@ export function buildToolMetadata(tool: ServerToolInfo): ToolMetadata {
|
||||
};
|
||||
}
|
||||
|
||||
export function buildToolMetadataList(
|
||||
tools: ServerToolInfo[],
|
||||
options: { readonly sort?: boolean } = {}
|
||||
): ToolMetadata[] {
|
||||
const result = tools.map((tool) => buildToolMetadata(tool));
|
||||
if (options.sort !== false) {
|
||||
result.sort((left, right) => left.tool.name.localeCompare(right.tool.name));
|
||||
}
|
||||
const methods = new Map<string, string>();
|
||||
for (const entry of result) {
|
||||
const previous = methods.get(entry.methodName);
|
||||
if (previous) {
|
||||
throw new Error(
|
||||
`Generated proxy method collision '${entry.methodName}' for tools '${previous}' and '${entry.tool.name}'.`
|
||||
);
|
||||
}
|
||||
methods.set(entry.methodName, entry.tool.name);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
export function buildEmbeddedSchemaMap(tools: ToolMetadata[]): Record<string, unknown> {
|
||||
const result: Record<string, unknown> = {};
|
||||
for (const entry of tools.toSorted((left, right) => left.tool.name.localeCompare(right.tool.name))) {
|
||||
|
||||
@ -93,6 +93,9 @@ function parseInspectFlags(args: string[]): InspectFlags {
|
||||
if (!artifactPath) {
|
||||
throw new Error('Usage: mcporter inspect-cli <artifact> [--json]');
|
||||
}
|
||||
if (args.length > 0) {
|
||||
throw new Error(`Unexpected inspect-cli argument '${args[0]}'.`);
|
||||
}
|
||||
return { artifactPath, format };
|
||||
}
|
||||
|
||||
|
||||
@ -266,5 +266,5 @@ function quoteCommandSegment(segment: string): string {
|
||||
if (/^[A-Za-z0-9_./:-]+$/.test(segment)) {
|
||||
return segment;
|
||||
}
|
||||
return JSON.stringify(segment);
|
||||
return `'${segment.replace(/'/g, `'\\''`)}'`;
|
||||
}
|
||||
|
||||
@ -1,4 +1,5 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { inspect } from 'node:util';
|
||||
import type { CallResult } from '../result-utils.js';
|
||||
import { logWarn } from './logger-context.js';
|
||||
@ -33,17 +34,8 @@ export function tailLogIfRequested(result: unknown, enabled: boolean): void {
|
||||
return;
|
||||
}
|
||||
const candidates: string[] = [];
|
||||
if (typeof result === 'string') {
|
||||
const idx = result.indexOf(':');
|
||||
if (idx !== -1) {
|
||||
const candidate = result.slice(idx + 1).trim();
|
||||
if (candidate) {
|
||||
candidates.push(candidate);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (result && typeof result === 'object') {
|
||||
const possibleKeys = ['logPath', 'logFile', 'logfile', 'path'];
|
||||
const possibleKeys = ['logPath', 'logFile', 'logfile'];
|
||||
for (const key of possibleKeys) {
|
||||
const value = (result as Record<string, unknown>)[key];
|
||||
if (typeof value === 'string') {
|
||||
@ -53,6 +45,10 @@ export function tailLogIfRequested(result: unknown, enabled: boolean): void {
|
||||
}
|
||||
|
||||
for (const candidate of candidates) {
|
||||
if (!path.isAbsolute(candidate)) {
|
||||
logWarn(`Refusing to tail non-absolute log path: ${candidate}`);
|
||||
continue;
|
||||
}
|
||||
if (!fs.existsSync(candidate)) {
|
||||
logWarn(`Log path not found: ${candidate}`);
|
||||
continue;
|
||||
|
||||
@ -1,16 +1,21 @@
|
||||
const DEFAULT_LIST_TIMEOUT_MS = 30_000;
|
||||
const DEFAULT_CALL_TIMEOUT_MS = 60_000;
|
||||
const POSITIVE_INTEGER_PATTERN = /^[1-9]\d*$/;
|
||||
|
||||
export function parsePositiveInteger(raw: string | undefined): number | undefined {
|
||||
if (!raw || !POSITIVE_INTEGER_PATTERN.test(raw)) {
|
||||
return undefined;
|
||||
}
|
||||
const parsed = Number(raw);
|
||||
return Number.isFinite(parsed) ? parsed : undefined;
|
||||
}
|
||||
|
||||
// parseTimeout reads timeout values from strings while honoring defaults.
|
||||
export function parseTimeout(raw: string | undefined, fallback: number): number {
|
||||
if (!raw) {
|
||||
return fallback;
|
||||
}
|
||||
const parsed = Number.parseInt(raw, 10);
|
||||
if (!Number.isFinite(parsed) || parsed <= 0) {
|
||||
return fallback;
|
||||
}
|
||||
return parsed;
|
||||
return parsePositiveInteger(raw) ?? fallback;
|
||||
}
|
||||
|
||||
export const LIST_TIMEOUT_MS = parseTimeout(process.env.MCPORTER_LIST_TIMEOUT, DEFAULT_LIST_TIMEOUT_MS);
|
||||
@ -58,8 +63,8 @@ export function consumeTimeoutFlag(
|
||||
if (!value) {
|
||||
throw new Error(missingValueMessage);
|
||||
}
|
||||
const parsed = Number.parseInt(value, 10);
|
||||
if (!Number.isFinite(parsed) || parsed <= 0) {
|
||||
const parsed = parsePositiveInteger(value);
|
||||
if (parsed === undefined) {
|
||||
throw new Error(`${flagName} must be a positive integer (milliseconds).`);
|
||||
}
|
||||
args.splice(index, 2);
|
||||
|
||||
@ -1,5 +1,5 @@
|
||||
import type { ListToolsOptions, Runtime } from '../runtime.js';
|
||||
import { buildToolMetadata, type ToolMetadata } from './generate/tools.js';
|
||||
import { buildToolMetadataList, type ToolMetadata } from './generate/tools.js';
|
||||
|
||||
interface LoadToolMetadataOptions {
|
||||
includeSchema?: boolean;
|
||||
@ -43,7 +43,7 @@ export async function loadToolMetadata(
|
||||
};
|
||||
const promise = runtime
|
||||
.listTools(serverName, listOptions)
|
||||
.then((tools) => tools.map((tool) => buildToolMetadata(tool)))
|
||||
.then((tools) => buildToolMetadataList(tools, { sort: false }))
|
||||
.catch((error) => {
|
||||
cache?.delete(key);
|
||||
throw error;
|
||||
|
||||
@ -121,12 +121,44 @@ function validateVaultPayload(value: unknown): VaultPayload {
|
||||
) {
|
||||
throw new CliUsageError("Vault payload 'clientInfo' must be an object.");
|
||||
}
|
||||
validateOAuthTokens(record.tokens as Record<string, unknown>);
|
||||
if (record.clientInfo !== undefined) {
|
||||
validateOAuthClientInfo(record.clientInfo as Record<string, unknown>);
|
||||
}
|
||||
return {
|
||||
tokens: record.tokens as OAuthTokens,
|
||||
...(record.clientInfo ? { clientInfo: record.clientInfo as OAuthClientInformationMixed } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
function validateOAuthTokens(tokens: Record<string, unknown>): void {
|
||||
if (typeof tokens.access_token !== 'string' || tokens.access_token.length === 0) {
|
||||
throw new CliUsageError('Vault payload tokens.access_token must be a non-empty string.');
|
||||
}
|
||||
if (typeof tokens.token_type !== 'string' || tokens.token_type.length === 0) {
|
||||
throw new CliUsageError('Vault payload tokens.token_type must be a non-empty string.');
|
||||
}
|
||||
for (const key of ['refresh_token', 'scope'] as const) {
|
||||
if (tokens[key] !== undefined && typeof tokens[key] !== 'string') {
|
||||
throw new CliUsageError(`Vault payload tokens.${key} must be a string.`);
|
||||
}
|
||||
}
|
||||
if (
|
||||
tokens.expires_in !== undefined &&
|
||||
(!Number.isFinite(tokens.expires_in) || typeof tokens.expires_in !== 'number')
|
||||
) {
|
||||
throw new CliUsageError('Vault payload tokens.expires_in must be a finite number.');
|
||||
}
|
||||
}
|
||||
|
||||
function validateOAuthClientInfo(clientInfo: Record<string, unknown>): void {
|
||||
for (const [key, value] of Object.entries(clientInfo)) {
|
||||
if (value !== undefined && value !== null && typeof value !== 'string') {
|
||||
throw new CliUsageError(`Vault payload clientInfo.${key} must be a string.`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function printVaultHelp(): void {
|
||||
const lines = [
|
||||
'Usage: mcporter vault <set|clear> ...',
|
||||
|
||||
@ -11,7 +11,7 @@ import { ensureInvocationDefaults, fetchTools, resolveServerDefinition } from '.
|
||||
import { resolveRuntimeKind } from './cli/generate/runtime.js';
|
||||
import { readPackageMetadata, writeTemplate } from './cli/generate/template.js';
|
||||
import type { ToolMetadata } from './cli/generate/tools.js';
|
||||
import { buildToolMetadata, toolsTestHelpers } from './cli/generate/tools.js';
|
||||
import { buildToolMetadataList, toolsTestHelpers } from './cli/generate/tools.js';
|
||||
import { type CliArtifactMetadata, serializeDefinition } from './cli-metadata.js';
|
||||
import { stableJsonStringify } from './cli/generate/stable-json.js';
|
||||
import type { ServerDefinition } from './config.js';
|
||||
@ -62,9 +62,7 @@ export async function generateCli(
|
||||
: { ...baseDefinition, description: derivedDescription };
|
||||
const embeddedDefinition = stripBuildSources(definition);
|
||||
const serializedDefinition = serializeDefinition(embeddedDefinition);
|
||||
const toolMetadata: ToolMetadata[] = tools
|
||||
.map((tool) => buildToolMetadata(tool))
|
||||
.toSorted((left, right) => left.tool.name.localeCompare(right.tool.name));
|
||||
const toolMetadata: ToolMetadata[] = buildToolMetadataList(tools);
|
||||
const generator = await readPackageMetadata();
|
||||
const baseInvocation = ensureInvocationDefaults(
|
||||
{
|
||||
|
||||
@ -119,6 +119,11 @@ describe('parseCallArguments', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('preserves whitespace-only generic long flag values', () => {
|
||||
const parsed = parseCallArguments(['server.tool', '--body', ' ']);
|
||||
expect(parsed.args.body).toBe(' ');
|
||||
});
|
||||
|
||||
it('uses @@ to preserve a literal leading @ without reading a file', () => {
|
||||
const parsed = parseCallArguments(['server.tool', 'body=@@literal']);
|
||||
expect(parsed.args.body).toBe('@literal');
|
||||
|
||||
@ -12,4 +12,10 @@ describe('inspect-cli flag parsing', () => {
|
||||
it('validates explicit format values', () => {
|
||||
expect(() => inspectInternals.parseInspectFlags(['--format', 'xml', 'artifact'])).toThrow(/format/);
|
||||
});
|
||||
|
||||
it('rejects extra positional arguments', () => {
|
||||
expect(() => inspectInternals.parseInspectFlags(['artifact', 'shadow'])).toThrow(
|
||||
/Unexpected inspect-cli argument 'shadow'/
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
42
tests/cli-metadata.test.ts
Normal file
42
tests/cli-metadata.test.ts
Normal file
@ -0,0 +1,42 @@
|
||||
import fs from 'node:fs/promises';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { metadataPathForArtifact, readCliMetadata } from '../src/cli-metadata.js';
|
||||
|
||||
describe('readCliMetadata', () => {
|
||||
it('prefers embedded metadata over stale sidecar metadata', async () => {
|
||||
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'mcporter-metadata-'));
|
||||
const artifact = path.join(tempDir, 'artifact');
|
||||
const embedded = metadataPayload('embedded');
|
||||
const sidecar = metadataPayload('sidecar');
|
||||
await fs.writeFile(
|
||||
artifact,
|
||||
`#!/usr/bin/env node\nconsole.log(${JSON.stringify(JSON.stringify(embedded))});\n`,
|
||||
'utf8'
|
||||
);
|
||||
await fs.chmod(artifact, 0o755);
|
||||
await fs.writeFile(metadataPathForArtifact(artifact), JSON.stringify(sidecar), 'utf8');
|
||||
|
||||
await expect(readCliMetadata(artifact)).resolves.toMatchObject({
|
||||
server: { name: 'embedded' },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function metadataPayload(name: string) {
|
||||
return {
|
||||
schemaVersion: 1,
|
||||
generatedAt: '1970-01-01T00:00:00.000Z',
|
||||
generator: { name: 'mcporter', version: 'test' },
|
||||
server: {
|
||||
name,
|
||||
definition: {
|
||||
name,
|
||||
command: { kind: 'stdio' as const, command: 'node', args: [], cwd: process.cwd() },
|
||||
},
|
||||
},
|
||||
artifact: { path: '', kind: 'template' as const },
|
||||
invocation: { runtime: 'node' as const, timeoutMs: 30_000, minify: false },
|
||||
};
|
||||
}
|
||||
@ -52,6 +52,13 @@ describe('mcporter --oauth-timeout flag', () => {
|
||||
createRuntimeSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('rejects malformed --oauth-timeout values', async () => {
|
||||
const { runCli } = await import('../src/cli.js');
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
await expect(runCli(['--oauth-timeout', '5000abc', 'list'])).rejects.toThrow(/process\.exit/);
|
||||
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('positive integer'));
|
||||
});
|
||||
|
||||
it('returns once runtime.listTools surfaces an OAuth timeout error', async () => {
|
||||
const definition = {
|
||||
name: 'fake',
|
||||
|
||||
26
tests/cli-timeouts.test.ts
Normal file
26
tests/cli-timeouts.test.ts
Normal file
@ -0,0 +1,26 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { consumeTimeoutFlag, parseTimeout } from '../src/cli/timeouts.js';
|
||||
|
||||
describe('CLI timeout parsing', () => {
|
||||
it('accepts positive integer millisecond values', () => {
|
||||
expect(parseTimeout('2500', 30_000)).toBe(2_500);
|
||||
|
||||
const args = ['--timeout', '7500', 'server'];
|
||||
expect(consumeTimeoutFlag(args, 0)).toBe(7_500);
|
||||
expect(args).toEqual(['server']);
|
||||
});
|
||||
|
||||
it('falls back for non-positive and partially numeric environment values', () => {
|
||||
for (const value of ['0', '-1', '1s', '10abc', '100.5']) {
|
||||
expect(parseTimeout(value, 30_000)).toBe(30_000);
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects non-positive and partially numeric CLI flag values', () => {
|
||||
for (const value of ['0', '-1', '1s', '10abc', '100.5']) {
|
||||
expect(() => consumeTimeoutFlag(['--timeout', value], 0)).toThrow(
|
||||
'--timeout must be a positive integer (milliseconds).'
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
@ -101,6 +101,10 @@ describe('emit-ts templates', () => {
|
||||
expect(source).toContain('wrapCallResult');
|
||||
expect(source).toContain('proxy.listComments');
|
||||
});
|
||||
|
||||
it('does not leave a .d suffix when importing generated declaration files', () => {
|
||||
expect(emitTsTestInternals.computeImportPath('/tmp/client.ts', '/tmp/client.d.ts')).toBe('./client');
|
||||
});
|
||||
});
|
||||
|
||||
describe('handleEmitTs', () => {
|
||||
|
||||
@ -4,6 +4,7 @@ import {
|
||||
buildFallbackLiteral,
|
||||
buildPlaceholder,
|
||||
buildToolMetadata,
|
||||
buildToolMetadataList,
|
||||
extractOptions,
|
||||
getDescriptorDefault,
|
||||
getDescriptorDescription,
|
||||
@ -45,6 +46,15 @@ describe('generate helpers', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects generated proxy method collisions', () => {
|
||||
expect(() =>
|
||||
buildToolMetadataList([
|
||||
{ name: 'some-tool', inputSchema: undefined, outputSchema: undefined },
|
||||
{ name: 'some_tool', inputSchema: undefined, outputSchema: undefined },
|
||||
])
|
||||
).toThrow(/Generated proxy method collision 'someTool'/);
|
||||
});
|
||||
|
||||
it('extracts detailed option information', () => {
|
||||
const options = extractOptions(sampleTool);
|
||||
const first = options.find((option) => option.property === 'firstValue');
|
||||
|
||||
@ -137,6 +137,17 @@ describe('list output helpers', () => {
|
||||
expect(entry.authCommand).toBe(buildAuthCommandHint(definition));
|
||||
});
|
||||
|
||||
it('shell-quotes auth hints for stdio commands', () => {
|
||||
const hint = buildAuthCommandHint({
|
||||
name: 'unsafe',
|
||||
command: { kind: 'stdio', command: 'node', args: ['server.js', '--name', "$(touch bad)'"], cwd: process.cwd() },
|
||||
auth: 'oauth',
|
||||
source: { kind: 'local', path: '<adhoc>' },
|
||||
});
|
||||
expect(hint).toContain('mcporter auth --stdio node server.js --name ');
|
||||
expect(hint).toContain("'$(touch bad)'\\'''");
|
||||
});
|
||||
|
||||
it('exposes source list in JSON only when includeSources is true', () => {
|
||||
const withSources: ServerDefinition = {
|
||||
...definition,
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
import path from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { templateTestHelpers } from '../src/cli/generate/template.js';
|
||||
import { renderTemplate, templateTestHelpers } from '../src/cli/generate/template.js';
|
||||
import type { CliArtifactMetadata } from '../src/cli-metadata.js';
|
||||
import type { ServerDefinition } from '../src/config.js';
|
||||
|
||||
const { computeRelativeStdioCwd } = templateTestHelpers;
|
||||
@ -49,3 +50,103 @@ describe('computeRelativeStdioCwd', () => {
|
||||
expect(computeRelativeStdioCwd(stdioDef({ cwd: 'relative-dir' }), outputPath)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe('renderTemplate', () => {
|
||||
it('rejects sanitized command name collisions before emitting a broken CLI', () => {
|
||||
expect(() =>
|
||||
renderTemplate({
|
||||
runtimeKind: 'node',
|
||||
timeoutMs: 30_000,
|
||||
definition: stdioDef(),
|
||||
serverName: 'demo',
|
||||
generator: { name: 'mcporter', version: 'test' },
|
||||
metadata: metadataFor('demo'),
|
||||
tools: [
|
||||
{
|
||||
tool: { name: 'foo/bar', inputSchema: undefined, outputSchema: undefined },
|
||||
methodName: 'fooSlash',
|
||||
options: [],
|
||||
},
|
||||
{
|
||||
tool: { name: 'foo_bar', inputSchema: undefined, outputSchema: undefined },
|
||||
methodName: 'fooUnderscore',
|
||||
options: [],
|
||||
},
|
||||
],
|
||||
})
|
||||
).toThrow(/Generated command name collision 'foo-bar'/);
|
||||
});
|
||||
|
||||
it('emits strict numeric parsing and empty required-string validation', () => {
|
||||
const source = renderTemplate({
|
||||
runtimeKind: 'node',
|
||||
timeoutMs: 30_000,
|
||||
definition: stdioDef(),
|
||||
serverName: 'demo',
|
||||
generator: { name: 'mcporter', version: 'test' },
|
||||
metadata: metadataFor('demo'),
|
||||
tools: [
|
||||
{
|
||||
tool: {
|
||||
name: 'sum',
|
||||
inputSchema: {
|
||||
type: 'object',
|
||||
properties: {
|
||||
count: { type: 'number' },
|
||||
coords: { type: 'array', items: { type: 'number' } },
|
||||
name: { type: 'string' },
|
||||
},
|
||||
required: ['name'],
|
||||
},
|
||||
outputSchema: undefined,
|
||||
},
|
||||
methodName: 'sum',
|
||||
options: [
|
||||
{
|
||||
property: 'count',
|
||||
cliName: 'count',
|
||||
required: false,
|
||||
type: 'number',
|
||||
placeholder: '<count:number>',
|
||||
},
|
||||
{
|
||||
property: 'coords',
|
||||
cliName: 'coords',
|
||||
required: false,
|
||||
type: 'array',
|
||||
arrayItemType: 'number',
|
||||
placeholder: '<coords:value1,value2>',
|
||||
},
|
||||
{
|
||||
property: 'name',
|
||||
cliName: 'name',
|
||||
required: true,
|
||||
type: 'string',
|
||||
placeholder: '<name>',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(source).toContain('parseFiniteNumber');
|
||||
expect(source).not.toContain('parseFloat');
|
||||
expect(source).toContain("typeof entry.value === 'string' && entry.value.trim() === ''");
|
||||
});
|
||||
});
|
||||
|
||||
function metadataFor(serverName: string): CliArtifactMetadata {
|
||||
return {
|
||||
schemaVersion: 1,
|
||||
generatedAt: '1970-01-01T00:00:00.000Z',
|
||||
generator: { name: 'mcporter', version: 'test' },
|
||||
server: {
|
||||
name: serverName,
|
||||
definition: {
|
||||
name: serverName,
|
||||
command: { kind: 'stdio' as const, command: 'node', args: [], cwd: process.cwd() },
|
||||
},
|
||||
},
|
||||
artifact: { path: '', kind: 'template' as const },
|
||||
invocation: { runtime: 'node' as const, timeoutMs: 30_000, minify: false },
|
||||
};
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user