feat: no-production-newtonsoft hook, on shared YAML/reporting helpers

.github/workflows/ci.yml

@@ -51,6 +51,7 @@ jobs:
           pip install .
           stevedore-release-scope --help > /dev/null
           service-yaml-check > /dev/null
+          no-production-newtonsoft --help > /dev/null
 
       - name: try-repo each hook against a fixture consumer
         run: |
@@ -76,10 +77,16 @@ jobs:
             image: pinpredict/oms
           YAML
 
+          # A tracked, clean production source so no-production-newtonsoft has
+          # something to actually scan — with zero matching files it would pass
+          # without exercising the git enumeration path.
+          mkdir -p "$fixture/Acme.Api"
+          printf 'using System.Text.Json;\n' > "$fixture/Acme.Api/Program.cs"
+
           git -c init.defaultBranch=main init -q --template= "$fixture"
           git -C "$fixture" add -A
 
-          for hook in stevedore-release-scope service-yaml-check; do
+          for hook in stevedore-release-scope service-yaml-check no-production-newtonsoft; do
             echo "::group::try-repo $hook"
             (cd "$fixture" && pre-commit try-repo "$GITHUB_WORKSPACE" "$hook" --all-files)
             echo "::endgroup::"

.pre-commit-hooks.yaml

@@ -39,6 +39,22 @@
   files: '^(\.stevedore\.yaml|\.github/workflows/ci\.yml|charts/[^/]+/Chart\.yaml)$'
   pass_filenames: false
 
+- id: no-production-newtonsoft
+  name: reject production Newtonsoft references
+  description: |
+    Case-insensitive scan of every tracked .NET source and build file for a
+    Newtonsoft reference, allowed only under the path prefixes named by
+    --allow-prefix (the approved test and benchmark projects) and on the single
+    central PackageVersion line named by --central-version-file. This is the
+    static half of the policy; the transitive package-graph half needs `dotnet
+    restore` and private feed credentials, so it stays in the consumer's CI.
+    Enumerates files with `git ls-files`, not the staged set, so a violation in
+    a file this commit did not touch still fails.
+  entry: no-production-newtonsoft
+  language: python
+  pass_filenames: false
+  always_run: true
+
 - id: check-go-version-sync
   name: check go version sync (go.mod ↔ .tool-versions)
   description: |

README.md

@@ -12,6 +12,7 @@ repos.
 | `csharpier-worktree-guard` | Run `dotnet csharpier format .` (or `check .` via `CSHARPIER_MODE=check`) but fail loudly if csharpier reports "0 files" while the repo actually contains tracked `.cs` files. Catches the silent no-op observed when csharpier runs from inside a git worktree. | `*.cs` |
 | `service-yaml-check` | Static checks for new/changed `.platform/services/<svc>.yaml` files: chart path resolves, ECR repo / PIA / GHA push-role exist in TF, networkPolicy ingress ports match the chart's declared health port. Catches the post-merge failure modes from [platform-gitops#544](https://github.com/pinpredict/platform-gitops/issues/544). | `.platform/services/*.yaml` |
 | `stevedore-release-scope` | Assert that onboarding or retiring a service does not widen the shared image build contract: named services pair an `.stevedore.yaml` image id with a name-matching sibling chart, `docker-release` and `chart-release` receive the same `only:` selector, and `change_detection.shared_paths` carries the all-image signal without listing paths every onboarding touches. | `.stevedore.yaml`, `.github/workflows/ci.yml`, `charts/*/Chart.yaml` |
+| `no-production-newtonsoft` | Reject Newtonsoft.Json references in production .NET sources: a case-insensitive scan of every tracked source and build file, permitted only under the `--allow-prefix` paths (the approved test/benchmark projects) and on the one central `PackageVersion` line. Static half only — the transitive package-graph half needs `dotnet restore` and stays in the consumer's CI. | every commit (whole-tree scan) |
 | `check-go-version-sync` | Fails when a `go.mod` `go` directive and the governing `.tool-versions` `golang` pin drift apart. | `go.mod`, `.tool-versions` |
 
 ## Using a hook
@@ -153,6 +154,51 @@ variant frozen; the docker/chart consistency check runs either way.
 All violations are collected and reported in one run rather than failing on the
 first, so a single `pre-commit run` shows the whole picture.
 
+## `no-production-newtonsoft`
+
+Production .NET code should use `System.Text.Json`; a `Newtonsoft.Json`
+reference is allowed only in explicitly approved test and benchmark projects.
+Nothing is exempt by default — name every permitted prefix:
+
+```yaml
+- repo: https://github.com/pinpredict/pre-commit-hooks
+  rev: v0.4.0
+  hooks:
+    - id: no-production-newtonsoft
+      args:
+        - --allow-prefix=PinPredict.Tests/
+        - --allow-prefix=PinPredict.ParlayManager.Tests/
+        - --allow-prefix=PinPredict.Benchmarks/
+        - --central-version-file=Directory.Packages.props
+```
+
+| Flag | Purpose |
+|---|---|
+| `--allow-prefix PREFIX` (repeatable) | path prefix where a reference is permitted; nothing is allowed by default |
+| `--central-version-file PATH` | the one file permitted to declare the package's central `PackageVersion` |
+| `--token TOKEN` | case-insensitive token that marks a violation (default `newtonsoft`) |
+| `--package ID` | package id for the central-version exemption (default `<token>.Json`) |
+| `--source-glob GLOB` (repeatable) | git pathspecs to scan (default `*.cs *.csproj *.props *.targets`) |
+| `--root PATH` | repository root to scan (default the working directory) |
+
+**This is the static half of the policy only.** Catching Newtonsoft that arrives
+*transitively* needs `dotnet list package --include-transitive`, which needs
+`dotnet restore` and the solution's private feed credentials — far too slow and
+too credential-bound for a commit hook. Keep that guard in your own CI beside
+the SDK install; this hook covers the direct references, which are the ones a
+commit actually introduces.
+
+Two contract details worth knowing before you tune it:
+
+- **The scan is whole-tree, not staged-files.** Files come from `git ls-files`
+  and the hook runs `pass_filenames: false` / `always_run: true`. The policy is a
+  property of the repository, so a violation sitting in a file your commit never
+  touched must still fail — scoping to changed files would let a pre-existing
+  reference stay invisible forever.
+- **The central-version exemption is scoped to one named file.** A
+  `PackageVersion` element copied into an ordinary project file is still a
+  violation, so the exemption can't be used to smuggle a reference in.
+
 ## Why this repo exists
 
 Pre-commit hooks defined as `repo: local` in a `.pre-commit-config.yaml` are

pinpredict_hooks/_common.py

@@ -0,0 +1,44 @@
+#!/usr/bin/env python3
+"""Helpers shared by the Python hooks in this repo.
+
+Private module — not exposed as a console script. Everything here is behavior
+that every hook needs to get identically right: reading an optional YAML file,
+and reporting failures so GitHub Actions annotates them while a local terminal
+stays readable.
+"""
+
+from __future__ import annotations
+
+import os
+from pathlib import Path
+from typing import Any
+
+import yaml
+
+# Distinguishes "the file is not there" from "the file parsed to None". A hook
+# must treat an absent surface as a clean no-op but an empty one as a real
+# (checkable) document — collapsing both to None loses that.
+MISSING = object()
+
+
+def load_yaml(path: Path, prog: str) -> Any:
+    """Parse `path`, returning MISSING when the file does not exist.
+
+    A malformed document is fatal rather than a skipped check: silently passing
+    on unparseable YAML is how a guard stops guarding without anyone noticing.
+    """
+    try:
+        text = path.read_text(encoding="utf-8")
+    except FileNotFoundError:
+        return MISSING
+    try:
+        return yaml.safe_load(text)
+    except yaml.YAMLError as error:
+        raise SystemExit(f"{prog}: {path} is not valid YAML: {error}")
+
+
+def emit(failures: list[str], label: str) -> None:
+    """Print each failure, as a GHA annotation under Actions and plainly elsewhere."""
+    prefix = "::error::" if os.environ.get("GITHUB_ACTIONS") == "true" else "ERROR: "
+    for failure in failures:
+        print(f"{prefix}{label}: {failure}")

pinpredict_hooks/no_production_newtonsoft.py

