A user reports that Pipeline stash does not preserve the executable bit on 64-bit AIX, apparently due to a missing JNR port. Probably the same applies to any use of TarArchiver.

      Since we now assume Java 7, we can use java.nio.file calls. In particular, making IOUtils.mode and FilePath._chmod use PosixFilePermission rather than JNR would be appropriate.

          [JENKINS-36088] Use NIO rather than JNR whenever possible

          Jesse Glick added a comment -

          Doing so would presumably also fix issues relating to loading JNR.

          Jesse Glick added a comment - Doing so would presumably also fix issues relating to loading JNR.

          Jesse Glick added a comment -

          Also when working on DockerToolInstaller I noticed that FilePath.renameTo does not check its return value!

          Jesse Glick added a comment - Also when working on DockerToolInstaller I noticed that FilePath.renameTo does not check its return value!

          Jesse Glick added a comment -

          Also a Pipeline sh step may fail in 2.32.2 when the workspace is not writable with this unhelpful message:

          java.io.IOException: Failed to mkdirs: …
          	at hudson.FilePath.mkdirs(FilePath.java:1169)
          	at org.jenkinsci.plugins.durabletask.FileMonitoringTask$FileMonitoringController.<init>(FileMonitoringTask.java:112)
          	at org.jenkinsci.plugins.durabletask.BourneShellScript$ShellController.<init>(BourneShellScript.java:167)
          	at org.jenkinsci.plugins.durabletask.BourneShellScript$ShellController.<init>(BourneShellScript.java:161)
          	at org.jenkinsci.plugins.durabletask.BourneShellScript.launchWithCookie(BourneShellScript.java:90)
          	at org.jenkinsci.plugins.durabletask.FileMonitoringTask.launch(FileMonitoringTask.java:63)
          	at org.jenkinsci.plugins.workflow.steps.durable_task.DurableTaskStep$Execution.start(DurableTaskStep.java:172)
          	at …
          

          Here the remote callable in FilePath.mkdirs is using File.mkdirs(), which merely returns false without reporting a particular reason.

          Jesse Glick added a comment - Also a Pipeline sh step may fail in 2.32.2 when the workspace is not writable with this unhelpful message: java.io.IOException: Failed to mkdirs: … at hudson.FilePath.mkdirs(FilePath.java:1169) at org.jenkinsci.plugins.durabletask.FileMonitoringTask$FileMonitoringController.<init>(FileMonitoringTask.java:112) at org.jenkinsci.plugins.durabletask.BourneShellScript$ShellController.<init>(BourneShellScript.java:167) at org.jenkinsci.plugins.durabletask.BourneShellScript$ShellController.<init>(BourneShellScript.java:161) at org.jenkinsci.plugins.durabletask.BourneShellScript.launchWithCookie(BourneShellScript.java:90) at org.jenkinsci.plugins.durabletask.FileMonitoringTask.launch(FileMonitoringTask.java:63) at org.jenkinsci.plugins.workflow.steps.durable_task.DurableTaskStep$Execution.start(DurableTaskStep.java:172) at … Here the remote callable in FilePath.mkdirs is using File.mkdirs() , which merely returns false without reporting a particular reason.

          Jesse Glick added a comment -

          ProcessTree / ProcessKiller could probably be implemented using JEP 102 in Java 9.

          Jesse Glick added a comment - ProcessTree / ProcessKiller could probably be implemented using JEP 102 in Java 9.

          pjdarton added a comment -

          One word of caution:

          • On Windows, it's commonplace^1^ to use a "Directory Junction"2 instead of a symbolic link to a directory.
          • As far as Java is concerned, a "Directory Junction" is not a symbolic link.
          • Jenkins MUST treat a "Directory Junction" in exactly the same way it treats a symbolic link.

          This has previously caused problems, e.g. when doing a recursive delete, Jenkins has to ask "is this a link?" before deleting a subdirectory's contents. We've previously seen bugs where Jenkins' code trusted the JVM to say "yes, this is a link" when the directory was any kind of link to "somewhere else", whereas Java actually only said "yes, this is a link" when it was a symbolic link (not a Directory Junction), causing recursive deletes to escape from the workspace and run rampant.

          I suspect that it'll be tricky to remove all of the JNA code and remain able to run on Windows...

          Note 1: Microsoft deemed the creation of a symbolic link to be a controlled-by-privilege operation, and then (due to buggy kernel code) prevented admins from granting that privilege, so you need to be an Administrator and running "as admin" (which is worse than "running as root" on Linux) to make a symbolic link. Microsoft placed no restrictions on creating "Directory Junction"s.
          Note 2: A Windows "Directory Junction" is functionally identical to a symbolic link that's pointing at (the absolute path to) a directory. It looks the same and it must be treated the same.

          pjdarton added a comment - One word of caution: On Windows, it's commonplace^1^ to use a "Directory Junction" 2 instead of a symbolic link to a directory. As far as Java is concerned, a "Directory Junction" is not a symbolic link. Jenkins MUST treat a "Directory Junction" in exactly the same way it treats a symbolic link. This has previously caused problems, e.g. when doing a recursive delete, Jenkins has to ask "is this a link?" before deleting a subdirectory's contents. We've previously seen bugs where Jenkins' code trusted the JVM to say "yes, this is a link" when the directory was any kind of link to "somewhere else", whereas Java actually only said "yes, this is a link" when it was a symbolic link (not a Directory Junction), causing recursive deletes to escape from the workspace and run rampant. I suspect that it'll be tricky to remove  all of the JNA code and remain able to run on Windows... Note 1: Microsoft deemed the creation of a symbolic link to be a controlled-by-privilege operation, and then (due to buggy kernel code) prevented admins from granting that privilege, so you need to be an Administrator and running "as admin" (which is worse than "running as root" on Linux) to make a symbolic link. Microsoft placed no restrictions on creating "Directory Junction"s. Note 2: A Windows "Directory Junction" is functionally identical to a symbolic link that's pointing at (the absolute path to) a directory. It looks the same and it must be treated the same.

          Devin Nusbaum added a comment -

          I've attempted to remove the native code for Util#isSymlink
          here. pjdarton Feel free to take a look and make sure the junction handling on Windows appears to be correct.

          Devin Nusbaum added a comment - I've attempted to remove the native code for Util#isSymlink here . pjdarton Feel free to take a look and make sure the junction handling on Windows appears to be correct.

          Devin Nusbaum added a comment - - edited

          While looking at updating IOUtils#mode and FilePath#chmod I realized that NIO provides no way to read/write the setgid, setuid, or sticky bits. I don't see any usages of chmod with the 4th octal digit or higher in a basic search, but there are some places that use chmod(mode()) while copying a file to keep the same permissions. We could use an NIO implementation of chmod for modes less than 0o777 and fall back to JNR if the user is trying to set other bits, but to maintain compatibility mode would always have to use JNR.

          I don't have any idea of how common setgid, setuid, or sticky bits are in practice for files in a workspace (They don't seem useful to me in this context at first glance). If this doesn't seem to be an issue I can submit a PR to use NIO implementations, but I wanted to get some feedback before proceeding.

          Devin Nusbaum added a comment - - edited While looking at updating IOUtils#mode and FilePath#chmod I realized that NIO provides no way to read/write the setgid, setuid, or sticky bits. I don't see any usages of chmod with the 4th octal digit or higher in a basic search , but there are some places that use chmod(mode()) while copying a file to keep the same permissions. We could use an NIO implementation of chmod for modes less than 0o777 and fall back to JNR if the user is trying to set other bits, but to maintain compatibility mode would always have to use JNR. I don't have any idea of how common setgid, setuid, or sticky bits are in practice for files in a workspace (They don't seem useful to me in this context at first glance). If this doesn't seem to be an issue I can submit a PR to use NIO implementations, but I wanted to get some feedback before proceeding.

          Jesse Glick added a comment -

          I would be fine with just throwing an exception if someone is actually trying to use those mode bits in workspace files. Seems very unlikely, and sounds like a security risk anyway.

          Jesse Glick added a comment - I would be fine with just throwing an exception if someone is actually trying to use those mode bits in workspace files. Seems very unlikely, and sounds like a security risk anyway.

          pjdarton added a comment - - edited

          Using setgid on folders is pretty standard when the user running Jenkins has access to more than one unix group and wants the files in the Jenkins area to be in a group other than their default group.
          e.g. if user "jenkins" has access to multiple groups, including "users" and "jobreaders" you might set the group of /home/jenkins/jobs/ to "jobreaders" and set the setgid bit on all the folders so that all subsequently-created subfolders within /home/jenkins/jobs/ have the same group.
          If you don't set setgid then all subsequently-created subfolders within /home/jenkins/jobs/ would be created with user "jenkins"'s default group.

          setgid on files is another matter entirely - that, I agree, is where the security aspects start to get worrisome.

          As long as Jenkins doesn't unset the bits it can't read/write then everything should "just work".

          Edit: Just to clarify, I'm not claiming that it'd be commonplace for the Jenkins software to go setting setgid bits of folders; what I'm saying is that a system administrator might set the setgid bit on selected folders in the area of the filesystem where Jenkins is, and that Jenkins shouldn't unset those bits if the filesystem defaulted them to "set".

          pjdarton added a comment - - edited Using setgid on folders is pretty standard when the user running Jenkins has access to more than one unix group and wants the files in the Jenkins area to be in a group other than their default group. e.g. if user "jenkins" has access to multiple groups, including "users" and "jobreaders" you might set the group of /home/jenkins/jobs/ to "jobreaders" and set the setgid bit on all the folders so that all subsequently-created subfolders within /home/jenkins/jobs/ have the same group. If you don't set setgid then all subsequently-created subfolders within /home/jenkins/jobs/ would be created with user "jenkins"'s default group. setgid on files is another matter entirely - that, I agree, is where the security aspects start to get worrisome. As long as Jenkins doesn't unset the bits it can't read/write then everything should "just work". Edit: Just to clarify, I'm not claiming that it'd be commonplace for the Jenkins software to go setting setgid bits of folders; what I'm saying is that a system administrator might set the setgid bit on selected folders in the area of the filesystem where Jenkins is, and that Jenkins shouldn't unset those bits if the filesystem defaulted them to "set".

          Devin Nusbaum added a comment - - edited

          Any attribute write done via NIO's setPosixFilePermissions will clear those bits. See JDK-8137404. However, from reading the OpenJDK sources it looks like calling Files#copy with StandardCopyOption.COPY_ATTRIBUTES would leave them untouched, so updating `copyToWithPermission` to use NIO at the same time would likely prevent most issues. I think that would just leave TarArchiver and ZipArchiver as the other places in core using something like chmod(mode()), so I'll investigate those further.

          Devin Nusbaum added a comment - - edited Any attribute write done via NIO's setPosixFilePermissions will clear those bits. See JDK-8137404 . However, from reading the OpenJDK sources it looks like calling Files#copy with StandardCopyOption.COPY_ATTRIBUTES would leave them untouched, so updating `copyToWithPermission` to use NIO at the same time would likely prevent most issues. I think that would just leave TarArchiver and ZipArchiver as the other places in core using something like chmod(mode()) , so I'll investigate those further.

          Code changed in jenkins
          User: Devin Nusbaum
          Path:
          core/src/main/java/hudson/Util.java
          core/src/main/java/hudson/util/jna/Kernel32Utils.java
          core/src/test/java/hudson/FilePathTest.java
          core/src/test/java/hudson/UtilTest.java
          http://jenkins-ci.org/commit/jenkins/52fa4d90b938243ccc273955caa7262154b9f688
          Log:
          JENKINS-39179 JENKINS-36088 Always use NIO to create and detect symbolic links and Windows junctions (#3133)

          • Always use NIO to detect symlinks
          • Make assertion failure message consistent
          • Catch NoSuchFileException to keep tests passing
          • Make method name more specific and simlify assumption
          • Remove obsolete comment and reword the main comment in isSymlink
          • Deprecate Kernel32Util#isJunctionOrSymlink
          • Use assumptions for junction creation and add messages to assumptions
          • Replace deprecated code with recommended alternative
          • Add comment explaining call to DosFileAttributes#isOther
          • Do not fall back to native code when creating symlinks
          • Log FileSystemExceptions when creating symbolic links
          • Catch InvalidPathException and rethrow as IOException
          • Deprecate Kernel32Utils#createSymbolicLink and #getWin32FileAttributes
          • Preserve original logging behavior on Windows and remove useless call to Util#displayIOException

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Devin Nusbaum Path: core/src/main/java/hudson/Util.java core/src/main/java/hudson/util/jna/Kernel32Utils.java core/src/test/java/hudson/FilePathTest.java core/src/test/java/hudson/UtilTest.java http://jenkins-ci.org/commit/jenkins/52fa4d90b938243ccc273955caa7262154b9f688 Log: JENKINS-39179 JENKINS-36088 Always use NIO to create and detect symbolic links and Windows junctions (#3133) Always use NIO to detect symlinks Make assertion failure message consistent Catch NoSuchFileException to keep tests passing Make method name more specific and simlify assumption Remove obsolete comment and reword the main comment in isSymlink Deprecate Kernel32Util#isJunctionOrSymlink Use assumptions for junction creation and add messages to assumptions Replace deprecated code with recommended alternative Add comment explaining call to DosFileAttributes#isOther Do not fall back to native code when creating symlinks Log FileSystemExceptions when creating symbolic links Catch InvalidPathException and rethrow as IOException Deprecate Kernel32Utils#createSymbolicLink and #getWin32FileAttributes Preserve original logging behavior on Windows and remove useless call to Util#displayIOException

          Oleg Nenashev added a comment -

          The change by dnusbaum has been integrated towards 2.91.
          dnusbaum is it enough to close this issue? I am not sure about that

          Oleg Nenashev added a comment - The change by dnusbaum has been integrated towards 2.91. dnusbaum is it enough to close this issue? I am not sure about that

          Devin Nusbaum added a comment -

          oleg_nenashev No, I don't think this issue should be closed yet. There is at least one more PR that should be finalized.

          Devin Nusbaum added a comment - oleg_nenashev No, I don't think this issue should be closed yet. There is at least one more PR that should be finalized.

          Code changed in jenkins
          User: Devin Nusbaum
          Path:
          core/src/main/java/hudson/FilePath.java
          core/src/main/java/hudson/Util.java
          core/src/main/java/hudson/util/IOUtils.java
          core/src/test/java/hudson/FilePathTest.java
          core/src/test/java/hudson/UtilTest.java
          core/src/test/java/hudson/util/io/TarArchiverTest.java
          http://jenkins-ci.org/commit/jenkins/fdccc0e8384370684e25063e95f4a704773c53dd
          Log:
          JENKINS-36088 Use NIO implementations of chmod and mode by default (#3135)

          • Use NIO for FilePath#chmod and IOUtils#mode
          • Add tests for NIO mode and chmod implementations
          • Add test, remove new method, and update JavaDoc
          • Provide system property to use native implementations of chmod and mode
          • Revert unrelated whitespace modification
          • Don't remove exception from throws and put imports in original location
          • Fix broken JavaDoc links
          • Ignore file type bits (above 0o7777) in Util#modeToPermission
          • Use octal for constants and don't include file type bits
          • Revert unnecessary changes to TarArchiverTest
          • Add assertion that non-permission bits are ignored by chmod
          • Use NIO copy with StandardCopyOption.COPY_ATTRIBUTES in copyToWithPermissions where possible
          • Catch InvalidPathException and convert it to IOException
          • Create utility method for File#toPath and use File#createDirectories after review
          • Remove useless calls to toAbsolutePath and getAbsoluteFile
          • Fix typos and use octal for constant after review
          • Add test for behavior of copyToWithPermission with special bits

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Devin Nusbaum Path: core/src/main/java/hudson/FilePath.java core/src/main/java/hudson/Util.java core/src/main/java/hudson/util/IOUtils.java core/src/test/java/hudson/FilePathTest.java core/src/test/java/hudson/UtilTest.java core/src/test/java/hudson/util/io/TarArchiverTest.java http://jenkins-ci.org/commit/jenkins/fdccc0e8384370684e25063e95f4a704773c53dd Log: JENKINS-36088 Use NIO implementations of chmod and mode by default (#3135) Use NIO for FilePath#chmod and IOUtils#mode Add tests for NIO mode and chmod implementations Add test, remove new method, and update JavaDoc Provide system property to use native implementations of chmod and mode Revert unrelated whitespace modification Don't remove exception from throws and put imports in original location Fix broken JavaDoc links Ignore file type bits (above 0o7777) in Util#modeToPermission Use octal for constants and don't include file type bits Revert unnecessary changes to TarArchiverTest Add assertion that non-permission bits are ignored by chmod Use NIO copy with StandardCopyOption.COPY_ATTRIBUTES in copyToWithPermissions where possible Catch InvalidPathException and convert it to IOException Create utility method for File#toPath and use File#createDirectories after review Remove useless calls to toAbsolutePath and getAbsoluteFile Fix typos and use octal for constant after review Add test for behavior of copyToWithPermission with special bits

          Oleg Nenashev added a comment -

          The fix has been integrated towards 2.93. I doubt it is backportable, but feel free to add the label if you feel it's reasonable

          Oleg Nenashev added a comment - The fix has been integrated towards 2.93. I doubt it is backportable, but feel free to add the label if you feel it's reasonable

          This actually breaks the docker-commons plugin: when it tries to 'materialize' the credentials, it relies on a chmod() call to create a folder (seems this is the behavior of native API), which now leads to an exception:

          15:38:55 java.nio.file.NoSuchFileException: /var/lib/jenkins/.docker/7d36d1a6-4917-4dc8-97b6-2babbb2ae3d7
          15:38:55 at sun.nio.fs.UnixException.translateToIOException(UnixException.java:86)
          15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
          15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
          15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setMode(UnixFileAttributeViews.java:238)
          15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setPermissions(UnixFileAttributeViews.java:260) 15:38:55 at java.nio.file.Files.setPosixFilePermissions(Files.java:2045)
          15:38:55 at hudson.FilePath._chmod(FilePath.java:1618) 15:38:55 at hudson.FilePath.access$1500(FilePath.java:197)
          15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1600)
          15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1597)
          15:38:55 at hudson.FilePath.act(FilePath.java:998)
          15:38:55 at hudson.FilePath.act(FilePath.java:976)
          15:38:55 at hudson.FilePath.chmod(FilePath.java:1597)
          15:38:55 at org.jenkinsci.plugins.docker.commons.impl.ServerKeyMaterialFactory.materialize(ServerKeyMaterialFactory.java:86)
          15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69)
          15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69)
          15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:459)
          15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:431)
          15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.buildAndTag(DockerBuilder.java:373)
          15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.exec(DockerBuilder.java:311)
          15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.access$100(DockerBuilder.java:291)
          15:38:55 at com.cloudbees.dockerpublish.DockerBuilder.perform(DockerBuilder.java:262)
          15:38:55 at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
          15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:744)
          15:38:55 at hudson.model.Build$BuildExecution.build(Build.java:206)
          15:38:55 at hudson.model.Build$BuildExecution.doRun(Build.java:163)
          15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:504)
          15:38:55 at hudson.model.Run.execute(Run.java:1727)
          15:38:55 at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
          15:38:55 at hudson.model.ResourceController.execute(ResourceController.java:97)
          15:38:55 at hudson.model.Executor.run(Executor.java:429)
          15:38:55 Build step 'Docker Build and Publish' marked build as failure

           

          Francois Ferrand added a comment - This actually breaks the docker-commons plugin: when it tries to 'materialize' the credentials, it relies on a chmod() call to create a folder (seems this is the behavior of native API), which now leads to an exception: 15:38:55 java.nio.file.NoSuchFileException: /var/lib/jenkins/.docker/7d36d1a6-4917-4dc8-97b6-2babbb2ae3d7 15:38:55 at sun.nio.fs.UnixException.translateToIOException(UnixException.java:86) 15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102) 15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107) 15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setMode(UnixFileAttributeViews.java:238) 15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setPermissions(UnixFileAttributeViews.java:260) 15:38:55 at java.nio.file.Files.setPosixFilePermissions(Files.java:2045) 15:38:55 at hudson.FilePath._chmod(FilePath.java:1618) 15:38:55 at hudson.FilePath.access$1500(FilePath.java:197) 15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1600) 15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1597) 15:38:55 at hudson.FilePath.act(FilePath.java:998) 15:38:55 at hudson.FilePath.act(FilePath.java:976) 15:38:55 at hudson.FilePath.chmod(FilePath.java:1597) 15:38:55 at org.jenkinsci.plugins.docker.commons.impl.ServerKeyMaterialFactory.materialize(ServerKeyMaterialFactory.java:86) 15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69) 15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:459) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:431) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.buildAndTag(DockerBuilder.java:373) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.exec(DockerBuilder.java:311) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.access$100(DockerBuilder.java:291) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder.perform(DockerBuilder.java:262) 15:38:55 at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20) 15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:744) 15:38:55 at hudson.model.Build$BuildExecution.build(Build.java:206) 15:38:55 at hudson.model.Build$BuildExecution.doRun(Build.java:163) 15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:504) 15:38:55 at hudson.model.Run.execute(Run.java:1727) 15:38:55 at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43) 15:38:55 at hudson.model.ResourceController.execute(ResourceController.java:97) 15:38:55 at hudson.model.Executor.run(Executor.java:429) 15:38:55 Build step 'Docker Build and Publish' marked build as failure  

          Devin Nusbaum added a comment -

          typz Thanks for reporting the bug. My guess is that previously the call to `FilePath#chmod` failed, but the return value was not checked and so no exception was thrown, and it went unnoticed because ServerKeyMaterialFactory#copyInto calls FilePath#write which created parent directories if they don't already exist.

          I will test to see what happens when the native chmod is used on a non-existent file or directory. If it fails silently before this change, I will file a PR against docker-commons to create the file before changing permissions. If it succeeds, I will change the new code to create the file or directory first if necessary.

          Devin Nusbaum added a comment - typz Thanks for reporting the bug. My guess is that previously the call to `FilePath#chmod` failed, but the return value was not checked and so no exception was thrown, and it went unnoticed because ServerKeyMaterialFactory#copyInto calls FilePath#write which created parent directories if they don't already exist. I will test to see what happens when the native chmod is used on a non-existent file or directory. If it fails silently before this change, I will file a PR against docker-commons to create the file before changing permissions. If it succeeds, I will change the new code to create the file or directory first if necessary.

          Devin Nusbaum added a comment -

          The following test passes against master, which implies that before PR #3135, FilePath#chmod on a non-existent file or directory would fail silently:

              @Test
              public void chmodFileDoesNotExist() throws Exception {
                  assumeFalse(Functions.isWindows());
                  Util.NATIVE_CHMOD_MODE = true; // Use native chmod
                  File f = File.createTempFile("chmod-test", ".tmp");
                  f.delete();
                  FilePath fp = new FilePath(f);
                  fp.chmod(0666);
                  assertFalse(f.exists());
              }
          

          Changing Util.NATIVE_CHMOD_MODE to false (to use NIO) causes the test to throw NoSuchFileException as noted by typz. The native implementation of IOUtils#mode threw an exception if the file did not exist, so it seems only the behavior of FilePath#chmod has changed.

          I searched through the jenkinsci org for chmod to see if any other uses were likely to break after this change.
          Almost all uses call one of mkdirs, touch, exists, write, or createTempFile/Dir immediately before chmod which should prevent them from seeing a NoSuchFileException.There are a few uses such as android-emulator-plugin and copy-data-to-workspace-plugin that could hit new exceptions if files are deleted while the directories are being traversed.

          I think there is value in throwing NoSuchFileException to catch legitimate bugs as in docker-commons, and it doesn't appear that there will be widespread issues with the exception being thrown in plugins where it wouldn't have been before, so I will file a PR against docker-commons to create the directory before calling chmod. If anyone feels strongly that that we should preserve the old behavior I am also happy to catch NoSuchFileException in FilePath#chmod.

          Devin Nusbaum added a comment - The following test passes against master, which implies that before PR #3135, FilePath#chmod on a non-existent file or directory would fail silently: @Test public void chmodFileDoesNotExist() throws Exception { assumeFalse(Functions.isWindows()); Util.NATIVE_CHMOD_MODE = true ; // Use native chmod File f = File.createTempFile( "chmod-test" , ".tmp" ); f.delete(); FilePath fp = new FilePath(f); fp.chmod(0666); assertFalse(f.exists()); } Changing Util.NATIVE_CHMOD_MODE to false (to use NIO) causes the test to throw NoSuchFileException as noted by typz . The native implementation of IOUtils#mode threw an exception if the file did not exist, so it seems only the behavior of FilePath#chmod has changed. I searched through the jenkinsci org for chmod to see if any other uses were likely to break after this change. Almost all uses call one of mkdirs , touch , exists , write , or createTempFile/Dir immediately before chmod which should prevent them from seeing a NoSuchFileException .There are a few uses such as android-emulator-plugin and  copy-data-to-workspace-plugin that could hit new exceptions if files are deleted while the directories are being traversed. I think there is value in throwing NoSuchFileException to catch legitimate bugs as in docker-commons, and it doesn't appear that there will be widespread issues with the exception being thrown in plugins where it wouldn't have been before, so I will file a PR against docker-commons to create the directory before calling chmod. If anyone feels strongly that that we should preserve the old behavior I am also happy to catch NoSuchFileException in FilePath#chmod .

          Code changed in jenkins
          User: Jesse Glick
          Path:
          src/main/java/org/jenkinsci/plugins/docker/commons/impl/ServerKeyMaterialFactory.java
          src/test/java/org/jenkinsci/plugins/docker/commons/credentials/DockerServerEndpointTest.java
          http://jenkins-ci.org/commit/docker-commons-plugin/c871b5e85fc96df8f70ef65aa96d2d73edf16615
          Log:
          Merge pull request #62 from dwnusbaum/JENKINS-36088-exposed-bug

          Create directory before calling chmod

          Compare: https://github.com/jenkinsci/docker-commons-plugin/compare/c0d9723d787a...c871b5e85fc9

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/docker/commons/impl/ServerKeyMaterialFactory.java src/test/java/org/jenkinsci/plugins/docker/commons/credentials/DockerServerEndpointTest.java http://jenkins-ci.org/commit/docker-commons-plugin/c871b5e85fc96df8f70ef65aa96d2d73edf16615 Log: Merge pull request #62 from dwnusbaum/ JENKINS-36088 -exposed-bug Create directory before calling chmod Compare: https://github.com/jenkinsci/docker-commons-plugin/compare/c0d9723d787a...c871b5e85fc9

          dnusbaum I linked the PR with this JIRA number in title in the Core, and assigned it to you to clarify.

          Please update/clarify if you shouldn't be the assignee. Thanks!

          Baptiste Mathus added a comment - dnusbaum I linked the PR with this JIRA number in title in the Core, and assigned it to you to clarify. Please update/clarify if you shouldn't be the assignee. Thanks!

            dnusbaum Devin Nusbaum
            jglick Jesse Glick
            Votes:
            1 Vote for this issue
            Watchers:
            9 Start watching this issue

              Created:
              Updated:
              Resolved: