From 97b09bdb82eb75d971f359da515826b2371da273 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 28 Aug 2026 08:29:43 +0200 Subject: [PATCH 1/4] Fix crash-tracking script/directory permissions and add regression tests Clear all permission bits before setting owner-only ones so stray group/other bits from the process umask are actually stripped, and fix the OOME notifier script lockdown to operate on the script file instead of the directory. --- .../CrashUploaderScriptInitializer.java | 5 ++ .../OOMENotifierScriptInitializer.java | 12 +++- .../ScriptInitializerSecurityTest.java | 56 +++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java index f2e7ac706bb..1a1e5b5c805 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java @@ -78,6 +78,11 @@ private static boolean copyCrashUploaderScript( scriptDirectory); return false; } + // first clear all privileges + scriptDirectory.setReadable(false, false); + scriptDirectory.setWritable(false, false); + scriptDirectory.setExecutable(false, false); + // then set them only for the owner scriptDirectory.setReadable(true, true); scriptDirectory.setWritable(true, true); scriptDirectory.setExecutable(true, true); diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java index 91420eda112..f5ff7aeba5e 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java @@ -86,6 +86,11 @@ private static boolean copyOOMEscript(File scriptFile) { scriptDirectory); return false; } + // first clear all privileges + scriptDirectory.setReadable(false, false); + scriptDirectory.setWritable(false, false); + scriptDirectory.setExecutable(false, false); + // then set them only for the owner scriptDirectory.setReadable(true, true); scriptDirectory.setWritable(true, true); scriptDirectory.setExecutable(true, true); @@ -95,8 +100,13 @@ private static boolean copyOOMEscript(File scriptFile) { // do not overwrite existing if (!scriptFile.exists()) { copyStream(getOomeNotifierTemplate(), scriptFile); - scriptFile.setReadable(true, true); + // first clear all privileges + scriptFile.setReadable(false, false); scriptFile.setWritable(false, false); + scriptFile.setExecutable(false, false); + // then set them only for the owner + scriptFile.setReadable(true, true); + // do not restore the writable scriptFile.setExecutable(true, true); } else { if (!isOwnedAndPrivate(scriptFile)) { diff --git a/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java b/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java index 8f0cc003948..4490caf3428 100644 --- a/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java +++ b/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java @@ -1,6 +1,7 @@ package datadog.crashtracking; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -12,6 +13,7 @@ import java.nio.file.attribute.PosixFilePermission; import java.nio.file.attribute.PosixFilePermissions; import java.util.Comparator; +import java.util.EnumSet; import java.util.Set; import java.util.stream.Stream; import org.junit.jupiter.api.AfterEach; @@ -108,6 +110,46 @@ void crashUploaderHijackedDirectoryIsRefused() throws Exception { assertFalse(Files.exists(scriptFile), "Script must not be written into a hijacked directory"); } + @Test + void crashUploaderFreshDirHasExactlyOwnerPermissions() throws Exception { + Path scriptFile = tempDir.resolve("dd_crash_uploader.sh"); + CrashUploaderScriptInitializer.initialize(scriptFile.toString(), "/tmp/hs_err.log"); + + // The directory is created by mkdirs() (subject to the process umask) before the + // owner-only bits are applied, so any stray group/other bits left over from that + // umask must have been cleared, not just overlaid with the owner bits. + assertPermissions( + scriptFile.getParent(), + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE)); + } + + @Test + void oomeNotifierFreshDirHasExactlyOwnerPermissions() throws Exception { + Path scriptFile = tempDir.resolve("dd_oome_notifier.sh"); + OOMENotifierScriptInitializer.initialize(scriptFile + " %p"); + + assertPermissions( + scriptFile.getParent(), + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE)); + } + + @Test + void oomeNotifierScriptFileIsNotOwnerWritable() throws Exception { + Path scriptFile = tempDir.resolve("dd_oome_notifier.sh"); + OOMENotifierScriptInitializer.initialize(scriptFile + " %p"); + + // The write bit is deliberately not restored after the clear/set-owner-only sequence, + // so the copied script must end up read+execute only, even for the owner. + assertPermissions( + scriptFile, EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE)); + } + @Test void oomeNotifierFreshDirIsOwnerRestricted() throws Exception { Path scriptFile = tempDir.resolve("dd_oome_notifier.sh"); @@ -176,6 +218,20 @@ void cleanPosixTreeEndToEndInitProducesScriptsAndConfigs() throws Exception { assertTrue(crashCfgWritten, "Crash uploader .cfg file must be written in the clean flow"); } + private static void assertPermissions(Path path, Set expected) + throws IOException { + Set actual = Files.getPosixFilePermissions(path); + assertEquals( + actual, + expected, + "Expected permissions " + + PosixFilePermissions.toString(expected) + + " but found " + + PosixFilePermissions.toString(actual) + + " on " + + path); + } + private static void assertNoGroupWorldWriteBit(Path path) throws IOException { Set perms = Files.getPosixFilePermissions(path); for (PosixFilePermission bit : From 1327f248fa95293908ee12c2c6bb3aea7b53049c Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 28 Aug 2026 10:35:24 +0200 Subject: [PATCH 2/4] sphinx: address review feedback on PR #12330 --- .../CrashUploaderScriptInitializer.java | 25 ++++---- .../datadog/crashtracking/Initializer.java | 57 ++++++++++++++++- .../OOMENotifierScriptInitializer.java | 16 +++-- .../ScriptInitializerSecurityTest.java | 62 ++++++++++++++++++- 4 files changed, 135 insertions(+), 25 deletions(-) diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java index 1a1e5b5c805..68e1977b44d 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java @@ -5,6 +5,8 @@ import static datadog.crashtracking.Initializer.findAgentJar; import static datadog.crashtracking.Initializer.getCrashUploaderTemplate; import static datadog.crashtracking.Initializer.isOwnedAndPrivate; +import static datadog.crashtracking.Initializer.isSafeToRepairDirectory; +import static datadog.crashtracking.Initializer.restrictDirectoryToOwnerOnly; import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; import static java.util.Locale.ROOT; @@ -78,23 +80,19 @@ private static boolean copyCrashUploaderScript( scriptDirectory); return false; } - // first clear all privileges - scriptDirectory.setReadable(false, false); - scriptDirectory.setWritable(false, false); - scriptDirectory.setExecutable(false, false); - // then set them only for the owner - scriptDirectory.setReadable(true, true); - scriptDirectory.setWritable(true, true); - scriptDirectory.setExecutable(true, true); + restrictDirectoryToOwnerOnly(scriptDirectory); } else { - if (!isOwnedAndPrivate(scriptDirectory)) { + if (!isSafeToRepairDirectory(scriptDirectory)) { LOG.warn( SEND_TELEMETRY, - "Untrusted crash tracking script folder {} (wrong owner or group/world bits set). " + "Untrusted crash tracking script folder {} (wrong owner or group/world-writable). " + SETUP_FAILURE_MESSAGE, scriptDirectory); return false; } + // owned by us but possibly left over from an older, less restrictive version: tighten it + // down to owner-only before trusting it, then validate the script inside it independently + restrictDirectoryToOwnerOnly(scriptDirectory); } if (!scriptDirectory.canWrite()) { LOG.warn(SEND_TELEMETRY, "Read only directory {}. " + SETUP_FAILURE_MESSAGE, scriptDirectory); @@ -142,8 +140,13 @@ private static void writeCrashUploaderScript( bw.newLine(); } } - scriptFile.setReadable(true, true); + // first clear all privileges + scriptFile.setReadable(false, false); scriptFile.setWritable(false, false); + scriptFile.setExecutable(false, false); + // then set them only for the owner + scriptFile.setReadable(true, true); + // do not restore the writable scriptFile.setExecutable(true, true); } else { if (!isOwnedAndPrivate(scriptFile)) { diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java index 8e1ba25f423..e8dc74373b2 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java @@ -460,9 +460,7 @@ static boolean isOwnedAndPrivate(File f) { } try { Path path = f.toPath(); - UserPrincipal owner = Files.getOwner(path); - UserPrincipal jvmUser = Files.getOwner(TempLocationManager.getInstance().getTempDir()); - if (!jvmUser.equals(owner)) { + if (!isJvmOwner(path)) { return false; } Set perms = Files.getPosixFilePermissions(path); @@ -472,4 +470,57 @@ static boolean isOwnedAndPrivate(File f) { return false; } } + + private static final Set GROUP_WORLD_WRITE_BITS = + EnumSet.of(PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_WRITE); + + /** + * Returns {@code true} when {@code dir} is owned by the current JVM user and has no group/world + * write bit set; on non-POSIX file systems always returns {@code true}. Unlike {@link + * #isOwnedAndPrivate(File)}, stray group/world read or execute bits (e.g. the + * {@code 0755} a pre-upgrade version of this initializer, which did not lock down permissions, + * could have left behind) do not disqualify the directory here: those bits are safe to tighten in + * place with {@link #restrictDirectoryToOwnerOnly(File)} rather than treating the directory as + * untrusted. A group/world write bit is still treated as a sign of possible tampering and causes + * this method to return {@code false}. + */ + static boolean isSafeToRepairDirectory(File dir) { + if (OperatingSystem.isWindows()) { + return true; + } + try { + Path path = dir.toPath(); + if (!isJvmOwner(path)) { + return false; + } + Set perms = Files.getPosixFilePermissions(path); + return perms.stream().noneMatch(GROUP_WORLD_WRITE_BITS::contains); + } catch (IOException | IllegalStateException e) { + LOG.debug("Unable to check ownership/permissions for {}: {}", dir, e.getMessage()); + return false; + } + } + + private static boolean isJvmOwner(Path path) throws IOException { + UserPrincipal owner = Files.getOwner(path); + UserPrincipal jvmUser = Files.getOwner(TempLocationManager.getInstance().getTempDir()); + return jvmUser.equals(owner); + } + + /** + * Clears all permission bits on {@code dir} and then sets read/write/execute for the owner only + * (effective {@code 0700}). Used both when a script directory is freshly created (to strip any + * group/world bits left over from the process umask) and to repair a pre-existing directory that + * is owned by the JVM user but was created by an older, less restrictive version. + */ + static void restrictDirectoryToOwnerOnly(File dir) { + // first clear all privileges + dir.setReadable(false, false); + dir.setWritable(false, false); + dir.setExecutable(false, false); + // then set them only for the owner + dir.setReadable(true, true); + dir.setWritable(true, true); + dir.setExecutable(true, true); + } } diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java index f5ff7aeba5e..ee52cfd68cf 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java @@ -6,7 +6,9 @@ import static datadog.crashtracking.Initializer.getOomeNotifierTemplate; import static datadog.crashtracking.Initializer.getScriptPathFromArg; import static datadog.crashtracking.Initializer.isOwnedAndPrivate; +import static datadog.crashtracking.Initializer.isSafeToRepairDirectory; import static datadog.crashtracking.Initializer.pidFromSpecialFileName; +import static datadog.crashtracking.Initializer.restrictDirectoryToOwnerOnly; import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; import datadog.trace.api.internal.VisibleForTesting; @@ -61,13 +63,16 @@ private static boolean copyOOMEscript(File scriptFile) { File scriptDirectory = scriptFile.getParentFile(); if (scriptDirectory.exists()) { - if (!isOwnedAndPrivate(scriptDirectory)) { + if (!isSafeToRepairDirectory(scriptDirectory)) { LOG.warn( SEND_TELEMETRY, "Untrusted OOME script folder {} (wrong owner or group/world bits set). OOME notification will not work properly.", scriptDirectory); return false; } + // owned by us but possibly left over from an older, less restrictive version: tighten it + // down to owner-only before trusting it, then validate the script inside it independently + restrictDirectoryToOwnerOnly(scriptDirectory); // cleanup all stale process-specific generated files in the parent folder of the given OOME // notifier script runScriptCleanup(scriptDirectory); @@ -86,14 +91,7 @@ private static boolean copyOOMEscript(File scriptFile) { scriptDirectory); return false; } - // first clear all privileges - scriptDirectory.setReadable(false, false); - scriptDirectory.setWritable(false, false); - scriptDirectory.setExecutable(false, false); - // then set them only for the owner - scriptDirectory.setReadable(true, true); - scriptDirectory.setWritable(true, true); - scriptDirectory.setExecutable(true, true); + restrictDirectoryToOwnerOnly(scriptDirectory); } try { diff --git a/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java b/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java index 4490caf3428..625e15031ea 100644 --- a/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java +++ b/dd-java-agent/agent-crashtracking/src/test/java/datadog/crashtracking/ScriptInitializerSecurityTest.java @@ -110,9 +110,51 @@ void crashUploaderHijackedDirectoryIsRefused() throws Exception { assertFalse(Files.exists(scriptFile), "Script must not be written into a hijacked directory"); } + @Test + void crashUploaderRepairsPreviouslyGeneratedDirectory() throws Exception { + // Simulate a directory left behind by a pre-upgrade version of the initializer that did not + // lock down permissions: owned by us, but group/world readable+executable (0755) with no + // write bits set for group/other. + Path scriptDir = tempDir.resolve("legacy_crash_dir"); + Files.createDirectories(scriptDir); + Files.setPosixFilePermissions(scriptDir, PosixFilePermissions.fromString("rwxr-xr-x")); + + Path scriptFile = scriptDir.resolve("dd_crash_uploader.sh"); + CrashUploaderScriptInitializer.initialize(scriptFile.toString(), "/tmp/hs_err.log"); + + assertTrue(Files.exists(scriptFile), "Script should have been created in the repaired dir"); + assertPermissions( + scriptDir, + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE)); + } + + @Test + void oomeNotifierRepairsPreviouslyGeneratedDirectory() throws Exception { + Path scriptDir = tempDir.resolve("legacy_oome_dir"); + Files.createDirectories(scriptDir); + Files.setPosixFilePermissions(scriptDir, PosixFilePermissions.fromString("rwxr-xr-x")); + + Path scriptFile = scriptDir.resolve("dd_oome_notifier.sh"); + OOMENotifierScriptInitializer.initialize(scriptFile + " %p"); + + assertTrue(Files.exists(scriptFile), "Script should have been created in the repaired dir"); + assertPermissions( + scriptDir, + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE)); + } + @Test void crashUploaderFreshDirHasExactlyOwnerPermissions() throws Exception { - Path scriptFile = tempDir.resolve("dd_crash_uploader.sh"); + // Place the script under a child directory that does not exist yet, so the initializer + // must go through its mkdirs()/permission-reset branch rather than the existing-directory + // (already owner-only) branch that tempDir itself would take. + Path scriptFile = tempDir.resolve("fresh-crash-dir").resolve("dd_crash_uploader.sh"); CrashUploaderScriptInitializer.initialize(scriptFile.toString(), "/tmp/hs_err.log"); // The directory is created by mkdirs() (subject to the process umask) before the @@ -128,7 +170,10 @@ void crashUploaderFreshDirHasExactlyOwnerPermissions() throws Exception { @Test void oomeNotifierFreshDirHasExactlyOwnerPermissions() throws Exception { - Path scriptFile = tempDir.resolve("dd_oome_notifier.sh"); + // Place the script under a child directory that does not exist yet, so the initializer + // must go through its mkdirs()/permission-reset branch rather than the existing-directory + // (already owner-only) branch that tempDir itself would take. + Path scriptFile = tempDir.resolve("fresh-oome-dir").resolve("dd_oome_notifier.sh"); OOMENotifierScriptInitializer.initialize(scriptFile + " %p"); assertPermissions( @@ -139,6 +184,19 @@ void oomeNotifierFreshDirHasExactlyOwnerPermissions() throws Exception { PosixFilePermission.OWNER_EXECUTE)); } + @Test + void crashUploaderScriptFileHasNoGroupOrWorldReadBit() throws Exception { + Path scriptFile = tempDir.resolve("dd_crash_uploader.sh"); + CrashUploaderScriptInitializer.initialize(scriptFile.toString(), "/tmp/hs_err.log"); + + // The script is created with FileOutputStream, so its initial mode is subject to the + // process umask (e.g. 0644 under a typical 0022 umask). The clear-then-set-owner-only + // sequence must strip any inherited group/other bits, not just overlay owner bits on top + // of them, otherwise a later JVM start rejects the script via isOwnedAndPrivate(). + assertPermissions( + scriptFile, EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE)); + } + @Test void oomeNotifierScriptFileIsNotOwnerWritable() throws Exception { Path scriptFile = tempDir.resolve("dd_oome_notifier.sh"); From 766939b374eb864c8a54fbd15bf63f3fa116ece5 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 28 Aug 2026 10:48:24 +0200 Subject: [PATCH 3/4] Spotless!!! --- .../CrashUploaderScriptInitializer.java | 8 ++++--- .../datadog/crashtracking/Initializer.java | 21 +++++++++++++++++++ .../OOMENotifierScriptInitializer.java | 8 ++++--- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java index 68e1977b44d..bd57687d643 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/CrashUploaderScriptInitializer.java @@ -7,6 +7,7 @@ import static datadog.crashtracking.Initializer.isOwnedAndPrivate; import static datadog.crashtracking.Initializer.isSafeToRepairDirectory; import static datadog.crashtracking.Initializer.restrictDirectoryToOwnerOnly; +import static datadog.crashtracking.Initializer.stripGroupAndWorldBits; import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; import static java.util.Locale.ROOT; @@ -90,9 +91,10 @@ private static boolean copyCrashUploaderScript( scriptDirectory); return false; } - // owned by us but possibly left over from an older, less restrictive version: tighten it - // down to owner-only before trusting it, then validate the script inside it independently - restrictDirectoryToOwnerOnly(scriptDirectory); + // owned by us but possibly left over from an older, less restrictive version: strip any + // stray group/world bits without touching the owner's own bits, so a directory an operator + // deliberately made non-writable stays non-writable + stripGroupAndWorldBits(scriptDirectory); } if (!scriptDirectory.canWrite()) { LOG.warn(SEND_TELEMETRY, "Read only directory {}. " + SETUP_FAILURE_MESSAGE, scriptDirectory); diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java index e8dc74373b2..228973a4378 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java @@ -523,4 +523,25 @@ static void restrictDirectoryToOwnerOnly(File dir) { dir.setWritable(true, true); dir.setExecutable(true, true); } + + /** + * Removes any group/world permission bits from {@code dir} while leaving the owner's own bits + * untouched. Unlike {@link #restrictDirectoryToOwnerOnly(File)}, this never adds a permission + * (e.g. owner write) that the directory did not already have, so a directory an operator + * deliberately made non-writable for the owner stays non-writable after repair. On non-POSIX + * file systems this is a no-op. + */ + static void stripGroupAndWorldBits(File dir) { + if (OperatingSystem.isWindows()) { + return; + } + try { + Path path = dir.toPath(); + Set perms = EnumSet.copyOf(Files.getPosixFilePermissions(path)); + perms.removeAll(GROUP_WORLD_BITS); + Files.setPosixFilePermissions(path, perms); + } catch (IOException | IllegalStateException e) { + LOG.debug("Unable to strip group/world permissions for {}: {}", dir, e.getMessage()); + } + } } diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java index ee52cfd68cf..e4ae6fda673 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/OOMENotifierScriptInitializer.java @@ -9,6 +9,7 @@ import static datadog.crashtracking.Initializer.isSafeToRepairDirectory; import static datadog.crashtracking.Initializer.pidFromSpecialFileName; import static datadog.crashtracking.Initializer.restrictDirectoryToOwnerOnly; +import static datadog.crashtracking.Initializer.stripGroupAndWorldBits; import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY; import datadog.trace.api.internal.VisibleForTesting; @@ -70,9 +71,10 @@ private static boolean copyOOMEscript(File scriptFile) { scriptDirectory); return false; } - // owned by us but possibly left over from an older, less restrictive version: tighten it - // down to owner-only before trusting it, then validate the script inside it independently - restrictDirectoryToOwnerOnly(scriptDirectory); + // owned by us but possibly left over from an older, less restrictive version: strip any + // stray group/world bits without touching the owner's own bits, so a directory an operator + // deliberately made non-writable stays non-writable + stripGroupAndWorldBits(scriptDirectory); // cleanup all stale process-specific generated files in the parent folder of the given OOME // notifier script runScriptCleanup(scriptDirectory); From f1e6f66c8e083e5025ea115f9080e25f88f0d726 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 28 Aug 2026 12:44:20 +0200 Subject: [PATCH 4/4] Reformat comment in Initializer.stripGroupAndWorldBits --- .../src/main/java/datadog/crashtracking/Initializer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java index 228973a4378..d0f7cac1ad8 100644 --- a/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java +++ b/dd-java-agent/agent-crashtracking/src/main/java/datadog/crashtracking/Initializer.java @@ -528,8 +528,8 @@ static void restrictDirectoryToOwnerOnly(File dir) { * Removes any group/world permission bits from {@code dir} while leaving the owner's own bits * untouched. Unlike {@link #restrictDirectoryToOwnerOnly(File)}, this never adds a permission * (e.g. owner write) that the directory did not already have, so a directory an operator - * deliberately made non-writable for the owner stays non-writable after repair. On non-POSIX - * file systems this is a no-op. + * deliberately made non-writable for the owner stays non-writable after repair. On non-POSIX file + * systems this is a no-op. */ static void stripGroupAndWorldBits(File dir) { if (OperatingSystem.isWindows()) {