• Icon: Bug Bug
    • Resolution: Fixed
    • Icon: Minor Minor
    • core
    • OS: RHEL 6
      Jenkins: 2.93
      ThinBackup: 1.9

      After upgrading to Jenkins 2.93, ThinBackup sets the permissions of installedPlugins.xml to 600.  Previously, the permission were 644, matching the original file.

      If I watch the permissions as a backup is running, the permissions start as 644, but change to 600 by the time the backup completes.

      The dnusbaum user on IRC recommended I create this issue. 

      EDIT: This issue was discovered when automation attempts to copy the backups files off of the server as non jenkins user. I assume since the permissions of the backups are different than JENKINS_HOME, that restoring would fail. I have not had a chance to test this theory.

          [JENKINS-48407] Permission issue after upgrade to 2.93

          James Nord added a comment -

          That's just crazy weird. surely if I ask for world writeable I mean world writable...

          Given I did not see that documented (may have missed it) I would not like to rely on that behaviour :-o

          James Nord added a comment - That's just crazy weird. surely if I ask for world writeable I mean world writable... Given I did not see that documented (may have missed it) I would not like to rely on that behaviour :-o

          Devin Nusbaum added a comment -

          Yeah it's definitely weird. The javadoc for PosixFileAttributesView#Setting Initial Permissions mentions it (not sure if that was the javadoc you were referring to earlier):

          When the access permissions are set at file creation time then the actual value of the permissions may differ that the value of the attribute object. The reasons for this are implementation specific. On UNIX systems, for example, a process has a umask that impacts the permission bits of newly created files. Where an implementation supports the setting of the access permissions, and the underlying file system supports access permissions, then it is required that the value of the actual access permissions will be equal or less than the value of the attribute provided to the createFile or createDirectory methods. In other words, the file may be more secure than requested.

          So I guess if you want exact permissions on a new file you have to create it, and then set the permissions to what you actually want...

          Devin Nusbaum added a comment - Yeah it's definitely weird. The javadoc for PosixFileAttributesView#Setting Initial Permissions  mentions it (not sure if that was the javadoc you were referring to earlier): When the access permissions are set at file creation time then the actual value of the permissions may differ that the value of the attribute object. The reasons for this are implementation specific. On UNIX systems, for example, a process has a umask that impacts the permission bits of newly created files. Where an implementation supports the setting of the access permissions, and the underlying file system supports access permissions, then it is required that the value of the actual access permissions will be equal or less than the value of the attribute provided to the createFile or createDirectory methods. In other words, the file may be more secure than requested. So I guess if you want exact permissions on a new file you have to create it, and then set the permissions to what you actually want...

          Baptiste Mathus added a comment - bharper dnusbaum teilo https://github.com/jenkinsci/jenkins/pull/3181 tests + fix is up for review.

          https://github.com/jenkinsci/jenkins/pull/3233 up for review, simpler and superseding the previous one.

          Baptiste Mathus added a comment - https://github.com/jenkinsci/jenkins/pull/3233  up for review, simpler and superseding the previous one.

          Code changed in jenkins
          User: Baptiste Mathus
          Path:
          core/src/main/java/hudson/util/AtomicFileWriter.java
          http://jenkins-ci.org/commit/jenkins/4e9376034546e9d18571cc02ea8fa5d1393418d5
          Log:
          [FIX JENKINS-48407] Permission issue after upgrade to 2.93

          Simply revert to using pre-NIO createTempFile for backward compatibility.
          There is no simple way to restore a similar way using NIO's createTempFile.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Baptiste Mathus Path: core/src/main/java/hudson/util/AtomicFileWriter.java http://jenkins-ci.org/commit/jenkins/4e9376034546e9d18571cc02ea8fa5d1393418d5 Log: [FIX JENKINS-48407] Permission issue after upgrade to 2.93 Simply revert to using pre-NIO createTempFile for backward compatibility. There is no simple way to restore a similar way using NIO's createTempFile.

          Fixed towards upcoming 2.103 (if no out of order release has to happen in between). Cf. this commit to find out the actual release if needed.

          Baptiste Mathus added a comment - Fixed towards upcoming 2.103 (if no out of order release has to happen in between). Cf. this commit  to find out the actual release if needed.

          b harper added a comment -

          I just upgraded to 2.103 and we are back in business. Thanks to all who helped fixed this issue.

          b harper added a comment - I just upgraded to 2.103 and we are back in business. Thanks to all who helped fixed this issue.

          Code changed in jenkins
          User: Daniel Beck
          Path:
          core/src/test/java/hudson/util/AtomicFileWriterTest.java
          http://jenkins-ci.org/commit/jenkins/631ed7a456fd8959c9399500db637edbaef77c9b
          Log:
          Merge pull request #3274 from batmat/JENKINS-48407-disable-test-temporarily

          Disable test temporarily to get the release out

          Compare: https://github.com/jenkinsci/jenkins/compare/66bb3dc6fd2a...631ed7a456fd

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Daniel Beck Path: core/src/test/java/hudson/util/AtomicFileWriterTest.java http://jenkins-ci.org/commit/jenkins/631ed7a456fd8959c9399500db637edbaef77c9b Log: Merge pull request #3274 from batmat/ JENKINS-48407 -disable-test-temporarily Disable test temporarily to get the release out Compare: https://github.com/jenkinsci/jenkins/compare/66bb3dc6fd2a...631ed7a456fd

          Code changed in jenkins
          User: Baptiste Mathus
          Path:
          core/src/test/java/hudson/util/AtomicFileWriterTest.java
          http://jenkins-ci.org/commit/jenkins/6cefcf17a8d92b43b738ec6cd5a2cf298cc1156e
          Log:
          JENKINS-48407 Re-enable AtomicFileWriterTest#checkPermissionsRespectUmask() (#3275)

          The previous test assumed permissions would always be the same,
          when they actually depend on umask settings.

          This change creates a file not using the temporary API, gets its
          permissions then compares it to the ones obtained using
          AtomicFileWriter.

          Note: we now only check the given permissions, not the "non-given".

          • Use assertThat(..., equalTo()) instead of a manual loop
          • Remove unused imports
          • Use TemporaryFolder instead of manual temporary dir creation

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Baptiste Mathus Path: core/src/test/java/hudson/util/AtomicFileWriterTest.java http://jenkins-ci.org/commit/jenkins/6cefcf17a8d92b43b738ec6cd5a2cf298cc1156e Log: JENKINS-48407 Re-enable AtomicFileWriterTest#checkPermissionsRespectUmask() (#3275) JENKINS-48407 Re-enable test The previous test assumed permissions would always be the same, when they actually depend on umask settings. This change creates a file not using the temporary API, gets its permissions then compares it to the ones obtained using AtomicFileWriter. Note: we now only check the given permissions, not the "non-given". Use assertThat(..., equalTo()) instead of a manual loop Remove unused imports Use TemporaryFolder instead of manual temporary dir creation

          Daniel Beck added a comment -

          This is in 2.105, so should be in the next LTS baseline even if we end up not going with 2.107.

          Daniel Beck added a comment - This is in 2.105, so should be in the next LTS baseline even if we end up not going with 2.107.

            batmat Baptiste Mathus
            bharper b harper
            Votes:
            1 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: