[JENKINS-70870] Save libraries as JAR files rather than unpacked #57
Conversation
…onicalize` any more
…cache maintenance
jglick
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
…ce more conservatively
openjdk/jdk#12871 though as of a14ed45 it no longer matters here. |
…rly deleting its source; anyway it needed to be refactored to be simpler and more efficient by directly archiving the desired files
…ning about missing sources
This comment was marked as resolved.
This comment was marked as resolved.
| // Util.escape translates \n but not \r, and we do not know what platform the library will be checked out on: | ||
| replace("\r\n", "\n")); | ||
| try { | ||
| return Jenkins.get().getMarkupFormatter().translate( |
There was a problem hiding this comment.
(suppress whitespace changes when reviewing)
…raryRecord.directoryName` to avoid use of `$JENKINS_HOME` in `program.dat`
| Path target = Path.of(dir.getRemote(), link).toRealPath(); | ||
| if (!target.startsWith(Path.of(root.getRemote()))) { |
There was a problem hiding this comment.
Does this behave as expected if JENKINS_HOME itself is a symlink? See for example jenkinsci/workflow-cps-global-lib-plugin#139. Also, toRealPath will throw an exception if the link target does not exist, which may be undesirable from a security standpoint.
There was a problem hiding this comment.
Does this behave as expected if JENKINS_HOME itself is a symlink?
No idea offhand; I suppose we have no test coverage for such cases.
toRealPathwill throw an exception if the link target does not exist, which may be undesirable
I think it is fine. If there is no such target, we want to fail one way or the other.
There was a problem hiding this comment.
I suppose we have no test coverage for such cases.
I don't think so. Based on the number of reports I got from the related security fix though, it is a surprisingly common scenario. Safest to use File.getCanonicalFile / File.getCanonicalPath on both the link and root.
If there is no such target, we want to fail one way or the other.
Yeah, but the interesting thing about toRealPath vs just comparing the canonical paths is that toRealPath allows an attacker to probe the existence of arbitrary files on the controller's file system.
src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Devin Nusbaum <[email protected]>
jglick
changed the title
jglick
changed the title
JENKINS-70870
Subsumes #55. Incremental diff: jglick/pipeline-groovy-lib-plugin@SCMSourceRetriever.clone...dir2Jar