Conversation
…s, and UI wizard Add 12 stages of functionality including: - Stage 9/17: Enhanced 8-step wizard UI with SeverityBadge, DiffViewer, TreePreview, FindingsFilter components and deeper page integration - Stage 10: History preservation prerequisites and dry-run reporting - Stage 11: Full lifecycle CLI commands (add, archive, migrate-branch) with plan types - Stage 12: Extended analysis (environment, tooling, CI, publishing, repo risks) with risk classification - Stage 13: Path-filtered GitHub Actions workflow generation - Stage 14: Configure engine for Prettier/ESLint/TypeScript scaffolding - Stage 15: Dependency enforcement via overrides/resolutions - Stage 16: Cross-platform path normalization - Stage 18: Smart defaults with evidence-based suggestions and error shaping - Stage 19: Multi-language detection and scaffolding (Go, Rust, Python) - Stage 20: Performance utilities (pMap concurrency, disk space check, progress emitter) 674 unit tests passing across 49 test files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ckers SEC-01: Sanitize Python injection in history-preserve commitPrefix SEC-02: Add path traversal prevention in apply command SEC-03: Add Bearer token auth to server API and WebSocket SEC-04: Allowlist install command executables in apply SEC-05: Replace shell exec() with execFile() in ui command SEC-06: Cap concurrent operations and event buffer in WsHub Publishing: rename to monotize, add LICENSE/SECURITY.md/CHANGELOG.md, add files field, author, repository metadata, semver and js-yaml deps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // Fetch | ||
| logger.info(`Fetching ${branch}...`); | ||
| await safeExecFile('git', ['fetch', remoteName, branch], { |
Check failure
Code scanning / CodeQL
Second order command injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 25 days ago
In general, to fix this issue we need to ensure that user-controlled values passed as arguments to git cannot be interpreted as options or otherwise malicious inputs. The standard approaches are: (1) validate branch names against a strict whitelist of allowed characters and forms (e.g., typical Git refname constraints), and reject anything that looks like an option (starts with -) or contains suspicious characters; and (2) avoid using tainted data in positions where git expects options. Validation at the HTTP boundary is ideal, but per the constraints we will apply it where the branch is consumed in src/strategies/migrate-branch.ts.
The single best fix here, without changing existing high-level functionality, is to introduce a small validateBranchName helper in src/strategies/migrate-branch.ts and call it in generateBranchPlan before the branch is ever used to build the BranchPlan. This ensures that any subsequent use of plan.branch in applySubtreeImport and applyPatchReplay is safe. The validator should: (a) reject empty strings, (b) reject names starting with - (to prevent option-like values), and (c) enforce a reasonable character set for branch names (e.g. letters, digits, /, _, -, ., and +) and a maximum length. If the name is invalid, we throw an error; the HTTP route already catches errors and reports them to the client. We don’t need external dependencies; a small inline function and a single call to it are sufficient.
Concretely:
- In
src/strategies/migrate-branch.ts, add avalidateBranchName(branch: string): voidfunction near the top of the file. - In
generateBranchPlan, before usingbranch, callvalidateBranchName(branch);. - This introduces no new imports (only uses built-in regex support) and doesn’t alter any other behavior besides rejecting invalid branch names early.
| @@ -4,6 +4,36 @@ | ||
| import { pathExists } from '../utils/fs.js'; | ||
|
|
||
| /** | ||
| * Validate a Git branch name to avoid passing unsafe values to git commands. | ||
| * This is intentionally stricter than Git's own refname rules: we reject names | ||
| * that start with '-' (which could be interpreted as options) and only allow | ||
| * a conservative set of characters. | ||
| */ | ||
| function validateBranchName(branch: string): void { | ||
| if (!branch || typeof branch !== 'string') { | ||
| throw new Error('Invalid branch name: must be a non-empty string'); | ||
| } | ||
|
|
||
| // Prevent option-like arguments such as "--upload-pack=..." or "-c ..." | ||
| if (branch.startsWith('-')) { | ||
| throw new Error(`Invalid branch name "${branch}": must not start with '-'`); | ||
| } | ||
|
|
||
| // Allow common characters in branch names: letters, digits, ".", "_", "-", "/", and "+" | ||
| // and keep the name at a reasonable length. | ||
| if (branch.length > 255) { | ||
| throw new Error(`Invalid branch name "${branch}": too long`); | ||
| } | ||
|
|
||
| const branchPattern = /^[A-Za-z0-9._\/+-]+$/; | ||
| if (!branchPattern.test(branch)) { | ||
| throw new Error( | ||
| `Invalid branch name "${branch}": contains unsupported characters`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Check prerequisites for branch migration | ||
| */ | ||
| export async function checkBranchMigratePrerequisites( | ||
| @@ -102,6 +132,9 @@ | ||
| strategy: BranchMigrateStrategy, | ||
| logger: Logger, | ||
| ): Promise<BranchPlan> { | ||
| // Validate the branch name early to avoid passing unsafe values to git. | ||
| validateBranchName(branch); | ||
|
|
||
| const srcPath = path.resolve(sourceRepo); | ||
| const targetPath = path.resolve(targetMonorepo); | ||
|
|
| // Use wmic on Windows | ||
| const drive = path.parse(path.resolve(dirPath)).root; | ||
| const { stdout } = await execFileAsync('wmic', [ | ||
| 'logicaldisk', 'where', `DeviceID='${drive.replace('\\', '')}'`, |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 25 days ago
In general, when removing or escaping a character from a string, use a global regular expression or a well-tested utility rather than string.replace with a plain string, which affects only the first occurrence. Here, we should replace all backslashes in drive before interpolating it into the wmic DeviceID filter.
The best targeted fix is to change drive.replace('\\', '') to use a global regex: drive.replace(/\\/g, ''). This preserves existing behavior for typical values like 'C:\\' (which still becomes 'C:'), while correctly handling any unexpected additional backslashes. No new imports are needed, and no other logic in checkDiskSpace needs to change. The edit is confined to line 18 in src/utils/disk.ts.
| @@ -15,7 +15,7 @@ | ||
| // Use wmic on Windows | ||
| const drive = path.parse(path.resolve(dirPath)).root; | ||
| const { stdout } = await execFileAsync('wmic', [ | ||
| 'logicaldisk', 'where', `DeviceID='${drive.replace('\\', '')}'`, | ||
| 'logicaldisk', 'where', `DeviceID='${drive.replace(/\\/g, '')}'`, | ||
| 'get', 'FreeSpace', '/format:value', | ||
| ]); | ||
| const match = stdout.match(/FreeSpace=(\d+)/); |
1 participant
Summary
Validation