Merge pull request #207 from morluto/agent/fix-native-command-edge-cases

fix(native): harden command capture edge cases

src/native/CommandRunner.ts

@@ -16,8 +16,10 @@ export interface NativeCommandCapture {
   readonly stderr: string;
   readonly stdoutBytes: number;
   readonly stderrBytes: number;
-  readonly stdoutTruncated: boolean;
-  readonly stderrTruncated: boolean;
+  /** Successful captures are exhaustive; output-limit exits return a failure. */
+  readonly stdoutTruncated: false;
+  /** Successful captures are exhaustive; output-limit exits return a failure. */
+  readonly stderrTruncated: false;
   readonly exitCode: number | null;
   readonly signal: string | null;
 }

src/native/NativeMacOSProvider.ts

@@ -199,10 +199,11 @@ class NativeMacOSClient implements AnalysisClient {
     );
     if (!capture.ok) return capture;
     const provenance = [invocation(capture.value, this.target.path)];
+    const architectures = parseLipoArchitectures(capture.value.stdout);
     const result = listArchitecturesSchema.parse({
       architectures: {
-        items: parseLipoArchitectures(capture.value.stdout),
-        total: parseLipoArchitectures(capture.value.stdout).length,
+        items: architectures,
+        total: architectures.length,
         exhaustive: true,
         limitations: [],
       },

src/native/NativeMachoInspection.ts

@@ -32,7 +32,7 @@ interface NativeMachoInspectionContext {
   ) => NativeCommandInvocation;
 }
 
-const COMMANDS = [
+const REQUIRED_COMMANDS = [
   ["file", ["-b"]],
   ["lipo", ["-detailed_info"]],
   ["otool", ["-l"]],
@@ -40,15 +40,18 @@ const COMMANDS = [
   ["dyld_info", ["-imports"]],
   ["dyld_info", ["-exports"]],
   ["dwarfdump", ["--uuid"]],
-  ["vtool", ["-show-build"]],
 ] as const;
 
+const OPTIONAL_VTOOL_COMMAND = ["vtool", ["-show-build"]] as const;
+const VTOOL_UNAVAILABLE_LIMITATION =
+  "vtool is unavailable; build metadata is normalized from otool only.";
+
 /** Inspect one Mach-O with bounded native commands and normalized output. */
 export const inspectNativeMacho = async (
   context: NativeMachoInspectionContext,
 ): Promise<Result<NativeMachoObservation, AnalysisError>> => {
   const captures: NativeCommandCapture[] = [];
-  for (const [tool, prefix] of COMMANDS) {
+  for (const [tool, prefix] of REQUIRED_COMMANDS) {
     const captured = await context.run(
       tool,
       [...prefix, context.target.path],
@@ -57,7 +60,18 @@ export const inspectNativeMacho = async (
     if (!captured.ok) return err(captured.error);
     captures.push(captured.value);
   }
-  const result = normalizeMacho(captures, context.invocation);
+  const limitations: string[] = [];
+  const [tool, prefix] = OPTIONAL_VTOOL_COMMAND;
+  const optional = await context.run(
+    tool,
+    [...prefix, context.target.path],
+    context.signal,
+  );
+  if (optional.ok) captures.push(optional.value);
+  else if (optional.error._tag === "AnalysisCapabilityUnavailableError")
+    limitations.push(VTOOL_UNAVAILABLE_LIMITATION);
+  else return err(optional.error);
+  const result = normalizeMacho(captures, context.invocation, limitations);
   return ok({
     result: jsonValueSchema.parse(result),
     provenance: result.provenance,
@@ -69,6 +83,7 @@ export const inspectNativeMacho = async (
 const normalizeMacho = (
   captures: readonly NativeCommandCapture[],
   toInvocation: (capture: NativeCommandCapture) => NativeCommandInvocation,
+  additionalLimitations: readonly string[],
 ) => {
   const byTool = captureLookup(captures);
   const architectures = parseLipoArchitectures(byTool("lipo").stdout);
@@ -88,7 +103,12 @@ const normalizeMacho = (
   const provenance = captures.map(toInvocation);
   const limitations = [
     "Imports and exports combine dyld_info and nm; stripped or toolchain-hidden symbols may be absent.",
-    "vtool output is retained as provenance but only otool build metadata is normalized.",
+    ...(captures.some(({ tool }) => tool === "vtool")
+      ? [
+          "vtool output is retained as provenance but only otool build metadata is normalized.",
+        ]
+      : []),
+    ...additionalLimitations,
   ];
   return inspectMachoSchema.parse({
     format: "mach-o",

tests/nativeProvider.test.ts

@@ -41,6 +41,22 @@ describe("native macOS provider", () => {
     expect(resolutions).toBe(2);
   });
 
+  it("fails closed when a native command exceeds its output limit", async () => {
+    const runner = new XcrunCommandRunner(() =>
+      Promise.resolve(ok({ path: process.execPath, sha256: "a".repeat(64) })),
+    );
+    const captured = await runner.run(
+      "file",
+      ["-e", "process.stdout.write('x'.repeat(4096))"],
+      { timeoutMs: 1_000, maxOutputBytes: 64 },
+    );
+
+    expect(captured).toMatchObject({
+      ok: false,
+      error: { reason: "output-limit" },
+    });
+  });
+
   it("normalizes comprehensive Mach-O inspection with exact bounded provenance", async () => {
     const runner = new FixtureRunner();
     const client = new NativeMacOSProvider(runner, "darwin").createClient(
@@ -80,6 +96,41 @@ describe("native macOS provider", () => {
     });
   });
 
+  it("keeps Mach-O inspection available when optional vtool is unavailable", async () => {
+    const client = new NativeMacOSProvider(
+      new VtoolFailingRunner("unavailable"),
+      "darwin",
+    ).createClient(machoTarget("/private/fixture"));
+
+    const execution = await client.execute("inspect_macho", {});
+
+    expect(execution.ok).toBe(true);
+    if (!execution.ok) return;
+    expect(execution.value.limitations).toEqual(
+      expect.arrayContaining([expect.stringMatching(/vtool.*unavailable/iu)]),
+    );
+    expect(execution.value.result).toMatchObject({
+      architectures: { total: 2 },
+      provenance: expect.not.arrayContaining([
+        expect.objectContaining({ tool: "vtool" }),
+      ]),
+    });
+  });
+
+  it("propagates optional vtool failures other than unavailability", async () => {
+    const client = new NativeMacOSProvider(
+      new VtoolFailingRunner("timeout"),
+      "darwin",
+    ).createClient(machoTarget("/private/fixture"));
+
+    const execution = await client.execute("inspect_macho", {});
+
+    expect(execution).toMatchObject({
+      ok: false,
+      error: { _tag: "AnalysisTimeoutError" },
+    });
+  });
+
   it("supports architectures, signatures, plists, and ordered demangling", async () => {
     directory = await mkdtemp(join(tmpdir(), "rea-native-"));
     const app = join(directory, "Fixture.app");
@@ -208,6 +259,18 @@ class FailingRunner implements NativeCommandRunner {
   }
 }
 
+class VtoolFailingRunner implements NativeCommandRunner {
+  readonly #fixture = new FixtureRunner();
+
+  constructor(private readonly reason: NativeCommandFailure["reason"]) {}
+
+  run(tool: string, arguments_: readonly string[]) {
+    return tool === "vtool"
+      ? Promise.resolve(err(new NativeCommandFailure(tool, this.reason)))
+      : this.#fixture.run(tool, arguments_);
+  }
+}
+
 class CountingRunner implements NativeCommandRunner {
   calls = 0;