Skip to content

fix: security hardening across SDK, CLI, and MCP #2

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

Closed
Hovessian wants to merge 1 commit into from
Closed

fix: security hardening across SDK, CLI, and MCP #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 26 additions & 1 deletion packages/cli/src/commands/tokenize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,32 @@ export function registerTokenizeCommand(program: Command): void {
category: { id: categoryId },
};
if (options.description) body.description = options.description;
if (options.image) body.image = options.image;
if (options.image) {
// Validate image URL scheme to prevent SSRF with file:// or other protocols
try {
const imageUrl = new URL(options.image);
if (imageUrl.protocol !== 'https:' && imageUrl.protocol !== 'http:') {
if (isJson) {
console.log(
JSON.stringify({ error: "--image must be an HTTP or HTTPS URL" }),
);
} else {
console.error("Error: --image must be an HTTP or HTTPS URL");
}
process.exit(1);
}
} catch {
if (isJson) {
console.log(
JSON.stringify({ error: "--image must be a valid URL" }),
);
} else {
console.error("Error: --image must be a valid URL");
}
process.exit(1);
}
body.image = options.image;
}
if (initialBuyAmount !== undefined)
body.initialBuyAmount = initialBuyAmount;

Expand Down
18 changes: 15 additions & 3 deletions packages/mcp/src/tools/agentverse.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as fs from 'fs';
import * as path from 'path';
import { deployAgent, updateAgent, buildOptimizationChecklist } from 'agentlaunch-sdk';
import type { AgentverseDeployResult, AgentMetadata, OptimizationCheckItem } from 'agentlaunch-sdk';

Expand Down Expand Up @@ -34,12 +35,23 @@ export async function deployToAgentverse(args: {
readme?: string;
shortDescription?: string;
}): Promise<DeployToAgentverseResult> {
// Security: resolve and validate agentFile is within cwd to prevent path traversal
const resolvedPath = path.resolve(args.agentFile);
const cwd = process.cwd();
if (!resolvedPath.startsWith(cwd + path.sep) && resolvedPath !== cwd) {
throw new Error(
`Security: agentFile must be within the current working directory.\n` +
` cwd: ${cwd}\n` +
` resolved: ${resolvedPath}`,
);
}

// Read agent source code
if (!fs.existsSync(args.agentFile)) {
throw new Error(`Agent file not found: ${args.agentFile}`);
if (!fs.existsSync(resolvedPath)) {
throw new Error(`Agent file not found: ${resolvedPath}`);
}

const sourceCode = fs.readFileSync(args.agentFile, 'utf8');
const sourceCode = fs.readFileSync(resolvedPath, 'utf8');
if (!sourceCode.trim()) {
throw new Error(`Agent file is empty: ${args.agentFile}`);
}
Expand Down
20 changes: 20 additions & 0 deletions packages/mcp/src/tools/scaffold.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ export async function scaffoldAgent(args: {
args.outputDir ?? path.join(process.cwd(), args.name.toLowerCase().replace(/\s+/g, '-')),
);

// Security: validate outputDir is within cwd to prevent arbitrary file writes
const cwd = process.cwd();
if (!outputDir.startsWith(cwd + path.sep) && outputDir !== cwd) {
throw new Error(
`Security: outputDir must be within the current working directory.\n` +
` cwd: ${cwd}\n` +
` resolved: ${outputDir}`,
);
}

// Create base directory and .claude/ subdirectory
fs.mkdirSync(outputDir, { recursive: true });
fs.mkdirSync(path.join(outputDir, '.claude'), { recursive: true });
Expand Down Expand Up @@ -148,6 +158,16 @@ export async function scaffoldSwarm(args: {
args.outputDir ?? path.join(process.cwd(), args.name.toLowerCase().replace(/\s+/g, '-')),
);

// Security: validate outputDir is within cwd to prevent arbitrary file writes
const cwd = process.cwd();
if (!outputDir.startsWith(cwd + path.sep) && outputDir !== cwd) {
throw new Error(
`Security: outputDir must be within the current working directory.\n` +
` cwd: ${cwd}\n` +
` resolved: ${outputDir}`,
);
}

// Create base directory and .claude/ subdirectory
fs.mkdirSync(outputDir, { recursive: true });
fs.mkdirSync(path.join(outputDir, '.claude'), { recursive: true });
Expand Down
8 changes: 6 additions & 2 deletions packages/sdk/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,13 @@ export class AgentLaunchClient {
const delayMs = 1000 * Math.pow(2, attempt);
attempt++;

// Respect Retry-After header if the server sends one
// Respect Retry-After header if the server sends one (capped at 30s to
// prevent a malicious server from stalling the client indefinitely).
const retryAfter = response.headers.get('Retry-After');
const waitMs = retryAfter ? parseInt(retryAfter, 10) * 1000 : delayMs;
const MAX_RETRY_WAIT_MS = 30_000;
const waitMs = retryAfter
? Math.min(parseInt(retryAfter, 10) * 1000, MAX_RETRY_WAIT_MS)
: delayMs;
Comment on lines +161 to +163
Copy link

@sentry sentry bot Mar 3, 2026

Choose a reason for hiding this comment

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

Bug: parseInt on a non-numeric Retry-After header returns NaN, causing sleep(NaN) which results in an immediate retry instead of waiting.
Severity: MEDIUM

Suggested Fix

Validate the result of parseInt(retryAfter, 10) before using it. If the result is NaN, the logic should fall back to the default exponential backoff delay (delayMs) instead of using the invalid value. A check like !isNaN(parsedRetryAfter) should be added.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/sdk/src/client.ts#L161-L163

Potential issue: The code calculates `waitMs` using `parseInt(retryAfter, 10)`. If the
`Retry-After` header contains a non-numeric value, such as an HTTP-date string,
`parseInt` returns `NaN`. This `NaN` value propagates through the `Math.min`
calculation, resulting in a call to `sleep(NaN)`. The underlying `setTimeout` treats
`NaN` as a 0ms delay, causing the client to bypass the intended rate-limiting and enter
a tight retry loop. This can lead to client-side resource exhaustion and excessive
requests to the server when a malformed header is received.

Did we get this right? 👍 / 👎 to inform future reviews.


await sleep(waitMs);

Expand Down
7 changes: 7 additions & 0 deletions packages/sdk/src/handoff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ export function generateTradeLink(
opts: TradeLinkOptions = {},
baseUrl?: string,
): string {
// Validate Ethereum address format to prevent URL injection
if (!/^0x[a-fA-F0-9]{40}$/.test(address)) {
throw new Error(
`generateTradeLink: address must be a valid 0x-prefixed Ethereum address (40 hex chars), got "${address}"`,
);
}

const base = resolveBaseUrl(baseUrl);
const params = new URLSearchParams();

Expand Down