fix(validator): reject frontmatter values starting with a YAML indicator

The frontmatter YAML checks catch an unquoted `: `, a tab indent and an
unterminated quote, but not a plain scalar that *begins* with a character YAML
reserves. A real parser rejects the whole file on those, which is the failure
the check exists to prevent.

The backtick is the one that bites this repo. Descriptions routinely name other
skills, and

    description: `other-skill` designs things. Use when …

is a natural way to start one. It passes the validator today and throws
`YAMLException: bad indentation of a mapping entry` in any consumer that parses
the frontmatter.

The rejected set was measured against a real parser rather than read off the
spec, every candidate indicator tried both with and without a following space,
and only the ones invalid in *both* forms with no legitimate single-line use
are rejected:

    always invalid:            ` @ %
    invalid before a space:    - ? &          (`:` was already covered)

Deliberately still accepted, because each parses cleanly and flagging it would
be a false positive on valid frontmatter: `:` or `-` when *not* followed by a
space (`:platform`, `-hyphenated`), a leading `,`, `]`, `}` or `#`, and `[`/`{`,
which open valid flow collections. `|` and `>` are left alone because they
legitimately open a multi-line block scalar. Those exclusions are pinned by a
test of their own, so the rule cannot be widened into false positives later —
both widening mutations are caught by exactly that test.

Cross-checked the finished rule against the parser over 38 values spanning every
indicator in both forms: **0 false positives, 11 remaining misses**, all of them
the deliberate exclusions above.

Seven mutations caught: removing either rule, dropping the backtick or the at
sign from the set, widening either rule (to anywhere in the value, or to ignore
the required space), and letting quoted values fall through to the new checks.

The repo's own 25 skills still pass with 0 errors and 0 warnings.
This commit is contained in:
Abhishek Sharma committed 2026-09-23 15:33:44 -07:00
1 parent bcab6a1b85
commit cdb801652d
2 files changed
+98 -2

No files matched your search

+67
View File
@@ -318,6 +318,73 @@ test('an unterminated quote is rejected', () => {
assert.match(yamlErrors(result)[0], /never closes/);
});
// A plain (unquoted) YAML scalar may not BEGIN with certain indicator characters.
// The set below was not read off the spec — it was measured against js-yaml, both
// with and without a following space, and only characters invalid in *both* forms
// with no legitimate single-line use are rejected here. Anything ambiguous is left
// alone on purpose, and the second test pins that so the rule cannot be widened
// into false positives later.
//
// The backtick is the one that actually bites this repo: descriptions routinely
// name other skills, and `\`alpha\` designs things` is a natural way to start one.
for (const [label, value] of [
['a backtick', '`alpha` designs things. Use when alpha.'],
['an at sign', '@team owns this. Use when alpha.'],
['a percent sign', '%complete coverage. Use when alpha.'],
]) {
test(`an unquoted value starting with ${label} is rejected`, () => {
const result = lintSkillContent(
'alpha',
fmLines(`description: ${value}`),
KNOWN,
);
assert.equal(yamlErrors(result).length, 1);
assert.match(yamlErrors(result)[0], /reserved|cannot begin|indicator/i);
});
}
for (const [label, value] of [
['a dash', '- Designs things. Use when alpha.'],
['a question mark', '? Designs things. Use when alpha.'],
['an ampersand', '& Designs things. Use when alpha.'],
]) {
test(`an unquoted value starting with ${label} and a space is rejected`, () => {
const result = lintSkillContent(
'alpha',
fmLines(`description: ${value}`),
KNOWN,
);
assert.equal(yamlErrors(result).length, 1);
});
}
test('quoting the value makes every reserved start valid again', () => {
for (const value of ['`alpha` x', '@team x', '%x', '- x', '? x', '& x']) {
const result = lintSkillContent(
'alpha',
fmLines(`description: "${value}"`),
KNOWN,
);
assert.deepEqual(yamlErrors(result), [], `quoted ${value} must be accepted`);
}
});
test('starts that YAML accepts are deliberately NOT rejected', () => {
// Each of these parses cleanly under js-yaml, so flagging them would be a
// false positive on valid frontmatter. Pinned so the rule stays narrow.
for (const value of [
':platform is fine', // a colon not followed by a space
'-hyphenated is fine', // a dash not followed by a space
'Designs `alpha` things', // a backtick anywhere but the first character
'Designs @team things', // an at sign anywhere but the first character
',leading comma is fine',
'#not-a-comment-here',
]) {
const result = lintSkillContent('alpha', fmLines(`description: ${value}`), KNOWN);
assert.deepEqual(yamlErrors(result), [], `${value} must be accepted`);
}
});
test('a duplicate key is not reported, because YAML accepts it', () => {
// Deliberate boundary: `safe_load` accepts duplicate keys, so flagging them
// here would fail files no host rejects. The rule tracks the parser, not taste.
+31 -2
View File
@@ -212,8 +212,37 @@ function frontmatterYamlErrors(content) {
return;
}
// Unquoted scalar. A colon followed by a space (or ending the line) makes
// YAML read a nested mapping where a plain string was meant.
// Unquoted scalar. A plain YAML scalar may not BEGIN with certain indicator
// characters. The set below was measured against a real parser rather than
// read off the spec, both with and without a following space, and only the
// characters invalid in *both* forms with no legitimate single-line use are
// rejected — a guard that flags valid frontmatter gets switched off.
//
// Deliberately not rejected, because each parses cleanly: `:` or `-` when NOT
// followed by a space (`:platform`, `-hyphenated`), a leading `,`, `]`, `}`
// or `#`, and `[`/`{`, which begin valid flow collections. `|` and `>` are
// left alone too, since they legitimately open a multi-line block scalar.
//
// The backtick is the one that bites this repo: descriptions routinely name
// other skills, and "`other-skill` designs things" is a natural way to start.
if (/^[`@%]/.test(value)) {
errors.push(
`Frontmatter line ${lineNo} starts an unquoted value with '${value[0]}' — YAML reserves ` +
`that character and cannot begin a plain scalar with it, so a host parsing this ` +
`frontmatter rejects the whole file; wrap the value in quotes`
);
return;
}
if (/^[-?&](\s|$)/.test(value)) {
errors.push(
`Frontmatter line ${lineNo} starts an unquoted value with '${value[0]} ' — YAML reads ` +
`that as an indicator rather than text and rejects the file; wrap the value in quotes`
);
return;
}
// A colon followed by a space (or ending the line) makes YAML read a nested
// mapping where a plain string was meant.
if (/:(\s|$)/.test(value)) {
errors.push(
`Frontmatter line ${lineNo} has an unquoted value containing ': ' — YAML reads that as a ` +