diff --git a/cli/__tests__/git-readonly.test.mjs b/cli/__tests__/git-readonly.test.mjs index fe62bd76c4..9c464c56c6 100644 --- a/cli/__tests__/git-readonly.test.mjs +++ b/cli/__tests__/git-readonly.test.mjs @@ -429,6 +429,36 @@ describe('read-only git runs no gpg and no textconv', () => { expect(ran('gpg')).toBe(false); }); + test('a %G placeholder behind a +, - or space modifier runs gpg (control), and is refused', async () => { + for (const format of ['%+G?', '%-GG', '% GS']) { + await cfgGit(['log', `--format=${format}`, '-1', signed]); + expect(ran('gpg')).toBe(true); + } + for (const arg of ['--format=%+G?', '--pretty=format:%-GK', '--pretty=tformat:%h% GF', '--format=%<(8)%+GP', '--format=%-GT']) { + expect((await run('log', [arg, '-1', signed])).text).toMatch(/%G placeholder runs gpg/); + } + expect(ran('gpg')).toBe(false); + const modified = await run('log', ['--format=%h%+s', '-1', signed]); + expect(modified.ok).toBe(true); + expect(modified.text).toContain('\nunsigned'); + }); + + // git 2.54 rejects every abbreviation of --pretty and --format, and a bare + // --format, so none of these has a control: the check holds without that. + test('an abbreviation is checked like the full name, and --format never takes the next argument', () => { + expect(refusal({ subcommand: 'log', args: ['--form=%G?'] })).toMatch(/%G placeholder runs gpg/); + expect(refusal({ subcommand: 'show', args: ['--pret=format:%+GS'] })).toMatch(/%G placeholder runs gpg/); + expect(refusal({ subcommand: 'log', args: ['--forma=sig'] })).toMatch(/name a built-in format/); + for (const option of ['--format', '--form', '--for']) { + expect(refusal({ subcommand: 'log', args: [option, '%G?'] })).toMatch(/give the format in the same argument/); + } + for (const args of [['--form=%h'], ['--pret=oneline'], ['--pretty'], ['--pretty', '-n1']]) { + expect(readonlyGitArgv({ subcommand: 'log', args, root: repo }).ok).toBe(true); + } + // blame reads --pr as its own --progress. + expect(readonlyGitArgv({ subcommand: 'blame', args: ['--pr', 'tracked.txt'], root: repo }).ok).toBe(true); + }); + test('log.showSignature in the repository config runs gpg (control); the guarded log and show do not', async () => { await cfgGit(['config', 'log.showSignature', 'true']); try { diff --git a/cli/src/lib/git-readonly.js b/cli/src/lib/git-readonly.js index 6cdc9825e4..509dd19808 100644 --- a/cli/src/lib/git-readonly.js +++ b/cli/src/lib/git-readonly.js @@ -127,16 +127,28 @@ const TOP_LEVEL = Object.freeze([ ]); // A %G placeholder (%G?, %GG, %GS, %GK, %GF, %GP, %GT) runs gpg on each signed -// commit. A value with no %, no format: and no tformat: is looked up by -// prefix among the built-in formats and the repository's `pretty.` -// aliases, and an alias can hold one, so only a built-in name is taken. -const FORMAT_ARGUMENT = /^--(?:pretty|format)=([\s\S]*)$/; +// commit, and git reads one behind a `+`, `-` or space modifier (`%+G?`, +// `%-GG`, `% GS`) as the same placeholder. A value with no %, no format: and +// no tformat: is looked up by prefix among the built-in formats and the +// repository's `pretty.` aliases, and an alias can hold one, so only a +// built-in name is taken. +const SIGNATURE_PLACEHOLDER = /%[-+ ]?G/; const BUILTIN_FORMATS = Object.freeze(['oneline', 'short', 'medium', 'full', 'fuller', 'reference', 'email', 'mboxrd', 'raw']); +// Git's revision parser reads --pretty and --format only by their exact names, +// and their format only after `=`: a bare --pretty means medium, and a bare +// --format is an unrecognized argument. The check does not depend on that. An +// abbreviation (`--form=`, `--pret=`) is checked like the full name, and a +// --format that would take its format from the next argument is refused. +const FORMAT_OPTIONS = Object.freeze(['--pretty', '--format']); const checkFormat = (arg) => { - const match = FORMAT_ARGUMENT.exec(arg); - if (!match) return null; - const value = match[1]; - if (value.includes('%G')) return `${arg} is refused: a %G placeholder runs gpg to check a signature`; + const [name] = arg.split('=', 1); + const option = name.length >= 3 && FORMAT_OPTIONS.find((full) => full.startsWith(name)); + if (!option) return null; + if (name === arg) { + return option === '--format' ? `${arg} is refused: give the format in the same argument, as --format=` : null; + } + const value = arg.slice(name.length + 1); + if (SIGNATURE_PLACEHOLDER.test(value)) return `${arg} is refused: a %G placeholder runs gpg to check a signature`; const userFormat = value === '' || value.startsWith('format:') || value.startsWith('tformat:') || value.includes('%'); if (userFormat || BUILTIN_FORMATS.includes(value.toLowerCase())) return null; return `${arg} is refused: name a built-in format (${BUILTIN_FORMATS.join(', ')}) or give format:`;