@@ -0,0 +1,201 @@
+#!/usr/bin/env python3
+"""Reject Newtonsoft.Json references in production .NET sources.
+
+Production code should use `System.Text.Json`; a Newtonsoft reference is allowed
+only in explicitly approved test and benchmark projects. This hook is the
+**static** half of that policy: a case-insensitive scan of every tracked source
+and build file, with no `dotnet` involvement, so it runs on every commit in well
+under a second.
+
+The **transitive** half — resolving each project's package graph via
+`dotnet list package --include-transitive` to catch Newtonsoft arriving through
+a dependency — deliberately does *not* live here. It needs `dotnet restore`,
+the solution's private package feeds, and credentials, so it belongs in CI
+alongside the SDK install. Consumers keep that guard in their own repo; this
+hook covers the direct references, which are the ones a commit introduces.
+
+Every repo-specific detail is a flag, so the hook carries no consumer's layout:
+
+  * `--allow-prefix` — path prefixes where a reference is permitted (the test
+    and benchmark projects). Nothing is allowed by default.
+  * `--central-version-file` — a central package-management file permitted to
+    declare the `PackageVersion` for the forbidden package, since pinning a
+    version centrally is how the approved test projects consume it at all.
+  * `--source-glob` / `--token` — override the file set and the matched token.
+
+Files are enumerated with `git ls-files` rather than from pre-commit's staged
+filenames, and the hook runs with `pass_filenames: false`. That is deliberate:
+the policy is a property of the whole tree, so a violation sitting in a file
+this commit did not touch must still fail. Scanning only changed files would
+let a pre-existing reference stay invisible forever.
+"""
+
+from __future__ import annotations
+
+import argparse
+import re
+import subprocess
+import sys
+from pathlib import Path
+
+from pinpredict_hooks._common import emit
+
+PROG = "no-production-newtonsoft"
+LABEL = "production newtonsoft"
+
+DEFAULT_TOKEN = "newtonsoft"
+DEFAULT_SOURCE_GLOBS = ("*.cs", "*.csproj", "*.props", "*.targets")
+
+
+def central_version_pattern(package: str) -> re.Pattern[str]:
+    """Match a `<PackageVersion Include="<package>" .../>` declaration."""
+    return re.compile(
+        rf"<\s*PackageVersion\b[^>]*\bInclude\s*=\s*['\"]{re.escape(package)}['\"][^>]*/?\s*>",
+        re.IGNORECASE,
+    )
+
+
+def is_allowed_path(path: Path, allowed_prefixes: tuple[str, ...]) -> bool:
+    return path.as_posix().startswith(allowed_prefixes)
+
+
+def is_allowed_central_version(
+    path: Path, line: str, central_file: str | None, pattern: re.Pattern[str]
+) -> bool:
+    """True for the one line that centrally pins the forbidden package's version.
+
+    Scoped to the named file so a `PackageVersion` element copied into an
+    ordinary project file is still a violation.
+    """
+    return (
+        central_file is not None
+        and path.as_posix() == central_file
+        and bool(pattern.search(line))
+    )
+
+
+def tracked_sources(root: Path, globs: tuple[str, ...]) -> list[Path]:
+    result = subprocess.run(
+        ["git", "-C", str(root), "ls-files", "--", *globs],
+        check=True,
+        capture_output=True,
+        text=True,
+    )
+    return [Path(line) for line in result.stdout.splitlines() if line]
+
+
+def find_static_violations(
+    root: Path,
+    paths: list[Path],
+    token: str,
+    allowed_prefixes: tuple[str, ...],
+    central_file: str | None,
+    pattern: re.Pattern[str],
+) -> list[tuple[Path, int]]:
+    needle = token.casefold()
+    failures: list[tuple[Path, int]] = []
+    for path in paths:
+        if is_allowed_path(path, allowed_prefixes):
+            continue
+        try:
+            text = (root / path).read_text(encoding="utf-8")
+        except (FileNotFoundError, UnicodeDecodeError):
+            # A tracked-but-absent path (mid-rebase) or a mislabeled binary is
+            # not this hook's failure to report.
+            continue
+        for number, line in enumerate(text.splitlines(), 1):
+            if needle in line.casefold() and not is_allowed_central_version(
+                path, line, central_file, pattern
+            ):
+                failures.append((path, number))
+    return failures
+
+
+def build_parser() -> argparse.ArgumentParser:
+    parser = argparse.ArgumentParser(
+        prog=PROG, description=__doc__ or "", allow_abbrev=False
+    )
+    parser.add_argument(
+        "--root",
+        default=".",
+        type=Path,
+        help="repository root to scan (default: the working directory)",
+    )
+    parser.add_argument(
+        "--token",
+        default=DEFAULT_TOKEN,
+        help=f"case-insensitive token that marks a violation (default: {DEFAULT_TOKEN})",
+    )
+    parser.add_argument(
+        "--package",
+        default=None,
+        metavar="ID",
+        help="package id for the central-version exemption (default: <token>.Json)",
+    )
+    parser.add_argument(
+        "--allow-prefix",
+        action="append",
+        default=[],
+        metavar="PREFIX",
+        help="path prefix where a reference is permitted, e.g. Acme.Tests/ (repeatable)",
+    )
+    parser.add_argument(
+        "--central-version-file",
+        default=None,
+        metavar="PATH",
+        help="file permitted to declare the package's central PackageVersion",
+    )
+    parser.add_argument(
+        "--source-glob",
+        action="append",
+        default=[],
+        metavar="GLOB",
+        help=f"git pathspec to scan (repeatable; default: {' '.join(DEFAULT_SOURCE_GLOBS)})",
+    )
+    # pre-commit may pass matched filenames even with pass_filenames: false;
+    # accept and ignore them so the hook stays robust.
+    parser.add_argument("filenames", nargs="*", help=argparse.SUPPRESS)
+    return parser
+
+
+def main(argv: list[str] | None = None) -> int:
+    args = build_parser().parse_args(argv)
+
+    globs = tuple(args.source_glob) or DEFAULT_SOURCE_GLOBS
+    package = args.package or f"{args.token}.Json"
+    pattern = central_version_pattern(package)
+    allowed_prefixes = tuple(args.allow_prefix)
+
+    sources = tracked_sources(args.root, globs)
+    failures = find_static_violations(
+        args.root,
+        sources,
+        args.token,
+        allowed_prefixes,
+        args.central_version_file,
+        pattern,
+    )
+
+    if failures:
+        emit(
+            [
+                f"{path}:{number} references {args.token!r} — production code must use "
+                "System.Text.Json; compatibility references are allowed only under "
+                f"{', '.join(allowed_prefixes) or 'no approved prefix'}"
+                for path, number in failures
+            ],
+            LABEL,
+        )
+        print(
+            f"\n{len(failures)} production {args.token} reference(s) in "
+            f"{len({path for path, _ in failures})} file(s).",
+            file=sys.stderr,
+        )
+        return 1
+
+    print(f"{PROG}: no direct production {args.token} references in {len(sources)} file(s).")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())

