fix(native): validate XML plist source admission

2026-10-26 14:25:49 -04:00 · 2026-10-24 10:46:42 +08:00
parent 2bd981b017
commit 9f5dd56e2f
3 changed files with 194 additions and 48 deletions
@@ -464,9 +464,10 @@ numbers requiring XML type facts retain the `plutil` route. Invalid UTF-8 and
 encoding-declaration mismatches remain explicit failures. Direct-source
 provenance records `file` without claiming conversion; conversion limitations
 and invocations appear only when conversion actually ran.
-Malformed XML structure remains an input failure; malformed scalar literals
-retain their exact source text, reason and counted unknown-scalar limitation,
-using the same scalar interpretation as archive readers.
+XML sources must also pass native format validation, with native refusals retaining
+their diagnostics and successful validation recorded in provenance. Original
+unknown scalar text, reasons and limitations survive successful validation;
+output-budget omissions remain omissions.

 Source input is limited to 64 MiB. Binary and direct XML decoding reserve an
 estimated 64 MiB decode budget before parsing; this is not a hard process-memory
@@ -1090,8 +1090,31 @@ class NativeMacOSClient implements AnalysisClient {
    const source = await readNativePlistBytes(fileInput);
    if (!source.ok) return source;
    let parsed: ReturnType<typeof parsePlistXmlSource>;
+    const provenance = [
+      invocation(classified, fileInput.selection.path, "$PLIST"),
+    ];
    if (format === "xml") {
      parsed = parsePlistXmlSource(source.value);
+      if (parsed.ok) {
+        const validated = await this.#run(
+          "inspect_plist",
+          "plutil",
+          ["-lint", "--", fileInput.selection.path],
+          {
+            signal: fileInput.signal,
+            acceptNonZero: true,
+            timeoutMs: fileInput.timeoutMs,
+          },
+        );
+        if (!validated.ok) return validated;
+        if (validated.value.exitCode !== 0)
+          return err(
+            nativePlistDecodeFailure(fileInput.selection, validated.value),
+          );
+        provenance.push(
+          invocation(validated.value, fileInput.selection.path, "$PLIST"),
+        );
+      }
      if (
        !parsed.ok &&
        parsed.error instanceof XmlPlistTextDecodeError &&
@@ -1125,9 +1148,7 @@ class NativeMacOSClient implements AnalysisClient {
        format,
        path: fileInput.selection.path,
        resourceRoot: this.bundleResourceRoot,
-        provenance: [
-          invocation(classified, fileInput.selection.path, "$PLIST"),
-        ],
+        provenance,
      }),
    );
  }
@@ -1168,24 +1189,7 @@ class NativeMacOSClient implements AnalysisClient {
      if (!converted.ok) return converted;
      xml = converted.value;
      if (xml.exitCode !== 0) {
-        const capture = failureCapture(xml, false);
-        return err(
-          unreadablePlist(
-            fileInput.selection,
-            `plutil could not decode it as a property list (${plutilDiagnostic(capture, path)})`,
-            {
-              capturedOutput: {
-                stdout: capture.stdout,
-                stderr: capture.stderr,
-                truncated: capture.truncated,
-                stdout_bytes: capture.stdoutBytes,
-                stderr_bytes: capture.stderrBytes,
-                exit_code: capture.exitCode,
-                signal: capture.signal,
-              },
-            },
-          ),
-        );
+        return err(nativePlistDecodeFailure(fileInput.selection, xml));
      }
      parsed =
        jsonAttempt === undefined
@@ -1560,7 +1564,29 @@ const parseEntitlements = (
  };
 };

-/** Failed plutil conversions can report on either stream; preserve both. */
+/** Native admission failures preserve the selected/default source and physical command output. */
+const nativePlistDecodeFailure = (
+  selection: NativePlistSelection,
+  command: NativeCommandCapture,
+): AnalysisError => {
+  const capture = failureCapture(command, false);
+  return unreadablePlist(
+    selection,
+    `plutil could not decode it as a property list (${plutilDiagnostic(capture, selection.path)})`,
+    {
+      capturedOutput: {
+        stdout: capture.stdout,
+        stderr: capture.stderr,
+        truncated: capture.truncated,
+        stdout_bytes: capture.stdoutBytes,
+        stderr_bytes: capture.stderrBytes,
+        exit_code: capture.exitCode,
+        signal: capture.signal,
+      },
+    },
+  );
+};
+
 const plutilDiagnostic = (
  capture: Pick<NativeCommandCapture, "stdout" | "stderr">,
  path: string,
@@ -148,12 +148,18 @@ describe.skipIf(process.platform !== "darwin")(
  "native plist CLI decode diagnostics",
  () => {
    cliTest.for([
-      "not a property list",
-      "<plist><dict><key>value</key><integer> 42 </integer></dict></plist>",
-      "<plist><dict><key>value</key><true>x</true></dict></plist>",
-    ])(
+      ["not a property list", "Property List error:"],
+      [
+        "<plist><dict><key>value</key><integer> 42 </integer></dict></plist>",
+        "Unknown character",
+      ],
+      [
+        "<plist><dict><key>value</key><true>x</true></dict></plist>",
+        "Encountered non-empty <true>",
+      ],
+    ] as const)(
      "preserves native decode diagnostics for %s",
-      async (source, { cli }) => {
+      async ([source, diagnostic], { cli }) => {
        const app = await fixtureApp();
        const malformed = join(app, "Contents/Broken.plist");
        await writeFile(malformed, source);
@@ -171,15 +177,11 @@ describe.skipIf(process.platform !== "darwin")(
        expect(selected.json).toMatchObject({
          code: "invalid_request",
          details: {
-            issues: [
-              { message: expect.stringContaining("Property List error:") },
-            ],
+            issues: [{ message: expect.stringContaining(diagnostic) }],
            captured_output: { exit_code: 1 },
          },
        });
-        expect(capturedDiagnostic(selected.json)).toContain(
-          "Property List error:",
-        );
+        expect(capturedDiagnostic(selected.json)).toContain(diagnostic);
        const directory = await createTestTempDirectory(
          "rea-plist-default-diagnostic-",
        );
@@ -196,13 +198,11 @@ describe.skipIf(process.platform !== "darwin")(
        expect(fallback.json).toMatchObject({
          code: "capability_unavailable",
          details: {
-            reason: expect.stringContaining("Property List error:"),
+            reason: expect.stringContaining(diagnostic),
            captured_output: { exit_code: 1 },
          },
        });
-        expect(capturedDiagnostic(fallback.json)).toContain(
-          "Property List error:",
-        );
+        expect(capturedDiagnostic(fallback.json)).toContain(diagnostic);
        expect(await readFile(malformed, "utf8")).toBe(source);
        expect(await readFile(defaultPlist, "utf8")).toBe(source);
      },
@@ -214,11 +214,27 @@ describe.skipIf(process.platform !== "darwin")(
  "native plist CLI direct target selection",
  () => {
    cliTest.for([
-      "<plist><dict><key>value</key><integer> 42 </integer></dict></plist>",
-      "<plist><dict><key>value</key><true>x</true></dict></plist>",
-    ])(
+      [
+        "<plist><dict><key>value</key><integer> 42 </integer></dict></plist>",
+        "Unknown character",
+      ],
+      [
+        "<plist><dict><key>value</key><true>x</true></dict></plist>",
+        "Encountered non-empty <true>",
+      ],
+      [
+        "<plist><dict><key>__proto__</key><true>x</true></dict></plist>",
+        "Encountered non-empty <true>",
+      ],
+      ["<plist><null/></plist>", "Encountered unknown tag null"],
+      ["<plist/>", "Encountered empty plist tag"],
+      [
+        "<plist><integer>1</integer><integer>2</integer></plist>",
+        "plist can only include one object",
+      ],
+    ] as const)(
      "classifies the selected malformed plist as input: %s",
-      async (source, { cli }) => {
+      async ([source, diagnostic], { cli }) => {
        const root = await createTestTempDirectory("rea-plist-target-input-");
        const path = join(root, "chosen.plist");
        await writeFile(path, source);
@@ -234,15 +250,13 @@ describe.skipIf(process.platform !== "darwin")(
              issues: [
                {
                  path: ["path"],
-                  message: expect.stringContaining("Property List error:"),
+                  message: expect.stringContaining(diagnostic),
                },
              ],
              captured_output: { exit_code: 1 },
            },
          });
-          expect(capturedDiagnostic(result.json)).toContain(
-            "Property List error:",
-          );
+          expect(capturedDiagnostic(result.json)).toContain(diagnostic);
        }
        expect(await readFile(path, "utf8")).toBe(source);
      },
@@ -250,6 +264,111 @@ describe.skipIf(process.platform !== "darwin")(
  },
 );

+describe.skipIf(process.platform !== "darwin")(
+  "native plist CLI source admission",
+  () => {
+    cliTest(
+      "accepts native nested plist syntax without imposing an XML grammar allowlist",
+      async ({ cli }) => {
+        const root = await createTestTempDirectory(
+          "rea-plist-nested-admission-",
+        );
+        const path = join(root, "chosen.plist");
+        const source = "<plist><plist><integer>1</integer></plist></plist>";
+        await writeFile(path, source);
+        const result = await cli.run({
+          arguments: ["inspect-plist", path, "--json"],
+          environment,
+        });
+        expect(result.exitCode, result.stderr).toBe(0);
+        expect(result.json).toMatchObject({
+          normalized_result: {
+            source_path: path,
+            provenance: expect.arrayContaining([
+              expect.objectContaining({
+                tool: "plutil",
+                command: ["/usr/bin/plutil", "-lint", "--", "$PLIST"],
+              }),
+            ]),
+          },
+        });
+        expect(await readFile(path, "utf8")).toBe(source);
+      },
+    );
+    cliTest(
+      "omits a valid prototype key after native validation",
+      async ({ cli }) => {
+        const root = await createTestTempDirectory("rea-plist-valid-omission-");
+        const path = join(root, "chosen.plist");
+        const source =
+          "<plist><dict><key>__proto__</key><integer>42</integer><key>kept</key><string>value</string></dict></plist>";
+        await writeFile(path, source);
+        const result = await cli.run({
+          arguments: ["inspect-plist", path, "--json"],
+          environment,
+        });
+        expect(result.exitCode, result.stderr).toBe(0);
+        expect(result.json).toMatchObject({
+          normalized_result: {
+            value: { kept: "value" },
+            provenance: [
+              expect.objectContaining({ tool: "file" }),
+              expect.objectContaining({
+                tool: "plutil",
+                command: ["/usr/bin/plutil", "-lint", "--", "$PLIST"],
+              }),
+            ],
+            limitations: expect.arrayContaining([
+              expect.stringContaining(
+                "1 dictionary entry keyed __proto__ was omitted",
+              ),
+            ]),
+          },
+        });
+        expect(await readFile(path, "utf8")).toBe(source);
+      },
+    );
+    cliTest(
+      "retains original unknown scalar facts after successful native validation",
+      async ({ cli }) => {
+        const root = await createTestTempDirectory(
+          "rea-plist-source-admission-",
+        );
+        const path = join(root, "chosen.plist");
+        const source =
+          "<plist><dict><key>value</key><real>1e999</real></dict></plist>";
+        await writeFile(path, source);
+        const result = await cli.run({
+          arguments: ["inspect-plist", path, "--json"],
+          environment,
+        });
+        expect(result.exitCode, result.stderr).toBe(0);
+        expect(result.json).toMatchObject({
+          normalized_result: {
+            format: "xml",
+            source_path: path,
+            value: {
+              value: {
+                $plist_type: "unknown",
+                element: "real",
+                literal: "1e999",
+                reason: "unrepresentable",
+              },
+            },
+            provenance: expect.arrayContaining([
+              expect.objectContaining({
+                tool: "plutil",
+                command: ["/usr/bin/plutil", "-lint", "--", "$PLIST"],
+              }),
+            ]),
+          },
+        });
+        expect(await readFile(path, "utf8")).toBe(source);
+      },
+    );
+  },
+);
+
 describe.skipIf(process.platform !== "darwin")(
  "native plist CLI fixed allowances",
  () => {