feat: configurable workspaces #172
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| private sessions = new Map<string, ShellSession>(); | ||
|
|
||
| createSession(options: CreateSessionOptions): ShellSession { | ||
| const { sessionId, webContents, cwd, initialCommand } = options; |
Check failure
Code scanning / CodeQL
Insecure randomness High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix this issue, replace all uses of Math.random() for selecting random elements in the worktree name generation logic (WorktreeManager.randomElement) with a cryptographically secure alternative. In Node.js, this means using crypto.randomInt or deriving randomness from crypto.randomBytes.
Specifically, in packages/agent/src/worktree-manager.ts, update the randomElement selection function to utilize crypto.randomInt to generate a random index for the array, rather than using a value derived from Math.random(). This update applies wherever random unique names are generated for worktrees.
- Import the Node.js
cryptomodule. - Modify the
randomElementmethod (not fully shown above, but implied from usage) to usecrypto.randomIntto select the random array index. - Ensure this applies to all places where pseudo-random selection of elements for worktree names is performed.
Only the file packages/agent/src/worktree-manager.ts requires modification, as the taint originates here.
| @@ -2,6 +2,7 @@ | ||
| import * as fs from "node:fs/promises"; | ||
| import * as path from "node:path"; | ||
| import { promisify } from "node:util"; | ||
| import * as crypto from "node:crypto"; | ||
| import type { WorktreeInfo } from "./types.js"; | ||
| import { Logger } from "./utils/logger.js"; | ||
|
|
| } | ||
|
|
||
| for (const command of commands) { | ||
| const sessionId = generateSessionId(taskId, scriptType); |
Check failure
Code scanning / CodeQL
Insecure randomness High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the problem, we need to replace the use of Math.random() in randomSuffix (in apps/array/src/shared/utils/id.ts) with a cryptographically secure PRNG. For Node.js and Electron (backed by Node.js), we can use require("crypto").randomBytes to generate random values securely. We'll replace the body of randomSuffix to use crypto.randomBytes to generate random bytes, then encode them in base36 and return the correct substring. This change will cascade securely across usages in generateSessionId and anywhere randomSuffix is used. Since randomSuffix exists in a shared utility file, only it needs changing. An import of crypto must be added to apps/array/src/shared/utils/id.ts, but no change is needed elsewhere.
| @@ -1,7 +1,14 @@ | ||
| import { randomBytes } from "crypto"; | ||
|
|
||
| export function randomSuffix(length = 8): string { | ||
| return Math.random() | ||
| .toString(36) | ||
| .substring(2, 2 + length); | ||
| // Generate enough random bytes to have at least the number of base36 characters we need | ||
| // Each byte gives up to two base36 chars; round up. | ||
| const bytesNeeded = Math.ceil(length * 0.625); // base36 chars ~ log2(36) = ~5.17 bits per char | ||
| const randomBuf = randomBytes(bytesNeeded); | ||
| // Convert buffer to base36 string and get the required length | ||
| return randomBuf.toString("base64") // base36 not directly supported for Buffer, so use base64 for compactness | ||
| .replace(/[^a-z0-9]/gi, "") // strip out non-alphanumeric to match base36 style | ||
| .substring(0, length); | ||
| } | ||
|
|
||
| export function generateId(prefix: string, length = 8): string { |
No description provided.