From 1c329f780c28c070294b8c5f2d32932a6cb42b7d Mon Sep 17 00:00:00 2001 From: thossullivan Date: Fri, 23 Oct 2026 18:22:50 -0500 Subject: [PATCH 1/4] Escape repository-controlled values in text and Markdown output alert --format markdown printed file paths, matched strings, and waiver owners and reasons as raw Markdown, so a crafted file name or waiver could inject links, images, headings, details blocks, or mentions into job summaries and PR comments. The text formats (check, inventory, schedule) and the stderr coverage notes passed control characters through, so a file name containing a newline could emit Actions workflow commands such as ::warning:: into the job log. Markdown alerts now render untrusted values as code spans whose fence outruns any backtick run in the value. Text output and stderr notes run those values through printable(). The GitHub annotation format keeps its existing escaping. The bot's hidden metadata comment now also encodes < and >, because HTML closes a comment at --!> as well as -->. Implemented by an Opus subagent; reviewed by Claude. --- bot/lib/common.mjs | 3 ++- bot/test/run.mjs | 3 +++ check.mjs | 2 +- lib/reports.mjs | 47 ++++++++++++++++++++++++---------------- test/run.mjs | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 88 insertions(+), 21 deletions(-) diff --git a/bot/lib/common.mjs b/bot/lib/common.mjs index 5f1e99b..33b3eb8 100644 --- a/bot/lib/common.mjs +++ b/bot/lib/common.mjs @@ -25,7 +25,8 @@ export const daysRemaining = (shutdown, now = new Date()) => { return Math.ceil((new Date(`${shutdown}T00:00:00Z`) - now) / 86400000) } -export const metadataLine = metadata => `', '\\u002d\\u002d\\u003e')} -->` +// HTML also closes a comment at --!>, so no raw angle bracket may reach the body +export const metadataLine = metadata => `', '\\u002d\\u002d\\u003e').replaceAll('<', '\\u003c').replaceAll('>', '\\u003e')} -->` export const hasMetadataMarker = body => String(body ?? '').includes(' b --> c' } +const hostileLine = metadataLine(hostileMetadata) +assert(!/[<>]/.test(hostileLine.slice(''.length)) && parseMetadata(hostileLine)?.replacement_note === hostileMetadata.replacement_note, 'metadata comments carry no raw angle bracket that could close them early and still round-trip') const clockDigestFeeds = path.join(tempRoot, 'clock-digest-feeds') fs.mkdirSync(clockDigestFeeds) const clockDigestRows = status => [ diff --git a/check.mjs b/check.mjs index 18ba6b9..1155a1e 100755 --- a/check.mjs +++ b/check.mjs @@ -474,7 +474,7 @@ if (incompleteNotes.length && (command === 'check' || command === 'plan')) { console.error(`model-eol: ${label}; ${incompleteNotes.length} coverage-loss note(s) recorded`) for (const note of incompleteNotes) { const location = note.file ? ` (${note.file})` : '' - console.error(` ${note.reason}${location}${note.message ? `: ${note.message}` : ''}`) + console.error(printable(` ${note.reason}${location}${note.message ? `: ${note.message}` : ''}`)) } if (!ALLOW_INCOMPLETE) return 2 } diff --git a/lib/reports.mjs b/lib/reports.mjs index eecbd76..f3a5d27 100644 --- a/lib/reports.mjs +++ b/lib/reports.mjs @@ -35,22 +35,31 @@ const lifecycleTail = (f, days) => f.status === 'watch' ? 'deprecation announced, no shutdown date yet' : 'no retirement scheduled' -const waiverTail = finding => !finding.waiver +const waiverTail = (finding, text) => !finding.waiver ? '' : finding.waiver.active - ? ` [waived until ${finding.waiver.expires} by ${finding.waiver.owner}: ${finding.waiver.reason}]` - : ` [waiver expired on ${finding.waiver.expires}; owner ${finding.waiver.owner}: ${finding.waiver.reason}]` + ? ` [waived until ${finding.waiver.expires} by ${text(finding.waiver.owner)}: ${text(finding.waiver.reason)}]` + : ` [waiver expired on ${finding.waiver.expires}; owner ${text(finding.waiver.owner)}: ${text(finding.waiver.reason)}]` -const captureTail = finding => { +const captureTail = (finding, text) => { if (!['retiring', 'retired'].includes(finding.status) || !finding.capture?.alternatives?.length) return '' const alternatives = finding.capture.alternatives.map(alternative => { const until = ['tentative', 'earliest'].includes(alternative.date_precision) ? 'at least until' : 'until' - return `via ${alternative.via} ${until} ${alternative.until}` + return `via ${text(alternative.via)} ${until} ${alternative.until}` }) return ` [still answers ${alternatives.join(', ')}]` } -export const findingTail = (finding, days) => `${lifecycleTail(finding, days)}${waiverTail(finding)}${captureTail(finding)}` +// text renders config and feed strings for the target format +export const findingTail = (finding, days, text = printable) => `${lifecycleTail(finding, days)}${waiverTail(finding, text)}${captureTail(finding, text)}` + +// a code span whose fence outruns every backtick run, so Markdown and HTML in the value stay literal +export const markdownCode = value => { + const text = printable(value) + const fence = '`'.repeat(Math.max(0, ...[...text.matchAll(/`+/g)].map(run => run[0].length)) + 1) + const pad = text === '' || /^[` ]|[` ]$/.test(text) ? ' ' : '' + return `${fence}${pad}${text}${pad}${fence}` +} const statusStyle = finding => finding.waiver?.active === true ? 'yellow' : @@ -225,8 +234,8 @@ export const formatCheck = ({ findings, bad, scannedFiles, days, scope }) => { out.push(color('bold', `model-eol: no tracked model IDs found in ${scannedFiles} files for --scope ${scope}`)) } for (const f of findings) { - const where = `${f.file}:${f.line}` - out.push(humanFinding(f, `${marker(f.status)} ${where} ${f.matched} ${findingTail(f, days)}${f.via && f.via !== 'publisher' ? ` [via ${f.via}]` : ''}${changedTail(f)}`)) + const where = `${printable(f.file)}:${f.line}` + out.push(humanFinding(f, `${marker(f.status)} ${where} ${printable(f.matched)} ${findingTail(f, days)}${f.via && f.via !== 'publisher' ? ` [via ${printable(f.via)}]` : ''}${changedTail(f)}`)) } if (bad.length) out.push(`\n${color('bold', `model-eol: ${bad.length} finding(s) at or past the ${days}-day threshold`)}`) return out.join('\n') @@ -239,20 +248,20 @@ export const formatInventory = (inventory, days) => { if (inventory.model_references.length) { out.push('\nModel references') for (const item of inventory.model_references) { - out.push(humanFinding(item, `${marker(item.status)} ${item.file}:${item.line} ${item.matched} ${item.publisher}/${item.id} ${item.usage} ${findingTail(item, days)}`)) + out.push(humanFinding(item, `${marker(item.status)} ${printable(item.file)}:${item.line} ${printable(item.matched)} ${printable(item.publisher)}/${printable(item.id)} ${item.usage} ${findingTail(item, days)}`)) } } if (inventory.candidate_model_references.length) { out.push('\nCandidate model references') for (const item of inventory.candidate_model_references) { - out.push(`? ${item.file}:${item.line} ${item.matched} ${item.usage} ${item.reason}`) + out.push(`? ${printable(item.file)}:${item.line} ${printable(item.matched)} ${item.usage} ${item.reason}`) } } if (inventory.integration_hints.length) { out.push('\nIntegration hints') for (const h of inventory.integration_hints) { const note = h.usage === 'direct-api' ? 'direct API signal' : 'resolver needed for exact deployed model' - out.push(color('dim', `? ${h.file}:${h.line} ${h.provider} ${h.evidence} (${note})`)) + out.push(color('dim', `? ${printable(h.file)}:${h.line} ${printable(h.provider)} ${printable(h.evidence)} (${note})`)) } } return out.join('\n') @@ -264,17 +273,17 @@ export const formatSchedule = (schedule, days) => { if (incompleteNotes.length) out.push(`WARNING: scan incomplete; ${incompleteNotes.length} coverage-loss note(s) recorded`) for (const item of schedule.items) { const safeUntil = item.safe_until ? ` [safe until ${item.safe_until}]` : '' - out.push(humanFinding(item, `${marker(item.status)} ${item.shutdown ? shutdownText(item) : 'TBD'} ${item.file}:${item.line} ${item.matched} ${findingTail(item, days)} [${item.usage}]${safeUntil}`)) + out.push(humanFinding(item, `${marker(item.status)} ${item.shutdown ? shutdownText(item) : 'TBD'} ${printable(item.file)}:${item.line} ${printable(item.matched)} ${findingTail(item, days)} [${item.usage}]${safeUntil}`)) } if (schedule.unresolved_integrations.length) { out.push('\nUnresolved cloud/gateway references') for (const h of schedule.unresolved_integrations) { - out.push(color('dim', `? ${h.file}:${h.line} ${h.provider} ${h.evidence} (needs provider/gateway resolver)`)) + out.push(color('dim', `? ${printable(h.file)}:${h.line} ${printable(h.provider)} ${printable(h.evidence)} (needs provider/gateway resolver)`)) } } for (const item of schedule._policy_guarantees ?? []) { const feedGenerated = String(item.feed_generated).slice(0, 10) - out.push(`guaranteed until ${item.safe_until} per ${item.publisher} stated policy (>=${item.policy.min_notice_days}d notice, feed generated ${feedGenerated})`) + out.push(`guaranteed until ${item.safe_until} per ${printable(item.publisher)} stated policy (>=${item.policy.min_notice_days}d notice, feed generated ${feedGenerated})`) } out.push(color('bold', schedule.earliest_risk ? `earliest risk: ${schedule.earliest_risk.safe_until} (${schedule.earliest_risk.id})` @@ -449,7 +458,7 @@ export const formatAlertGithub = (alert, days) => { if (incompleteNotes.length) out.push(`::warning title=${annotationEscape('model-eol scan')}::${annotationEscape(`Scan incomplete; ${incompleteNotes.length} coverage-loss note(s) recorded`)}`) for (const item of alert.errors) { const title = `model-eol ${item.status}: ${item.matched}` - out.push(`::error file=${annotationEscape(item.file)},line=${item.line},title=${annotationEscape(title)}::${annotationEscape(findingTail(item, days))}`) + out.push(`::error file=${annotationEscape(item.file)},line=${item.line},title=${annotationEscape(title)}::${annotationEscape(findingTail(item, days, String))}`) } for (const item of alert.warnings) { const title = item.kind === 'integration-hint' @@ -459,7 +468,7 @@ export const formatAlertGithub = (alert, days) => { ? `${item.evidence}; provider/gateway resolver needed` : item.status === 'unknown' ? `${item.matched} is model-like but not present in loaded feeds` - : findingTail(item, days) + : findingTail(item, days, String) out.push(`::warning file=${annotationEscape(item.file)},line=${item.line},title=${annotationEscape(title)}::${annotationEscape(message)}`) } if (!out.length) out.push('model-eol alert: no findings') @@ -475,7 +484,7 @@ export const formatAlertMarkdown = (alert, days) => { if (alert.errors.length) { out.push('', '## Errors') for (const item of alert.errors) { - out.push(`- ${item.file}:${item.line} \`${item.matched}\` - ${findingTail(item, days)}`) + out.push(`- ${markdownCode(`${item.file}:${item.line}`)} ${markdownCode(item.matched)} - ${findingTail(item, days, markdownCode)}`) } } if (alert.warnings.length) { @@ -486,8 +495,8 @@ export const formatAlertMarkdown = (alert, days) => { ? `${item.evidence}; resolver needed` : item.status === 'unknown' ? item.reason - : findingTail(item, days) - out.push(`- ${item.file}:${item.line} \`${label}\` - ${detail}`) + : findingTail(item, days, markdownCode) + out.push(`- ${markdownCode(`${item.file}:${item.line}`)} ${markdownCode(label)} - ${detail}`) } } return out.join('\n') diff --git a/test/run.mjs b/test/run.mjs index f42add6..e2cc775 100644 --- a/test/run.mjs +++ b/test/run.mjs @@ -247,6 +247,60 @@ assert(cyclonedx.metadata?.properties?.some(item => item.name === 'model-eol:gen const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'model-eol-test-')) +// Repository file names, scanned strings, and waiver text are untrusted in every human report. +const escapeDir = path.join(tempRoot, 'escape-names') +fs.mkdirSync(path.join(escapeDir, 'waived'), { recursive: true }) +for (const name of ['x\n## injected\n![p](https:example.com).py', 'esc\x1b[2Kok.py', path.join('waived', 'tick`code`.py')]) { + fs.writeFileSync(path.join(escapeDir, name), 'MODEL = "o3-deep-research"\n') +} +fs.writeFileSync(path.join(escapeDir, 'candidate.py'), 'model = "a` `b"\n') +fs.writeFileSync(path.join(escapeDir, '.model-eol.json'), JSON.stringify({ + waivers: [{ model: 'o3-deep-research', paths: ['waived/**'], owner: '', reason: 'see [link](https://example.com)\n# heading', expires: '2099-01-01' }], +})) +assert(spawnSync('git', ['init', '-q'], { cwd: escapeDir }).status === 0, 'escape fixture initializes a git root') +const outsideCodeSpans = line => { + let rest = '' + let index = 0 + while (index < line.length) { + const open = line.slice(index).match(/`+/) + if (!open) return rest + line.slice(index) + const start = index + open.index + const fence = open[0] + let close = -1 + for (let probe = start + fence.length; probe < line.length;) { + const next = line.slice(probe).match(/`+/) + if (!next) break + if (next[0].length === fence.length) { + close = probe + next.index + break + } + probe += next.index + next[0].length + } + if (close < 0) return rest + line.slice(index) + rest += line.slice(index, start) + index = close + fence.length + } + return rest +} +const escapeMarkdown = run(['alert', escapeDir, '--format', 'markdown', '--scope', 'all'], { env: colorEnv }) +const escapeMarkdownLines = escapeMarkdown.out.split('\n') +assert(escapeMarkdownLines.filter(line => line.startsWith('#')).join('|') === '# model-eol alert|## Errors|## Warnings', 'markdown alert file names cannot add headings or lines') +assert(escapeMarkdownLines.filter(line => line.startsWith('- ') && !/^- (Errors|Warnings): /.test(line)).length === 4, 'markdown alert keeps one list line per crafted finding') +assert(escapeMarkdownLines.every(line => !/[<]|!\[|\]\(/.test(outsideCodeSpans(line))), 'markdown alert keeps paths, matched strings, and waiver text inside code spans') +assert(escapeMarkdown.out.includes('``') && escapeMarkdown.out.includes('waived/tick`code`.py:1``'), 'markdown alert fences a backtick path with a longer run') +assert(escapeMarkdown.out.includes('by ``: `see [link](https://example.com)\\n# heading`'), 'markdown alert code-spans waiver owner and reason') +assert(!escapeMarkdown.out.includes('\x1b'), 'markdown alert shows control characters as escapes') +for (const command of ['check', 'inventory', 'schedule']) { + const human = run([command, escapeDir, '--scope', 'all', ...(command === 'inventory' ? ['--format', 'text'] : [])], { env: colorEnv }) + const lines = human.out.split('\n') + assert(!human.out.includes('\x1b') && !lines.some(line => /^(## injected|!\[p\]|# heading)/.test(line)), `${command} text shows control characters in names and waiver text as escapes`) +} +const noteDir = path.join(tempRoot, 'escape-note') +fs.mkdirSync(noteDir) +fs.writeFileSync(path.join(noteDir, 'bad\n::warning::forged.py'), Buffer.from([0xff, 0xfe, 0x0a])) +const noteCheck = run(['check', noteDir], { env: colorEnv }) +assert(noteCheck.code === 2 && noteCheck.err.includes('bad\\n::warning::forged.py') && !noteCheck.err.split('\n').some(line => line.startsWith('::')), 'incomplete-scan notes cannot start a workflow command line') + const anthropicAliasConsumerDir = path.join(tempRoot, 'anthropic-alias-consumer') const anthropicAliasConsumerFeeds = path.join(tempRoot, 'anthropic-alias-feeds') fs.mkdirSync(anthropicAliasConsumerDir) From 91ce931387bc6d150c952efde0d9f017b4d7391a Mon Sep 17 00:00:00 2001 From: thossullivan Date: Fri, 23 Oct 2026 20:08:34 -0500 Subject: [PATCH 2/4] Escape untrusted values on the remaining CLI and bot output paths Review of the first pass found the same bug class on paths it did not touch: config and policy errors on stderr echoed .model-eol.json text, apply printed plan file paths raw, bot issue evidence and stale-work comments kept newlines, mentions, table pipes, and autolinks, U+2028 or U+2029 in feed text broke the bot's metadata parse (duplicate issues and permanent PR conflicts), and the runner's legacy ##[ command form matches anywhere in a line. printable() moves to lib/printable.mjs and also escapes bidi controls, zero-width marks, and line separators, and breaks ##[. printableLines() keeps intended line breaks in multi-line errors but stops any line from starting a :: command. The bot's markdownText now produces one inline line with @ and | encoded and bare autolinks broken; repository paths, matched strings, and waiver text in bot bodies are code spans. metadataLine encodes U+2028 and U+2029. JSON outputs, GitHub annotations, and ordinary-input text are byte-identical to main. Implemented by an Opus subagent; reviewed by Claude. --- bot/bot.mjs | 9 +++++---- bot/lib/common.mjs | 28 ++++++++++++++++------------ bot/test/run.mjs | 30 +++++++++++++++++++++++++++++- check.mjs | 17 +++++++++-------- lib/apply.mjs | 13 +++++++------ lib/printable.mjs | 13 +++++++++++++ lib/reports.mjs | 15 +++++++++------ lib/verify.mjs | 8 ++------ test/run.mjs | 41 ++++++++++++++++++++++++++++++++++++++++- 9 files changed, 130 insertions(+), 44 deletions(-) create mode 100644 lib/printable.mjs diff --git a/bot/bot.mjs b/bot/bot.mjs index 7dd06b8..9c22bc8 100755 --- a/bot/bot.mjs +++ b/bot/bot.mjs @@ -37,6 +37,7 @@ import { import { assertValidPlanDocument } from '../lib/validate-document.mjs' import { captureWindowFor, lifecycleFor } from '../lib/feeds.mjs' import { parseCliArgs } from '../lib/cli.mjs' +import { printable, printableLines } from '../lib/printable.mjs' import { EVAL_RESULTS_SCHEMA as EVAL_SCHEMA, assertValidEvalResults } from '../lib/receipts.mjs' import { loadConfig } from './lib/config.mjs' import { downloadFeeds } from './lib/feeds.mjs' @@ -508,7 +509,7 @@ export const buildIssueBody = ({ group, now = new Date() }) => { ...(group.channel ? { channel: group.channel } : {}), } const evidence = group.issues - .map(issue => `- ${markdownText(`${repoPath(issue.file, group.root || '.')}:${issue.line} - ${issue.reason}${issue.matched ? ` (${issue.matched})` : ''}`)}`) + .map(issue => `- ${markdownCode(`${repoPath(issue.file, group.root || '.')}:${issue.line}`)} - ${markdownText(issue.reason)}${issue.matched ? ` (${markdownCode(issue.matched)})` : ''}`) .join('\n') const sections = [ metadataLine(metadata), @@ -1285,7 +1286,7 @@ export const formatDecisions = (decisions, { degraded = false } = {}) => { if (record.branch) lines.push(` branch: ${record.branch}`) if (record.body) lines.push(' body:', record.body.split('\n').map(line => ` ${line}`).join('\n')) } - return lines.join('\n') + return printableLines(lines.join('\n')) } const BLOCKED_ACTIONS = new Set(['conflict', 'eval-failed', 'label-unavailable', 'report-only', 'stand-down']) @@ -1531,7 +1532,7 @@ export const main = async (argv = process.argv.slice(2), env = process.env) => { if (options.dryRun) console.log(formatDecisions(result.decisions, { degraded: result.degraded })) else { if (result.degraded) console.log('model-eol status: degraded - vendored feed fallback enabled; no GitHub writes') - for (const item of result.decisions) console.log(`${item.action} ${item.group.publisher}/${item.group.kind === 'model' ? item.group.id : item.group.subject}`) + for (const item of result.decisions) console.log(printable(`${item.action} ${item.group.publisher}/${item.group.kind === 'model' ? item.group.id : item.group.subject}`)) } appendDecisionSummary(env.GITHUB_STEP_SUMMARY, result.decisions, { degraded: result.degraded }) return result.degraded || result.decisions.some(item => BLOCKED_ACTIONS.has(item.action)) ? 1 : 0 @@ -1548,7 +1549,7 @@ const invokedFile = (() => { if (invokedFile === fs.realpathSync(fileURLToPath(import.meta.url))) { main().then(code => process.exit(code)).catch(error => { - console.error(`model-eol bot: ${error.message}`) + console.error(printableLines(`model-eol bot: ${error.message}`)) process.exit(2) }) } diff --git a/bot/lib/common.mjs b/bot/lib/common.mjs index 33b3eb8..4f4f0cc 100644 --- a/bot/lib/common.mjs +++ b/bot/lib/common.mjs @@ -1,4 +1,5 @@ import { sha256 } from '../../lib/digests.mjs' +import { printable } from '../../lib/printable.mjs' export { matchesAnyGlob, matchesGlob } from '../../lib/glob.mjs' export { activeWaiver, itemDigest, repoPath, sha256, sortedJsonDigest, stableJson } from '../../lib/digests.mjs' @@ -25,8 +26,8 @@ export const daysRemaining = (shutdown, now = new Date()) => { return Math.ceil((new Date(`${shutdown}T00:00:00Z`) - now) / 86400000) } -// HTML also closes a comment at --!>, so no raw angle bracket may reach the body -export const metadataLine = metadata => `', '\\u002d\\u002d\\u003e').replaceAll('<', '\\u003c').replaceAll('>', '\\u003e')} -->` +// HTML also closes a comment at --!>, and parseMetadata's . stops at U+2028 and U+2029 +export const metadataLine = metadata => `', '\\u002d\\u002d\\u003e').replaceAll('<', '\\u003c').replaceAll('>', '\\u003e').replace(/[\u2028\u2029]/g, character => `\\u${character.charCodeAt(0).toString(16)}`)} -->` export const hasMetadataMarker = body => String(body ?? '').includes(' b --> c' } const hostileLine = metadataLine(hostileMetadata) assert(!/[<>]/.test(hostileLine.slice(''.length)) && parseMetadata(hostileLine)?.replacement_note === hostileMetadata.replacement_note, 'metadata comments carry no raw angle bracket that could close them early and still round-trip') +const lineSeparator = String.fromCharCode(0x2028) +const separatorMetadata = { ...hostileMetadata, replacement_note: `a${lineSeparator}b${String.fromCharCode(0x2029)}c` } +const separatorLine = metadataLine(separatorMetadata) +assert(!separatorLine.includes(lineSeparator) && parseMetadata(separatorLine)?.replacement_note === separatorMetadata.replacement_note, 'metadata comments encode U+2028 and U+2029 so the metadata line still parses') +const hostileText = markdownText('azure/x\n---\n@thossullivan see https://evil.example www.evil.example |a|b| \x1b[2K') +assert(!/[\n\r@|\x1b]|:\/\/|www\./i.test(hostileText) && hostileText.includes('@thossullivan') && hostileText.includes('|'), 'markdown text stays on one line and neutralizes mentions, table pipes, autolinks, and controls') +const hostileIssue = buildIssueBody({ + group: { + kind: 'issue', + id: 'o3-deep-research-2025-06-26', + subject: 'o3-deep-research-2025-06-26', + publisher: 'openai', + shutdown: '2026-07-23', + via: null, + root: '.', + feedDigest: 'd', + context: { announced: null, notes: [], entry: null }, + issues: [{ file: 'azure/x\n---\n@thossullivan see https://evil.example now.py', line: 3, reason: 'not-direct-api', status: 'retired', matched: 'o3-deep-research' }], + }, + now: new Date('2026-08-01T00:00:00Z'), +}) +const hostileIssueLines = hostileIssue.split('\n') +const evidenceLines = hostileIssueLines.slice(hostileIssueLines.indexOf('## Evidence') + 1, hostileIssueLines.indexOf('## Sources') - 1) +assert(evidenceLines.length === 1 && evidenceLines[0] === '- `azure/x --- @thossullivan see https:/evil.example now.py:3` - not-direct-api (`o3-deep-research`)' && !hostileIssueLines.includes('---'), 'issue evidence code-spans repository paths so they cannot add rules, mentions, or links') +const waivedComment = staleWorkComment('issue', { expires: '2099-01-01', owner: '@thossullivan', reason: 'see [x](https://evil.example)\n# h' }) +assert(waivedComment.includes('owned by `@thossullivan`') && waivedComment.includes('records: `see [x](https://evil.example) # h`') && !waivedComment.includes('\n'), 'stale-work comments code-span waiver owner and reason') +const decisionLog = formatDecisions([{ action: 'create', group: { kind: 'issue', publisher: 'openai', subject: 'x\n::error::forged' }, body: 'line\n::warning title=forged::x\n ##[error]legacy\n\x1b[2K' }]) +assert(decisionLog.startsWith('model-eol decision table\n') && !decisionLog.split('\n').some(line => line.trimStart().startsWith('::') || line.includes('##[') || line.includes('\x1b')), 'dry-run decision tables cannot open workflow commands') const clockDigestFeeds = path.join(tempRoot, 'clock-digest-feeds') fs.mkdirSync(clockDigestFeeds) const clockDigestRows = status => [ diff --git a/check.mjs b/check.mjs index 1155a1e..55937ff 100755 --- a/check.mjs +++ b/check.mjs @@ -52,6 +52,7 @@ import { BaseUnavailableError, inCheckScope, selectChangedFindings } from './lib import { gitRootFor, incompleteScanNotes, scanTargets } from './lib/scanner.mjs' import { DOCUMENT_TYPES, formatDocumentErrors, validateDocumentFile } from './lib/validate-document.mjs' import { SUPPORTED_RECEIPT_SCHEMAS, VERIFY_INPUTS, VerifyError, formatVerification, printable, verifyReceipt } from './lib/verify.mjs' +import { printableLines } from './lib/printable.mjs' const rootForTarget = target => { const absolute = path.resolve(target) @@ -349,7 +350,7 @@ if (command === 'apply') { const result = applyPlan({ planPath: PLAN_FILE, dryRun: DRY_RUN, rootDir: process.cwd() }) return result.failed ? 1 : 0 } catch (e) { - console.error(`failed to apply plan ${PLAN_FILE}: ${e.message}`) + console.error(printableLines(`failed to apply plan ${PLAN_FILE}: ${e.message}`)) return 2 } } @@ -371,13 +372,13 @@ if (command === 'validate') { try { const result = validateDocumentFile(file, { type: values.type ?? null }) if (result.errors.length) { - for (const error of formatDocumentErrors(result.errors)) console.error(`${file}: ${error}`) + for (const error of formatDocumentErrors(result.errors)) console.error(printableLines(`${file}: ${error}`)) failed = true continue } console.log(`${file}: valid ${result.type} document`) } catch (error) { - console.error(`${file}: validation failed: ${error.message}`) + console.error(printableLines(`${file}: validation failed: ${error.message}`)) failed = true } } @@ -397,7 +398,7 @@ try { ? loadConfig(configLocation.file, { defaults: DEFAULT_CONFIG, allowMissing: false }) : normalizeConfig({}, { defaults: CLI_DEFAULT_CONFIG }) } catch (e) { - console.error(`failed to load repository config: ${e.message}`) + console.error(printable(`failed to load repository config: ${e.message}`)) return 2 } @@ -431,7 +432,7 @@ let feedData try { feedData = loadFeeds(FEEDS_DIR) } catch (e) { - console.error(`failed to load feeds from ${FEEDS_DIR}: ${e.message}`) + console.error(printableLines(`failed to load feeds from ${FEEDS_DIR}: ${e.message}`)) return 2 } if (feedData.entries.size === 0) { @@ -441,7 +442,7 @@ if (feedData.entries.size === 0) { const headPolicyError = policyError(repositoryConfig, feedData, values.via) if (headPolicyError) { - console.error(headPolicyError) + console.error(printable(headPolicyError)) return 2 } @@ -464,7 +465,7 @@ try { includeDocs: INCLUDE_DOCS, })) } catch (e) { - console.error(`scan failed: ${e.message}`) + console.error(printableLines(`scan failed: ${e.message}`)) return 2 } @@ -541,7 +542,7 @@ if (CHANGED_BASE !== null) { changedFindings = selection.findings changedStats = selection.stats } catch (e) { - console.error(`--changed failed: ${e.message}`) + console.error(printableLines(`--changed failed: ${e.message}`)) return 2 } const statsFile = process.env.MODEL_EOL_CHANGED_STATS diff --git a/lib/apply.mjs b/lib/apply.mjs index 3915821..7b81d9d 100644 --- a/lib/apply.mjs +++ b/lib/apply.mjs @@ -3,6 +3,7 @@ import fs from 'node:fs' import path from 'node:path' import { assertValidPlanDocument, readJsonDocument, validatePlanItems } from './validate-document.mjs' +import { printable } from './printable.mjs' export { validatePlanItems } @@ -21,10 +22,10 @@ const occurrenceIndex = (line, value, occurrence) => { return -1 } -const itemLabel = item => `${item.file}:${item.line}` +const itemLabel = item => `${printable(item.file)}:${printable(item.line)}` const printError = (item, message) => { - console.error(`model-eol: apply refused ${itemLabel(item)}: ${message}`) + console.error(`model-eol: apply refused ${itemLabel(item)}: ${printable(message)}`) } const isInside = (root, candidate) => { @@ -319,7 +320,7 @@ const reportAlready = (analyses, excluded = new Set()) => { const items = sortedItems(analyses.flatMap(analysis => analysis.already)) for (const item of items) { if (excluded.has(item)) continue - console.log(`already-applied ${itemLabel(item)}: ${item.matched} -> ${item.replacement}`) + console.log(`already-applied ${itemLabel(item)}: ${printable(item.matched)} -> ${printable(item.replacement)}`) alreadyApplied++ } return alreadyApplied @@ -398,7 +399,7 @@ const cleanupFailureDetail = failures => failures.length const reportCleanupFailures = failures => { for (const { temporaryFile, error } of failures) { - console.error(`model-eol: apply cleanup failed ${temporaryFile}: ${error.message}`) + console.error(printable(`model-eol: apply cleanup failed ${temporaryFile}: ${error.message}`)) } } @@ -451,7 +452,7 @@ export const applyPlan = ({ planPath, dryRun = false, rootDir, _test } = {}) => if (dryRun) { const alreadyApplied = reportAlready(initial.analyses) for (const item of sortedItems(initiallyReady)) { - console.log(`would change ${itemLabel(item)}: ${item.matched} -> ${item.replacement}`) + console.log(`would change ${itemLabel(item)}: ${printable(item.matched)} -> ${printable(item.replacement)}`) } return { failed: 0, applied: 0, alreadyApplied } } @@ -565,7 +566,7 @@ export const applyPlan = ({ planPath, dryRun = false, rootDir, _test } = {}) => reportCleanupFailures(cleanupFailures) const alreadyApplied = reportAlready(current.analyses) for (const item of sortedItems(currentReady)) { - console.log(`applied ${itemLabel(item)}: ${item.matched} -> ${item.replacement}`) + console.log(`applied ${itemLabel(item)}: ${printable(item.matched)} -> ${printable(item.replacement)}`) } return { failed: cleanupFailures.length, applied: currentReady.length, alreadyApplied } } diff --git a/lib/printable.mjs b/lib/printable.mjs new file mode 100644 index 0000000..d968ea2 --- /dev/null +++ b/lib/printable.mjs @@ -0,0 +1,13 @@ +const NAMED_CONTROLS = new Map([['\n', '\\n'], ['\r', '\\r'], ['\t', '\\t']]) + +// controls, bidi overrides, zero-width marks, and line separators all print as escapes +const HIDDEN = /[\u0000-\u001f\u007f-\u009f\u061c\u200b-\u200f\u2028-\u202e\u2060-\u2064\u2066-\u2069\ufeff]/g + +// the Actions runner reads ##[ as a legacy workflow command anywhere in a line +export const printable = value => String(value).replace(HIDDEN, character => { + const code = character.charCodeAt(0) + return NAMED_CONTROLS.get(character) ?? (code < 0x80 ? `\\x${code.toString(16).padStart(2, '0')}` : `\\u${code.toString(16).padStart(4, '0')}`) +}).replaceAll('##[', '#\\x23[') + +// keeps intentional line breaks, but no line may start a :: workflow command +export const printableLines = value => String(value).split('\n').map(line => printable(line).replace(/^(\s*):(?=:)/, '$1\\x3a')).join('\n') diff --git a/lib/reports.mjs b/lib/reports.mjs index f3a5d27..c6112d8 100644 --- a/lib/reports.mjs +++ b/lib/reports.mjs @@ -1,7 +1,7 @@ import { isBad } from './feeds.mjs' import { color } from './color.mjs' import { incompleteScanNotes } from './scanner.mjs' -import { printable } from './verify.mjs' +import { printable } from './printable.mjs' export const marker = status => status === 'retired' ? '✗' : @@ -16,8 +16,11 @@ const shutdownText = finding => const tentativeFloor = finding => finding.date_precision === 'tentative' -const policyTail = finding => finding.safe_until && finding.shutdown && finding.safe_until > finding.shutdown - ? `, guaranteed until ${finding.safe_until} by ${finding.publisher} policy` +// plain identifiers stay as they are, so only unusual feed text changes shape +const word = (value, text) => /^[A-Za-z0-9][A-Za-z0-9.-]*$/.test(value) ? value : text(value) + +const policyTail = (finding, text) => finding.safe_until && finding.shutdown && finding.safe_until > finding.shutdown + ? `, guaranteed until ${finding.safe_until} by ${word(finding.publisher, text)} policy` : '' const replacementTail = finding => finding.replacement @@ -26,11 +29,11 @@ const replacementTail = finding => finding.replacement ? ` options: ${finding.replacement_options.join(' | ')}` : '' -const lifecycleTail = (f, days) => +const lifecycleTail = (f, days, text) => f.status === 'retired' && !f.shutdown ? `RETIRED (shutdown date unavailable)${replacementTail(f)}` : f.status === 'retired' ? `RETIRED ${shutdownText(f)} (${-f.days} days ago)${replacementTail(f)}` : f.status === 'retiring' ? `RETIRES ${shutdownText(f)} (${f.days} days)${replacementTail(f)}` : - tentativeFloor(f) ? `scheduled - tentative, not announced: ${shutdownText(f)}${policyTail(f)}` : + tentativeFloor(f) ? `scheduled - tentative, not announced: ${shutdownText(f)}${policyTail(f, text)}` : f.status === 'scheduled' ? `scheduled ${shutdownText(f)} (${f.days} days, outside --days ${f.threshold_days ?? days})` : f.status === 'watch' ? 'deprecation announced, no shutdown date yet' : 'no retirement scheduled' @@ -51,7 +54,7 @@ const captureTail = (finding, text) => { } // text renders config and feed strings for the target format -export const findingTail = (finding, days, text = printable) => `${lifecycleTail(finding, days)}${waiverTail(finding, text)}${captureTail(finding, text)}` +export const findingTail = (finding, days, text = printable) => `${lifecycleTail(finding, days, text)}${waiverTail(finding, text)}${captureTail(finding, text)}` // a code span whose fence outruns every backtick run, so Markdown and HTML in the value stay literal export const markdownCode = value => { diff --git a/lib/verify.mjs b/lib/verify.mjs index 4ab83d5..8e42480 100644 --- a/lib/verify.mjs +++ b/lib/verify.mjs @@ -3,6 +3,7 @@ import path from 'node:path' import { TextDecoder } from 'node:util' import { loadConfig } from './config.mjs' +import { printable } from './printable.mjs' import { effectiveEval, evalConfigDigest, evalIdentity, feedContext, modelGroups, planDigest, sha256 } from './digests.mjs' import { EVAL_RESULTS_SCHEMA, @@ -26,12 +27,7 @@ export class VerifyError extends Error { } } -const NAMED_CONTROLS = new Map([['\n', '\\n'], ['\r', '\\r'], ['\t', '\\t']]) - -export const printable = value => String(value).replace(/[\u0000-\u001f\u007f-\u009f]/g, character => { - const code = character.charCodeAt(0) - return NAMED_CONTROLS.get(character) ?? (code < 0x80 ? `\\x${code.toString(16).padStart(2, '0')}` : `\\u${code.toString(16).padStart(4, '0')}`) -}) +export { printable } const refuse = message => { throw new VerifyError(message) diff --git a/test/run.mjs b/test/run.mjs index e2cc775..9f65210 100644 --- a/test/run.mjs +++ b/test/run.mjs @@ -14,7 +14,7 @@ import { color, colorEnabled } from '../lib/color.mjs' import { assertIsoDate, buildModelPattern, captureWindowFor, findingFromRef, lifecycleFor, loadFeeds } from '../lib/feeds.mjs' import { buildPlan } from '../lib/plan.mjs' import { JsonSchemaRegistry, validateJsonSchema } from '../lib/json-schema.mjs' -import { formatCheck, formatSchedule } from '../lib/reports.mjs' +import { formatAlertMarkdown, formatCheck, formatSchedule } from '../lib/reports.mjs' import { SKIP_DIRS, changedLinesForTargets, isScannable, parseDiffPath } from '../lib/scanner.mjs' import { validateDocument } from '../lib/validate-document.mjs' import { PROVIDERS, loadProviderSources, mergeFeed } from '../refresh/providers.mjs' @@ -300,6 +300,45 @@ fs.mkdirSync(noteDir) fs.writeFileSync(path.join(noteDir, 'bad\n::warning::forged.py'), Buffer.from([0xff, 0xfe, 0x0a])) const noteCheck = run(['check', noteDir], { env: colorEnv }) assert(noteCheck.code === 2 && noteCheck.err.includes('bad\\n::warning::forged.py') && !noteCheck.err.split('\n').some(line => line.startsWith('::')), 'incomplete-scan notes cannot start a workflow command line') +const opensCommand = text => text.split('\n').some(line => line.trimStart().startsWith('::') || line.includes('##[')) +const hostileConfigDir = path.join(tempRoot, 'escape-config') +fs.mkdirSync(hostileConfigDir) +fs.writeFileSync(path.join(hostileConfigDir, 'a.py'), 'MODEL = "o3-deep-research"\n') +for (const [label, config] of [ + ['an unknown lifecycle channel', JSON.stringify({ via: 'x\n::warning::forged ##[error]legacy' })], + ['a route model missing from feeds', JSON.stringify({ routes: [{ paths: ['**'], via: 'aws-bedrock', match: 'm', model: 'x\n ::error::forged' }] })], + ['an unknown config key', JSON.stringify({ 'k\n::error::forged': 1 })], + ['a JSON.parse excerpt of raw bytes', 'x\n::error::forged'], +]) { + fs.writeFileSync(path.join(hostileConfigDir, '.model-eol.json'), config) + const result = run(['check', hostileConfigDir], { env: colorEnv }) + assert(result.code === 2 && result.err.length > 0 && !opensCommand(result.err), `config errors for ${label} cannot start a workflow command`) +} +const hostileFeedsDir = path.join(tempRoot, 'escape-feeds') +fs.mkdirSync(hostileFeedsDir) +fs.writeFileSync(path.join(hostileFeedsDir, 'x.json'), JSON.stringify({ spec: 'model-eol/0.1', publisher: 'p\n::warning::forged', generated: '2026-01-01T00:00:00Z', models: [{ id: 'bad\n::error::forged' }] })) +fs.writeFileSync(path.join(tempRoot, 'escape-empty.json'), '{}') +const hostileFeeds = run(['check', path.join(hostileConfigDir, 'a.py'), '--config', path.join(tempRoot, 'escape-empty.json'), '--feeds', hostileFeedsDir], { env: colorEnv }) +assert(hostileFeeds.code === 2 && hostileFeeds.err.includes('failed to load') && !opensCommand(hostileFeeds.err), 'feed load errors keep their lines but cannot start a workflow command') +const escapeApplyDir = path.join(tempRoot, 'escape-apply') +fs.mkdirSync(escapeApplyDir) +fs.writeFileSync(path.join(escapeApplyDir, 'x\n::warning title=forged::apply ##[warning]legacy.py'), 'from openai import OpenAI\nclient = OpenAI()\nclient.responses.create(model="o3-deep-research")\n') +const applyPlanRun = run(['plan', '.', '--days', '90'], { cwd: escapeApplyDir, env: colorEnv }) +fs.writeFileSync(path.join(tempRoot, 'escape-apply-plan.json'), applyPlanRun.out) +const applyDry = run(['apply', '--plan', path.join(tempRoot, 'escape-apply-plan.json'), '--dry-run'], { cwd: escapeApplyDir, env: colorEnv }) +assert(applyDry.code === 0 && applyDry.out.includes('would change x\\n') && !opensCommand(applyDry.out), 'apply prints plan file paths with control characters escaped') +const legacyDir = path.join(tempRoot, 'escape-legacy') +fs.mkdirSync(legacyDir) +fs.writeFileSync(path.join(legacyDir, 'a ##[warning]forged \u202e.py'), 'MODEL = "o3-deep-research"\n') +for (const args of [['check'], ['inventory', '--format', 'text'], ['schedule'], ['alert', '--format', 'markdown']]) { + const human = run([...args, legacyDir], { env: colorEnv }) + assert(human.out.includes('a #\\x23[warning]forged \\u202e.py') && !opensCommand(human.out), `${args.join(' ')} neutralizes legacy ##[ commands and bidi overrides in names`) +} +const legacyJson = JSON.parse(run(['check', legacyDir, '--json'], { env: colorEnv }).out) +assert(legacyJson.findings[0]?.file.endsWith('a ##[warning]forged \u202e.py'), 'JSON output keeps names byte-for-byte') +const tentativeHostile = { file: 'a.py', line: 1, matched: 'm', status: 'scheduled', date_precision: 'tentative', shutdown: '2027-01-01', safe_until: '2027-06-01', publisher: '[x](https://evil.example)\n# h' } +const tentativeMarkdown = formatAlertMarkdown({ scan_notes: [], errors: [], warnings: [tentativeHostile] }, 30) +assert(tentativeMarkdown.includes('by `[x](https://evil.example)\\n# h` policy') && formatAlertMarkdown({ scan_notes: [], errors: [], warnings: [{ ...tentativeHostile, publisher: 'openai' }] }, 30).includes('by openai policy'), 'markdown alert code-spans only unusual feed publishers in the policy tail') const anthropicAliasConsumerDir = path.join(tempRoot, 'anthropic-alias-consumer') const anthropicAliasConsumerFeeds = path.join(tempRoot, 'anthropic-alias-feeds') From 22032f3c4e4a98bfd59930d8b9f9719feda7cf42 Mon Sep 17 00:00:00 2001 From: thossullivan Date: Fri, 23 Oct 2026 20:42:16 -0500 Subject: [PATCH 3/4] Stop mentions, JSON ##[ commands, and raw warnings reaching GitHub GitHub resolves @mentions and #references after decoding HTML entities, so the @ and # encoding from the previous commit still produced live mentions and issue links when rendered. markdownText now inserts a zero-width space after @ and # (and in www. and ://) instead. The Action prints JSON reports into the job log, and the runner's legacy command form matches ##[ anywhere in a line, so a file named "x ##[set-output ...]" reached the log through check --json, plan, inventory, schedule, alert, and verify --json. Printed JSON now writes the second hash as #, which JSON.parse reads back unchanged; objects, digests, receipts, and files written by apply are untouched. The bot's evaluate output, its feed-download and GitHub API warnings, and markdownLink labels (which could show github.com while linking elsewhere) now go through the same escaping. Implemented by an Opus subagent; reviewed by Claude. --- bot/bot.mjs | 11 +++++++---- bot/lib/common.mjs | 14 ++++++++------ bot/lib/feeds.mjs | 3 ++- bot/lib/github.mjs | 4 +++- bot/test/run.mjs | 41 ++++++++++++++++++++++++++++++++++++++++- check.mjs | 20 ++++++++++---------- lib/printable.mjs | 3 +++ test/run.mjs | 24 ++++++++++++++++++++++++ 8 files changed, 97 insertions(+), 23 deletions(-) diff --git a/bot/bot.mjs b/bot/bot.mjs index 9c22bc8..c21ca8e 100755 --- a/bot/bot.mjs +++ b/bot/bot.mjs @@ -12,6 +12,7 @@ import { branchFor, daysRemaining, hasMetadataMarker, + logWarning, markdownCode, markdownLink, markdownText, @@ -165,7 +166,7 @@ const validateRepo = repo => { return repo } -const spawnPlan = ({ workDir, config, configPath = null, feedsDir, checkerPath = CHECKER, warn = console.error }) => { +const spawnPlan = ({ workDir, config, configPath = null, feedsDir, checkerPath = CHECKER, warn = logWarning }) => { const args = [checkerPath, 'plan', '.'] if (configPath) args.push('--config', configPath) else { @@ -741,7 +742,7 @@ export const evaluatePlan = async ({ fetchImpl = globalThis.fetch, checkerPath = CHECKER, vendoredFeeds = VENDORED_FEEDS, - warn = message => console.error(message), + warn = logWarning, now = new Date(), } = {}) => { if (!planFile) throw new Error('--plan-file is required for evaluate') @@ -1349,7 +1350,7 @@ export const runBot = async ({ evalResultsFile = null, evalCommandOverride = null, now = new Date(), - warn = message => console.error(message), + warn = logWarning, } = {}) => { if (deprecatedInlineEval || evalEnabled) { throw new Error('inline --eval was removed because publication is write-capable; run `model-eol-bot evaluate` in a read-only job and pass its manifest with MODEL_EOL_EVAL_RESULTS_FILE') @@ -1489,6 +1490,8 @@ export const runBot = async ({ } } +export const evalResultLines = results => results.map(item => printable(`${item.status} ${item.publisher}/${item.id}`)) + export const main = async (argv = process.argv.slice(2), env = process.env) => { if (argv[0] === 'evaluate') { const options = parseEvaluateArgs(argv.slice(1)) @@ -1505,7 +1508,7 @@ export const main = async (argv = process.argv.slice(2), env = process.env) => { commandOverride: env.MODEL_EOL_EVAL_COMMAND || null, }) appendEvalSummary(env.GITHUB_STEP_SUMMARY, result.artifact) - for (const item of result.artifact.results) console.log(`${item.status} ${item.publisher}/${item.id}`) + for (const line of evalResultLines(result.artifact.results)) console.log(line) return 0 } const options = parseArgs(argv) diff --git a/bot/lib/common.mjs b/bot/lib/common.mjs index 4f4f0cc..6fc121f 100644 --- a/bot/lib/common.mjs +++ b/bot/lib/common.mjs @@ -1,5 +1,5 @@ import { sha256 } from '../../lib/digests.mjs' -import { printable } from '../../lib/printable.mjs' +import { printable, printableLines } from '../../lib/printable.mjs' export { matchesAnyGlob, matchesGlob } from '../../lib/glob.mjs' export { activeWaiver, itemDigest, repoPath, sha256, sortedJsonDigest, stableJson } from '../../lib/digests.mjs' @@ -72,12 +72,12 @@ const entityMap = new Map([ [']', ']'], ['(', '('], [')', ')'], - ['#', '#'], + ['#', '#​'], ['*', '*'], ['_', '_'], ['~', '~'], ['!', '!'], - ['@', '@'], + ['@', '@​'], ['|', '|'], ]) @@ -87,8 +87,8 @@ const markdownInline = value => printable(String(value ?? '').replace(/\r\n|[\r\ // one line of text: no block structure, table cells, or bare autolinks export const markdownText = value => markdownInline(value) - .replace(/(www)\./gi, '$1.') - .replaceAll('://', '://') + .replace(/(www)\./gi, '$1.​') + .replaceAll('://', ':​//') // code spans show entities literally, so drop backticks and line breaks instead of escaping export const markdownCode = value => { @@ -102,10 +102,12 @@ export const markdownLink = (label, value) => { const url = new URL(text) if (url.protocol !== 'http:' && url.protocol !== 'https:') return markdownText(text) const href = url.toString().replaceAll('\\', '%5C').replaceAll('(', '%28').replaceAll(')', '%29') - return `[${markdownInline(label)}](${href})` + return `[${markdownInline(label === text ? href : label)}](${href})` } catch { return markdownText(text) } } export const markdownFenceText = value => String(value ?? '').replaceAll('`', '') + +export const logWarning = message => console.error(printableLines(message)) diff --git a/bot/lib/feeds.mjs b/bot/lib/feeds.mjs index ea9b468..ba27f6e 100644 --- a/bot/lib/feeds.mjs +++ b/bot/lib/feeds.mjs @@ -3,6 +3,7 @@ import os from 'node:os' import path from 'node:path' import { assertValidFeed } from '../../lib/validate-feed.mjs' +import { logWarning } from './common.mjs' const responseText = async response => { if (typeof response?.text === 'function') return response.text() @@ -16,7 +17,7 @@ export const downloadFeeds = async ({ fetchImpl = globalThis.fetch, vendoredDir, allowVendoredFallback = false, - warn = console.error, + warn = logWarning, }) => { if (!urls.length) return { dir: vendoredDir, cleanup: () => {} } const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'model-eol-feeds-')) diff --git a/bot/lib/github.mjs b/bot/lib/github.mjs index 18dbc23..49e618d 100644 --- a/bot/lib/github.mjs +++ b/bot/lib/github.mjs @@ -1,3 +1,5 @@ +import { logWarning } from './common.mjs' + const linkFor = (headers, relation) => { const value = headers?.get?.('link') || headers?.get?.('Link') || headers?.link || headers?.Link || '' const match = value.split(',').map(part => part.trim()).find(part => part.endsWith(`rel="${relation}"`)) @@ -32,7 +34,7 @@ const MODEL_EOL_LABEL = { } export class GitHubClient { - constructor({ repo, apiUrl = 'https://api.github.com', token, transport = globalThis.fetch, warn = console.error }) { + constructor({ repo, apiUrl = 'https://api.github.com', token, transport = globalThis.fetch, warn = logWarning }) { this.repo = repo this.apiUrl = apiUrl.replace(/\/$/, '') this.token = token diff --git a/bot/test/run.mjs b/bot/test/run.mjs index 40b84de..119680b 100644 --- a/bot/test/run.mjs +++ b/bot/test/run.mjs @@ -20,6 +20,8 @@ import { import { branchFor, itemDigest, markdownCode, markdownText, metadataLine, parseMetadata, slugFor, staleWorkComment } from '../lib/common.mjs' import { loadConfig } from '../lib/config.mjs' import { downloadFeeds } from '../lib/feeds.mjs' +import { GitHubClient } from '../lib/github.mjs' +import * as botModule from '../bot.mjs' import { capReport, readReportCapped, reportForBody, runEvalHook } from '../lib/eval.mjs' import { cloneRepository, deleteRemoteBranch, gitAuthentication, originFor } from '../lib/git.mjs' import { assertValidEvalResults } from '../../lib/receipts.mjs' @@ -441,7 +443,25 @@ const separatorMetadata = { ...hostileMetadata, replacement_note: `a${lineSepara const separatorLine = metadataLine(separatorMetadata) assert(!separatorLine.includes(lineSeparator) && parseMetadata(separatorLine)?.replacement_note === separatorMetadata.replacement_note, 'metadata comments encode U+2028 and U+2029 so the metadata line still parses') const hostileText = markdownText('azure/x\n---\n@thossullivan see https://evil.example www.evil.example |a|b| \x1b[2K') -assert(!/[\n\r@|\x1b]|:\/\/|www\./i.test(hostileText) && hostileText.includes('@thossullivan') && hostileText.includes('|'), 'markdown text stays on one line and neutralizes mentions, table pipes, autolinks, and controls') +assert(!/[\n\r|\x1b]|@(?!​)|:\/\/|www\.(?!​)/i.test(hostileText) && hostileText.includes('@​thossullivan') && hostileText.includes('|'), 'markdown text stays on one line and neutralizes mentions, table pipes, autolinks, and controls') +const referenceText = markdownText('ping @thossullivan about #1, owner/repo#1, and user@example.com') +assert(referenceText === 'ping @​thossullivan about #​1, owner/repo#​1, and user@​example.com', 'markdown text breaks mentions, issue references, and emails with a zero-width space, which GitHub does not resolve through') +const spoofedSource = buildIssueBody({ + group: { + kind: 'issue', + id: 'o3-deep-research-2025-06-26', + subject: 'o3-deep-research-2025-06-26', + publisher: 'openai', + shutdown: '2026-07-23', + via: null, + root: '.', + feedDigest: 'd', + context: { announced: null, notes: [], entry: null }, + issues: [{ file: 'a.py', line: 1, reason: 'not-direct-api', status: 'retired', matched: 'o3-deep-research', sources: ['https://evil.example\\@github.com/advisory'] }], + }, + now: new Date('2026-08-01T00:00:00Z'), +}) +assert(spoofedSource.includes('- [https://evil.example/@​github.com/advisory](https://evil.example/@github.com/advisory)'),'source link labels show the normalized URL the link actually opens') const hostileIssue = buildIssueBody({ group: { kind: 'issue', @@ -464,6 +484,25 @@ const waivedComment = staleWorkComment('issue', { expires: '2099-01-01', owner: assert(waivedComment.includes('owned by `@thossullivan`') && waivedComment.includes('records: `see [x](https://evil.example) # h`') && !waivedComment.includes('\n'), 'stale-work comments code-span waiver owner and reason') const decisionLog = formatDecisions([{ action: 'create', group: { kind: 'issue', publisher: 'openai', subject: 'x\n::error::forged' }, body: 'line\n::warning title=forged::x\n ##[error]legacy\n\x1b[2K' }]) assert(decisionLog.startsWith('model-eol decision table\n') && !decisionLog.split('\n').some(line => line.trimStart().startsWith('::') || line.includes('##[') || line.includes('\x1b')), 'dry-run decision tables cannot open workflow commands') +const opensWorkflowCommand = text => String(text).split('\n').some(line => line.trimStart().startsWith('::') || line.includes('##[')) +const evalLines = botModule.evalResultLines?.([{ status: 'pass', publisher: 'p\n::warning title=forged::eval', id: 'x ##[error]y' }]) ?? [] +assert(evalLines.length === 1 && !opensWorkflowCommand(evalLines.join('\n')), 'evaluate result lines escape plan publisher and id') +const capturedWarnings = [] +const originalConsoleError = console.error +console.error = (...values) => capturedWarnings.push(values.join(' ')) +try { + await downloadFeeds({ + urls: ['https://feeds.invalid/forged.json'], + vendoredDir: path.join(root, 'feeds'), + allowVendoredFallback: true, + fetchImpl: async () => ({ status: 200, ok: true, text: async () => JSON.stringify({ spec: 'x\n::warning title=forged::feed', publisher: 'p', generated: '2026-08-01T00:00:00Z', models: [] }) }), + }) + const labelClient = new GitHubClient({ repo: 'example/repo', token: 't', transport: async () => ({ status: 500, ok: false, data: 'x\n::warning title=forged::api ##[error]y' }) }) + await labelClient.ensureModelEolLabel() +} finally { + console.error = originalConsoleError +} +assert(capturedWarnings.length === 2 && capturedWarnings.every(message => message.includes('forged') && !opensWorkflowCommand(message)), 'default feed and GitHub API warnings cannot open workflow commands') const clockDigestFeeds = path.join(tempRoot, 'clock-digest-feeds') fs.mkdirSync(clockDigestFeeds) const clockDigestRows = status => [ diff --git a/check.mjs b/check.mjs index 55937ff..5952de2 100755 --- a/check.mjs +++ b/check.mjs @@ -52,7 +52,7 @@ import { BaseUnavailableError, inCheckScope, selectChangedFindings } from './lib import { gitRootFor, incompleteScanNotes, scanTargets } from './lib/scanner.mjs' import { DOCUMENT_TYPES, formatDocumentErrors, validateDocumentFile } from './lib/validate-document.mjs' import { SUPPORTED_RECEIPT_SCHEMAS, VERIFY_INPUTS, VerifyError, formatVerification, printable, verifyReceipt } from './lib/verify.mjs' -import { printableLines } from './lib/printable.mjs' +import { logSafeJson, printableLines } from './lib/printable.mjs' const rootForTarget = target => { const absolute = path.resolve(target) @@ -331,7 +331,7 @@ if (command === 'verify') { : `model-eol verify: unexpected error, nothing was checked: ${error?.message ?? error}`)) return 2 } - console.log(AS_JSON ? JSON.stringify(report, null, 2) : formatVerification(report)) + console.log(AS_JSON ? logSafeJson(JSON.stringify(report, null, 2)) : formatVerification(report)) if (report.result === 'mismatch') return 1 return report.result === 'partial' && values['require-all'] ? 1 : 0 } @@ -560,21 +560,21 @@ const schedule = () => buildSchedule(inventory()) const alert = () => buildAlert(schedule()) if (command === 'plan') { - console.log(JSON.stringify(buildPlan({ + console.log(logSafeJson(JSON.stringify(buildPlan({ scan, findings, entries: feedData.entries, days: DAYS, via: VIA, scope: SCOPE, - }), null, 2)) + }), null, 2))) return 0 } if (command === 'check') { if (AS_JSON) { // capture stays out of the 0.1 document until the contract grows; the human line and the bot carry it - console.log(JSON.stringify({ schema: 'model-eol/check@0.1', threshold_days: DAYS, distributor: VIA, scope: SCOPE, scan_notes: scan.notes, findings: changedFindings.map(({ capture, ...finding }) => finding) }, null, 2)) + console.log(logSafeJson(JSON.stringify({ schema: 'model-eol/check@0.1', threshold_days: DAYS, distributor: VIA, scope: SCOPE, scan_notes: scan.notes, findings: changedFindings.map(({ capture, ...finding }) => finding) }, null, 2))) } else { console.log(formatCheck({ findings: changedFindings, bad, scannedFiles: scan.files.length, days: DAYS, scope: SCOPE })) } @@ -585,9 +585,9 @@ if (command === 'inventory') { const inv = inventory() const inventoryFormat = FORMAT ?? 'json' if (inventoryFormat === 'cyclonedx') { - console.log(JSON.stringify(formatInventoryCycloneDX(inv), null, 2)) + console.log(logSafeJson(JSON.stringify(formatInventoryCycloneDX(inv), null, 2))) } else if (AS_JSON || inventoryFormat === 'json') { - console.log(JSON.stringify(inv, null, 2)) + console.log(logSafeJson(JSON.stringify(inv, null, 2))) } else { console.log(formatInventory(inv, DAYS)) } @@ -597,7 +597,7 @@ if (command === 'inventory') { if (command === 'schedule') { const sched = schedule() if (AS_JSON) { - console.log(JSON.stringify(sched, null, 2)) + console.log(logSafeJson(JSON.stringify(sched, null, 2))) } else { console.log(formatSchedule(sched, DAYS)) } @@ -607,9 +607,9 @@ if (command === 'schedule') { if (command === 'alert') { const payload = alert() if (AS_JSON || FORMAT === 'json') { - console.log(JSON.stringify(payload, null, 2)) + console.log(logSafeJson(JSON.stringify(payload, null, 2))) } else if (FORMAT === 'badge') { - console.log(JSON.stringify(formatAlertBadge(payload), null, 2)) + console.log(logSafeJson(JSON.stringify(formatAlertBadge(payload), null, 2))) } else if (FORMAT === 'markdown') { console.log(formatAlertMarkdown(payload, DAYS)) } else { diff --git a/lib/printable.mjs b/lib/printable.mjs index d968ea2..f9591c4 100644 --- a/lib/printable.mjs +++ b/lib/printable.mjs @@ -11,3 +11,6 @@ export const printable = value => String(value).replace(HIDDEN, character => { // keeps intentional line breaks, but no line may start a :: workflow command export const printableLines = value => String(value).split('\n').map(line => printable(line).replace(/^(\s*):(?=:)/, '$1\\x3a')).join('\n') + +// JSON text for a log: an escaped hash parses back the same, but the runner never sees ##[ +export const logSafeJson = text => String(text).replaceAll('##[', '#\\u0023[') diff --git a/test/run.mjs b/test/run.mjs index 9f65210..cfe4d60 100644 --- a/test/run.mjs +++ b/test/run.mjs @@ -8,6 +8,7 @@ import { spawn, spawnSync } from 'node:child_process' import fs from 'node:fs' import os from 'node:os' import path from 'node:path' +import { isDeepStrictEqual } from 'node:util' import { applyPlan } from '../lib/apply.mjs' import { color, colorEnabled } from '../lib/color.mjs' @@ -339,6 +340,29 @@ assert(legacyJson.findings[0]?.file.endsWith('a ##[warning]forged \u202e.py'), ' const tentativeHostile = { file: 'a.py', line: 1, matched: 'm', status: 'scheduled', date_precision: 'tentative', shutdown: '2027-01-01', safe_until: '2027-06-01', publisher: '[x](https://evil.example)\n# h' } const tentativeMarkdown = formatAlertMarkdown({ scan_notes: [], errors: [], warnings: [tentativeHostile] }, 30) assert(tentativeMarkdown.includes('by `[x](https://evil.example)\\n# h` policy') && formatAlertMarkdown({ scan_notes: [], errors: [], warnings: [{ ...tentativeHostile, publisher: 'openai' }] }, 30).includes('by openai policy'), 'markdown alert code-spans only unusual feed publishers in the policy tail') +const jsonLogDir = path.join(tempRoot, 'escape-json-log') +const jsonLogName = 'x ##[set-output name=pwned;]yes.py' +fs.mkdirSync(jsonLogDir) +fs.writeFileSync(path.join(jsonLogDir, jsonLogName), 'from openai import OpenAI\nclient = OpenAI()\nclient.responses.create(model="o3-deep-research")\n') +const savedReports = [] +for (const args of [['check', '--json'], ['plan'], ['inventory', '--format', 'json'], ['inventory', '--format', 'cyclonedx'], ['schedule', '--json'], ['alert', '--json']]) { + const printed = run([args[0], '.', ...args.slice(1), '--days', '90'], { cwd: jsonLogDir, env: colorEnv }) + const document = JSON.parse(printed.out) + const unescaped = JSON.parse(printed.out.replaceAll('#\\u0023[', '##[')) + assert(!printed.out.includes('##[') && JSON.stringify(document).includes(jsonLogName) && isDeepStrictEqual(document, unescaped), `${args.join(' ')} prints JSON with no ##[ that parses to the unescaped document`) + const saved = path.join(jsonLogDir, `report-${args.join('-').replaceAll('--', '')}.json`) + fs.writeFileSync(saved, printed.out) + if (args[1] !== '--format' || args[2] !== 'cyclonedx') savedReports.push(saved) +} +const rescan = JSON.parse(run(['check', '.', '--json', '--days', '90'], { cwd: jsonLogDir, env: colorEnv }).out) +assert(rescan.findings.length > 0 && rescan.findings.every(finding => finding.file === jsonLogName), 'a rescan still skips printed model-eol JSON reports as its own documents') +const validatedReports = run(['validate', ...savedReports], { cwd: jsonLogDir, env: colorEnv }) +assert(validatedReports.code === 0 && validatedReports.out.split('\n').filter(line => line.includes(': valid ')).length === savedReports.length, 'validate accepts every printed JSON report with an escaped hash') +const receiptHead = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8' }).stdout.trim() +const receiptPath = path.join(tempRoot, 'r ##[set-output name=pwned;]yes.json') +const receiptCreate = spawnSync(process.execPath, [path.join(root, 'scripts/feed-refresh-receipt.mjs'), 'create', '--repo-dir', root, '--out', receiptPath, '--state', 'clean', '--checked-at', '2026-10-19T05:23:00Z', '--refresh-run-url', 'https://github.com/thossullivan/model-eol/actions/runs/1', '--refresh-sha', receiptHead], { encoding: 'utf8' }) +const verifyJson = run(['verify', receiptPath, '--json'], { env: colorEnv }) +assert(receiptCreate.status === 0 && verifyJson.out.includes('"path"') && !verifyJson.out.includes('##[') && JSON.parse(verifyJson.out).receipt.path.endsWith('r ##[set-output name=pwned;]yes.json'), 'verify --json escapes ##[ in the printed receipt path only') const anthropicAliasConsumerDir = path.join(tempRoot, 'anthropic-alias-consumer') const anthropicAliasConsumerFeeds = path.join(tempRoot, 'anthropic-alias-feeds') From 895cd98fd507b1ebb34ee83dea4a556c80d0d129 Mon Sep 17 00:00:00 2001 From: thossullivan Date: Fri, 23 Oct 2026 21:05:46 -0500 Subject: [PATCH 4/4] Break issue paths, GH- references, and leading block markers in bot text Rendering bot text through GitHub's Markdown API showed three gaps. owner/repo/issues/N paths (with or without a scheme), /issues/N, and GH-N still became issue links, which leave cross-references on the target issue; markdownText now puts a zero-width space after every slash and inside GH-N. GitHub detects www. on the raw text, so the zero-width space moves before the dot. A value starting with -, +, =, leading spaces, or N. could still open a rule, list, or code block inside a list item; the leading marker is now encoded and leading spaces dropped. Fifteen hostile and ordinary samples rendered through the API produce no links, mentions, rules, or lists, and read unchanged. Written by Claude after three implementer rounds. --- bot/lib/common.mjs | 12 ++++++++---- bot/test/run.mjs | 7 ++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/bot/lib/common.mjs b/bot/lib/common.mjs index 6fc121f..1750e37 100644 --- a/bot/lib/common.mjs +++ b/bot/lib/common.mjs @@ -80,15 +80,19 @@ const entityMap = new Map([ ['@', '@​'], ['|', '|'], ]) +const leadingMarker = new Map([['-', '-'], ['+', '+'], ['=', '=']]) const markdownInline = value => printable(String(value ?? '').replace(/\r\n|[\r\n]/g, ' ')) .replace(/[&<>`\\[\]()#*_~!@|]/g, character => entityMap.get(character)) - .replace(/^(\s*)([-+>]\s|\d+\.\s)/, '$1- ') + .replace(/^\s+/, '') + .replace(/^[-+=]/, character => leadingMarker.get(character)) + .replace(/^(\d+)\./, '$1.') -// one line of text: no block structure, table cells, or bare autolinks +// one line of text: no block structure, table cells, bare autolinks, or issue references export const markdownText = value => markdownInline(value) - .replace(/(www)\./gi, '$1.​') - .replaceAll('://', ':​//') + .replace(/(www)\./gi, '$1​.') + .replaceAll('/', '/​') + .replace(/\b(gh)(?=-\d)/gi, '$1​') // code spans show entities literally, so drop backticks and line breaks instead of escaping export const markdownCode = value => { diff --git a/bot/test/run.mjs b/bot/test/run.mjs index 119680b..64bb952 100644 --- a/bot/test/run.mjs +++ b/bot/test/run.mjs @@ -445,7 +445,12 @@ assert(!separatorLine.includes(lineSeparator) && parseMetadata(separatorLine)?.r const hostileText = markdownText('azure/x\n---\n@thossullivan see https://evil.example www.evil.example |a|b| \x1b[2K') assert(!/[\n\r|\x1b]|@(?!​)|:\/\/|www\.(?!​)/i.test(hostileText) && hostileText.includes('@​thossullivan') && hostileText.includes('|'), 'markdown text stays on one line and neutralizes mentions, table pipes, autolinks, and controls') const referenceText = markdownText('ping @thossullivan about #1, owner/repo#1, and user@example.com') -assert(referenceText === 'ping @​thossullivan about #​1, owner/repo#​1, and user@​example.com', 'markdown text breaks mentions, issue references, and emails with a zero-width space, which GitHub does not resolve through') +assert(referenceText === 'ping @​thossullivan about #​1, owner/​repo#​1, and user@​example.com', 'markdown text breaks mentions, issue references, and emails with a zero-width space, which GitHub does not resolve through') +const issuePathText = markdownText('see octocat/Hello-World/issues/1, https://github.com/o/r/pull/2, /issues/3, GH-4, and www.evil.example') +assert(!/\/(?!​)|www(?!​)|gh-/i.test(issuePathText), 'markdown text breaks issue paths, GH- references, and www. links before GitHub can link them') +for (const [block, rendered] of [['---', '---'], [' ---', '---'], ['+', '+'], ['1. one', '1. one'], [' code', 'code'], ['===', '===']]) { + assert(markdownText(block) === rendered, `markdown text cannot start a block element: ${JSON.stringify(block)}`) +} const spoofedSource = buildIssueBody({ group: { kind: 'issue',