pinact: fix SARIF fixes corrupting lines on apply

eslint.config.cjs

@@ -126,6 +126,8 @@ module.exports = [
     },
     rules: {
       ...jestPlugin.configs.recommended.rules,
+      // conditionalTest (tests/utils) is the repo's it()/it.skip() wrapper.
+      "jest/no-standalone-expect": ["error", { additionalTestBlockFunctions: ["conditionalTest"] }],
     },
   },
   {

linters/pinact/pinact.test.ts

@@ -1,8 +1,10 @@
+import { execFileSync } from "child_process";
 import * as fs from "fs";
+import * as os from "os";
 import * as path from "path";
-import { customLinterCheckTest } from "tests";
+import { customLinterCheckTest, setupLintDriver } from "tests";
 import { TrunkLintDriver } from "tests/driver";
-import { TEST_DATA } from "tests/utils";
+import { conditionalTest, TEST_DATA } from "tests/utils";
 
 const moveWorkflowFile =
   (filename: string, disableGhAuth = false) =>
@@ -58,6 +60,18 @@ const skipIfMissingGitHubToken = () => {
   return false;
 };
 
+const resolvePython = (): string | undefined => {
+  for (const bin of ["python3", "python"]) {
+    try {
+      execFileSync(bin, ["--version"], { stdio: "ignore" });
+      return bin;
+    } catch {
+      // Try the next candidate.
+    }
+  }
+  return undefined;
+};
+
 const preCheckBadConfig = async (driver: TrunkLintDriver) => {
   process.env.PINACT_DISABLE_GH_AUTH = "1";
   driver.moveFile(path.join(TEST_DATA, "bad.pinact.yaml"), path.join(".pinact.yaml"));
@@ -104,3 +118,182 @@ customLinterCheckTest({
   preCheck: enablePinactCommand("upgrade", moveWorkflowFile("unpinned.in.yaml")),
   skipTestIf: skipIfMissingGitHubToken,
 });
+
+// The snapshot tests above never apply a fix (the driver's runCheck forces
+// `-n`), so none of them caught pinact SARIF whose `deletedRegion` was
+// line-only: Trunk read that as a zero-width insert and concatenated the pinned
+// `uses:` with the original one on a single line. This applies the fix for real
+// and asserts the rewrite is clean. Kept online (a resolvable SHA is required to
+// exercise the fix path) and version-independent (no snapshot of the volatile
+// SHA) — it only asserts the structural invariant the bug violated.
+describe("Testing linter pinact fix application", () => {
+  const driver = setupLintDriver(
+    __dirname,
+    {},
+    "pinact",
+    undefined,
+    moveWorkflowFile("unpinned.in.yaml"),
+  );
+
+  conditionalTest(
+    skipIfMissingGitHubToken(),
+    "pins to a SHA without corrupting the line",
+    async () => {
+      await driver
+        .runTrunkCmd("check --filter=pinact --fix -y --no-progress --ignore-git-state .github")
+        .catch(() => undefined);
+
+      const fixed = driver.readFile(".github/workflows/unpinned.in.yaml");
+      // The action is pinned to a full 40-char SHA with its version comment...
+      expect(fixed).toMatch(/uses: actions\/checkout@[0-9a-f]{40} # v\d/);
+      // ...and no line carries the concatenated `<pinned> # … <original>` corruption.
+      expect(fixed).not.toMatch(/uses:.*#.*uses:/);
+      // The single input `uses:` stays single — the corruption doubled it.
+      expect(fixed.match(/uses:/g)).toHaveLength(1);
+    },
+  );
+});
+
+interface FixRegion {
+  startLine: number;
+  startColumn?: number;
+  endLine?: number;
+  endColumn?: number;
+}
+
+interface FixSarif {
+  runs: {
+    results: {
+      fixes: { artifactChanges: { replacements: { deletedRegion: FixRegion }[] }[] }[];
+    }[];
+  }[];
+}
+
+// Deterministic, offline coverage of the SARIF fix-region normalization that
+// pinact_run.py applies before Trunk consumes it. pinact can only pin online
+// (it resolves tags -> SHAs via the GitHub API), so the end-to-end fix test
+// above is token-gated; this drives the pure transformation directly, so the
+// invariant is locked on every CI run with no token or network.
+describe("pinact SARIF fix-region normalization", () => {
+  const pythonBin = resolvePython();
+  // A representative unpinned step; the trailing `@v4` is what pinact rewrites.
+  const line = "      - uses: actions/checkout@v4";
+  let sandbox: string;
+
+  beforeAll(() => {
+    sandbox = fs.mkdtempSync(path.join(os.tmpdir(), "pinact-normalize-"));
+    const workflowDir = path.join(sandbox, ".github", "workflows");
+    fs.mkdirSync(workflowDir, { recursive: true });
+    fs.writeFileSync(path.join(workflowDir, "wf.yaml"), `jobs:\n  a:\n    steps:\n${line}\n`);
+  });
+
+  afterAll(() => {
+    if (sandbox) {
+      fs.rmSync(sandbox, { recursive: true, force: true });
+    }
+  });
+
+  // Invoke pinact_run.normalize_fix_regions on `sarif` with cwd at the sandbox,
+  // so the relative artifact URI resolves to the fixture workflow above.
+  const normalize = (sarif: unknown): FixSarif => {
+    const script =
+      "import sys; sys.path.insert(0, sys.argv[1]); import pinact_run; " +
+      "sys.stdout.write(pinact_run.normalize_fix_regions(sys.stdin.read()))";
+    const out = execFileSync(pythonBin ?? "python3", ["-c", script, __dirname], {
+      input: JSON.stringify(sarif),
+      cwd: sandbox,
+      encoding: "utf8",
+    });
+    return JSON.parse(out) as FixSarif;
+  };
+
+  const sarifWithRegion = (deletedRegion: FixRegion) => ({
+    runs: [
+      {
+        results: [
+          {
+            fixes: [
+              {
+                artifactChanges: [
+                  {
+                    artifactLocation: { uri: ".github/workflows/wf.yaml" },
+                    replacements: [
+                      {
+                        deletedRegion,
+                        insertedContent: { text: line.replace("@v4", "@<sha> # v4.4.0") },
+                      },
+                    ],
+                  },
+                ],
+              },
+            ],
+          },
+        ],
+      },
+    ],
+  });
+
+  const regionOf = (sarif: FixSarif): FixRegion =>
+    sarif.runs[0].results[0].fixes[0].artifactChanges[0].replacements[0].deletedRegion;
+
+  conditionalTest(
+    pythonBin === undefined,
+    "widens a line-only region to span the whole original line",
+    () => {
+      const out = normalize(sarifWithRegion({ startLine: 4 }));
+      // Full-line replacement covers columns 1..len(line); endColumn is exclusive.
+      expect(regionOf(out)).toEqual({
+        startLine: 4,
+        startColumn: 1,
+        endLine: 4,
+        endColumn: line.length + 1,
+      });
+    },
+  );
+
+  conditionalTest(
+    pythonBin === undefined,
+    "leaves an already fully-specified region unchanged",
+    () => {
+      const region: FixRegion = { startLine: 4, startColumn: 5, endLine: 4, endColumn: 10 };
+      expect(regionOf(normalize(sarifWithRegion(region)))).toEqual(region);
+    },
+  );
+
+  conditionalTest(
+    pythonBin === undefined,
+    "preserves an explicit startColumn while backfilling the line end",
+    () => {
+      const out = normalize(sarifWithRegion({ startLine: 4, startColumn: 9 }));
+      expect(regionOf(out)).toEqual({
+        startLine: 4,
+        startColumn: 9,
+        endLine: 4,
+        endColumn: line.length + 1,
+      });
+    },
+  );
+
+  conditionalTest(
+    pythonBin === undefined,
+    "backfills endColumn when only startLine and endLine are given",
+    () => {
+      const out = normalize(sarifWithRegion({ startLine: 4, endLine: 4 }));
+      expect(regionOf(out)).toEqual({
+        startLine: 4,
+        startColumn: 1,
+        endLine: 4,
+        endColumn: line.length + 1,
+      });
+    },
+  );
+
+  conditionalTest(
+    pythonBin === undefined,
+    "leaves a line-only region untouched when the target line is out of range",
+    () => {
+      const region: FixRegion = { startLine: 999 };
+      expect(regionOf(normalize(sarifWithRegion(region)))).toEqual(region);
+    },
+  );
+});

linters/pinact/pinact_run.py

@@ -105,6 +105,73 @@ def build_pinact_args(mode: str) -> list[str]:
     return args
 
 
+def normalize_fix_regions(sarif_text: str) -> str:
+    """Backfill a concrete line end on pinact's SARIF fix regions.
+
+    pinact emits each replacement's ``deletedRegion`` without an end column
+    (typically just ``{"startLine": N}``). Trunk's fix applier reads a region
+    with no explicit end as a zero-width insertion point, so it *prepends* the
+    pinned ``uses:`` and never deletes the original line — concatenating both
+    onto one line. Backfill only what's missing — preserving any explicit
+    ``startColumn``/``endLine`` — so the region carries a concrete end
+    (``endColumn`` at the end of its end line) that Trunk replaces rather than
+    inserts, matching the fully-specified regions the ruff/sqlfluff converters
+    already rely on. Regions that already carry an ``endColumn`` or an
+    offset-based span (``charOffset``/``charLength``) are left untouched.
+    """
+    try:
+        sarif = json.loads(sarif_text)
+    except (json.JSONDecodeError, TypeError):
+        return sarif_text
+
+    line_cache: dict[str, list[str]] = {}
+
+    def lines_for(uri: str) -> list[str] | None:
+        if uri not in line_cache:
+            try:
+                line_cache[uri] = Path(uri).read_text(encoding="utf-8").splitlines()
+            except OSError:
+                line_cache[uri] = []
+        return line_cache[uri] or None
+
+    for run in sarif.get("runs", []):
+        for result in run.get("results", []):
+            for fix in result.get("fixes", []):
+                for change in fix.get("artifactChanges", []):
+                    uri = change.get("artifactLocation", {}).get("uri")
+                    if not uri:
+                        continue
+                    for replacement in change.get("replacements", []):
+                        region = replacement.get("deletedRegion")
+                        if not region or "startLine" not in region:
+                            continue
+                        # An explicit end column or an offset-based span is
+                        # unambiguous — Trunk applies it as-is, so leave it alone.
+                        if any(
+                            key in region
+                            for key in ("endColumn", "charOffset", "charLength")
+                        ):
+                            continue
+                        lines = lines_for(uri)
+                        if lines is None:
+                            continue
+                        start_index = region["startLine"] - 1
+                        end_line = region.get("endLine", region["startLine"])
+                        end_index = end_line - 1
+                        if not 0 <= start_index < len(
+                            lines
+                        ) or not 0 <= end_index < len(lines):
+                            continue
+                        # Preserve any explicit start/end line; only backfill what's
+                        # missing so the region carries a concrete end (end of its end
+                        # line) that Trunk won't read as a zero-width insert.
+                        region.setdefault("startColumn", 1)
+                        region["endLine"] = end_line
+                        region["endColumn"] = len(lines[end_index]) + 1
+
+    return json.dumps(sarif, indent=2)
+
+
 def strip_ansi(text: str) -> str:
     return ANSI_ESCAPE.sub("", text)
 
@@ -183,8 +250,9 @@ def run_pinact(mode: str, targets: list[str]) -> int:
         return 2
 
     if stdout:
-        sys.stdout.write(stdout)
-        if not stdout.endswith("\n"):
+        normalized = normalize_fix_regions(stdout)
+        sys.stdout.write(normalized)
+        if not normalized.endswith("\n"):
             sys.stdout.write("\n")
     if stderr:
         sys.stderr.write(stderr)