fix: strip HTML comments before preview/routing; refuse apply when the sealed sidecar is gone

bin/fde.js

@@ -214,8 +214,14 @@ const PRIVATE_TAG = /<(\/)?private\b[^>]*>/gi
 // Depth-aware split of a markdown body into public text and sealed blocks. A
 // nested block seals to the outermost close, an unclosed one seals to EOF, and a
 // stray close is dropped - a regex pair cannot do any of those safely.
-function splitPrivate(md) {
-  const text = String(md || '')
+// HTML comments go first: template hints and pasted notes hide content there, and
+// `clean` is what debrief/ingest preview to a human and route into memory.
+// opts.sealDangling seals an unterminated `<!--` to EOF. Only untrusted input
+// gets that: on the read path a stray `<!--` already stored in memory would
+// otherwise hide every line after it from every view.
+function splitPrivate(md, opts = {}) {
+  let text = String(md || '').replace(/<!--[\s\S]*?-->/g, '')
+  if (opts.sealDangling) text = text.replace(/<!--[\s\S]*$/, '')
   const blocks = []
   let out = ''
   let cursor = 0
@@ -253,7 +259,25 @@ function splitPrivate(md) {
 }
 
 function stripPrivate(md) {
-  return stripControlChars(splitPrivate(md).clean.replace(/<!--[\s\S]*?-->/g, ''))
+  return stripControlChars(splitPrivate(md).clean)
+}
+
+// Persisted blocks must be balanced. splitPrivate() seals an unclosed block to
+// EOF and hands it back exactly as written; storing that would leave a dangling
+// opener that swallows every note appended to the file afterwards.
+function sealedText(blocks) {
+  return blocks.map((b) => {
+    let open = 0
+    let m
+    PRIVATE_TAG.lastIndex = 0
+    while ((m = PRIVATE_TAG.exec(b))) {
+      if (m[1]) open = Math.max(0, open - 1)
+      else open++
+    }
+    // Balance by count, not by suffix: one block can hold several unclosed
+    // openers, and each needs its own closer or the tail still dangles.
+    return `${b}\n${'</private>\n'.repeat(open)}`
+  }).join('')
 }
 
 // Read + redact in one step - the default way dashboard code should ever touch
@@ -392,7 +416,10 @@ function atomicWriteFile(p, content, opts = {}) {
   }
   const tmp = `${p}.${process.pid}.${Date.now()}.tmp`
   try {
-    fs.writeFileSync(tmp, content)
+    // opts.mode is set at create time: a secret must never exist world-readable,
+    // not even for the window between rename and a follow-up chmod.
+    fs.writeFileSync(tmp, content, opts.mode ? { mode: opts.mode } : undefined)
+    if (opts.mode) fs.chmodSync(tmp, opts.mode)
     fs.renameSync(tmp, p)
   } catch (e) {
     try { fs.unlinkSync(tmp) } catch (_) {}
@@ -429,6 +456,7 @@ const DEBRIEF_PROPOSE = '.debrief-propose'
 // The agent is told to open and rewrite .debrief-propose, so sealed blocks are
 // held out of it in an owner-only sidecar that only --apply reads back.
 const DEBRIEF_PRIVATE = '.debrief-private'
+const DEBRIEF_SEAL = '.debrief-seal'
 
 function gitBinOk() {
   try {
@@ -1337,19 +1365,36 @@ function previewLine(text, max = 240) {
 }
 
 function writeProposal(eng, text) {
-  const { clean, blocks } = splitPrivate(text)
+  const { clean, blocks } = splitPrivate(text, { sealDangling: true })
   const proposePath = path.join(eng, DEBRIEF_PROPOSE)
   const privatePath = path.join(eng, DEBRIEF_PRIVATE)
-  withFileLock(proposePath, () => { atomicWriteFile(proposePath, clean) })
+  // Seal first. A refused or failed sidecar write must not leave behind a
+  // proposal whose (private - redacted) marker has nothing left behind it.
   if (blocks.length) {
-    withFileLock(privatePath, () => { atomicWriteFile(privatePath, `${blocks.join('\n')}\n`) })
+    const blocked = refuseSymlinkWrite(privatePath, { soft: true })
+    if (blocked) { console.error(blocked); process.exit(1) }
+    withFileLock(privatePath, () => { atomicWriteFile(privatePath, sealedText(blocks), { mode: 0o600 }) })
     try { fs.chmodSync(privatePath, 0o600) } catch (_) {}
   } else {
     try { fs.unlinkSync(privatePath) } catch (_) {}
   }
+  withFileLock(proposePath, () => { atomicWriteFile(proposePath, clean) })
+  // Receipt, so apply knows how many blocks the human actually approved. Counting
+  // (private - redacted) markers in the proposal instead would refuse forever on
+  // notes that merely quote the wording - the CLI prints it, so it gets pasted back.
+  withFileLock(path.join(eng, DEBRIEF_SEAL), () => {
+    atomicWriteFile(path.join(eng, DEBRIEF_SEAL), `${blocks.length}\n`)
+  })
   return { proposePath, clean, blocks }
 }
 
+function readSealCount(eng) {
+  try {
+    const n = parseInt(fs.readFileSync(path.join(eng, DEBRIEF_SEAL), 'utf8').trim(), 10)
+    return Number.isInteger(n) && n >= 0 ? n : null
+  } catch (_) { return null }
+}
+
 function readSealedProposal(eng) {
   try {
     return splitPrivate(stripControlChars(fs.readFileSync(path.join(eng, DEBRIEF_PRIVATE), 'utf8'))).blocks
@@ -1367,7 +1412,7 @@ function routeDebriefInput(eng, input, { dry, force, sealed = [] }) {
   // lines are never previewed and never routed into decisions/risks/stakeholders
   // unsealed. They land verbatim in context.md instead: the preview a human
   // approves is exactly what --apply writes.
-  const { clean: routable, blocks: inlinePrivate } = splitPrivate(input)
+  const { clean: routable, blocks: inlinePrivate } = splitPrivate(input, { sealDangling: true })
   const privateBlocks = [...inlinePrivate, ...sealed]
   for (const raw of routable.split('\n')) {
     let line = raw.trim()
@@ -1408,7 +1453,7 @@ function routeDebriefInput(eng, input, { dry, force, sealed = [] }) {
     if (dry) ctxLines.forEach(l => console.log(`→ context.md  - ${previewLine(l)}`))
     else {
       const bullets = ctxLines.length ? `${ctxLines.map(l => `- ${l}`).join('\n')}\n` : ''
-      const sealed = privateBlocks.length ? `${privateBlocks.join('\n')}\n` : ''
+      const sealed = privateBlocks.length ? sealedText(privateBlocks) : ''
       lockedAppendFile(path.join(eng, 'context.md'), `\n## Debrief - ${stamp}\n${bullets}${sealed}`)
     }
   }
@@ -1441,6 +1486,12 @@ function cmdDebrief(args) {
       process.exit(1)
     }
     sealed = readSealedProposal(eng)
+    const expected = readSealCount(eng)
+    if (expected === null ? (!sealed.length && input.includes(PRIVATE_MARKER)) : sealed.length < expected) {
+      console.error(`refused: the proposal seals a private note but ${DEBRIEF_PRIVATE} is missing or unreadable - applying now would drop it silently.`)
+      console.error('re-run the propose step (fde debrief --smart <notes> | fde ingest propose <id>).')
+      process.exit(1)
+    }
   } else {
     input = readDebriefInput(args)
   }
@@ -1466,6 +1517,7 @@ function cmdDebrief(args) {
     })
     try { fs.unlinkSync(path.join(eng, DEBRIEF_PROPOSE)) } catch (_) {}
     try { fs.unlinkSync(path.join(eng, DEBRIEF_PRIVATE)) } catch (_) {}
+  try { fs.unlinkSync(path.join(eng, DEBRIEF_SEAL)) } catch (_) {}
     if (hash) console.log(`memory @${hash}`)
   }
   const plural = {

bin/lib/memory.js

@@ -3,7 +3,7 @@
 const { execFileSync } = require('child_process')
 
 // Ephemeral sidecar files - never treated as "manual tamper" dirt.
-const MEMORY_EPHEMERAL = new Set(['.last-write', '.debrief-propose', '.debrief-private'])
+const MEMORY_EPHEMERAL = new Set(['.last-write', '.debrief-propose', '.debrief-private', '.debrief-seal'])
 
 function createMemoryApi(deps) {
   const { fs, path, gitBinOk, writeOwnerIfMissing, atomicWriteFile } = deps
@@ -131,7 +131,7 @@ function createMemoryApi(deps) {
       execFileSync('git', ['init'], { cwd: eng, stdio: 'ignore', timeout: 10000 })
       atomicWriteFile(
         path.join(eng, '.gitignore'),
-        ['*.lock', '*.tmp', '.last-write', '.debrief-propose', '.debrief-private', ''].join('\n')
+        ['*.lock', '*.tmp', '.last-write', '.debrief-propose', '.debrief-private', '.debrief-seal', ''].join('\n')
       )
       const owner = writeOwnerIfMissing(eng)
       configureMemoryGitIdentity(eng, owner)

docs/schema.md

@@ -55,7 +55,7 @@ After confirm, `fde ingest apply` writes thin dated facts into `.fde/` (same rou
 
 ## Rules
 
-1. **`<private>...</private>`** - redacted from CLI, dashboard, and hook-injected context. Nothing inside a block is ever routed into `decisions.md`/`risks.md`/`delivery.md`/`stakeholders.md`; `fde debrief`/`fde ingest` seal it verbatim into `context.md` instead, and hold it out of the agent-facing `.debrief-propose` in an owner-only `.debrief-private` sidecar that only `--apply` reads. Do not load raw blocks into the model via file tools or paste.
+1. **`<private>...</private>`** - redacted from CLI, dashboard, and hook-injected context. Nothing inside a block is ever routed into `decisions.md`/`risks.md`/`delivery.md`/`stakeholders.md`; `fde debrief`/`fde ingest` seal it verbatim into `context.md` instead, and hold it out of the agent-facing `.debrief-propose` in an owner-only (`0600`) `.debrief-private` sidecar that only `--apply` reads; a `.debrief-seal` receipt records how many blocks were sealed, so `--apply` refuses rather than silently dropping one if the sidecar disappears. Do not load raw blocks into the model via file tools or paste.
 2. Phases load files **on demand**, not the whole directory.
 3. **Do not** mix two customers in one `.fde/`.
 4. **Deliverable = memory:** `--init` creates only the core files; phase artifacts (`audit.md`, `chaos-log.md`, `handoff.md`, `evals.md`, …) are created by their phases when they run - formats live in [skills/fde/references/](../skills/fde/references/).

test/fde-cli.test.js

@@ -1824,6 +1824,124 @@ test('routable lines inside a <private> block are sealed, never routed unsealed'
   assert.doesNotMatch(fs.readFileSync(html, 'utf8'), /12345678/)
 })
 
+test('a secret hidden in an HTML comment never reaches a preview or memory', () => {
+  const sandbox = makeSandbox('private-comment')
+  assert.equal(runFde(sandbox, ['resume', '--init', 'commentco']).status, 0)
+  const eng = engagementPath(sandbox, 'commentco')
+  const notes = path.join(sandbox.workspace, 'notes.md')
+  fs.writeFileSync(notes, [
+    'Decided: renew the support contract.',
+    '<!-- Bank account for payout: 12345678 -->',
+    '<!-- unterminated comment hiding 87654321',
+    '',
+  ].join('\n'))
+
+  const smart = runFde(sandbox, ['debrief', '--smart', notes])
+  assert.equal(smart.status, 0, smart.stderr)
+  assert.doesNotMatch(smart.stdout, /12345678|87654321/)
+  assert.doesNotMatch(fs.readFileSync(path.join(eng, '.debrief-propose'), 'utf8'), /12345678|87654321/)
+
+  const dry = runFde(sandbox, ['debrief', '--dry-run'], { input: fs.readFileSync(notes, 'utf8') })
+  assert.doesNotMatch(dry.stdout, /12345678|87654321/)
+
+  assert.equal(runFde(sandbox, ['debrief', '--apply']).status, 0)
+  for (const f of ['context.md', 'decisions.md', 'risks.md']) {
+    assert.doesNotMatch(fs.readFileSync(path.join(eng, f), 'utf8'), /12345678|87654321/)
+  }
+  assert.match(fs.readFileSync(path.join(eng, 'decisions.md'), 'utf8'), /renew the support contract/)
+})
+
+test('apply refuses when the sealed sidecar went missing instead of dropping it', () => {
+  const sandbox = makeSandbox('sidecar-loss')
+  assert.equal(runFde(sandbox, ['resume', '--init', 'lossco']).status, 0)
+  const eng = engagementPath(sandbox, 'lossco')
+  const notes = path.join(sandbox.workspace, 'notes.md')
+  fs.writeFileSync(notes, [
+    'decision: ship the pilot in March',
+    '<private>',
+    'Bank account for payout: 12345678',
+    '</private>',
+    '',
+  ].join('\n'))
+
+  assert.equal(runFde(sandbox, ['debrief', '--smart', notes]).status, 0)
+  fs.unlinkSync(path.join(eng, '.debrief-private'))
+  const apply = runFde(sandbox, ['debrief', '--apply'])
+  assert.equal(apply.status, 1)
+  assert.match(apply.stderr, /missing or unreadable/)
+  assert.doesNotMatch(fs.readFileSync(path.join(eng, 'decisions.md'), 'utf8'), /ship the pilot/)
+
+  // a symlinked sidecar is refused without leaving an unbacked proposal behind
+  const outside = path.join(sandbox.dir, 'outside.md')
+  fs.writeFileSync(outside, 'untouched\n')
+  fs.unlinkSync(path.join(eng, '.debrief-propose'))
+  fs.symlinkSync(outside, path.join(eng, '.debrief-private'))
+  const refused = runFde(sandbox, ['debrief', '--smart', notes])
+  assert.equal(refused.status, 1)
+  assert.match(refused.stderr, /symlink/)
+  assert.equal(fs.readFileSync(outside, 'utf8'), 'untouched\n')
+  assert.equal(fs.existsSync(path.join(eng, '.debrief-propose')), false)
+  assert.equal(fs.existsSync(path.join(eng, '.debrief-private.lock')), false)
+})
+
+test('a stray <!-- already in memory does not hide later notes, and quoting the redaction marker still applies', () => {
+  const sandbox = makeSandbox('comment-scope')
+  assert.equal(runFde(sandbox, ['resume', '--init', 'scopeco']).status, 0)
+  const eng = engagementPath(sandbox, 'scopeco')
+
+  // stored memory: a dangling comment opener must not blank the rest of the file
+  fs.appendFileSync(path.join(eng, 'context.md'), '\n- note with a stray <!-- opener\n- later visible note about rollout\n')
+  const resume = runFde(sandbox, ['resume', '--full'])
+  assert.match(resume.stdout, /later visible note about rollout/)
+
+  // notes quoting "(private - redacted)" sealed nothing, so apply must not refuse
+  const notes = path.join(sandbox.workspace, 'notes.md')
+  fs.writeFileSync(notes, `decision: keep the audit trail\nresume printed (private - redacted) for that entry\n`)
+  assert.equal(runFde(sandbox, ['debrief', '--smart', notes]).status, 0)
+  const apply = runFde(sandbox, ['debrief', '--apply'])
+  assert.equal(apply.status, 0, apply.stderr)
+  assert.match(fs.readFileSync(path.join(eng, 'decisions.md'), 'utf8'), /keep the audit trail/)
+  assert.equal(fs.existsSync(path.join(eng, '.debrief-seal')), false)
+})
+
+test('an unclosed private note is balanced before storage and cannot swallow later notes', () => {
+  const sandbox = makeSandbox('unclosed-seal')
+  assert.equal(runFde(sandbox, ['resume', '--init', 'unclosedco']).status, 0)
+  const eng = engagementPath(sandbox, 'unclosedco')
+  const notes = path.join(sandbox.workspace, 'notes.md')
+  fs.writeFileSync(notes, [
+    'decision: ship the pilot in March',
+    '<private>',
+    'Bank account for payout: 12345678',
+    '',
+  ].join('\n'))
+
+  assert.equal(runFde(sandbox, ['debrief', '--smart', notes]).status, 0)
+  const sidecar = fs.readFileSync(path.join(eng, '.debrief-private'), 'utf8')
+  assert.match(sidecar, /<\/private>/)
+  assert.equal(fs.statSync(path.join(eng, '.debrief-private')).mode & 0o777, 0o600)
+  assert.equal(runFde(sandbox, ['debrief', '--apply']).status, 0)
+  assert.match(fs.readFileSync(path.join(eng, 'context.md'), 'utf8'), /12345678[\s\S]*<\/private>/)
+
+  fs.writeFileSync(notes, [
+    '<private>',
+    'first secret 12345678',
+    '<private>',
+    'second secret 87654321',
+    '',
+  ].join('\n'))
+  assert.equal(runFde(sandbox, ['debrief', notes]).status, 0)
+  const ctx = fs.readFileSync(path.join(eng, 'context.md'), 'utf8')
+  const tags = ctx.match(/<(\/)?private\b[^>]*>/gi) || []
+  assert.equal(tags.filter(t => t.startsWith('</')).length, tags.length / 2, ctx)
+
+  fs.writeFileSync(notes, 'later public note about the March rollout\n')
+  assert.equal(runFde(sandbox, ['debrief', notes]).status, 0)
+  const resume = runFde(sandbox, ['resume', '--full'])
+  assert.doesNotMatch(resume.stdout, /12345678/)
+  assert.match(resume.stdout, /later public note about the March rollout/)
+})
+
 test('near-miss <private> tags still seal instead of failing open', () => {
   const sandbox = makeSandbox('private-tags')
   assert.equal(runFde(sandbox, ['resume', '--init', 'tagco']).status, 0)