fix(#2873): close review findings across fences, sanitizer and docs

Isolated security review found resolveSpecRootReference's fence tracker
toggled on any delimiter, so a backtick fence could be closed by a tilde
one and an include in the gap was rewritten inside a code block. Fixed by
reusing scanFencedBlocks - the canonical engine already behind
stripFencedCode and extractFencedBlock - rather than carrying a fourth
copy of fence detection, which also closes the duplication the standards
review flagged.

sanitizeForRender now strips combining marks and zero-width characters
alongside the ANSI, control and bidi classes it already handled.

Adds the C, E and F matrix rows the spec review found missing, including
installer-level coverage that spawns the real install rather than calling
the report builder. Ships the how-to, the reference and command docs in
five locales, the changeset, the inventory and glossary entries, and
regenerates health.md for the new W028 rule.

Refs #2873
This commit is contained in:
sim
2026-08-14 23:09:36 -04:00
parent 2641e6cb67
commit 147856040b
25 changed files with 747 additions and 34 deletions

View File

@@ -175,26 +175,42 @@ const CONTROL_RE = /[\x00-\x1f\x7f-\x9f]/g;
* #13 names. */
const BIDI_RE = /[‪-‮⁦-⁩]/g;
/** Combining marks (U+0300-U+036F) — "zalgo" text. Stacked onto the
* preceding base character, an unbounded run visually overflows into
* adjacent terminal cells/rows even though the string stays within the
* 64-char cap `readInstallManifest` enforces. Written as `\u{...}` escapes
* (not literal combining characters) so the source itself stays plain
* ASCII and does not visually combine in editors/diffs. */
const COMBINING_MARK_RE = /[\u{0300}-\u{036F}]/gu;
/** Zero-width characters: ZWSP (U+200B), ZWNJ (U+200C), ZWJ (U+200D), and
* BOM/ZWNBSP (U+FEFF). None of these are JS `\s`, so they survive both the
* char-count cap and the whitespace-collapse step below undetected. */
const ZERO_WIDTH_RE = /[\u{200B}-\u{200D}\u{FEFF}]/gu;
/**
* Sanitize a `declaredRuntime` (or any similarly attacker-influenceable
* string) for terminal/console rendering. `null` passes through as `null`;
* `''` passes through as `''`. Strips ANSI escapes, C0/C1 controls, and
* Unicode bidi overrides/isolates (replacing each stripped run with
* nothing — never a space), then collapses any remaining whitespace run
* (including adjacent spaces left behind by a removed newline) to a single
* space and trims.
* `''` passes through as `''`. Strips ANSI escapes, C0/C1 controls, Unicode
* bidi overrides/isolates, combining marks (zalgo), and zero-width
* characters (replacing each stripped run with nothing — never a space),
* then collapses any remaining whitespace run (including adjacent spaces
* left behind by a removed newline) to a single space and trims.
*
* Idempotent by construction: once ANSI/control/bidi bytes are gone and
* whitespace is collapsed to single internal spaces with no leading/
* trailing space, a second pass finds nothing left to strip or collapse.
* Never truncates — `readInstallManifest` already caps at 64 chars.
* Idempotent by construction: once ANSI/control/bidi/combining/zero-width
* bytes are gone and whitespace is collapsed to single internal spaces with
* no leading/trailing space, a second pass finds nothing left to strip or
* collapse. Pure character-class filter — never truncates; `readInstallManifest`
* already caps at 64 chars.
*/
export function sanitizeForRender(value: string | null): string | null {
if (value === null) return null;
const stripped = value
.replace(ANSI_RE, '')
.replace(CONTROL_RE, '')
.replace(BIDI_RE, '');
.replace(BIDI_RE, '')
.replace(COMBINING_MARK_RE, '')
.replace(ZERO_WIDTH_RE, '');
return stripped.replace(/\s+/g, ' ').trim();
}

View File

@@ -27,6 +27,7 @@ import capabilityRegistry = require('./capability-registry.cjs');
import hostIntegration = require('./host-integration.cjs');
import { posixNormalize } from './shell-command-projection.cjs';
import { escapeRegex as escapeRegExp } from './pattern.cjs';
import { scanFencedBlocks } from './markdown-sectionizer.cjs';
// #2870: install-scope.cts is a leaf-tier sibling (imports only
// runtime-homes.cjs + node builtins, never this module) — no cycle. See the
// isGlobal sites below for why the boolean projection is centralized here too.
@@ -513,13 +514,6 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c
// preserved rather than dropped.
const WORKFLOW_SPEC_ROOT_INCLUDE_RE = /^@~\/\.claude\/gsd-core\/workflows\/([A-Za-z0-9._-]+)\.md[ \t]*(\r?)$/gm;
// Matches a fenced code-block delimiter line (``` or ~~~, any info string)
// so occurrences of the include shape used as *documentation* inside a fence
// are left untouched — Claude Code documents backticks as the way to
// *prevent* an `@`-import, so rewriting a fenced example would corrupt
// documentation-of-the-syntax.
const FENCE_DELIMITER_RE = /^(```|~~~)[^\r\n]*$/gm;
/**
* Rewrite a static global-scope Claude skill `@`-include of the command's own
* workflow spec into an imperative two-step resolution the agent performs at
@@ -548,29 +542,38 @@ const FENCE_DELIMITER_RE = /^(```|~~~)[^\r\n]*$/gm;
* Idempotent: the replacement text never begins with `@` and never matches
* `WORKFLOW_SPEC_ROOT_INCLUDE_RE`, so re-applying this function to its own
* output is a no-op.
*
* Fence detection reuses `scanFencedBlocks` (markdown-sectionizer.cts) — the
* same CommonMark-correct state machine `stripFencedCode`/`extractFencedBlock`
* are built on — instead of a hand-rolled "any delimiter line toggles
* open/closed" tracker. A naive toggle is wrong under CommonMark: a fence
* opened with ``` is NOT closed by a ~~~ line (closer must share the
* opener's delimiter character and have run length >= the opener's), so a
* mismatched delimiter is fence CONTENT, not a boundary. #2873 review.
*/
function resolveSpecRootReference(body) {
if (typeof body !== 'string' || body.length === 0) return body;
if (!body.includes('@~/.claude/gsd-core/workflows/')) return body;
// Collect [start, end) offset ranges covered by fenced code blocks so
// matches inside them are skipped. An unterminated trailing fence covers
// to the end of the string (still "inside a fence").
const fenceRanges = [];
// Collect [start, end) character-offset ranges covered by fenced code
// blocks so matches inside them are skipped. An unterminated trailing
// fence covers to the end of the string (still "inside a fence").
const lines = body.split('\n');
const lineStartOffsets = [];
{
let m;
let openStart = null;
FENCE_DELIMITER_RE.lastIndex = 0;
while ((m = FENCE_DELIMITER_RE.exec(body)) !== null) {
if (openStart === null) {
openStart = m.index;
} else {
fenceRanges.push([openStart, m.index + m[0].length]);
openStart = null;
}
let offset = 0;
for (const line of lines) {
lineStartOffsets.push(offset);
offset += line.length + 1; // +1 for the '\n' separator
}
if (openStart !== null) fenceRanges.push([openStart, body.length]);
}
const fenceRanges = scanFencedBlocks(lines).map(({ openLineIdx, closeLineIdx }) => {
const start = lineStartOffsets[openLineIdx];
const end = closeLineIdx === -1
? body.length
: lineStartOffsets[closeLineIdx] + lines[closeLineIdx].length;
return [start, end];
});
const isInsideFence = (offset) => fenceRanges.some(([start, end]) => offset >= start && offset < end);
return body.replace(WORKFLOW_SPEC_ROOT_INCLUDE_RE, (match, stem, cr, offset) => {