[JENKINS-40533] Allow definition of sandboxed libraries at global scope #129
Conversation
src/main/java/org/jenkinsci/plugins/workflow/libs/AbstractGlobalLibraries.java
Outdated
Show resolved
Hide resolved
src/main/resources/org/jenkinsci/plugins/workflow/libs/Messages.properties
Outdated
Show resolved
Hide resolved
| return Jenkins.MANAGE; | ||
| } | ||
|
|
||
| @Extension(ordinal=0) public static class ForJob extends AbstractForJob { |
There was a problem hiding this comment.
I think this implementation needs to wrap the returned libraries in ResolvedLibraryConfiguration like FolderLibraries.ForJob to avoid introducing a new security issue like what was fixed by ace0de3.
There was a problem hiding this comment.
Maybe the trust flag is enough though, I am not sure. Either way I would look at old security fixes here carefully to see if anything extra needs to be done to separate the two global configurations in all contexts.
There was a problem hiding this comment.
Indeed, this is addressed in 98322fb
There was a problem hiding this comment.
Looking at the old fix in more detail, I think it was probably fine as you had it originally because of this line:
I think folder-based libraries just needed special treatment to distinguish between folders with the same name defined at different levels since they use the same LibraryResolver class. Since the new untrusted global libraries use a distinct resolver class and only exist in a single place, things should be fine without wrapping LibraryConfigurations. Your changes use getClass().getName() anyway so the behavior should be identical. I didn't try testing it though.
|
@dwnusbaum best to “request changes” if in doubt whether there might be a security issue. |
| return getConfiguration() | ||
| .getLibraries() | ||
| .stream() | ||
| .map(this::mayWrapLibrary) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| protected LibraryConfiguration mayWrapLibrary(LibraryConfiguration library) { | ||
| return library; | ||
| } |
There was a problem hiding this comment.
Looking more carefully, it is probably unnecessary to wrap these, see #129 (comment).
src/main/java/org/jenkinsci/plugins/workflow/libs/AbstractGlobalLibraries.java
Outdated
Show resolved
Hide resolved
|
@dwnusbaum can you merge this ? |
This lets users with
Jenkins.MANAGEpermission define untrusted global libraries.They can be used in a similar way than folder libraries, except they are available globally.
Testing done
Submitter checklist