Update host id resource detector for macos and full paths

instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/HostIdResource.java

@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.resources;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
 import static java.util.logging.Level.FINE;
 
@@ -25,11 +26,22 @@
 import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 
 /**
  * {@link ResourceProvider} for automatically configuring <code>host.id</code> according to <a
  * href="https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/host.md#non-privileged-machine-id-lookup">the
- * semantic conventions</a>
+ * semantic conventions</a>.
+ *
+ * <p>The lookup is non-privileged and OS specific:
+ *
+ * <ul>
+ *   <li>Linux: {@code /etc/machine-id}, falling back to {@code /var/lib/dbus/machine-id}
+ *   <li>BSD: {@code /etc/hostid}, falling back to {@code /bin/kenv -q smbios.system.uuid}
+ *   <li>macOS: the {@code IOPlatformUUID} from {@code /usr/sbin/ioreg -rd1 -c
+ *       IOPlatformExpertDevice}
+ *   <li>Windows: the {@code MachineGuid} registry value
+ * </ul>
  */
 public final class HostIdResource {
 
@@ -38,27 +50,46 @@ public final class HostIdResource {
   // copied from HostIncubatingAttributes
   static final AttributeKey<String> HOST_ID = AttributeKey.stringKey("host.id");
 
-  public static final String REGISTRY_QUERY =
-      "reg query HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Cryptography /v MachineGuid";
+  // Non-privileged machine-id sources per the semantic conventions. Commands are invoked with
+  // absolute paths to avoid resolving them through a potentially attacker controlled PATH, see
+  // https://github.com/open-telemetry/semantic-conventions/pull/3896
+  private static final List<String> LINUX_MACHINE_ID_PATHS =
+      asList("/etc/machine-id", "/var/lib/dbus/machine-id");
+  private static final String BSD_HOSTID_PATH = "/etc/hostid";
+  private static final List<String> BSD_KENV_COMMAND =
+      asList("/bin/kenv", "-q", "smbios.system.uuid");
+  private static final List<String> MACOS_IOREG_COMMAND =
+      asList("/usr/sbin/ioreg", "-rd1", "-c", "IOPlatformExpertDevice");
+
+  // Prefer the SystemRoot/windir environment variables to locate the Windows directory, falling
+  // back to the conventional install path only if neither is set.
+  private static final String[] WINDOWS_ROOT_ENV_VARS = {"SystemRoot", "windir"};
+  private static final String WINDOWS_ROOT_FALLBACK = "C:\\Windows";
+  private static final List<String> WINDOWS_REGISTRY_QUERY_ARGS =
+      asList("query", "HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Cryptography", "/v", "MachineGuid");
 
   private static final HostIdResource INSTANCE =
       new HostIdResource(
           HostIdResource::getOsTypeSystemProperty,
-          HostIdResource::readMachineIdFile,
-          HostIdResource::queryWindowsRegistry);
+          HostIdResource::readFileLines,
+          HostIdResource::queryWindowsRegistry,
+          HostIdResource::runCommand);
 
   private final Supplier<String> getOsType;
-  private final Function<Path, List<String>> machineIdReader;
+  private final Function<Path, List<String>> fileReader;
   private final Supplier<List<String>> queryWindowsRegistry;
+  private final Function<List<String>, List<String>> commandExecutor;
 
   // Visible for testing
   HostIdResource(
       Supplier<String> getOsType,
-      Function<Path, List<String>> machineIdReader,
-      Supplier<List<String>> queryWindowsRegistry) {
+      Function<Path, List<String>> fileReader,
+      Supplier<List<String>> queryWindowsRegistry,
+      Function<List<String>, List<String>> commandExecutor) {
     this.getOsType = getOsType;
-    this.machineIdReader = machineIdReader;
+    this.fileReader = fileReader;
     this.queryWindowsRegistry = queryWindowsRegistry;
+    this.commandExecutor = commandExecutor;
   }
 
   /** Returns a {@link Resource} containing the {@code host.id} resource attribute. */
@@ -74,6 +105,12 @@ Resource createResource() {
     if (runningLinux()) {
       return readLinuxMachineId();
     }
+    if (runningMacOs()) {
+      return readMacOsUuid();
+    }
+    if (runningBsd()) {
+      return readBsdHostId();
+    }
     logger.log(FINE, "Unsupported OS type: {0}", getOsType.get());
     return Resource.empty();
   }
@@ -86,6 +123,16 @@ private boolean runningWindows() {
     return getOsType.get().startsWith("Windows");
   }
 
+  private boolean runningMacOs() {
+    // os.name is "Mac OS X" on the JVM (not "Darwin" as reported by uname)
+    String osType = getOsType.get().toLowerCase(Locale.ROOT);
+    return osType.contains("mac") || osType.contains("darwin");
+  }
+
+  private boolean runningBsd() {
+    return getOsType.get().endsWith("BSD");
+  }
+
   // see
   // https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/SystemUtils.java
   // for values
@@ -94,53 +141,125 @@ private static String getOsTypeSystemProperty() {
   }
 
   private Resource readLinuxMachineId() {
-    Path path = FileSystems.getDefault().getPath("/etc/machine-id");
-    List<String> lines = machineIdReader.apply(path);
-    if (lines.isEmpty()) {
-      return Resource.empty();
+    // /etc/machine-id is the primary source, /var/lib/dbus/machine-id is the fallback
+    for (String path : LINUX_MACHINE_ID_PATHS) {
+      String machineId =
+          firstNonBlankLine(fileReader.apply(FileSystems.getDefault().getPath(path)));
+      if (machineId != null) {
+        return Resource.create(Attributes.of(HOST_ID, machineId));
+      }
     }
-    return Resource.create(Attributes.of(HOST_ID, lines.get(0)));
+    return Resource.empty();
   }
 
-  private static List<String> readMachineIdFile(Path path) {
-    try {
-      List<String> lines = Files.readAllLines(path);
-      if (lines.isEmpty()) {
-        logger.fine("Failed to read /etc/machine-id: empty file");
+  private Resource readBsdHostId() {
+    // /etc/hostid is the primary source, kenv is the fallback
+    String hostId =
+        firstNonBlankLine(fileReader.apply(FileSystems.getDefault().getPath(BSD_HOSTID_PATH)));
+    if (hostId == null) {
+      hostId = firstNonBlankLine(commandExecutor.apply(BSD_KENV_COMMAND));
+    }
+    if (hostId != null) {
+      return Resource.create(Attributes.of(HOST_ID, hostId));
+    }
+    logger.fine("Failed to read host id on BSD: no value found in /etc/hostid or kenv output");
+    return Resource.empty();
+  }
+
+  private Resource readMacOsUuid() {
+    List<String> lines = commandExecutor.apply(MACOS_IOREG_COMMAND);
+
+    for (String line : lines) {
+      if (line.contains("IOPlatformUUID")) {
+        // line looks like: "IOPlatformUUID" = "0123456789ABCDEF"
+        int equalsIndex = line.indexOf('=');
+        if (equalsIndex >= 0) {
+          String uuid = line.substring(equalsIndex + 1).trim().replace("\"", "").trim();
+          if (!uuid.isEmpty()) {
+            return Resource.create(Attributes.of(HOST_ID, uuid));
+          }
+        }
+        break;
       }
-      return lines;
-    } catch (IOException e) {
-      logger.log(FINE, "Failed to read /etc/machine-id", e);
-      return emptyList();
     }
+    logger.fine("Failed to read macOS host id: no IOPlatformUUID found in ioreg output");
+    return Resource.empty();
   }
 
   private Resource readWindowsGuid() {
     List<String> lines = queryWindowsRegistry.get();
 
     for (String line : lines) {
       if (line.contains("MachineGuid")) {
+        // line looks like: MachineGuid    REG_SZ    0123456789ABCDEF
         String[] parts = line.trim().split("\\s+");
         if (parts.length == 3) {
           return Resource.create(Attributes.of(HOST_ID, parts[2]));
         }
       }
     }
-    logger.fine("Failed to read Windows registry: No MachineGuid found in output: " + lines);
+    logger.fine("Failed to read Windows registry: No MachineGuid found in output");
     return Resource.empty();
   }
 
   private static List<String> queryWindowsRegistry() {
+    List<String> command = new ArrayList<>();
+    command.add(windowsRegPath());
+    command.addAll(WINDOWS_REGISTRY_QUERY_ARGS);
+    return runCommand(command);
+  }
+
+  private static String windowsRegPath() {
+    String root = null;
+    for (String envVar : WINDOWS_ROOT_ENV_VARS) {
+      root = System.getenv(envVar);
+      if (root != null && !root.isEmpty()) {
+        break;
+      }
+    }
+    if (root == null || root.isEmpty()) {
+      root = WINDOWS_ROOT_FALLBACK;
+    }
+    return root + "\\System32\\reg.exe";
+  }
+
+  @Nullable
+  private static String firstNonBlankLine(List<String> lines) {
+    for (String line : lines) {
+      String trimmed = line.trim();
+      if (!trimmed.isEmpty()) {
+        return trimmed;
+      }
+    }
+    return null;
+  }
+
+  private static List<String> readFileLines(Path path) {
+    try {
+      List<String> lines = Files.readAllLines(path);
+      if (lines.isEmpty()) {
+        logger.log(FINE, "Failed to read {0}: empty file", path);
+      }
+      return lines;
+    } catch (IOException e) {
+      logger.log(FINE, "Failed to read " + path, e);
+      return emptyList();
+    }
+  }
+
+  private static List<String> runCommand(List<String> command) {
     try {
-      ProcessBuilder processBuilder = new ProcessBuilder("cmd", "/c", REGISTRY_QUERY);
+      ProcessBuilder processBuilder = new ProcessBuilder(command);
       processBuilder.redirectErrorStream(true);
       Process process = processBuilder.start();
 
       List<String> output = getProcessOutput(process);
       int exitedValue = process.waitFor();
       if (exitedValue != 0) {
         logger.fine(
-            "Failed to read Windows registry. Exit code: "
+            "Failed to run command "
+                + command
+                + ". Exit code: "
                 + exitedValue
                 + " Output: "
                 + String.join("\n", output));
@@ -153,7 +272,7 @@ private static List<String> queryWindowsRegistry() {
       if (e instanceof InterruptedException) {
         Thread.currentThread().interrupt();
       }
-      logger.log(FINE, "Failed to read Windows registry", e);
+      logger.log(FINE, "Failed to run command " + command, e);
       return emptyList();
     }
   }

instrumentation/resources/library/src/test/java/io/opentelemetry/instrumentation/resources/HostIdResourceTest.java

@@ -33,15 +33,23 @@ class HostIdResourceTest {
 
   @ParameterizedTest
   @MethodSource("createResourceLinuxCases")
-  void createResourceLinux(String expectedValue, Function<Path, List<String>> pathReader) {
-    HostIdResource hostIdResource = new HostIdResource(() -> "linux", pathReader, null);
+  void createResourceLinux(String expectedValue, Function<Path, List<String>> fileReader) {
+    HostIdResource hostIdResource = new HostIdResource(() -> "linux", fileReader, null, null);
     assertHostId(expectedValue, hostIdResource);
   }
 
   private static Stream<Arguments> createResourceLinuxCases() {
     return Stream.of(
         argumentSet(
             "default", "test", (Function<Path, List<String>>) path -> singletonList("test")),
+        argumentSet(
+            "dbus fallback",
+            "dbus-id",
+            (Function<Path, List<String>>)
+                path ->
+                    path.endsWith("machine-id") && path.toString().contains("dbus")
+                        ? singletonList("dbus-id")
+                        : emptyList()),
         argumentSet(
             "empty file or error reading",
             null,
@@ -52,7 +60,7 @@ private static Stream<Arguments> createResourceLinuxCases() {
   @MethodSource("createResourceWindowsCases")
   void createResourceWindows(String expectedValue, Supplier<List<String>> queryWindowsRegistry) {
     HostIdResource hostIdResource =
-        new HostIdResource(() -> "Windows 95", null, queryWindowsRegistry);
+        new HostIdResource(() -> "Windows 95", null, queryWindowsRegistry, null);
     assertHostId(expectedValue, hostIdResource);
   }
 
@@ -69,6 +77,61 @@ private static Stream<Arguments> createResourceWindowsCases() {
         argumentSet("short output", null, (Supplier<List<String>>) Collections::emptyList));
   }
 
+  @ParameterizedTest
+  @MethodSource("createResourceMacOsCases")
+  void createResourceMacOs(String expectedValue, Function<List<String>, List<String>> command) {
+    HostIdResource hostIdResource = new HostIdResource(() -> "Mac OS X", null, null, command);
+    assertHostId(expectedValue, hostIdResource);
+  }
+
+  private static Stream<Arguments> createResourceMacOsCases() {
+    return Stream.of(
+        argumentSet(
+            "default",
+            "0123456789ABCDEF",
+            (Function<List<String>, List<String>>)
+                command ->
+                    asList(
+                        "+-o IOPlatformExpertDevice  <class IOPlatformExpertDevice>",
+                        "    \"IOPlatformUUID\" = \"0123456789ABCDEF\"")),
+        argumentSet(
+            "no uuid",
+            null,
+            (Function<List<String>, List<String>>)
+                command -> singletonList("+-o IOPlatformExpertDevice")),
+        argumentSet(
+            "empty output", null, (Function<List<String>, List<String>>) command -> emptyList()));
+  }
+
+  @ParameterizedTest
+  @MethodSource("createResourceBsdCases")
+  void createResourceBsd(
+      String expectedValue,
+      Function<Path, List<String>> fileReader,
+      Function<List<String>, List<String>> command) {
+    HostIdResource hostIdResource = new HostIdResource(() -> "FreeBSD", fileReader, null, command);
+    assertHostId(expectedValue, hostIdResource);
+  }
+
+  private static Stream<Arguments> createResourceBsdCases() {
+    return Stream.of(
+        argumentSet(
+            "hostid file",
+            "hostid-value",
+            (Function<Path, List<String>>) path -> singletonList("hostid-value"),
+            (Function<List<String>, List<String>>) command -> emptyList()),
+        argumentSet(
+            "kenv fallback",
+            "kenv-uuid",
+            (Function<Path, List<String>>) path -> emptyList(),
+            (Function<List<String>, List<String>>) command -> singletonList("kenv-uuid")),
+        argumentSet(
+            "nothing found",
+            null,
+            (Function<Path, List<String>>) path -> emptyList(),
+            (Function<List<String>, List<String>>) command -> emptyList()));
+  }
+
   private static void assertHostId(String expectedValue, HostIdResource hostIdResource) {
     MapAssert<AttributeKey<?>, Object> that =
         assertThat(hostIdResource.createResource().getAttributes().asMap());