`gitOutputAsArray` trims file paths, so `pullapprove verify` can report coverage for a file no group owns

[bilguunbicktivism]

What happens

utils/git/git-client.ts turns git's stdout into a file list:

function gitOutputAsArray(gitCommandResult: SpawnSyncReturns<string>): string[] {
  return gitCommandResult.stdout
    .split('\n')
    .map((x) => x.trim())
    .filter((x) => !!x);
}

.trim() is applied to what are paths, not display strings. Two consequences, both reachable with an
ordinary file:

1. A trailing or leading space is silently removed. src/sneaky.ts becomes src/sneaky.ts,
   which names a different file — usually one that does not exist.
2. Quoted paths keep their quotes. git ls-files renders a non-ASCII path as
   "caf\303\251.ts" under the default core.quotePath=true, and that whole string, quotes and
   octal escapes included, becomes the list entry.

Every caller of allFiles, allStagedFiles and allChangesFilesSince gets the mangled list.

Why it matters — pullapprove verify

pullapprove/verify.ts builds its coverage decision from that list:

const REPO_FILES = git.allFiles();
...
REPO_FILES.forEach((file: string) => {
  if (groupsWithConditions.filter((group) => group.testFile(file)).length) {

so the check tests the trimmed string, while PullApprove itself matches the real path it gets from the
GitHub API. The check exists to guarantee that every file has a reviewer group, and it can be made to
say so when that is not true.

Reproduction

A repository with two groups, every ordinary file owned, plus one file whose name ends in a single
space:

=== the repository, as git sees it ===
    .gitignore
    .ng-dev/config.mjs
    .pullapprove.yml
    README.md
    package.json
    src/owned.ts
    src/sneaky.ts 

=== the glob src/*.ts, asked about the real path and about the trimmed string ===
    OWNED    "src/owned.ts"
    UNOWNED  "src/sneaky.ts "
    OWNED    "src/sneaky.ts"

=== ng-dev pullapprove verify ===
    PullApprove verification succeeded!
    Matched Files (7 files)
    Unmatched Files (0 files)

The middle block is minimatch with the pattern src/*.ts — the same matcher group.testFile uses —
asked about the real path and about the trimmed one. It owns the trimmed string and does not own the
file that is actually in the repository. verify reports full coverage regardless.

The same primitive on its own, for the quoting half:

=== what git ls-files actually emits ===
    "caf\303\251.ts"
    plain.ts
    trailing.ts 

=== does each entry name a file that exists on disk? ===
    NOT FOUND "\"caf\\303\\251.ts\""
    exists    "plain.ts"
    NOT FOUND "trailing.ts"

Two of three entries name files that do not exist.

What this is not

I went looking for the stronger version of this and it does not hold, so it is worth writing down.
angular/angular's .pullapprove.yml ends with a required-minimum-review group that has no
conditions, and verify.ts:30 skips condition-less groups precisely because they always match. In
PullApprove that group is always active, so a file left unowned by every conditional group still
requires one review from the team.

So this is not a way to merge code without review. What it costs is the specialist owner: the
group whose globs were meant to cover that path is not engaged, and a generic approval satisfies the
requirement instead. That is a correctness bug in the check, which is why I am filing it here rather
than anywhere else.

Suggested fix

Three small changes in the same place:

• Drop .trim(). Split on \n and remove only the trailing empty element — a path's own whitespace
  is part of it.
• Run the underlying commands with -z and split on \0. That is the only rendering that survives
  every legal filename, including one containing a newline.
• Pass -c core.quotePath=false so non-ASCII paths come back as bytes rather than as C-quoted
  strings. With -z this is already implied for ls-files and diff --name-only, but stating it
  makes the intent explicit.

Happy to send a PR if that shape is agreeable.

Note on matchers

Unrelated to the trimming, but adjacent: verify evaluates contains_any_globs with minimatch,
while PullApprove evaluates the same expression with wcmatch. #46589 already records one incident
caused by matcher inconsistency in this config ("contains any globs uses wcmatch, while
files.exclude and files.include uses fnmatch"). A third engine in the verifier is a standing
source of the same class of disagreement, even after the trimming is fixed.