Skip to content

Commit cbf386b

Browse files
zied-jlassiTabishB
andauthored
fix(adapters): escape carriage returns in YAML frontmatter and dedupe escapeYamlValue (#1240)
escapeYamlValue detected \r as a character requiring quoting but never escaped it, leaving a literal carriage return inside the double-quoted scalar. A literal CR there is subject to YAML line folding/normalization and could silently corrupt the round-tripped value (realistic with CRLF-authored command descriptions). - Escape \r as \r alongside the existing \, " and \n handling. - Extract the helper, previously duplicated verbatim across five adapters (bob, claude, cursor, pi, windsurf), into a shared command-generation/yaml.ts module. - Add unit tests covering the escaping rules and a round-trip through a real YAML parser. Refs #1205, #1204 Co-authored-by: Tabish Bidiwale <30385142+TabishB@users.noreply.github.com>
1 parent bb1f18c commit cbf386b

8 files changed

Lines changed: 124 additions & 75 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@fission-ai/openspec": patch
3+
---
4+
5+
fix(adapters): escape carriage returns in generated YAML frontmatter
6+
7+
`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.

src/core/command-generation/adapters/bob.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,7 @@
88
import path from 'path';
99
import type { CommandContent, ToolCommandAdapter } from '../types.js';
1010
import { transformToHyphenCommands } from '../../../utils/command-references.js';
11-
12-
/**
13-
* Escapes a string value for safe YAML output.
14-
* Quotes the string if it contains special YAML characters.
15-
*/
16-
function escapeYamlValue(value: string): string {
17-
// Check if value needs quoting (contains special YAML characters or starts/ends with whitespace)
18-
const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value);
19-
if (needsQuoting) {
20-
// Use double quotes and escape internal double quotes and backslashes
21-
const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n');
22-
return `"${escaped}"`;
23-
}
24-
return value;
25-
}
11+
import { escapeYamlValue } from '../yaml.js';
2612

2713
/**
2814
* Bob Shell adapter for command generation.

src/core/command-generation/adapters/claude.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,7 @@
66

77
import path from 'path';
88
import type { CommandContent, ToolCommandAdapter } from '../types.js';
9-
10-
/**
11-
* Escapes a string value for safe YAML output.
12-
* Quotes the string if it contains special YAML characters.
13-
*/
14-
function escapeYamlValue(value: string): string {
15-
// Check if value needs quoting (contains special YAML characters or starts/ends with whitespace)
16-
const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value);
17-
if (needsQuoting) {
18-
// Use double quotes and escape internal double quotes and backslashes
19-
const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n');
20-
return `"${escaped}"`;
21-
}
22-
return value;
23-
}
9+
import { escapeYamlValue } from '../yaml.js';
2410

2511
/**
2612
* Formats a tags array as a YAML array with proper escaping.

src/core/command-generation/adapters/cursor.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,7 @@
77

88
import path from 'path';
99
import type { CommandContent, ToolCommandAdapter } from '../types.js';
10-
11-
/**
12-
* Escapes a string value for safe YAML output.
13-
* Quotes the string if it contains special YAML characters.
14-
*/
15-
function escapeYamlValue(value: string): string {
16-
// Check if value needs quoting (contains special YAML characters or starts/ends with whitespace)
17-
const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value);
18-
if (needsQuoting) {
19-
// Use double quotes and escape internal double quotes and backslashes
20-
const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n');
21-
return `"${escaped}"`;
22-
}
23-
return value;
24-
}
10+
import { escapeYamlValue } from '../yaml.js';
2511

2612
/**
2713
* Cursor adapter for command generation.

src/core/command-generation/adapters/pi.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import path from 'path';
99
import type { CommandContent, ToolCommandAdapter } from '../types.js';
1010
import { transformToHyphenCommands } from '../../../utils/command-references.js';
11+
import { escapeYamlValue } from '../yaml.js';
1112

1213
const PI_INPUT_HEADING = /^\*\*Input\*\*:[^\n]*$/m;
1314

@@ -22,21 +23,6 @@ function injectPiArgs(body: string): string {
2223
);
2324
}
2425

25-
/**
26-
* Escapes a string value for safe YAML output.
27-
* Quotes the string if it contains special YAML characters.
28-
*/
29-
function escapeYamlValue(value: string): string {
30-
// Check if value needs quoting (contains special YAML characters or starts/ends with whitespace)
31-
const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value);
32-
if (needsQuoting) {
33-
// Use double quotes and escape internal double quotes and backslashes
34-
const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n');
35-
return `"${escaped}"`;
36-
}
37-
return value;
38-
}
39-
4026
/**
4127
* Pi adapter for prompt template generation.
4228
* File path: .pi/prompts/opsx-<id>.md

src/core/command-generation/adapters/windsurf.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,7 @@
77

88
import path from 'path';
99
import type { CommandContent, ToolCommandAdapter } from '../types.js';
10-
11-
/**
12-
* Escapes a string value for safe YAML output.
13-
* Quotes the string if it contains special YAML characters.
14-
*/
15-
function escapeYamlValue(value: string): string {
16-
// Check if value needs quoting (contains special YAML characters or starts/ends with whitespace)
17-
const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value);
18-
if (needsQuoting) {
19-
// Use double quotes and escape internal double quotes and backslashes
20-
const escaped = value.replace(/\\/g, '\\\\').replace(/"/g, '\\"').replace(/\n/g, '\\n');
21-
return `"${escaped}"`;
22-
}
23-
return value;
24-
}
10+
import { escapeYamlValue } from '../yaml.js';
2511

2612
/**
2713
* Formats a tags array as a YAML array with proper escaping.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Shared YAML frontmatter helpers for command adapters.
3+
*
4+
* Several tool adapters emit YAML frontmatter and need to escape
5+
* user-facing strings (name, description, category, tags) so the
6+
* generated file stays valid YAML. This module centralizes that logic
7+
* so the behavior is identical across adapters and fixed in one place.
8+
*/
9+
10+
/**
11+
* Escapes a string value for safe YAML output.
12+
*
13+
* Quotes the value with double quotes when it contains characters that
14+
* carry special meaning in YAML (or leading/trailing whitespace), and
15+
* escapes the characters that are not representable verbatim inside a
16+
* double-quoted scalar: backslash, double quote, line feed and carriage
17+
* return. Values without special characters are returned unquoted.
18+
*
19+
* @param value - The raw string to embed in YAML frontmatter.
20+
* @returns The value, double-quoted and escaped when necessary.
21+
*/
22+
export function escapeYamlValue(value: string): string {
23+
// Check if value needs quoting (contains special YAML characters or starts/ends with whitespace)
24+
const needsQuoting = /[:\n\r#{}[\],&*!|>'"%@`]|^\s|\s$/.test(value);
25+
if (needsQuoting) {
26+
// Use double quotes and escape characters that are not safe to emit
27+
// verbatim inside a double-quoted YAML scalar. Carriage returns must be
28+
// escaped too: a literal CR inside double quotes is subject to YAML line
29+
// folding/normalization and would silently corrupt the round-tripped value.
30+
const escaped = value
31+
.replace(/\\/g, '\\\\')
32+
.replace(/"/g, '\\"')
33+
.replace(/\n/g, '\\n')
34+
.replace(/\r/g, '\\r');
35+
return `"${escaped}"`;
36+
}
37+
return value;
38+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { parse as parseYaml } from 'yaml';
3+
import { escapeYamlValue } from '../../../src/core/command-generation/yaml.js';
4+
5+
/**
6+
* Parses a single-key YAML document and returns the round-tripped value.
7+
*
8+
* @param value - The raw string to escape and round-trip through YAML.
9+
* @returns The value as read back by a real YAML parser.
10+
*/
11+
function roundTrip(value: string): unknown {
12+
const doc = `key: ${escapeYamlValue(value)}\n`;
13+
return parseYaml(doc).key;
14+
}
15+
16+
describe('command-generation/yaml escapeYamlValue', () => {
17+
it('returns the value unquoted when no special characters are present', () => {
18+
expect(escapeYamlValue('Enter explore mode for thinking')).toBe(
19+
'Enter explore mode for thinking'
20+
);
21+
});
22+
23+
it('quotes values containing a colon', () => {
24+
expect(escapeYamlValue('Fix: regression')).toBe('"Fix: regression"');
25+
});
26+
27+
it('escapes embedded double quotes', () => {
28+
expect(escapeYamlValue('Fix the "auth" feature')).toBe(
29+
'"Fix the \\"auth\\" feature"'
30+
);
31+
});
32+
33+
it('escapes backslashes before other characters', () => {
34+
expect(escapeYamlValue('path\\to:thing')).toBe('"path\\\\to:thing"');
35+
});
36+
37+
it('escapes line feeds', () => {
38+
expect(escapeYamlValue('Line 1\nLine 2')).toBe('"Line 1\\nLine 2"');
39+
});
40+
41+
it('escapes carriage returns', () => {
42+
// Regression: \r is detected as needing quoting but was previously left
43+
// as a literal CR inside the double-quoted scalar.
44+
expect(escapeYamlValue('Line 1\rLine 2')).toBe('"Line 1\\rLine 2"');
45+
});
46+
47+
it('escapes CRLF sequences', () => {
48+
expect(escapeYamlValue('Line 1\r\nLine 2')).toBe('"Line 1\\r\\nLine 2"');
49+
});
50+
51+
it('quotes values with leading or trailing whitespace', () => {
52+
expect(escapeYamlValue(' leading')).toBe('" leading"');
53+
expect(escapeYamlValue('trailing ')).toBe('"trailing "');
54+
});
55+
56+
describe('round-trips through a real YAML parser', () => {
57+
const cases: Array<[string, string]> = [
58+
['plain', 'Enter explore mode'],
59+
['colon', 'Fix: regression in parser'],
60+
['double quotes', 'Fix the "auth" feature'],
61+
['backslash', 'path\\to\\thing'],
62+
['line feed', 'Line 1\nLine 2'],
63+
['carriage return', 'Line 1\rLine 2'],
64+
['crlf', 'Line 1\r\nLine 2'],
65+
['mixed special', 'a: "b"\r\n#c\\d'],
66+
];
67+
68+
for (const [label, value] of cases) {
69+
it(`preserves the value: ${label}`, () => {
70+
expect(roundTrip(value)).toBe(value);
71+
});
72+
}
73+
});
74+
});

0 commit comments

Comments
 (0)