diff --git a/.changeset/escape-yaml-carriage-return.md b/.changeset/escape-yaml-carriage-return.md new file mode 100644 index 000000000..23594df89 --- /dev/null +++ b/.changeset/escape-yaml-carriage-return.md @@ -0,0 +1,7 @@ +--- +"@fission-ai/openspec": patch +--- + +fix(adapters): escape carriage returns in generated YAML frontmatter + +`escapeYamlValue` flagged `\r` as a character requiring quoting but never escaped it, leaving a literal carriage return inside the double-quoted scalar where YAML line folding/normalization could silently corrupt the value (realistic with CRLF-authored command descriptions). Carriage returns are now escaped as `\r`. The helper — previously duplicated verbatim across five adapters (bob, claude, cursor, pi, windsurf) — is extracted into a shared `command-generation/yaml.ts` module so the behavior stays consistent and is fixed in one place. diff --git a/src/core/command-generation/adapters/bob.ts b/src/core/command-generation/adapters/bob.ts index 53426fc4e..8acb32beb 100644 --- a/src/core/command-generation/adapters/bob.ts +++ b/src/core/command-generation/adapters/bob.ts @@ -8,21 +8,7 @@ import path from 'path'; import type { CommandContent, ToolCommandAdapter } from '../types.js'; import { transformToHyphenCommands } from '../../../utils/command-references.js'; - -/** - * Escapes a string value for safe YAML output. - * Quotes the string if it contains special YAML characters. - */ -function escapeYamlValue(value: string): string { - // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) - const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); - if (needsQuoting) { - // Use double quotes and escape internal double quotes and backslashes - const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); - return `"${escaped}"`; - } - return value; -} +import { escapeYamlValue } from '../yaml.js'; /** * Bob Shell adapter for command generation. diff --git a/src/core/command-generation/adapters/claude.ts b/src/core/command-generation/adapters/claude.ts index 532b3a47b..b0f03a08e 100644 --- a/src/core/command-generation/adapters/claude.ts +++ b/src/core/command-generation/adapters/claude.ts @@ -6,21 +6,7 @@ import path from 'path'; import type { CommandContent, ToolCommandAdapter } from '../types.js'; - -/** - * Escapes a string value for safe YAML output. - * Quotes the string if it contains special YAML characters. - */ -function escapeYamlValue(value: string): string { - // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) - const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); - if (needsQuoting) { - // Use double quotes and escape internal double quotes and backslashes - const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); - return `"${escaped}"`; - } - return value; -} +import { escapeYamlValue } from '../yaml.js'; /** * Formats a tags array as a YAML array with proper escaping. diff --git a/src/core/command-generation/adapters/cursor.ts b/src/core/command-generation/adapters/cursor.ts index 85adedb03..d540a479b 100644 --- a/src/core/command-generation/adapters/cursor.ts +++ b/src/core/command-generation/adapters/cursor.ts @@ -7,21 +7,7 @@ import path from 'path'; import type { CommandContent, ToolCommandAdapter } from '../types.js'; - -/** - * Escapes a string value for safe YAML output. - * Quotes the string if it contains special YAML characters. - */ -function escapeYamlValue(value: string): string { - // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) - const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); - if (needsQuoting) { - // Use double quotes and escape internal double quotes and backslashes - const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); - return `"${escaped}"`; - } - return value; -} +import { escapeYamlValue } from '../yaml.js'; /** * Cursor adapter for command generation. diff --git a/src/core/command-generation/adapters/pi.ts b/src/core/command-generation/adapters/pi.ts index fa11d9d8e..80963ec81 100644 --- a/src/core/command-generation/adapters/pi.ts +++ b/src/core/command-generation/adapters/pi.ts @@ -8,6 +8,7 @@ import path from 'path'; import type { CommandContent, ToolCommandAdapter } from '../types.js'; import { transformToHyphenCommands } from '../../../utils/command-references.js'; +import { escapeYamlValue } from '../yaml.js'; const PI_INPUT_HEADING = /^\*\*Input\*\*:[^\n]*$/m; @@ -22,21 +23,6 @@ function injectPiArgs(body: string): string { ); } -/** - * Escapes a string value for safe YAML output. - * Quotes the string if it contains special YAML characters. - */ -function escapeYamlValue(value: string): string { - // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) - const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); - if (needsQuoting) { - // Use double quotes and escape internal double quotes and backslashes - const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); - return `"${escaped}"`; - } - return value; -} - /** * Pi adapter for prompt template generation. * File path: .pi/prompts/opsx-.md diff --git a/src/core/command-generation/adapters/windsurf.ts b/src/core/command-generation/adapters/windsurf.ts index 59c86d8e0..a7fe4febe 100644 --- a/src/core/command-generation/adapters/windsurf.ts +++ b/src/core/command-generation/adapters/windsurf.ts @@ -7,21 +7,7 @@ import path from 'path'; import type { CommandContent, ToolCommandAdapter } from '../types.js'; - -/** - * Escapes a string value for safe YAML output. - * Quotes the string if it contains special YAML characters. - */ -function escapeYamlValue(value: string): string { - // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) - const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); - if (needsQuoting) { - // Use double quotes and escape internal double quotes and backslashes - const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n'); - return `"${escaped}"`; - } - return value; -} +import { escapeYamlValue } from '../yaml.js'; /** * Formats a tags array as a YAML array with proper escaping. diff --git a/src/core/command-generation/yaml.ts b/src/core/command-generation/yaml.ts new file mode 100644 index 000000000..dc4354add --- /dev/null +++ b/src/core/command-generation/yaml.ts @@ -0,0 +1,38 @@ +/** + * Shared YAML frontmatter helpers for command adapters. + * + * Several tool adapters emit YAML frontmatter and need to escape + * user-facing strings (name, description, category, tags) so the + * generated file stays valid YAML. This module centralizes that logic + * so the behavior is identical across adapters and fixed in one place. + */ + +/** + * Escapes a string value for safe YAML output. + * + * Quotes the value with double quotes when it contains characters that + * carry special meaning in YAML (or leading/trailing whitespace), and + * escapes the characters that are not representable verbatim inside a + * double-quoted scalar: backslash, double quote, line feed and carriage + * return. Values without special characters are returned unquoted. + * + * @param value - The raw string to embed in YAML frontmatter. + * @returns The value, double-quoted and escaped when necessary. + */ +export function escapeYamlValue(value: string): string { + // Check if value needs quoting (contains special YAML characters or starts/ends with whitespace) + const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value); + if (needsQuoting) { + // Use double quotes and escape characters that are not safe to emit + // verbatim inside a double-quoted YAML scalar. Carriage returns must be + // escaped too: a literal CR inside double quotes is subject to YAML line + // folding/normalization and would silently corrupt the round-tripped value. + const escaped = value + .replace(/\\/g, '\\\\') + .replace(/"/g, '\\"') + .replace(/\n/g, '\\n') + .replace(/\r/g, '\\r'); + return `"${escaped}"`; + } + return value; +} diff --git a/test/core/command-generation/yaml.test.ts b/test/core/command-generation/yaml.test.ts new file mode 100644 index 000000000..937209ae1 --- /dev/null +++ b/test/core/command-generation/yaml.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect } from 'vitest'; +import { parse as parseYaml } from 'yaml'; +import { escapeYamlValue } from '../../../src/core/command-generation/yaml.js'; + +/** + * Parses a single-key YAML document and returns the round-tripped value. + * + * @param value - The raw string to escape and round-trip through YAML. + * @returns The value as read back by a real YAML parser. + */ +function roundTrip(value: string): unknown { + const doc = `key: ${escapeYamlValue(value)}\n`; + return parseYaml(doc).key; +} + +describe('command-generation/yaml escapeYamlValue', () => { + it('returns the value unquoted when no special characters are present', () => { + expect(escapeYamlValue('Enter explore mode for thinking')).toBe( + 'Enter explore mode for thinking' + ); + }); + + it('quotes values containing a colon', () => { + expect(escapeYamlValue('Fix: regression')).toBe('"Fix: regression"'); + }); + + it('escapes embedded double quotes', () => { + expect(escapeYamlValue('Fix the "auth" feature')).toBe( + '"Fix the \\"auth\\" feature"' + ); + }); + + it('escapes backslashes before other characters', () => { + expect(escapeYamlValue('path\\to:thing')).toBe('"path\\\\to:thing"'); + }); + + it('escapes line feeds', () => { + expect(escapeYamlValue('Line 1\nLine 2')).toBe('"Line 1\\nLine 2"'); + }); + + it('escapes carriage returns', () => { + // Regression: \r is detected as needing quoting but was previously left + // as a literal CR inside the double-quoted scalar. + expect(escapeYamlValue('Line 1\rLine 2')).toBe('"Line 1\\rLine 2"'); + }); + + it('escapes CRLF sequences', () => { + expect(escapeYamlValue('Line 1\r\nLine 2')).toBe('"Line 1\\r\\nLine 2"'); + }); + + it('quotes values with leading or trailing whitespace', () => { + expect(escapeYamlValue(' leading')).toBe('" leading"'); + expect(escapeYamlValue('trailing ')).toBe('"trailing "'); + }); + + describe('round-trips through a real YAML parser', () => { + const cases: Array<[string, string]> = [ + ['plain', 'Enter explore mode'], + ['colon', 'Fix: regression in parser'], + ['double quotes', 'Fix the "auth" feature'], + ['backslash', 'path\\to\\thing'], + ['line feed', 'Line 1\nLine 2'], + ['carriage return', 'Line 1\rLine 2'], + ['crlf', 'Line 1\r\nLine 2'], + ['mixed special', 'a: "b"\r\n#c\\d'], + ]; + + for (const [label, value] of cases) { + it(`preserves the value: ${label}`, () => { + expect(roundTrip(value)).toBe(value); + }); + } + }); +});