fix(exec): don't treat shell_exec() null return as an error (#3723)

Leiyks · web-flow · commit 55eb9e5d3426 · 2026-03-25T14:46:55.000+01:00
PHP's shell_exec() returns null both when a command produces no output AND when execution fails — the two cases are indistinguishable. Flagging null as an error causes false-positive error spans for any intentionally silent command (e.g. Symfony's `stty 2>/dev/null` called internally by Terminal::hasSttyAvailable()), which is especially harmful when DD_APM_FEATURES=error_rare_sample_tracer_drop is set, since those spans cannot be sampled out. The fix from cff2034 (v1.8.0) added hooks on two specific Symfony methods to suppress their spans, but the root defect in postHookShell() remained. Any other shell_exec() call returning null — from a Symfony bundle, third-party package, or application code — still produced an error span. Per PHP docs: "It is not possible to detect execution failures using this function." Remove the null check entirely; the false return check above it already handles the case where PHP itself cannot invoke the shell. Adds a regression test asserting that shell_exec('true') — which returns null due to no output — produces a clean span with no error. Fixes: APMS-18967
diff --git a/src/DDTrace/Integrations/Exec/ExecIntegration.php b/src/DDTrace/Integrations/Exec/ExecIntegration.php
@@ -255,8 +255,6 @@ private static function postHookShell($variant)
                 $span->exception = $hook->exception;
             } elseif ($hook->returned === false) {
                 $span->meta[Tag::ERROR_MSG] = "$variant() returned false";
-            } elseif ($hook->returned === null && $variant === 'shell_exec') {
-                $span->meta[Tag::ERROR_MSG] = "shell_exec() returned null";
             } elseif (
                 !empty($retCodeArg) &&
                 isset($hook->args[$retCodeArg]) &&
diff --git a/tests/Integrations/Exec/ExecIntegrationTest.php b/tests/Integrations/Exec/ExecIntegrationTest.php
@@ -490,6 +490,32 @@ public function testShellOtherBadArguments($sf)
         }
     }
 
+    /**
+     * shell_exec() returning null must NOT produce an error span.
+     *
+     * PHP docs state: "It is not possible to detect execution failures using
+     * this function." A null return means either no output was produced or the
+     * command failed — the two cases are indistinguishable. Flagging null as an
+     * error causes false-positive error spans for any intentionally silent
+     * command (e.g. Symfony's `stty 2>/dev/null`).
+     */
+    public function testShellExecNullReturnIsNotAnError()
+    {
+        $traces = $this->isolateTracer(function () {
+            // 'true' exits 0 with no output → shell_exec returns null.
+            $res = shell_exec('true');
+            $this->assertNull($res);
+        });
+
+        $this->assertSpans($traces, [
+            SpanAssertion::build('command_execution', $traces[0][0]['service'], 'system', 'sh')
+                ->withExactTags([
+                    'cmd.shell' => 'true',
+                    'component' => 'subprocess',
+                ])
+        ]);
+    }
+
     /**
      * Test that the shell command is redacted.
      * @dataProvider allShellFunctionsProvider
