From e608b0518ea34679c0ea67e187feb72a223c9389 Mon Sep 17 00:00:00 2001 From: Kevin Lydon Date: Thu, 1 Aug 2024 16:28:33 -0400 Subject: [PATCH 1/4] Added a couple warnings for situations that could cause the tmp-dir to not be executable, to help troubleshooting if that causes issues. --- .../cmdline/CommandLineProgram.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java index 8835707c556..182f4522fba 100644 --- a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java +++ b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java @@ -25,6 +25,7 @@ import org.broadinstitute.hellbender.utils.io.IOUtils; import org.broadinstitute.hellbender.utils.runtime.RuntimeUtils; +import java.io.File; import java.io.IOException; import java.net.InetAddress; import java.nio.file.*; @@ -167,6 +168,33 @@ public Object instanceMainPostParseArgs() { final Path p = tmpDir.toPath(); try { p.getFileSystem().provider().checkAccess(p, AccessMode.READ, AccessMode.WRITE); + + // Warn if there's anything that prevents execution in the tmp dir because some tools need that + // Write an empty file to the tempdir + final Path tempFilePath = Files.createTempFile(p, "gatk_exec_test", null); + final File tempFile = tempFilePath.toFile(); + tempFile.deleteOnExit(); + // Add execute permissions + if(!tempFile.setExecutable(true, false)){ + logger.warn( + "Cannot create executable files within the configured temporary directory. It is possible " + + "this user does not have the proper permissions to execute files within this directory. " + + "This can cause issues for some GATK tools. You can specify a different directory using " + + "--tmp-dir" + ); + } + // Now check if the file can be executed + else if(!tempFile.canExecute()) { + logger.warn( + "User has permissions to create executable files within the configured temporary directory, " + + "but cannot execute those files. It is possible the directory has been mounted using the " + + "'noexec' flag. This can cause issues for some GATK tools. You can specify a different " + + "directory using --tmp-dir" + ); + } + + + System.setProperty("java.io.tmpdir", IOUtils.getAbsolutePathWithoutFileProtocol(p)); } catch (final AccessDeniedException | NoSuchFileException e) { // TODO: it may be that the program does not need a tmp dir From 6ba4be7c182357db4129abf8cc5ce60bc3ac29ac Mon Sep 17 00:00:00 2001 From: Kevin Lydon Date: Tue, 13 Aug 2024 12:49:09 -0400 Subject: [PATCH 2/4] Minor formatting change --- .../broadinstitute/hellbender/cmdline/CommandLineProgram.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java index 182f4522fba..fadea828ace 100644 --- a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java +++ b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java @@ -193,8 +193,6 @@ else if(!tempFile.canExecute()) { ); } - - System.setProperty("java.io.tmpdir", IOUtils.getAbsolutePathWithoutFileProtocol(p)); } catch (final AccessDeniedException | NoSuchFileException e) { // TODO: it may be that the program does not need a tmp dir From 29100fde28a96a33b875b97ed1ee4cf125feb955 Mon Sep 17 00:00:00 2001 From: Kevin Lydon Date: Tue, 13 Aug 2024 15:52:38 -0400 Subject: [PATCH 3/4] Modified the temp dir execution checking to only use nio --- .../cmdline/CommandLineProgram.java | 63 ++++++++++++------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java index fadea828ace..dd4e7e3f081 100644 --- a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java +++ b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java @@ -25,10 +25,10 @@ import org.broadinstitute.hellbender.utils.io.IOUtils; import org.broadinstitute.hellbender.utils.runtime.RuntimeUtils; -import java.io.File; import java.io.IOException; import java.net.InetAddress; import java.nio.file.*; +import java.nio.file.attribute.PosixFilePermission; import java.text.DecimalFormat; import java.time.Duration; import java.time.ZonedDateTime; @@ -166,31 +166,42 @@ public Object instanceMainPostParseArgs() { // set the temp directory as a java property, checking for existence and read/write access final Path p = tmpDir.toPath(); + Path tempFilePath = null; try { p.getFileSystem().provider().checkAccess(p, AccessMode.READ, AccessMode.WRITE); // Warn if there's anything that prevents execution in the tmp dir because some tools need that - // Write an empty file to the tempdir - final Path tempFilePath = Files.createTempFile(p, "gatk_exec_test", null); - final File tempFile = tempFilePath.toFile(); - tempFile.deleteOnExit(); - // Add execute permissions - if(!tempFile.setExecutable(true, false)){ - logger.warn( - "Cannot create executable files within the configured temporary directory. It is possible " + - "this user does not have the proper permissions to execute files within this directory. " + - "This can cause issues for some GATK tools. You can specify a different directory using " + - "--tmp-dir" - ); - } - // Now check if the file can be executed - else if(!tempFile.canExecute()) { - logger.warn( - "User has permissions to create executable files within the configured temporary directory, " + - "but cannot execute those files. It is possible the directory has been mounted using the " + - "'noexec' flag. This can cause issues for some GATK tools. You can specify a different " + - "directory using --tmp-dir" + // This test relies on the file system supporting posix file permissions + if(p.getFileSystem().supportedFileAttributeViews().contains("posix")) { + // Write an empty file to the tempdir + tempFilePath = Files.createTempFile(p, "gatk_exec_test", null); + // Add execute permissions + final Set executePermissions = EnumSet.of( + PosixFilePermission.OWNER_EXECUTE, + PosixFilePermission.GROUP_EXECUTE, + PosixFilePermission.OTHERS_EXECUTE ); + final Set newPermissions = Files.getPosixFilePermissions(tempFilePath); + newPermissions.addAll(executePermissions); + try{ + Files.setPosixFilePermissions(tempFilePath, newPermissions); + if(!Files.isExecutable(tempFilePath)) { + logger.warn( + "User has permissions to create executable files within the configured temporary directory, " + + "but cannot execute those files. It is possible the directory has been mounted using the " + + "'noexec' flag. This can cause issues for some GATK tools. You can specify a different " + + "directory using --tmp-dir" + ); + } + } + catch(IOException e) { + logger.warn( + "Cannot create executable files within the configured temporary directory. It is possible " + + "this user does not have the proper permissions to execute files within this directory. " + + "This can cause issues for some GATK tools. You can specify a different directory using " + + "--tmp-dir" + ); + } } System.setProperty("java.io.tmpdir", IOUtils.getAbsolutePathWithoutFileProtocol(p)); @@ -203,6 +214,16 @@ else if(!tempFile.canExecute()) { // other exceptions with the tmp directory throw new UserException.BadTempDir(p, e.getMessage(), e); } + finally { + // Make sure we clean up the test file + try { + if (tempFilePath != null) + Files.deleteIfExists(tempFilePath); + } + catch(Exception e) { + logger.warn("Failed to delete temp file for testing temp dir", e); + } + } //Set defaults (note: setting them here means they are not controllable by the user) if (! useJdkDeflater) { From 4c123d0b479d7037092bad65f5d19e22bf94c0d4 Mon Sep 17 00:00:00 2001 From: Kevin Lydon Date: Fri, 18 Oct 2024 12:00:02 -0400 Subject: [PATCH 4/4] Moved the tempdir exec permission checking into its own method --- .../cmdline/CommandLineProgram.java | 89 ++++++++++--------- 1 file changed, 46 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java index dd4e7e3f081..bc7e7eb9f3e 100644 --- a/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java +++ b/src/main/java/org/broadinstitute/hellbender/cmdline/CommandLineProgram.java @@ -166,43 +166,11 @@ public Object instanceMainPostParseArgs() { // set the temp directory as a java property, checking for existence and read/write access final Path p = tmpDir.toPath(); - Path tempFilePath = null; try { p.getFileSystem().provider().checkAccess(p, AccessMode.READ, AccessMode.WRITE); // Warn if there's anything that prevents execution in the tmp dir because some tools need that - // This test relies on the file system supporting posix file permissions - if(p.getFileSystem().supportedFileAttributeViews().contains("posix")) { - // Write an empty file to the tempdir - tempFilePath = Files.createTempFile(p, "gatk_exec_test", null); - // Add execute permissions - final Set executePermissions = EnumSet.of( - PosixFilePermission.OWNER_EXECUTE, - PosixFilePermission.GROUP_EXECUTE, - PosixFilePermission.OTHERS_EXECUTE - ); - final Set newPermissions = Files.getPosixFilePermissions(tempFilePath); - newPermissions.addAll(executePermissions); - try{ - Files.setPosixFilePermissions(tempFilePath, newPermissions); - if(!Files.isExecutable(tempFilePath)) { - logger.warn( - "User has permissions to create executable files within the configured temporary directory, " + - "but cannot execute those files. It is possible the directory has been mounted using the " + - "'noexec' flag. This can cause issues for some GATK tools. You can specify a different " + - "directory using --tmp-dir" - ); - } - } - catch(IOException e) { - logger.warn( - "Cannot create executable files within the configured temporary directory. It is possible " + - "this user does not have the proper permissions to execute files within this directory. " + - "This can cause issues for some GATK tools. You can specify a different directory using " + - "--tmp-dir" - ); - } - } + tryToWriteAnExecutableFileAndWarnOnFailure(p); System.setProperty("java.io.tmpdir", IOUtils.getAbsolutePathWithoutFileProtocol(p)); } catch (final AccessDeniedException | NoSuchFileException e) { @@ -214,16 +182,6 @@ public Object instanceMainPostParseArgs() { // other exceptions with the tmp directory throw new UserException.BadTempDir(p, e.getMessage(), e); } - finally { - // Make sure we clean up the test file - try { - if (tempFilePath != null) - Files.deleteIfExists(tempFilePath); - } - catch(Exception e) { - logger.warn("Failed to delete temp file for testing temp dir", e); - } - } //Set defaults (note: setting them here means they are not controllable by the user) if (! useJdkDeflater) { @@ -541,4 +499,49 @@ public final CommandLineParser getCommandLineParser() { protected interface AutoCloseableNoCheckedExceptions extends AutoCloseable{ @Override void close(); } + + private void tryToWriteAnExecutableFileAndWarnOnFailure(final Path p) { + Path tempFilePath = null; + try { + // This test relies on the file system supporting posix file permissions + if(p.getFileSystem().supportedFileAttributeViews().contains("posix")) { + // Write an empty file to the tempdir + tempFilePath = Files.createTempFile(p, "gatk_exec_test", null); + // Add execute permissions + final Set executePermissions = EnumSet.of( + PosixFilePermission.OWNER_EXECUTE, + PosixFilePermission.GROUP_EXECUTE, + PosixFilePermission.OTHERS_EXECUTE + ); + final Set newPermissions = Files.getPosixFilePermissions(tempFilePath); + newPermissions.addAll(executePermissions); + + Files.setPosixFilePermissions(tempFilePath, newPermissions); + if(!Files.isExecutable(tempFilePath)) { + logger.warn( + "User has permissions to create executable files within the configured temporary directory, " + + "but cannot execute those files. It is possible the directory has been mounted using the " + + "'noexec' flag. This can cause issues for some GATK tools. You can specify a different " + + "directory using --tmp-dir" + ); + } + } + } catch(Exception e) { + logger.warn( + "Cannot create executable files within the configured temporary directory. It is possible " + + "this user does not have the proper permissions to execute files within this directory. " + + "This can cause issues for some GATK tools. You can specify a different directory using " + + "--tmp-dir" + ); + logger.debug(e); + } finally { + // Make sure we clean up the test file + try { + Files.deleteIfExists(tempFilePath); + } catch(Exception e) { + logger.warn("Failed to delete temp file for testing temp dir", e); + } + } + + } }