Skip to content

ci(policy): read git path listings with -z so quoted names reach the gates #1473

Description

@doublewhy

Gap Claim

Three helpers that feed the repository's gates read git path listings without -z and split the output on newlines:

  • _git_lines in tools/nox_support/runner.py (:96-104), used by _changed_paths (:110-115) and _tracked_repo_paths (:303-304). _normalize_paths (:307-319) then drops every entry that is not a file.
  • changed_paths in tools/policy/common.py (:97-112), which returns each line as a path.
  • _git_paths in tools/check_schema_publication.py (:221-230), which keeps the lines that end in .json.

Without -z, git quotes "unusual" path names: it encloses the name in double quotes and escapes the characters with backslashes, as C does. Bytes above 0x80 are unusual under the default core.quotePath=true. Double quotes, backslashes and control characters always are (git help config, core.quotePath; git help diff, -z). For such a name, the runner keeps nothing, the policy helper returns git's quoted text, and the schema publication helper drops it.

Reproduction on dev (35122105) with git 2.54.0 (Apple Git-157) and core.quotePath unset. A scratch repository commits README.md, then adds and commits contracts/schemas/naïve.json, docs/"quoted".md, docs/plain.md, docs/tab<TAB>name.md and a 501-line implementations/python/packages/raes/naïve.py. The helpers come from the dev checkout, with the runner's REPO_ROOT pointed at the scratch repository (script below):

$ git diff --name-only --diff-filter=d --cached
"contracts/schemas/na\303\257ve.json"
"docs/\"quoted\".md"
docs/plain.md
"docs/tab\tname.md"
"implementations/python/packages/raes/na\303\257ve.py"

tools.policy.common.changed_paths(repo, base_rev='HEAD~1'):
  ['"contracts/schemas/na\\303\\257ve.json"', '"docs/\\"quoted\\".md"', 'docs/plain.md', '"docs/tab\\tname.md"', '"implementations/python/packages/raes/na\\303\\257ve.py"']
tools.nox_support.runner._changed_paths(base_rev='HEAD~1'):
  ['docs/plain.md']
tools.nox_support.runner._tracked_repo_paths():
  ['README.md', 'docs/plain.md']
tools.check_schema_publication._git_paths(repo, 'HEAD', 'contracts/schemas'):
  []

With -z, git diff, git ls-files and git ls-tree list every one of these names verbatim.

The gates then miss the files:

Gate Listing Result in the scratch repository
Hygiene stages of verify-fast-feedback and of the canonical verify-static-lane, both run with --base-rev (.github/workflows/ci.yml:257, .github/workflows/canonical-verification.yml:260-266) runner _changed_paths _parse_hygiene_posargs(["--base-rev", "HEAD~1"]) selects only docs/plain.md. The trailing-whitespace, end-of-file, YAML and JSON syntax, large-file, merge-conflict, private-key and gitleaks stages never read the other four files.
Hygiene with --all-files runner _tracked_repo_paths README.md and docs/plain.md only.
500-line source cap in tools/check_repo_policy.py changed_paths _check_oversized (tools/policy/repo_policy.py:462) reports nothing for the quoted module path, which does not end in .py. For the verbatim path it reports [oversized-source-file] implementations/python/packages/raes/naïve.py: file is 501 lines, exceeding the 500-line cap; ....
JSON artifact validation in the contracts lane (tools/check_json_artifacts.py) changed_paths _changed_path_target (:228) returns None for the quoted schema path and a metaschema target for the verbatim one.
Base-revision manifest in tools/check_schema_publication.py _git_paths _load_base_manifest (:399) would omit such a ledger record. Record names must match their schema identity (:140-141), so a schema with such a name gets a ledger record with one too.

The gap is latent. None of the 4,497 paths tracked at 35122105 contains a character git quotes. The #1344 fixture-provenance audit found it (finding CL-2 in the record #1461 proposes), and the tests that drive these listings use ASCII names.

Reproduction script (run with the project environment's Python and -I, passing the checkout path)
import importlib
import shutil
import subprocess
import sys
import tempfile
from pathlib import Path
from types import SimpleNamespace

checkout = Path(sys.argv[1]).resolve()
sys.path.insert(0, str(checkout))

from tools.policy.common import changed_paths  # noqa: E402

sys.modules["nox"] = SimpleNamespace(Session=object)  # the project env has no nox; runner only annotates with it
runner = importlib.import_module("tools.nox_support.runner")

NAMES = ("contracts/schemas/naïve.json", "docs/tab\tname.md", 'docs/"quoted".md', "docs/plain.md")
MODULE = "implementations/python/packages/raes/naïve.py"
repo = Path(tempfile.mkdtemp(prefix="cl2-repro-"))
try:

    def git(*args: str) -> str:
        return subprocess.run(["git", *args], cwd=repo, capture_output=True, text=True, check=True).stdout

    git("init", "-q")
    git("config", "user.email", "repro@example.invalid")
    git("config", "user.name", "Repro")
    git("config", "commit.gpgsign", "false")
    (repo / "README.md").write_text("base\n", encoding="utf-8")
    git("add", "README.md")
    git("commit", "-q", "-m", "base")
    for name in NAMES:
        path = repo / name
        path.parent.mkdir(parents=True, exist_ok=True)
        path.write_text("{}\n", encoding="utf-8")
    module = repo / MODULE
    module.parent.mkdir(parents=True, exist_ok=True)
    module.write_text("value = 0\n" * 501, encoding="utf-8")
    git("add", ".")
    print(git("diff", "--name-only", "--diff-filter=d", "--cached"), end="")
    print(changed_paths(repo, staged=True))
    git("commit", "-q", "-m", "names")
    print(changed_paths(repo, base_rev="HEAD~1"))

    runner.REPO_ROOT = repo
    print(runner._changed_paths(base_rev="HEAD~1"))
    print(runner._tracked_repo_paths())
    from tools import check_schema_publication

    print(check_schema_publication._git_paths(repo, "HEAD", "contracts/schemas"))
    selection = runner._parse_hygiene_posargs(["--base-rev", "HEAD~1"], default_all_files=False)
    print(selection.paths)
    from tools import check_json_artifacts
    from tools.policy import repo_policy
    from tools.policy.common import load_yaml

    listed = changed_paths(repo, base_rev="HEAD~1")
    print(check_json_artifacts._changed_path_target(repo, listed[0]))
    print(check_json_artifacts._changed_path_target(repo, NAMES[0]))
    policy = load_yaml(checkout / "tools/policy/adr_policy.yaml")
    config, errors = repo_policy._validate_oversized_config(policy.get("oversized_source_files"))
    for path in (listed[-1], MODULE):
        print([failure.render() for failure in repo_policy._check_oversized(repo, config, frozenset(), [path])])
finally:
    shutil.rmtree(repo)

Existing Surface Audit

  • The repository already reads these listings with -z elsewhere and splits on NUL: tools/check_requirement_governance.py:318-322 (diff --name-only --diff-filter=d -z), tools/verification_plan.py:127 and :147 (diff --name-status -z, ls-files --others --exclude-standard -z), tools/check_identity_cutover.py:322 and tools/check_tooling_artifact_policy.py:65 (ls-files -z).
  • verify-fast-feedback classifies changes with verification_plan.collect_git_changes (tools/nox_support/graph.py:214), so its changed-file lint and changed-test selection already see these names. Only its hygiene and policy stages go through the helpers above.
  • Callers: the runner helpers feed only _parse_hygiene_posargs (tools/nox_support/runner.py:288-300). changed_paths feeds tools/check_repo_policy.py:48, tools/check_json_artifacts.py:403 and tools/check_requirement_governance.py:304. _git_paths feeds _load_base_manifest (tools/check_schema_publication.py:399).
  • tools/policy/requirement_scope.py:156 runs git ls-files -- <path> without -z, but it only tests whether the output is empty (:165). Quoting cannot change that answer.

Alternatives

  1. Run the listings with -c core.quotePath=false: rejected. Double quotes, backslashes and control characters are still quoted, and the output stays newline-delimited.
  2. Undo git's C-style quoting in Python: rejected. It re-implements git's quoting rules where git can print names verbatim.
  3. Pass -z and split the output on NUL, as the listings above already do: selected.

Chosen Architecture

  • tools/nox_support/runner.py: _git_lines becomes _git_paths, which adds -z after the git subcommand and splits the output on NUL. _changed_paths and _tracked_repo_paths keep their git arguments, and _normalize_paths keeps its filtering.
  • tools/policy/common.py: changed_paths adds -z to each of its three listings and splits on NUL. run_git is unchanged.
  • tools/check_schema_publication.py: _git_paths adds -z and splits on NUL. Its .json filter and sorting stay.
  • The output stays verbatim, so names are no longer stripped of surrounding whitespace. It is still decoded as text, so a name that is not valid UTF-8 raises UnicodeDecodeError instead of being skipped, as the -z listings in check_requirement_governance.py and verification_plan.py already do.
  • No gate's selection rules change, and listings of names git does not quote are unchanged, except that a space at either end of a path is now kept.

Verification Plan

  • A regression test, implementations/python/tests/test_issue_<this issue>_nul_path_listings.py, builds a real git repository in a temporary directory. Next to a plain name, it holds names git quotes: a non-ASCII one, one with a tab and one with a double quote. The test checks that each helper returns them verbatim: changed_paths for staged changes, for a base revision and for the working tree; the runner's _changed_paths in the same three modes; _tracked_repo_paths; and check_schema_publication._git_paths. It fails on dev.
  • Run the existing tests that cover these helpers' callers (test_repo_policy_tools.py, test_requirement_governance.py, test_issue_1204_repository_governance.py, test_issue_1237_requirement_scope.py, test_issue_1330_schema_coverage.py), with default markers and with -m integration.
  • Run nox -s verify-fast-feedback -- --base-rev origin/dev, nox -s lint and make policy.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions