pass the model#392
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| log.warn(`Could not resolve path for '${commandName}', spawn may fail`); | ||
| } | ||
|
|
||
| // Track the effective model for this spawn. |
Check failure
Code scanning / CodeQL
Insecure randomness High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general terms, the fix is to stop using Math.random() for generating agent names and replace it with a cryptographically secure pseudo-random source. On Node.js, this means using the crypto module (for example, crypto.randomInt, or deriving an index from crypto.randomBytes) instead of Math.random(). We must ensure that the selection of indices remains uniform and within array bounds, and that we keep the public API (generateAgentName, generateUniqueAgentName, isValidAgentName) unchanged.
Concretely, we will modify packages/utils/src/name-generator.ts:
- Add an import of Node’s built-in
cryptomodule at the top of the file. - Replace the use of
Math.floor(Math.random() * ADJECTIVES.length)and the analogous call forNOUNSingenerateAgentNamewithcrypto.randomInt(ADJECTIVES.length)andcrypto.randomInt(NOUNS.length), respectively.crypto.randomInt(max)returns an integer in[0, max), so this is a drop-in replacement for indexing arrays. - Replace the fallback suffix generation in
generateUniqueAgentNamethat currently usesMath.floor(Math.random() * 1000)withcrypto.randomInt(1000).
These changes stay entirely within packages/utils/src/name-generator.ts, do not affect the external interface, and remove all insecure Math.random() usages that CodeQL traces into the spawner. No changes to src/cli/index.ts, packages/config/src/shadow-config.ts, or packages/bridge/src/spawner.ts are required because they simply consume the already-generated names.
| @@ -3,6 +3,8 @@ | ||
| * Inspired by mcp_agent_mail's approach. | ||
| */ | ||
|
|
||
| import crypto from 'node:crypto'; | ||
|
|
||
| const ADJECTIVES = [ | ||
| 'Blue', 'Green', 'Red', 'Purple', 'Golden', 'Silver', 'Crystal', 'Amber', | ||
| 'Coral', 'Jade', 'Ruby', 'Sapphire', 'Emerald', 'Onyx', 'Pearl', 'Copper', | ||
| @@ -29,8 +31,8 @@ | ||
| * Generate a random agent name (AdjectiveNoun format). | ||
| */ | ||
| export function generateAgentName(): string { | ||
| const adjective = ADJECTIVES[Math.floor(Math.random() * ADJECTIVES.length)]; | ||
| const noun = NOUNS[Math.floor(Math.random() * NOUNS.length)]; | ||
| const adjective = ADJECTIVES[crypto.randomInt(ADJECTIVES.length)]; | ||
| const noun = NOUNS[crypto.randomInt(NOUNS.length)]; | ||
| return `${adjective}${noun}`; | ||
| } | ||
|
|
||
| @@ -45,7 +47,7 @@ | ||
| } | ||
| } | ||
| // Fallback: append random suffix | ||
| return `${generateAgentName()}${Math.floor(Math.random() * 1000)}`; | ||
| return `${generateAgentName()}${crypto.randomInt(1000)}`; | ||
| } | ||
|
|
||
| /** |
| log.warn(`Could not resolve path for '${commandName}', spawn may fail`); | ||
| } | ||
|
|
||
| // Track the effective model for this spawn. |
Check failure
Code scanning / CodeQL
Insecure randomness High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
General approach: Replace usage of Math.random() in name-generator.ts with a cryptographically secure random source. In Node.js we can use crypto.randomInt (or crypto.randomBytes), which provides unbiased, cryptographically strong integers. This keeps the API the same (still returns a deterministic string format) while strengthening the randomness. No changes are needed in src/cli/index.ts, packages/config/src/shadow-config.ts, or packages/bridge/src/spawner.ts; they can continue to consume generated names exactly as before.
Best concrete fix:
- In
packages/utils/src/name-generator.ts, importrandomIntfrom Node’scryptomodule. - Replace
Math.floor(Math.random() * ADJECTIVES.length)andMath.floor(Math.random() * NOUNS.length)withrandomInt(ADJECTIVES.length)andrandomInt(NOUNS.length). - Replace the fallback suffix
Math.floor(Math.random() * 1000)withrandomInt(1000).
These are minimal edits that preserve all external behavior (same ranges, same string format) while using a cryptographically secure PRNG.
No changes are needed to spawner.ts, since it simply receives cli as a parameter; strengthening the randomness at source (name generation) handles all the variants CodeQL reported.
| @@ -3,6 +3,8 @@ | ||
| * Inspired by mcp_agent_mail's approach. | ||
| */ | ||
|
|
||
| import { randomInt } from 'node:crypto'; | ||
|
|
||
| const ADJECTIVES = [ | ||
| 'Blue', 'Green', 'Red', 'Purple', 'Golden', 'Silver', 'Crystal', 'Amber', | ||
| 'Coral', 'Jade', 'Ruby', 'Sapphire', 'Emerald', 'Onyx', 'Pearl', 'Copper', | ||
| @@ -29,8 +31,8 @@ | ||
| * Generate a random agent name (AdjectiveNoun format). | ||
| */ | ||
| export function generateAgentName(): string { | ||
| const adjective = ADJECTIVES[Math.floor(Math.random() * ADJECTIVES.length)]; | ||
| const noun = NOUNS[Math.floor(Math.random() * NOUNS.length)]; | ||
| const adjective = ADJECTIVES[randomInt(ADJECTIVES.length)]; | ||
| const noun = NOUNS[randomInt(NOUNS.length)]; | ||
| return `${adjective}${noun}`; | ||
| } | ||
|
|
||
| @@ -45,7 +47,7 @@ | ||
| } | ||
| } | ||
| // Fallback: append random suffix | ||
| return `${generateAgentName()}${Math.floor(Math.random() * 1000)}`; | ||
| return `${generateAgentName()}${randomInt(1000)}`; | ||
| } | ||
|
|
||
| /** |
| log.warn(`Could not resolve path for '${commandName}', spawn may fail`); | ||
| } | ||
|
|
||
| // Track the effective model for this spawn. |
Check failure
Code scanning / CodeQL
Insecure randomness High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
General fix: Replace all uses of Math.random() in packages/utils/src/name-generator.ts with a cryptographically secure random source. On Node, use crypto.randomInt for unbiased integer selection and suffix generation.
Best concrete fix:
- In
packages/utils/src/name-generator.ts, import Node’scryptomodule. - Implement a small helper
getRandomInt(max: number): numberthat callscrypto.randomInt(max). - Use
getRandomInt(ADJECTIVES.length)andgetRandomInt(NOUNS.length)instead ofMath.floor(Math.random() * ...). - For the numeric suffix in
generateUniqueAgentName, usegetRandomInt(1000)instead ofMath.floor(Math.random() * 1000). - Do not change any function signatures or exported names; the only behavioral change is that randomness becomes cryptographically secure.
Only packages/utils/src/name-generator.ts needs code changes; the spawner and CLI code can stay as is.
| @@ -3,6 +3,8 @@ | ||
| * Inspired by mcp_agent_mail's approach. | ||
| */ | ||
|
|
||
| import crypto from 'node:crypto'; | ||
|
|
||
| const ADJECTIVES = [ | ||
| 'Blue', 'Green', 'Red', 'Purple', 'Golden', 'Silver', 'Crystal', 'Amber', | ||
| 'Coral', 'Jade', 'Ruby', 'Sapphire', 'Emerald', 'Onyx', 'Pearl', 'Copper', | ||
| @@ -25,12 +27,16 @@ | ||
| 'Star', 'Moon', 'Sun', 'Comet', 'Cloud', 'Storm', 'Thunder', 'Lightning', | ||
| ]; | ||
|
|
||
| function getRandomInt(max: number): number { | ||
| return crypto.randomInt(max); | ||
| } | ||
|
|
||
| /** | ||
| * Generate a random agent name (AdjectiveNoun format). | ||
| */ | ||
| export function generateAgentName(): string { | ||
| const adjective = ADJECTIVES[Math.floor(Math.random() * ADJECTIVES.length)]; | ||
| const noun = NOUNS[Math.floor(Math.random() * NOUNS.length)]; | ||
| const adjective = ADJECTIVES[getRandomInt(ADJECTIVES.length)]; | ||
| const noun = NOUNS[getRandomInt(NOUNS.length)]; | ||
| return `${adjective}${noun}`; | ||
| } | ||
|
|
||
| @@ -45,7 +47,7 @@ | ||
| } | ||
| } | ||
| // Fallback: append random suffix | ||
| return `${generateAgentName()}${Math.floor(Math.random() * 1000)}`; | ||
| return `${generateAgentName()}${getRandomInt(1000)}`; | ||
| } | ||
|
|
||
| /** |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Addresses PR review feedback — validateModelForCli now checks the normalized model name against validModels rather than the raw input, ensuring aliases like 'claude-opus-4' are correctly validated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
modelpassthrough in spawn() #389