From 4c04b111080cb777684c1b1ee46c898c9c2bd023 Mon Sep 17 00:00:00 2001 From: Sarah Schneider Date: Thu, 8 Jan 2026 14:43:05 -0500 Subject: [PATCH] Rename liquid-tags script (#59068) --- package.json | 2 +- src/ai-tools/scripts/ai-tools.ts | 31 ++-- .../{resolve-liquid.ts => liquid-tags.ts} | 174 ++++++++++++------ .../{resolve-liquid.ts => liquid-tags.ts} | 24 +-- 4 files changed, 142 insertions(+), 89 deletions(-) rename src/content-render/scripts/{resolve-liquid.ts => liquid-tags.ts} (78%) rename src/content-render/tests/{resolve-liquid.ts => liquid-tags.ts} (72%) diff --git a/package.json b/package.json index 07747e17f3f6..2d2a39b21897 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,7 @@ "release-banner": "tsx src/ghes-releases/scripts/release-banner.ts", "repo-sync": "./src/workflows/local-repo-sync.sh", "reusables": "tsx src/content-render/scripts/reusables-cli.ts", - "resolve-liquid": "tsx src/content-render/scripts/resolve-liquid.ts", + "liquid-tags": "tsx src/content-render/scripts/liquid-tags.ts", "rendered-content-link-checker": "tsx src/links/scripts/rendered-content-link-checker.ts", "rendered-content-link-checker-cli": "tsx src/links/scripts/rendered-content-link-checker-cli.ts", "rest-dev": "tsx src/rest/scripts/update-files.ts", diff --git a/src/ai-tools/scripts/ai-tools.ts b/src/ai-tools/scripts/ai-tools.ts index 8ac78ed62c33..39f150292b4e 100644 --- a/src/ai-tools/scripts/ai-tools.ts +++ b/src/ai-tools/scripts/ai-tools.ts @@ -195,11 +195,11 @@ program const relativePath = path.relative(process.cwd(), fileToProcess) spinner.text = `Processing: ${relativePath}` try { - // Resolve Liquid references before processing + // Expand Liquid references before processing if (options.verbose) { - console.log(`Resolving Liquid references in: ${relativePath}`) + console.log(`Expanding Liquid references in: ${relativePath}`) } - runResolveLiquid('resolve', [fileToProcess], options.verbose || false) + runLiquidTagsScript('expand', [fileToProcess], options.verbose || false) const content = fs.readFileSync(fileToProcess, 'utf8') const answer = await callEditor( @@ -235,7 +235,7 @@ program if (options.verbose) { console.log(`Restoring Liquid references in: ${relativePath}`) } - runResolveLiquid('restore', [fileToProcess], options.verbose || false) + runLiquidTagsScript('restore', [fileToProcess], options.verbose || false) } catch (err) { const error = err as Error spinner.fail(`Error processing ${relativePath}: ${error.message}`) @@ -243,7 +243,7 @@ program // Still try to restore Liquid references on error try { - runResolveLiquid('restore', [fileToProcess], false) + runLiquidTagsScript('restore', [fileToProcess], false) } catch (restoreError) { // Log restore failures in verbose mode for debugging if (options.verbose) { @@ -275,35 +275,32 @@ program program.parse(process.argv) /** - * Run resolve-liquid command on specified file paths + * Run liquid-tags command on specified file paths */ -function runResolveLiquid( - command: 'resolve' | 'restore', +function runLiquidTagsScript( + command: 'expand' | 'restore', filePaths: string[], verbose: boolean = false, ): void { const args = [command, '--paths', ...filePaths] - if (command === 'resolve') { - args.push('--recursive') - } if (verbose) { args.push('--verbose') } try { - // Run resolve-liquid via tsx - const resolveLiquidPath = path.join( + // Run liquid-tags script via tsx + const liquidTagsScriptPath = path.join( process.cwd(), - 'src/content-render/scripts/resolve-liquid.ts', + 'src/content-render/scripts/liquid-tags.ts', ) - execFileSync('npx', ['tsx', resolveLiquidPath, ...args], { + execFileSync('npx', ['tsx', liquidTagsScriptPath, ...args], { stdio: verbose ? 'inherit' : 'pipe', }) } catch (error) { if (verbose) { - console.error(`Error running resolve-liquid ${command}:`, error) + console.error(`Error running liquid-tags ${command}:`, error) } - // Don't fail the entire process if resolve-liquid fails + // Don't fail the entire process if liquid-tags fails } } diff --git a/src/content-render/scripts/resolve-liquid.ts b/src/content-render/scripts/liquid-tags.ts similarity index 78% rename from src/content-render/scripts/resolve-liquid.ts rename to src/content-render/scripts/liquid-tags.ts index b76905842b4f..7854b05b7d68 100644 --- a/src/content-render/scripts/resolve-liquid.ts +++ b/src/content-render/scripts/liquid-tags.ts @@ -1,9 +1,9 @@ /* * @purpose Writer tool - * @description Resolve and unresolve Liquid data references in content files + * @description Expand and restore Liquid data references in content files */ -// Usage: npm run resolve-liquid -- resolve --paths content/pull-requests/about.md -// Usage: npm run resolve-liquid -- restore --paths content/pull-requests/about.md +// Usage: npm run liquid-tags -- expand --paths content/pull-requests/about.md +// Usage: npm run liquid-tags -- restore --paths content/pull-requests/about.md import { Command } from 'commander' import fs from 'fs' @@ -12,14 +12,14 @@ import yaml from 'js-yaml' import chalk from 'chalk' // Type definitions -interface ResolveOptions { +interface ExpandOptions { paths: string[] verbose?: boolean markers?: boolean dryRun?: boolean reusablesOnly?: boolean variablesOnly?: boolean - recursive?: boolean + shallow?: boolean } interface LiquidReference { @@ -36,38 +36,88 @@ const DATA_ROOT = path.resolve(path.join(ROOT, 'data')) const REUSABLES_ROOT = path.join(DATA_ROOT, 'reusables') const VARIABLES_ROOT = path.join(DATA_ROOT, 'variables') -// Regex pattern to match resolved content blocks -const RESOLVED_PATTERN = - /(.+?)/gs +// Regex pattern to match expanded content blocks +const EXPANDED_PATTERN = /(.+?)/gs /** * Get the file path for a data reference + * + * Validates and normalizes the incoming dataPath to prevent path traversal + * and ensure the final resolved path remains within the expected root. */ function getDataFilePath(type: 'reusable' | 'variable', dataPath: string): string { + // Basic validation of the raw dataPath + if (path.isAbsolute(dataPath)) { + throw new Error(`Invalid ${type} data path: absolute paths are not allowed: ${dataPath}`) + } + + // Disallow path traversal and empty segments + const segments = dataPath.split(/[\\/]/) + if (segments.some((segment) => segment === '..' || segment === '')) { + throw new Error(`Invalid ${type} data path: contains disallowed segments: ${dataPath}`) + } + + // Restrict allowed characters to a conservative safe set + if (!/^[A-Za-z0-9_.\-/]+$/.test(dataPath)) { + throw new Error(`Invalid ${type} data path: contains disallowed characters: ${dataPath}`) + } + if (type === 'reusable') { - return path.join(REUSABLES_ROOT, `${dataPath.replace(/\./g, '/')}.md`) + const baseRoot = path.resolve(REUSABLES_ROOT) + const relativePath = dataPath.replace(/\./g, '/') + const candidatePath = path.resolve(baseRoot, `${relativePath}.md`) + + if (!candidatePath.startsWith(baseRoot + path.sep)) { + throw new Error(`Invalid reusable data path: escapes reusables root: ${dataPath}`) + } + + return candidatePath } else { + const baseRoot = path.resolve(VARIABLES_ROOT) const fileName = dataPath.split('.')[0] - return path.join(VARIABLES_ROOT, `${fileName}.yml`) + const candidatePath = path.resolve(baseRoot, `${fileName}.yml`) + + if (!candidatePath.startsWith(baseRoot + path.sep)) { + throw new Error(`Invalid variable data path: escapes variables root: ${dataPath}`) + } + + return candidatePath + } +} + +/** + * Convert a file path back to data path format (for consistent verbose output) + */ +function convertFilePathToDataPath(filePath: string): string { + const normalizedPath = path.normalize(filePath) + + if (normalizedPath.includes('reusables')) { + const relativePath = path.relative(REUSABLES_ROOT, normalizedPath) + return `reusables.${relativePath.replace(/\.md$/, '').replace(/[\\/]/g, '.')}` + } else if (normalizedPath.includes('variables')) { + const relativePath = path.relative(VARIABLES_ROOT, normalizedPath) + return `variables.${relativePath.replace(/\.yml$/, '').replace(/[\\/]/g, '.')}` } + + return filePath } const program = new Command() program - .name('resolve-liquid') - .description('Tools to resolve and unresolve Liquid data references in content files') + .name('liquid-tags') + .description('Tools to expand and restore Liquid data references in content files') program - .command('resolve') - .description('Resolve {% data reusables %} and {% data variables %} statements to their content') + .command('expand') + .description('Expand {% data reusables %} and {% data variables %} statements to their content') .option('--paths ', 'Content file paths to process', []) .option('-v, --verbose', 'Verbose output', false) .option('--no-markers', 'Skip HTML comment markers (output cannot be restored to Liquid)', true) .option('--reusables-only', 'Process only reusables (skip variables)', false) .option('--variables-only', 'Process only variables (skip reusables)', false) - .option('-r, --recursive', 'Keep resolving until no references remain (max 10 iterations)', false) - .action((options: ResolveOptions) => resolveReferences(options)) + .option('--shallow', 'Expand only one level (do not expand nested references)', false) + .action((options: ExpandOptions) => expandReferences(options)) program .command('restore') @@ -76,14 +126,14 @@ program .option('-v, --verbose', 'Verbose output', false) .option('--reusables-only', 'Process only reusables (skip variables)', false) .option('--variables-only', 'Process only variables (skip reusables)', false) - .action((options: ResolveOptions) => restoreReferences(options)) + .action((options: ExpandOptions) => restoreReferences(options)) program.parse() /** * Get allowed types based on command options */ -function getAllowedTypes(options: ResolveOptions): Array<'reusable' | 'variable'> { +function getAllowedTypes(options: ExpandOptions): Array<'reusable' | 'variable'> { if (options.reusablesOnly && options.variablesOnly) { console.log( chalk.yellow( @@ -106,14 +156,15 @@ function getAllowedTypes(options: ResolveOptions): Array<'reusable' | 'variable' } /** - * Resolve Liquid data references in content files + * Expand Liquid data references in content files */ -async function resolveReferences(options: ResolveOptions): Promise { - const { paths, verbose, markers, recursive } = options +async function expandReferences(options: ExpandOptions): Promise { + const { paths, verbose, markers, shallow } = options // markers will be true by default, false when --no-markers is used const withMarkers = markers !== false + const recursive = !shallow // Recursive by default unless --shallow is specified const allowedTypes = getAllowedTypes(options) - const maxIterations = 10 // Safety limit for recursive resolution + const maxIterations = 10 // Safety limit for recursive expansion if (paths.length === 0) { console.error(chalk.red('Error: No paths provided. Use --paths option.')) @@ -143,7 +194,7 @@ async function resolveReferences(options: ResolveOptions): Promise { } const content = fs.readFileSync(filePath, 'utf-8') - const resolvedContent = await resolveFileContent( + const expandedContent = await expandFileContent( content, filePath, verbose, @@ -151,10 +202,10 @@ async function resolveReferences(options: ResolveOptions): Promise { allowedTypes, ) - if (resolvedContent !== content) { - fs.writeFileSync(filePath, resolvedContent, 'utf-8') + if (expandedContent !== content) { + fs.writeFileSync(filePath, expandedContent, 'utf-8') if (iteration === 1 || !recursive) { - console.log(chalk.green(`✓ Resolved references in: ${filePath}`)) + console.log(chalk.green(`✓ Expanded references in: ${filePath}`)) } } else { if (verbose && iteration === 1) { @@ -163,11 +214,11 @@ async function resolveReferences(options: ResolveOptions): Promise { } // Check for remaining references - const remainingRefs = findLiquidReferences(resolvedContent, allowedTypes) + const remainingRefs = findLiquidReferences(expandedContent, allowedTypes) hasRemainingRefs = remainingRefs.length > 0 - if (!recursive) { - // Non-recursive mode: show remaining references and break + if (shallow) { + // Shallow mode: show remaining references and break if (hasRemainingRefs) { console.log( chalk.yellow( @@ -180,7 +231,9 @@ async function resolveReferences(options: ResolveOptions): Promise { ), ) console.log( - chalk.yellow(' Run the resolve command again to resolve them, or use --recursive'), + chalk.yellow( + ' Run the expand command again to expand them, or omit --shallow for full expansion', + ), ) if (verbose) { for (const ref of remainingRefs) { @@ -210,7 +263,7 @@ async function resolveReferences(options: ResolveOptions): Promise { } else if (!hasRemainingRefs && iteration > 1) { console.log( chalk.green( - `✓ Fully resolved all references in: ${filePath} (${iteration} iterations)`, + `✓ Fully expanded all references in: ${filePath} (${iteration} iterations)`, ), ) } @@ -224,7 +277,7 @@ async function resolveReferences(options: ResolveOptions): Promise { /** * Restore content by restoring original Liquid statements from HTML comments */ -async function restoreReferences(options: ResolveOptions): Promise { +async function restoreReferences(options: ExpandOptions): Promise { const { paths, verbose } = options const allowedTypes = getAllowedTypes(options) @@ -254,11 +307,11 @@ async function restoreReferences(options: ResolveOptions): Promise { if (hasEdits) { console.log( chalk.blue( - `ℹ️ Info: ${filePath} contains resolved references that will be preserved by updating data files`, + `${filePath} contains expanded references; any edits made will be preserved in data files`, ), ) if (!verbose) { - console.log(chalk.dim(' Use --verbose to see details of the edits')) + console.log(chalk.dim(' Use --verbose to see details of the edits')) } // Update data files with the edited content before restoring @@ -266,7 +319,8 @@ async function restoreReferences(options: ResolveOptions): Promise { // Automatically restore any updated data files back to liquid tags if (updatedDataFiles.length > 0) { - console.log(chalk.blue(' Restoring updated data files back to liquid tags...')) + if (verbose) + console.log(chalk.blue(' Restoring updated data files back to liquid tags...')) for (const dataFile of updatedDataFiles) { try { const dataContent = fs.readFileSync(dataFile, 'utf-8') @@ -274,12 +328,14 @@ async function restoreReferences(options: ResolveOptions): Promise { if (restoredDataContent !== dataContent) { fs.writeFileSync(dataFile, restoredDataContent, 'utf-8') if (verbose) { - console.log(chalk.green(` Restored: ${dataFile}`)) + const dataPath = convertFilePathToDataPath(dataFile) + console.log(chalk.green(` Restored: ${dataPath}`)) } } } catch (error) { if (verbose) { - console.log(chalk.yellow(` Could not restore ${dataFile}: ${error}`)) + const dataPath = convertFilePathToDataPath(dataFile) + console.log(chalk.yellow(` Could not restore ${dataPath}: ${error}`)) } } } @@ -293,7 +349,7 @@ async function restoreReferences(options: ResolveOptions): Promise { fs.writeFileSync(filePath, restoredContent, 'utf-8') console.log(chalk.green(`✓ Restored references in: ${filePath}`)) } else { - console.log(chalk.gray(`No resolved references found in: ${filePath}`)) + console.log(chalk.gray(`No expanded references found in: ${filePath}`)) } } catch (error: any) { console.error(chalk.red(`Error restoring ${filePath}: ${error.message}`)) @@ -302,9 +358,9 @@ async function restoreReferences(options: ResolveOptions): Promise { } /** - * Resolve all Liquid data references in file content + * Expand all Liquid data references in file content */ -async function resolveFileContent( +async function expandFileContent( content: string, filePath: string, verbose?: boolean, @@ -317,7 +373,7 @@ async function resolveFileContent( return content } - let resolvedContent = content + let expandedContent = content let offset = 0 for (const ref of references) { @@ -329,8 +385,8 @@ async function resolveFileContent( let replacement: string if (withMarkers) { - const commentStart = `` - const commentEnd = `` + const commentStart = `` + const commentEnd = `` replacement = `${commentStart}${resolvedValue}${commentEnd}` } else { replacement = resolvedValue @@ -339,33 +395,33 @@ async function resolveFileContent( const startPos = ref.startIndex + offset const endPos = ref.endIndex + offset - resolvedContent = - resolvedContent.substring(0, startPos) + replacement + resolvedContent.substring(endPos) + expandedContent = + expandedContent.substring(0, startPos) + replacement + expandedContent.substring(endPos) offset += replacement.length - originalText.length if (verbose) { - console.log(chalk.green(` Resolved: ${ref.type}s.${ref.path}`)) + console.log(chalk.green(` Expanded: ${ref.type}s.${ref.path}`)) } } else { if (verbose) { - console.log(chalk.yellow(` Warning: Could not resolve ${ref.type}s.${ref.path}`)) + console.log(chalk.yellow(` Warning: Could not expand ${ref.type}s.${ref.path}`)) } } } catch (error: any) { if (verbose) { - console.log(chalk.red(` Error resolving ${ref.type}s.${ref.path}: ${error.message}`)) + console.log(chalk.red(` Error expanding ${ref.type}s.${ref.path}: ${error.message}`)) } } } - // Note: Remaining reference detection is now handled in resolveReferences function for recursive mode + // Note: Remaining reference detection is now handled in expandReferences function for recursive mode - return resolvedContent + return expandedContent } /** - * Detect if resolved content has been edited by comparing with original data + * Detect if expanded content has been edited by comparing with original data */ async function detectContentEdits( content: string, @@ -375,7 +431,7 @@ async function detectContentEdits( let hasEdits = false let match - while ((match = RESOLVED_PATTERN.exec(content)) !== null) { + while ((match = EXPANDED_PATTERN.exec(content)) !== null) { const [, type, dataPath, resolvedContent] = match const refType = type as 'reusable' | 'variable' @@ -463,7 +519,7 @@ function restoreFileContent( verbose?: boolean, allowedTypes?: Array<'reusable' | 'variable'>, ): string { - return content.replace(RESOLVED_PATTERN, (match, type, dataPath) => { + return content.replace(EXPANDED_PATTERN, (match, type, dataPath) => { const refType = type as 'reusable' | 'variable' // Only restore if this type is allowed @@ -483,7 +539,7 @@ function restoreFileContent( } /** - * Update data files with content from resolved blocks + * Update data files with content from expanded blocks * Returns array of file paths that were updated */ function updateDataFiles( @@ -533,7 +589,7 @@ function updateDataFiles( } /** - * Extract data updates from resolved content blocks + * Extract data updates from expanded content blocks */ function extractDataUpdates( content: string, @@ -542,7 +598,7 @@ function extractDataUpdates( const updates: Array<{ type: 'reusable' | 'variable'; path: string; newContent: string }> = [] let match - while ((match = RESOLVED_PATTERN.exec(content)) !== null) { + while ((match = EXPANDED_PATTERN.exec(content)) !== null) { const [, type, dataPath, resolvedContent] = match const refType = type as 'reusable' | 'variable' @@ -632,7 +688,7 @@ function applyDataUpdates( fs.writeFileSync(targetPath, newContent) if (verbose) { - console.log(chalk.green(` Updated: ${targetPath}`)) + console.log(chalk.green(` Updated: ${type}s.${dataPath}`)) } } else { // For variables, update YAML structure @@ -669,13 +725,13 @@ function applyDataUpdates( // Write back to file fs.writeFileSync(targetPath, finalYaml) if (verbose) { - console.log(chalk.green(` Updated: ${targetPath}`)) + console.log(chalk.green(` Updated: ${type}s.${dataPath}`)) } } return targetPath } catch (error: any) { if (verbose) { - console.log(chalk.red(` Error updating ${targetPath}: ${error.message}`)) + console.log(chalk.red(` Error updating ${type}s.${dataPath}: ${error.message}`)) } return null } diff --git a/src/content-render/tests/resolve-liquid.ts b/src/content-render/tests/liquid-tags.ts similarity index 72% rename from src/content-render/tests/resolve-liquid.ts rename to src/content-render/tests/liquid-tags.ts index 70e23a627ad6..5505f28c43c5 100644 --- a/src/content-render/tests/resolve-liquid.ts +++ b/src/content-render/tests/liquid-tags.ts @@ -6,7 +6,7 @@ import { execSync } from 'child_process' const rootDir = path.join(__dirname, '../../..') const testContentDir = path.join(rootDir, 'content/test-integration') -describe('resolve-liquid script integration tests', () => { +describe('liquid-tags script integration tests', () => { vi.setConfig({ testTimeout: 60 * 1000 }) beforeEach(async () => { @@ -20,12 +20,12 @@ describe('resolve-liquid script integration tests', () => { }) // Helper function to run script commands - async function runResolveScript(args: string): Promise<{ output: string; exitCode: number }> { + async function runScript(args: string): Promise<{ output: string; exitCode: number }> { let output = '' let exitCode = 0 try { - output = execSync(`tsx src/content-render/scripts/resolve-liquid.ts ${args}`, { + output = execSync(`tsx src/content-render/scripts/liquid-tags.ts ${args}`, { encoding: 'utf8', cwd: rootDir, stdio: 'pipe', @@ -39,7 +39,7 @@ describe('resolve-liquid script integration tests', () => { return { output, exitCode } } - test('resolve command should complete successfully with basic content', async () => { + test('expand command should complete successfully with basic content', async () => { // Create a test file with liquid reference const testFile = path.join(testContentDir, 'basic-test.md') const testContent = `--- @@ -51,16 +51,16 @@ This uses {% data variables.product.prodname_dotcom %} in content. await fs.writeFile(testFile, testContent) - const { output, exitCode } = await runResolveScript(`resolve --paths "${testFile}"`) + const { output, exitCode } = await runScript(`expand --paths "${testFile}"`) // Should complete without error expect(exitCode, `Script failed with output: ${output}`).toBe(0) expect(output.length).toBeGreaterThan(0) // Check that the file was modified - const resolvedContent = await fs.readFile(testFile, 'utf8') - expect(resolvedContent).not.toBe(testContent) - expect(resolvedContent).toContain('GitHub') // Should resolve to actual fixture value + const expandedContent = await fs.readFile(testFile, 'utf8') + expect(expandedContent).not.toBe(testContent) + expect(expandedContent).toContain('GitHub') // Should expand to actual fixture value }) test('restore command should complete successfully', async () => { @@ -74,11 +74,11 @@ This uses {% data variables.product.prodname_dotcom %} in content. await fs.writeFile(testFile, originalContent) - // First resolve - await runResolveScript(`resolve --paths "${testFile}"`) + // First expand + await runScript(`expand --paths "${testFile}"`) // Then restore - const { output, exitCode } = await runResolveScript(`restore --paths "${testFile}"`) + const { output, exitCode } = await runScript(`restore --paths "${testFile}"`) expect(exitCode, `Restore script failed with output: ${output}`).toBe(0) expect(output.length).toBeGreaterThan(0) @@ -90,7 +90,7 @@ This uses {% data variables.product.prodname_dotcom %} in content. }) test('help command should display usage information', async () => { - const { output, exitCode } = await runResolveScript('resolve --help') + const { output, exitCode } = await runScript('expand --help') expect(exitCode, `Help command failed with output: ${output}`).toBe(0) expect(output).toMatch(/resolve|usage|help|command/i)