golem15/msd-core
3
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(#2450): CRLF-tolerant session-section rewrite + no-op-detection guard (#2482)

Browse Source 
* fix(#2444): re-resolve body-parser to 2.3.0 in lockfile (GHSA-v422-hmwv-36x6)

GHSA-v422-hmwv-36x6 (body-parser DoS via invalid limit value, low severity,
published 2026-07-20T23:23:26Z) made tests/npm-integrity-gate.test.cjs
(#3588: root workspace production tree has no advisories) fail any subsequent
npm audit --omit=dev. The advisory affects body-parser >=2.0.0 <2.3.0 pulled
transitively via @anthropic-ai/claude-agent-sdk -> @modelcontextprotocol/sdk
-> express -> body-parser@2.2.2.

express@5.2.1 already declares body-parser as ^2.2.1, so 2.3.0 is a valid
re-resolution within express's own compatibility range — no override needed.
Regenerated the lockfile via 'npm audit fix --omit=dev' which re-resolves
transitive deps within their declared ranges; package.json is unchanged.

Verified: npm audit --omit=dev reports 0/0/0/0/0 advisories; body-parser
now reads as 2.3.0 in 'npm ls body-parser --omit=dev'.

* test(#2450): failing-first CRLF regression for record-session insert path

cmdStateRecordSession's section-rewrite regexes (src/state.cts:1166,
:1195) used literal \\n which cannot match CRLF STATE.md delimiters.
The detector regex (CRLF-tolerant via $ under /m) entered the rewrite
branch, the writer regex silently no-op'd, but updated.push(...)/
sessionCreated=true ran unconditionally. Result: caller reported
recorded:true with 'Resume File' in updated, but the field was never
written to disk. With core.autocrlf=input, the CRLF working-tree file
produces no git diff, so the bug was invisible.

Adds three regression tests covering all three rewrite paths:
- CRLF STATE.md with ## Session and Resume file absent
- CRLF STATE.md with ## Session and Stopped at absent
- CRLF STATE.md with ## Session Continuity (bootstrap shape)

Each asserts the field IS on disk (the bug discriminator: the pre-fix
command's JSON output looked identical to a successful write).

* fix(#2450): CRLF-tolerant session-section rewrite + no-op-detection guard

Two regexes in cmdStateRecordSession used literal \\n which cannot match
CRLF STATE.md, silently no-op'ing the section rewrite while the CRLF-
tolerant detector above entered the branch. The reporter's exact repro:
on a CRLF STATE.md with one canonical session field absent, the command
returned recorded:true + updated:['Resume File'] but the field was never
written to disk.

Three changes:

1. src/state.cts:1166 (canonical ## Session rewrite regex): \\n -> \\r?\\n
2. src/state.cts:1195 (## Session Continuity insert regex): \\n -> \\r?\\n
3. Defensive invariant (#2450 class fix per reporter's suggestion): track
   whether the chosen branch's replace actually matched via callback flag.
   Only set sessionCreated=true and push to updated when rewriteMatched.
   Unreachable post-fix, but fail-loud is the right posture for a silent-
   success gate. If a future drift between the detector and writer regexes
   reintroduces the asymmetry, the caller will not see false updated entries.

Same canonical CRLF-tolerant form already in use at check-command-router.cts
:205 (extractPlanDesignatedSections). Same bug class previously fixed in
#1658, #1668, #2206, #2449.

* fix(#2450): address review followups + add changeset

Code-review + security-review both flagged the unreachable else at the
Session Continuity branch (defaulted rewriteMatched=true in dead code,
re-arming the bug class for future drift). Removed the else; the
remaining code path leaves rewriteMatched=false if linesToInsert is
empty, preserving the fail-loud posture.

Added scope-limitation doc to the rewriteMatched gate: it covers the
INSERT path only, not the earlier in-place stateReplaceField successes
(which DID land on disk and correctly push to updated unconditionally).

Tests:
- Normalized STATE_CRLF_SESSION_MISSING_RESUME fixture to match the
  canonical 6-key frontmatter of STATE_WITH_SESSION (code-review I2).
- Added mixed-ending test (LF frontmatter + CRLF body) to close
  CONTRIBUTING.md:490 'Mixed CRLF/LF newlines' requirement (I1).

Added Fixed changeset (code-review H1).

* docs(changeset): backfill PR number to 2482
This commit is contained in:
Tom Boucher
2026-07-21 09:53:14 -04:00
committed by GitHub
parent bf0d715733
commit 04a0eb8d63
3 changed files with 231 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/eight-foxes-cheer.md Normal file
View File

@@ -0,0 +1,5 @@
---
type: Fixed
pr: 2482
---
**`state record-session` no longer silently drops inserted fields on a CRLF `STATE.md`** — the section-rewrite regexes in `cmdStateRecordSession` used literal `\n` which couldn't match a CRLF STATE.md (`---\r\n`), so when a canonical session field (`Resume file` / `Stopped at` / `Last session`) was missing and had to be **inserted** via the section-rewrite path, the CRLF-tolerant detector entered the branch, the writer regex silently no-op'd, but `updated.push(...)` ran unconditionally. The command returned `{"recorded": true, "updated": ["Resume File"]}` while the field was never written to disk. With `core.autocrlf=input`, the CRLF working-tree file produced no `git diff`/`git status` change, so the bug was invisible. Both regexes now use the CRLF-tolerant `\r?\n` form (same canonical pattern already in use elsewhere), and a new defensive invariant gates `updated.push(...)` on the replace callback actually firing — so a future detector/writer drift will surface as missing `updated` entries rather than re-arming this silent-success class.

81
src/state.cts
View File

@@ -1155,6 +1155,16 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
const existingCanonicalSession = /^## Session[ \t]*$/im.test(content);
const existingSessionContinuity = /^## Session Continuity[ \t]*$/im.test(content);
// Track whether the chosen branch's rewrite actually matched. The detector
// regexes (existingCanonicalSession/existingSessionContinuity) are CRLF-
// tolerant ($ under /m treats \r as a line terminator); the writer regexes
// below must be too. If a writer regex silently fails to match (line-ending
// mismatch, unexpected heading shape, ...), do NOT report success — the
// caller would believe fields were persisted that were silently dropped
// (#2450). The append branch always sets rewriteMatched=true (it always
// mutates content).
let rewriteMatched = false;
if (existingCanonicalSession) {
// Normalize in place: replace the ENTIRE BODY of the existing ## Session
// section (heading + all content up to the next ## heading or EOF) with
@@ -1162,17 +1172,27 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
// `(?!^## )[\s\S]` consumes every line that doesn't start with "## ",
// which correctly stops at the next section boundary without consuming it.
// A trailing blank line is added so the next ## heading keeps its spacing.
//
// CRLF-tolerant (`\r?\n` after `[ \t]*`): the prior literal `\n` could not
// match a CRLF STATE.md (`---\r\n`), silently no-op'ing the replace while
// updated.push(...) reported success — #2450. The detector regex on the
// line above (`/^## Session[ \t]*$/im`) was already CRLF-tolerant, so the
// asymmetry armed the bug.
const canonicalReplacement = [
'## Session',
'',
`**Last session:** ${now}`,
`**Stopped at:** ${stoppedAtValue}`,
`**Resume file:** ${resumeValue}`,
'',
'',
].join('\n');
content = content.replace(
/^(## Session[ \t]*\n(?:(?!^## )[\s\S])*)/m,
[
'## Session',
'',
`**Last session:** ${now}`,
`**Stopped at:** ${stoppedAtValue}`,
`**Resume file:** ${resumeValue}`,
'',
'',
].join('\n'),
/^(## Session[ \t]*\r?\n(?:(?!^## )[\s\S])*)/m,
() => {
rewriteMatched = true;
return canonicalReplacement;
},
);
} else if (existingSessionContinuity) {
// #1101: a `## Session Continuity` section already exists (bootstrap
@@ -1183,6 +1203,8 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
// (e.g. prose like "Next recommended action"). Fields already updated in
// place above (needs* false) are not re-inserted. A function replacement
// is used so `$`-bearing caller values are inserted literally (#3454).
//
// CRLF-tolerant (`\r?\n`): same #2450 fix as the canonical branch above.
const linesToInsert: string[] = [];
if (needsLastSession) linesToInsert.push(`**Last session:** ${now}`);
if (needsStoppedAt) linesToInsert.push(`**Stopped at:** ${stoppedAtValue}`);
@@ -1192,10 +1214,20 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
// above (#1101 review F3) — otherwise a lowercase heading would detect
// but no-op the insert while still reporting the fields as updated.
content = content.replace(
/^(## Session Continuity[ \t]*\n)/im,
(_m, heading: string) => heading + linesToInsert.join('\n') + '\n',
/^(## Session Continuity[ \t]*\r?\n)/im,
(_m, heading: string) => {
rewriteMatched = true;
return heading + linesToInsert.join('\n') + '\n';
},
);
}
// No `else` branch: if linesToInsert.length === 0 the outer guard at
// :1144 (callerSuppliedValues && (needsStoppedAt || needsResumeFile
// || needsLastSession)) could not have fired, so this whole block is
// unreachable. Leaving `rewriteMatched = false` here is the fail-loud
// posture — a future change to the outer guard or needs* computation
// that makes this branch reachable will surface as a missing
// updated[] entry (silent recorded:false) rather than re-arming #2450.
} else {
// No session heading exists at all — append a new canonical section.
const scaffold = [
@@ -1208,13 +1240,28 @@ function cmdStateRecordSession(cwd: string, options: StateRecordSessionOptions,
'',
].join('\n');
content = content.trimEnd() + '\n' + scaffold;
rewriteMatched = true;
}
sessionCreated = true;
if (needsLastSession) updated.push('Last session');
if (needsStoppedAt) updated.push('Stopped At');
if (needsResumeFile) updated.push('Resume File');
// #2450 defensive invariant: only report sessionCreated/updated when the
// chosen branch's rewrite actually mutated content. Unreachable when the
// writer regexes above stay in sync with the CRLF-tolerant detector —
// but unreachable-defensive is the right posture for a silent-success
// gate. A no-op replace here means a future line-ending or shape drift
// between detector and writer; fail to record rather than claim success.
//
// Scope limitation (not a regression of this fix): the gate covers only
// the section-rewrite block. The earlier in-place stateReplaceField
// successes at :1081/:1083/:1089/:1101/:1108/:1114 push to `updated`
// unconditionally — those represent fields that DID land on disk via
// same-line replace (CRLF-agnostic seam), so unconditional push is
// correct. The class-defect防御 here is for the INSERT path only.
if (rewriteMatched) {
sessionCreated = true;
if (needsLastSession) updated.push('Last session');
if (needsStoppedAt) updated.push('Stopped At');
if (needsResumeFile) updated.push('Resume File');
}
}
return content;

162
tests/state.test.cjs
View File

@@ -4806,6 +4806,168 @@ describe('T6 section-splice characterization — record-session', () => {
cleanup(d);
}
});
// ─── #2450: CRLF STATE.md must not silently drop inserted session fields ─────
//
// Bug class: cmdStateRecordSession's section-rewrite regex used literal \n
// for the `## Session` and `## Session Continuity` heading boundaries, which
// cannot match a CRLF STATE.md (---\r\n style). The detector regex (using
// /^## Session[ \t]*$/im) WAS CRLF-tolerant ($ under /m treats \r as a line
// terminator), so the bug fired when a canonical session field was missing
// and had to be INSERTED via the section-rewrite path: the detector entered
// the branch, the writer regex silently no-op'd, but `updated.push('Resume File')`
// ran unconditionally → reported as updated but never written to disk.
//
// With core.autocrlf=input, the CRLF working-tree file produces no git diff,
// so the contributor cannot tell their STATE.md was misclassified.
const STATE_CRLF_SESSION_MISSING_RESUME = [
'---',
"gsd_state_version: '1.0'",
'milestone: v1.0',
'milestone_name: TestMilestone',
'status: executing',
"last_updated: '2026-01-01T00:00:00.000Z'",
"last_activity: '2026-01-01'",
'---',
'',
'# Project State',
'',
'## Session',
'',
'**Last session:** 2026-01-01T00:00:00.000Z',
'**Stopped at:** None',
// Resume file absent — forces the section-rewrite insert path (the buggy block)
'',
].join('\r\n');
test('record-session --resume-file on CRLF STATE.md with field absent: field IS written (#2450)', () => {
const d = createTempProject();
try {
fs.writeFileSync(path.join(d, '.planning', 'STATE.md'), STATE_CRLF_SESSION_MISSING_RESUME);
const result = runGsdTools(['state', 'record-session', '--resume-file', 'plan-3.md'], d);
assert.ok(result.success, `Command failed: ${result.error}`);
const output = JSON.parse(result.output);
// Bug discriminator: command reported recorded:true + 'Resume File' in
// updated, but the field was never written to disk. The disk assertion
// below is what fails pre-fix and passes post-fix.
assert.ok(Array.isArray(output.updated) && output.updated.includes('Resume File'),
`updated must include 'Resume File': ${JSON.stringify(output.updated)}`);
const after = fs.readFileSync(path.join(d, '.planning', 'STATE.md'), 'utf-8');
assert.ok(
/\*\*Resume file:\*\*\s*plan-3\.md/.test(after),
`Resume file field must be on disk after the call; STATE.md head:\n${after.slice(0, 500)}`,
);
} finally {
cleanup(d);
}
});
test('record-session --stopped-at on CRLF STATE.md with field absent: field IS written (#2450)', () => {
// Same bug class, different field. The bug fires whenever a canonical
// session field must be INSERTED (vs. same-line replaced) on a CRLF STATE.md.
const STATE_CRLF_SESSION_MISSING_STOPPED = [
'---',
'status: executing',
'---',
'',
'## Session',
'',
'**Last session:** 2026-01-01T00:00:00.000Z',
// Stopped at absent — forces the section-rewrite insert path
'**Resume file:** None',
'',
].join('\r\n');
const d = createTempProject();
try {
fs.writeFileSync(path.join(d, '.planning', 'STATE.md'), STATE_CRLF_SESSION_MISSING_STOPPED);
const result = runGsdTools(['state', 'record-session', '--stopped-at', '14.3'], d);
assert.ok(result.success, `Command failed: ${result.error}`);
const after = fs.readFileSync(path.join(d, '.planning', 'STATE.md'), 'utf-8');
assert.ok(
/\*\*Stopped at:\*\*\s*14\.3/.test(after),
`Stopped at field must be on disk; STATE.md head:\n${after.slice(0, 500)}`,
);
} finally {
cleanup(d);
}
});
test('record-session --resume-file on CRLF STATE.md with Session Continuity heading: field IS inserted (#2450)', () => {
// Same bug class, different heading: `## Session Continuity` is the
// bootstrap-template shape. The rewrite regex at the second bug site used
// the same literal-\n pattern.
const STATE_CRLF_CONTINUITY = [
'---',
'status: executing',
'---',
'',
'## Session Continuity',
'',
'Next recommended action: resume phase 2.',
'',
].join('\r\n');
const d = createTempProject();
try {
fs.writeFileSync(path.join(d, '.planning', 'STATE.md'), STATE_CRLF_CONTINUITY);
const result = runGsdTools(['state', 'record-session', '--resume-file', 'plan-9.md'], d);
assert.ok(result.success, `Command failed: ${result.error}`);
const after = fs.readFileSync(path.join(d, '.planning', 'STATE.md'), 'utf-8');
assert.ok(
/\*\*Resume file:\*\*\s*plan-9\.md/.test(after),
`Resume file field must be inserted under Session Continuity; STATE.md head:\n${after.slice(0, 500)}`,
);
// Existing prose must be preserved (#1101 invariant)
assert.ok(/Next recommended action: resume phase 2\./.test(after),
'Session Continuity prose must be preserved');
} finally {
cleanup(d);
}
});
test('record-session --resume-file on mixed-ending STATE.md (LF frontmatter + CRLF body): field IS written (#2450)', () => {
// CONTRIBUTING.md §"Parser and project-file inputs" lists "Mixed CRLF/LF
// newlines" as a required adversarial fixture class. Realistic when a
// Windows editor normalizes frontmatter bytes but preserves body text,
// or when tooling concatenates LF + CRLF fragments. The bug class is
// body-section-rewrite, so CRLF body + LF frontmatter is the worst case:
// the frontmatter delimiter is LF (syncStateFrontmatter parses either)
// but the `## Session` heading is CRLF, exercising the writer regex.
const STATE_MIXED_LF_FM_CRLF_BODY = [
'---',
'status: executing',
'---',
'', // LF after frontmatter
].join('\n') + [
'## Session',
'',
'**Last session:** 2026-01-01T00:00:00.000Z',
'**Stopped at:** None',
// Resume file absent
'',
].join('\r\n');
const d = createTempProject();
try {
fs.writeFileSync(path.join(d, '.planning', 'STATE.md'), STATE_MIXED_LF_FM_CRLF_BODY);
const result = runGsdTools(['state', 'record-session', '--resume-file', 'plan-mix.md'], d);
assert.ok(result.success, `Command failed: ${result.error}`);
const after = fs.readFileSync(path.join(d, '.planning', 'STATE.md'), 'utf-8');
assert.ok(
/\*\*Resume file:\*\*\s*plan-mix\.md/.test(after),
`Resume file field must be on disk despite mixed line endings; STATE.md head:\n${after.slice(0, 500)}`,
);
} finally {
cleanup(d);
}
});
});
describe('T6 section-splice characterization — add-decision', () => {