diff --git a/agents/honker/work/queue-283/BUILD.md b/agents/honker/work/queue-283/BUILD.md index ac01f1c55..937fc888e 100644 --- a/agents/honker/work/queue-283/BUILD.md +++ b/agents/honker/work/queue-283/BUILD.md @@ -144,3 +144,36 @@ Fixture grep: `git grep -nE "\beditor\b|\bauthor\b" -- 'packages/library/tests/* leaving out mapping.test.mjs, finds nothing. packages/business/tests has no fixtures directory. The library fixture mapping.json has no `editor`. The positive control (`vaultPrefix` in mapping.json) matches once. No fixture maps an editor or author with surrounding whitespace. + +## 2026-10-23 E4 (webby-0230, sage-3606), same departure as E2 + +Claimed: `queue move 283 in-progress`, op honker-283-start, rev 3642, 19:10:49Z (briefed→in-progress). + +Git's ident.c `crud()` strips any byte <= 0x20 and `, : ; < > " \ '` from both ends of an ident. I +checked that with git 2.55.0: `Jane,` `"Jane"` `'Jane'` `Jane;` `Jane:` `Jane\` and `,Jane` all +commit as `Jane`, while `Jane.` and `Ja,ne` are kept. mapping.mjs now takes the set from crud() +rather than from that list: `IDENT_CRUD_END = /^[\u0000-\u0020,:;<>"\\']|[\u0000-\u0020,:;<>"\\']$/`. +The E2 check became `if (value.trim() !== value || IDENT_CRUD_END.test(value))`, which refuses +library-mapping-invalid with "editor. must not start or end with whitespace or any of , : ; " ' \". +`trim()` still catches Unicode whitespace (U+00A0, U+3000), which crud() leaves alone. + +Inner characters: IDENT_BAD (`/[\u0000-\u001f\u007f-\u009f<>]/`) already refuses `<`, `>`, newline, +tab and every other control anywhere in the value. T3 now pins that at the start, the end and the +middle. Inner `, : ; " ' \` are kept, since git keeps them, and T3 pins that too. + +Tests: +- T3 has one case per character in [space, U+00A0, U+3000, `,`, `:`, `;`, `"`, `'`, `\`], at the start + and at the end, for each key. `<`, `>`, newline and tab are pinned at the start, end and middle. + Four accepted names have inner crud characters. +- New round-trip test, "an accepted editor round-trips through a real git commit unchanged". It + commits in a scratch repo with editor `Doe, Jane Q.` / `jane.doe@example.invalid.` as author and + committer, with GIT_AUTHOR_* and GIT_COMMITTER_* stripped from the env, and asserts `git log` + shows exactly the saved values. I preflighted that commit with git 2.55.0 by hand: both values came + back unchanged. + +M6c now replaces the whole end check (this supersedes the earlier M6c row): + +| Mutant | File | Replace | With | Killed by | +|---|---|---|---|---| +| M6c | src/mapping.mjs | `if (value.trim() !== value \|\| IDENT_CRUD_END.test(value))` | `if (false)` | mapping.test "editor is optional" (end cases) | +| M6d | src/mapping.mjs | `if (value.trim() !== value \|\| IDENT_CRUD_END.test(value))` | `if (value.trim() !== value)` | mapping.test "editor is optional" (`,` `:` `;` `"` `'` `\` cases) | diff --git a/packages/library/src/mapping.mjs b/packages/library/src/mapping.mjs index 18a62a2f5..d67975ca6 100644 --- a/packages/library/src/mapping.mjs +++ b/packages/library/src/mapping.mjs @@ -16,6 +16,9 @@ const KINDS = ['hybrid', 'draft']; // C2-01d (F41): the editor ident her Console saves commit under. Git drops `<` and `>` from an ident // and commits under a different name, so they refuse here (webby-0169 R1). const IDENT_BAD = /[\u0000-\u001f\u007f-\u009f<>]/; +// Git's ident.c crud() strips any byte <= 0x20 and , : ; < > " \ ' from both ends of an ident, so a +// value that starts or ends with one would save one name and commit another (sage-3601, sage-3606). +const IDENT_CRUD_END = /^[\u0000-\u0020,:;<>"\\']|[\u0000-\u0020,:;<>"\\']$/; // A folder segment from her vault: spaces and capitals are fine, separators, control characters // and dot names are not. @@ -63,8 +66,8 @@ export function checkMapping(entry, where = 'library', { fail = raise } = {}) { const value = entry.editor[key]; if (typeof value !== 'string' || !value.trim() || IDENT_BAD.test(value)) bad(`editor.${key} must be non-empty text with no control character, < or >`); - // Git trims an ident at commit, so a padded value would save one name and commit another (sage-3601). - if (value.trim() !== value) bad(`editor.${key} must not start or end with whitespace`); + if (value.trim() !== value || IDENT_CRUD_END.test(value)) + bad(`editor.${key} must not start or end with whitespace or any of , : ; " ' \\`); } } diff --git a/packages/library/tests/mapping.test.mjs b/packages/library/tests/mapping.test.mjs index 62124d647..f446d3d82 100644 --- a/packages/library/tests/mapping.test.mjs +++ b/packages/library/tests/mapping.test.mjs @@ -1,5 +1,9 @@ import test from 'node:test'; import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { checkMapping, classify, LibraryError } from '../src/index.mjs'; import { mapping, mappingEntry } from './helpers.mjs'; @@ -121,12 +125,33 @@ test('C2-01d: editor is optional, kept when given, and refuses anything but a pl invalid({ ...editor, name: bad }, /editor.name must be non-empty text with no control character, < or >/); invalid({ ...editor, email: bad }, /editor.email must be non-empty text with no control character, < or >/); } - // sage-3601: git trims an ident at commit, so surrounding whitespace refuses. - for (const padded of [' Her Name', 'Her Name ', ' Her Name ', '\u00a0Her Name', 'Her Name\u3000']) { - invalid({ ...editor, name: padded }, /editor.name must not start or end with whitespace/); - invalid({ ...editor, email: padded }, /editor.email must not start or end with whitespace/); + // sage-3601, sage-3606: git's crud() strips whitespace and , : ; < > " \ ' from both ends of an + // ident, so a value starting or ending with one refuses. Inner < > and controls refuse above. + const ends = [' ', '\u00a0', '\u3000', ',', ':', ';', '"', "'", '\\']; + for (const c of ends) for (const padded of [`${c}Her Name`, `Her Name${c}`]) { + invalid({ ...editor, name: padded }, /editor.name must not start or end with whitespace or any of , : ; " ' \\/); + invalid({ ...editor, email: padded }, /editor.email must not start or end with whitespace or any of , : ; " ' \\/); } + for (const c of ['<', '>', '\n', '\t']) for (const padded of [`${c}Her Name`, `Her Name${c}`, `Her${c}Name`]) + invalid({ ...editor, name: padded }, /editor.name must be non-empty text with no control character, < or >/); assert.equal(mapping(e => ({ ...e, editor: { name: 'Her Middle Name', email: 'her@example.invalid' } })).editor.name, 'Her Middle Name'); + for (const name of ['Doe, Jane Q.', 'Jane: "Q" Doe', "O'Neil", 'Back\\slash Name']) + assert.equal(mapping(e => ({ ...e, editor: { name, email: 'her@example.invalid' } })).editor.name, name); +}); + +// sage-3606: an accepted editor commits under exactly the saved ident, as author and committer. A +// future git that strips more from an ident fails here. +test('C2-01d: an accepted editor round-trips through a real git commit unchanged', t => { + const editor = mapping(e => ({ ...e, editor: { name: 'Doe, Jane Q.', email: 'jane.doe@example.invalid.' } })).editor; + const root = mkdtempSync(join(tmpdir(), 'library-ident-')); + t.after(() => rmSync(root, { recursive: true, force: true })); + // An ident in the environment would win over -c, so the commit runs without one. + const env = Object.fromEntries(Object.entries(process.env).filter(([k]) => !/^GIT_(AUTHOR|COMMITTER)_/.test(k))); + const git = (...args) => execFileSync('git', ['-c', `user.name=${editor.name}`, '-c', `user.email=${editor.email}`, + '-c', 'commit.gpgsign=false', '-c', 'core.hooksPath=/dev/null', '-C', root, ...args], { encoding: 'utf8', env }); + git('init', '-q'); + git('commit', '-q', '--allow-empty', '-m', 'ident'); + assert.equal(git('log', '-1', '--format=%an%n%ae%n%cn%n%ce'), `${editor.name}\n${editor.email}\n${editor.name}\n${editor.email}\n`); }); test('no playground prefix holds another, and no book folder holds another', () => {