pinpredict_hooks/stevedore_release_scope.py

@@ -29,26 +29,17 @@
 from __future__ import annotations
 
 import argparse
-import os
 import sys
 from pathlib import Path
 from typing import Any
 
-import yaml
+from pinpredict_hooks._common import MISSING, emit as _emit, load_yaml as _load_yaml
 
-MISSING = object()
+PROG = "stevedore-release-scope"
 
 
 def load_yaml(path: Path) -> Any:
-    """Parse `path`, returning MISSING when the file does not exist."""
-    try:
-        text = path.read_text(encoding="utf-8")
-    except FileNotFoundError:
-        return MISSING
-    try:
-        return yaml.safe_load(text)
-    except yaml.YAMLError as error:
-        raise SystemExit(f"stevedore-release-scope: {path} is not valid YAML: {error}")
+    return _load_yaml(path, PROG)
 
 
 def image_ids(catalog: Any) -> list[str]:
@@ -157,10 +148,7 @@ def check_selectors(
 
 
 def emit(failures: list[str]) -> None:
-    github = os.environ.get("GITHUB_ACTIONS") == "true"
-    for failure in failures:
-        prefix = "::error::" if github else "ERROR: "
-        print(f"{prefix}stevedore release scope: {failure}")
+    _emit(failures, "stevedore release scope")
 
 
 def build_parser() -> argparse.ArgumentParser: