From 1e6799d5e4f8f256c802742d1aca2524444817c7 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Tue, 20 Jan 2026 21:17:11 -0600 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Improve=20preview=20command=20with?= =?UTF-8?q?=20smart=20exclusions=20and=20dry-run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix form-data upload issue by using native FormData/Blob for fetch compatibility - Add smart default exclusions for non-static files (node_modules, tests, configs, etc.) - Add --dry-run flag to show what would be uploaded without uploading - Add --exclude/-x flag to add custom exclusion patterns - Add --include/-i flag to override default exclusions - Show helpful error message when path argument is missing - Skip API token validation for dry-run mode --- src/api/endpoints.js | 11 +- src/cli.js | 26 ++- src/commands/preview.js | 321 +++++++++++++++++++++++++++++---- tests/api/endpoints.test.js | 6 +- tests/commands/preview.test.js | 201 +++++++++++++++++++++ 5 files changed, 520 insertions(+), 45 deletions(-) diff --git a/src/api/endpoints.js b/src/api/endpoints.js index f0e5610..5c1fc1b 100644 --- a/src/api/endpoints.js +++ b/src/api/endpoints.js @@ -333,18 +333,15 @@ export async function finalizeParallelBuild(client, parallelId) { * @returns {Promise} Upload result with preview URL */ export async function uploadPreviewZip(client, buildId, zipBuffer) { - // Create form data with the ZIP file - let FormData = (await import('form-data')).default; + // Use native FormData (Node 18+) with Blob for proper fetch compatibility let formData = new FormData(); - formData.append('file', zipBuffer, { - filename: 'preview.zip', - contentType: 'application/zip', - }); + let blob = new Blob([zipBuffer], { type: 'application/zip' }); + formData.append('file', blob, 'preview.zip'); return client.request(`/api/sdk/builds/${buildId}/preview/upload-zip`, { method: 'POST', body: formData, - headers: formData.getHeaders(), + // Let fetch set the Content-Type with boundary automatically }); } diff --git a/src/cli.js b/src/cli.js index ef108ea..5b3ed50 100644 --- a/src/cli.js +++ b/src/cli.js @@ -572,14 +572,38 @@ program program .command('preview') .description('Upload static files as a preview for a build') - .argument('', 'Path to static files (dist/, build/, out/)') + .argument('[path]', 'Path to static files (dist/, build/, out/)') .option('-b, --build ', 'Build ID to attach preview to') .option('-p, --parallel-id ', 'Look up build by parallel ID') .option('--base ', 'Override auto-detected base path') .option('--open', 'Open preview URL in browser after upload') + .option('--dry-run', 'Show what would be uploaded without uploading') + .option( + '-x, --exclude ', + 'Exclude files/dirs (repeatable, e.g. -x "*.log" -x "temp/")', + (val, prev) => (prev ? [...prev, val] : [val]) + ) + .option( + '-i, --include ', + 'Override default exclusions (repeatable, e.g. -i package.json -i tests/)', + (val, prev) => (prev ? [...prev, val] : [val]) + ) .action(async (path, options) => { const globalOptions = program.opts(); + // Show helpful error if path is missing + if (!path) { + output.error('Path to static files is required'); + output.blank(); + output.print(' Upload your build output directory:'); + output.blank(); + output.print(' vizzly preview ./dist'); + output.print(' vizzly preview ./build'); + output.print(' vizzly preview ./out'); + output.blank(); + process.exit(1); + } + // Validate options const validationErrors = validatePreviewOptions(path, options); if (validationErrors.length > 0) { diff --git a/src/commands/preview.js b/src/commands/preview.js index b946a08..ee97ccc 100644 --- a/src/commands/preview.js +++ b/src/commands/preview.js @@ -27,6 +27,9 @@ import { let execAsync = promisify(exec); +// Maximum files to show in dry-run output (use --verbose for all) +let DRY_RUN_FILE_LIMIT = 50; + /** * Validate path for shell safety - prevents command injection * @param {string} path - Path to validate @@ -71,14 +74,89 @@ function getZipCommand() { return { command: null, available: false }; } +// Default directories to exclude from preview uploads +let DEFAULT_EXCLUDED_DIRS = [ + 'node_modules', + '__pycache__', + '.git', + '.svn', + '.hg', + '.vizzly', + 'coverage', + '.nyc_output', + '.cache', + '.turbo', + '.next/cache', + '.nuxt', + '.output', + '.vercel', + '.netlify', + 'tests', + 'test', + '__tests__', + 'spec', + '__mocks__', + 'playwright-report', + 'cypress', + '.playwright', +]; + +// Default file patterns to exclude from preview uploads +let DEFAULT_EXCLUDED_FILES = [ + 'package.json', + 'package-lock.json', + 'yarn.lock', + 'pnpm-lock.yaml', + 'bun.lockb', + '*.config.js', + '*.config.ts', + '*.config.mjs', + '*.config.cjs', + 'tsconfig.json', + 'jsconfig.json', + '.eslintrc*', + '.prettierrc*', + 'Makefile', + 'Dockerfile', + 'docker-compose*.yml', + '*.md', + '*.log', + '*.map', +]; + +/** + * Check if a filename matches any of the exclusion patterns + * @param {string} filename - Filename to check + * @param {string[]} patterns - Patterns to match against + * @returns {boolean} + */ +function matchesPattern(filename, patterns) { + for (let pattern of patterns) { + if (pattern.includes('*')) { + // Simple glob matching - convert to regex + let regex = new RegExp( + `^${pattern.replace(/\./g, '\\.').replace(/\*/g, '.*')}$` + ); + if (regex.test(filename)) return true; + } else { + if (filename === pattern) return true; + } + } + return false; +} + /** * Create a ZIP file from a directory using system commands * @param {string} sourceDir - Directory to zip * @param {string} outputPath - Path for output ZIP file + * @param {Object} exclusions - Exclusion patterns + * @param {string[]} exclusions.dirs - Directory names to exclude + * @param {string[]} exclusions.files - File patterns to exclude * @returns {Promise} */ -async function createZipWithSystem(sourceDir, outputPath) { +async function createZipWithSystem(sourceDir, outputPath, exclusions = {}) { let { command, available } = getZipCommand(); + let { dirs = [], files = [] } = exclusions; if (!available) { throw new Error( @@ -95,23 +173,62 @@ async function createZipWithSystem(sourceDir, outputPath) { ); } + // Validate exclusion patterns to prevent command injection + // Only allow safe characters in patterns: alphanumeric, dots, asterisks, underscores, hyphens, slashes + let safePatternRegex = /^[a-zA-Z0-9.*_\-/]+$/; + for (let pattern of [...dirs, ...files]) { + if (!safePatternRegex.test(pattern)) { + throw new Error( + `Exclusion pattern contains unsafe characters: ${pattern}. Only alphanumeric, ., *, _, -, / are allowed.` + ); + } + } + if (command === 'zip') { // Standard zip command - create ZIP from directory contents // Using cwd option is safe as it's not part of the command string - // -r: recursive, -q: quiet - await execAsync(`zip -r -q "${outputPath}" .`, { + // -r: recursive, -q: quiet, -x: exclude patterns + let excludeArgs = [ + ...dirs.map(dir => `-x "${dir}/*"`), + ...files.map(pattern => `-x "${pattern}"`), + ].join(' '); + await execAsync(`zip -r -q "${outputPath}" . ${excludeArgs}`, { cwd: sourceDir, maxBuffer: 1024 * 1024 * 100, // 100MB buffer }); } else if (command === 'powershell') { - // Windows PowerShell - use -LiteralPath for safer path handling - // Escape single quotes in paths by doubling them + // Windows PowerShell - Compress-Archive doesn't support exclusions, + // so we create a temp directory with only the files we want let safeSrcDir = sourceDir.replace(/'/g, "''"); let safeOutPath = outputPath.replace(/'/g, "''"); - await execAsync( - `powershell -Command "Compress-Archive -LiteralPath '${safeSrcDir}\\*' -DestinationPath '${safeOutPath}' -Force"`, - { maxBuffer: 1024 * 1024 * 100 } - ); + + // Build exclusion filter for PowerShell + // We use Get-ChildItem with -Exclude and pipe to Compress-Archive + let excludePatterns = [...dirs, ...files].map(p => `'${p}'`).join(','); + + if (excludePatterns) { + // Use robocopy to copy files excluding patterns, then zip + // This is more reliable than PowerShell's native filtering + await execAsync( + `powershell -Command "` + + `$src = '${safeSrcDir}'; ` + + `$dst = '${safeOutPath}'; ` + + `$exclude = @(${excludePatterns}); ` + + `$items = Get-ChildItem -Path $src -Recurse -File | Where-Object { ` + + `$rel = $_.FullName.Substring($src.Length + 1); ` + + `$dominated = $false; ` + + `foreach ($ex in $exclude) { if ($rel -like $ex -or $rel -like \\"$ex/*\\" -or $_.Name -like $ex) { $dominated = $true; break } }; ` + + `-not $dominated ` + + `}; ` + + `if ($items) { $items | Compress-Archive -DestinationPath $dst -Force }"`, + { maxBuffer: 1024 * 1024 * 100 } + ); + } else { + await execAsync( + `powershell -Command "Compress-Archive -LiteralPath '${safeSrcDir}\\*' -DestinationPath '${safeOutPath}' -Force"`, + { maxBuffer: 1024 * 1024 * 100 } + ); + } } } @@ -119,12 +236,23 @@ async function createZipWithSystem(sourceDir, outputPath) { * Count files in a directory recursively * Skips symlinks to prevent path traversal attacks * @param {string} dir - Directory path - * @returns {Promise<{ count: number, totalSize: number }>} + * @param {Object} options - Options + * @param {boolean} options.collectPaths - Whether to collect file paths + * @param {string[]} options.excludedDirs - Directory names to exclude + * @param {string[]} options.excludedFiles - File patterns to exclude + * @returns {Promise<{ count: number, totalSize: number, files?: Array<{path: string, size: number}> }>} */ -async function countFiles(dir) { +async function countFiles(dir, options = {}) { let { readdir } = await import('node:fs/promises'); + let { + collectPaths = false, + excludedDirs = DEFAULT_EXCLUDED_DIRS, + excludedFiles = DEFAULT_EXCLUDED_FILES, + } = options; + let count = 0; let totalSize = 0; + let files = collectPaths ? [] : null; // Resolve the base directory to an absolute path for traversal checks let baseDir = await realpath(resolve(dir)); @@ -141,12 +269,8 @@ async function countFiles(dir) { } if (entry.isDirectory()) { - // Skip hidden directories and common non-content directories - if ( - entry.name.startsWith('.') || - entry.name === 'node_modules' || - entry.name === '__pycache__' - ) { + // Skip hidden directories and excluded directories + if (entry.name.startsWith('.') || excludedDirs.includes(entry.name)) { continue; } @@ -162,15 +286,27 @@ async function countFiles(dir) { if (entry.name.startsWith('.')) { continue; } + + // Skip excluded file patterns + if (matchesPattern(entry.name, excludedFiles)) { + continue; + } + count++; let fileStat = await stat(fullPath); totalSize += fileStat.size; + + if (files) { + // Store relative path from base directory + let relativePath = fullPath.slice(baseDir.length + 1); + files.push({ path: relativePath, size: fileStat.size }); + } } } } await walk(baseDir); - return { count, totalSize }; + return { count, totalSize, files }; } /** @@ -220,8 +356,8 @@ export async function previewCommand( let allOptions = { ...globalOptions, ...options }; let config = await loadConfig(globalOptions.config, allOptions); - // Validate API token - if (!config.apiKey) { + // Validate API token (skip for dry-run) + if (!options.dryRun && !config.apiKey) { output.error( 'API token required. Use --token or set VIZZLY_TOKEN environment variable' ); @@ -300,19 +436,65 @@ export async function previewCommand( return { success: false, reason: 'no-build' }; } - // Check for zip command availability - let zipInfo = getZipCommand(); - if (!zipInfo.available) { - output.error( - 'No zip command found. Please install zip (macOS/Linux) or ensure PowerShell is available (Windows).' - ); - exit(1); - return { success: false, reason: 'no-zip-command' }; + // Check for zip command availability (skip for dry-run) + if (!options.dryRun) { + let zipInfo = getZipCommand(); + if (!zipInfo.available) { + output.error( + 'No zip command found. Please install zip (macOS/Linux) or ensure PowerShell is available (Windows).' + ); + exit(1); + return { success: false, reason: 'no-zip-command' }; + } + } + + // Build exclusion lists from defaults and user options + let excludedDirs = [...DEFAULT_EXCLUDED_DIRS]; + let excludedFiles = [...DEFAULT_EXCLUDED_FILES]; + + // Add user-specified exclusions + if (options.exclude) { + let userExcludes = Array.isArray(options.exclude) + ? options.exclude + : [options.exclude]; + for (let pattern of userExcludes) { + // If pattern ends with /, treat as directory + if (pattern.endsWith('/')) { + excludedDirs.push(pattern.slice(0, -1)); + } else { + excludedFiles.push(pattern); + } + } } + // Remove patterns that user explicitly wants to include + if (options.include) { + let userIncludes = Array.isArray(options.include) + ? options.include + : [options.include]; + for (let pattern of userIncludes) { + if (pattern.endsWith('/')) { + let dirName = pattern.slice(0, -1); + excludedDirs = excludedDirs.filter(d => d !== dirName); + } else { + excludedFiles = excludedFiles.filter(f => f !== pattern); + } + } + } + + let exclusions = { dirs: excludedDirs, files: excludedFiles }; + // Count files and calculate size output.startSpinner('Scanning files...'); - let { count: fileCount, totalSize } = await countFiles(path); + let { + count: fileCount, + totalSize, + files, + } = await countFiles(path, { + collectPaths: options.dryRun, + excludedDirs, + excludedFiles, + }); if (fileCount === 0) { output.stopSpinner(); @@ -321,9 +503,82 @@ export async function previewCommand( return { success: false, reason: 'no-files' }; } - output.updateSpinner( - `Found ${fileCount} files (${formatBytes(totalSize)})` - ); + output.stopSpinner(); + + // Dry run - show what would be uploaded and exit + if (options.dryRun) { + let colors = output.getColors(); + + if (globalOptions.json) { + output.data({ + dryRun: true, + path: resolve(path), + fileCount, + totalSize, + excludedDirs, + excludedFiles, + files: files.map(f => ({ path: f.path, size: f.size })), + }); + } else { + output.info( + `Dry run - would upload ${fileCount} files (${formatBytes(totalSize)})` + ); + output.blank(); + output.print( + ` ${colors.brand.textTertiary('Source')} ${resolve(path)}` + ); + output.print( + ` ${colors.brand.textTertiary('Files')} ${fileCount}` + ); + output.print( + ` ${colors.brand.textTertiary('Total size')} ${formatBytes(totalSize)}` + ); + output.blank(); + + // Show exclusions in verbose mode + if (globalOptions.verbose) { + output.print( + ` ${colors.brand.textTertiary('Excluded directories:')}` + ); + for (let dir of excludedDirs) { + output.print(` ${colors.dim(dir)}`); + } + output.blank(); + + output.print( + ` ${colors.brand.textTertiary('Excluded file patterns:')}` + ); + for (let pattern of excludedFiles) { + output.print(` ${colors.dim(pattern)}`); + } + output.blank(); + } + + // Show files (limit in non-verbose mode) + let displayFiles = globalOptions.verbose + ? files + : files.slice(0, DRY_RUN_FILE_LIMIT); + let hasMore = files.length > displayFiles.length; + + output.print(` ${colors.brand.textTertiary('Files to upload:')}`); + for (let file of displayFiles) { + output.print( + ` ${file.path} ${colors.dim(`(${formatBytes(file.size)})`)}` + ); + } + + if (hasMore) { + output.print( + ` ${colors.dim(`... and ${files.length - DRY_RUN_FILE_LIMIT} more (use --verbose to see all)`)}` + ); + } + } + + output.cleanup(); + return { success: true, dryRun: true, fileCount, totalSize, files }; + } + + output.startSpinner(`Found ${fileCount} files (${formatBytes(totalSize)})`); // Create ZIP using system command output.updateSpinner('Compressing files...'); @@ -336,7 +591,7 @@ export async function previewCommand( let zipBuffer; try { - await createZipWithSystem(path, zipPath); + await createZipWithSystem(path, zipPath, exclusions); zipBuffer = await readFile(zipPath); } catch (zipError) { output.stopSpinner(); diff --git a/tests/api/endpoints.test.js b/tests/api/endpoints.test.js index be67904..540aedd 100644 --- a/tests/api/endpoints.test.js +++ b/tests/api/endpoints.test.js @@ -525,10 +525,8 @@ describe('api/endpoints', () => { await uploadPreviewZip(client, 'build-123', zipBuffer); let call = client.getLastCall(); - // Should have form-data headers - assert.ok( - call.options.headers['content-type']?.includes('multipart/form-data') - ); + // Should send body as native FormData (fetch sets Content-Type automatically) + assert.ok(call.options.body instanceof FormData); }); }); diff --git a/tests/commands/preview.test.js b/tests/commands/preview.test.js index 823347d..c5e346e 100644 --- a/tests/commands/preview.test.js +++ b/tests/commands/preview.test.js @@ -361,4 +361,205 @@ describe('previewCommand', () => { assert.strictEqual(openedUrl, 'https://preview.test'); }); + + it('dry-run shows files without uploading', async () => { + let output = createMockOutput(); + let uploadCalled = false; + + let result = await previewCommand( + distDir, + { dryRun: true }, + {}, + { + loadConfig: async () => ({ + apiKey: null, // No API key needed for dry-run + apiUrl: 'https://api.test', + }), + uploadPreviewZip: async () => { + uploadCalled = true; + return {}; + }, + output, + exit: () => {}, + } + ); + + assert.strictEqual(uploadCalled, false, 'Should not upload in dry-run'); + assert.strictEqual(result.dryRun, true); + assert.strictEqual(result.fileCount, 3); // index.html, app.js, assets/style.css + assert.ok(result.files.length > 0, 'Should return file list'); + }); + + it('dry-run outputs JSON when --json flag is set', async () => { + let output = createMockOutput(); + + await previewCommand( + distDir, + { dryRun: true }, + { json: true }, + { + loadConfig: async () => ({ + apiKey: null, + apiUrl: 'https://api.test', + }), + output, + exit: () => {}, + } + ); + + let dataCall = output.calls.find(c => c.method === 'data'); + assert.ok(dataCall, 'Should output JSON data'); + assert.strictEqual(dataCall.args[0].dryRun, true); + assert.ok(dataCall.args[0].files.length > 0); + assert.ok(Array.isArray(dataCall.args[0].excludedDirs)); + assert.ok(Array.isArray(dataCall.args[0].excludedFiles)); + }); + + it('excludes files matching --exclude patterns', async () => { + let output = createMockOutput(); + + let result = await previewCommand( + distDir, + { dryRun: true, exclude: ['*.js'] }, + {}, + { + loadConfig: async () => ({ + apiKey: null, + apiUrl: 'https://api.test', + }), + output, + exit: () => {}, + } + ); + + let filePaths = result.files.map(f => f.path); + assert.ok( + !filePaths.some(p => p.endsWith('.js')), + 'Should exclude .js files' + ); + assert.ok( + filePaths.some(p => p.endsWith('.html')), + 'Should include .html files' + ); + }); + + it('excludes directories matching --exclude patterns with trailing slash', async () => { + let output = createMockOutput(); + + let result = await previewCommand( + distDir, + { dryRun: true, exclude: ['assets/'] }, + {}, + { + loadConfig: async () => ({ + apiKey: null, + apiUrl: 'https://api.test', + }), + output, + exit: () => {}, + } + ); + + let filePaths = result.files.map(f => f.path); + assert.ok( + !filePaths.some(p => p.startsWith('assets/')), + 'Should exclude assets directory' + ); + assert.ok( + filePaths.some(p => p === 'index.html'), + 'Should include index.html' + ); + }); + + it('includes files matching --include patterns that override defaults', async () => { + // Create a package.json file (normally excluded by default) + writeFileSync(join(distDir, 'package.json'), '{}'); + + let output = createMockOutput(); + + let result = await previewCommand( + distDir, + { dryRun: true, include: ['package.json'] }, + {}, + { + loadConfig: async () => ({ + apiKey: null, + apiUrl: 'https://api.test', + }), + output, + exit: () => {}, + } + ); + + let filePaths = result.files.map(f => f.path); + assert.ok( + filePaths.includes('package.json'), + 'Should include package.json when --include is used' + ); + }); + + it('excludes node_modules by default', async () => { + // Create a node_modules directory with a file + mkdirSync(join(distDir, 'node_modules', 'some-package'), { + recursive: true, + }); + writeFileSync( + join(distDir, 'node_modules', 'some-package', 'index.js'), + 'module.exports = {}' + ); + + let output = createMockOutput(); + + let result = await previewCommand( + distDir, + { dryRun: true }, + {}, + { + loadConfig: async () => ({ + apiKey: null, + apiUrl: 'https://api.test', + }), + output, + exit: () => {}, + } + ); + + let filePaths = result.files.map(f => f.path); + assert.ok( + !filePaths.some(p => p.includes('node_modules')), + 'Should exclude node_modules by default' + ); + }); + + it('excludes config files by default', async () => { + // Create config files + writeFileSync(join(distDir, 'vite.config.js'), 'export default {}'); + writeFileSync(join(distDir, 'tsconfig.json'), '{}'); + + let output = createMockOutput(); + + let result = await previewCommand( + distDir, + { dryRun: true }, + {}, + { + loadConfig: async () => ({ + apiKey: null, + apiUrl: 'https://api.test', + }), + output, + exit: () => {}, + } + ); + + let filePaths = result.files.map(f => f.path); + assert.ok( + !filePaths.includes('vite.config.js'), + 'Should exclude *.config.js' + ); + assert.ok( + !filePaths.includes('tsconfig.json'), + 'Should exclude tsconfig.json' + ); + }); });