Skip to content

Commit 403a8d1

Browse files
authored
Merge branch 'master' into fix-cache-deletion
2 parents ce0e7ba + 24fa0ab commit 403a8d1

File tree

10 files changed

+266
-22
lines changed

10 files changed

+266
-22
lines changed

.github/workflows/cd.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ jobs:
4343
if: needs.validate.outputs.should_release == 'true'
4444
steps:
4545
- name: Check out
46-
uses: actions/checkout@v2.4.0
46+
uses: actions/checkout@v3
4747
with:
4848
fetch-depth: 0
4949
- name: Set up JDK 8
50-
uses: actions/setup-java@v2
50+
uses: actions/setup-java@v3
5151
with:
5252
distribution: 'adopt'
5353
java-version: 8

pom.xml

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
<parent>
2929
<groupId>org.jenkins-ci.plugins</groupId>
3030
<artifactId>plugin</artifactId>
31-
<version>4.33</version>
31+
<version>4.40</version>
3232
<relativePath />
3333
</parent>
3434
<groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -124,31 +124,36 @@
124124
<artifactId>workflow-support</artifactId>
125125
</dependency>
126126

127-
<!-- test only plugins -->
128127
<dependency>
129-
<groupId>org.jenkins-ci.plugins.workflow</groupId>
130-
<artifactId>workflow-support</artifactId>
131-
<classifier>tests</classifier>
132-
<scope>test</scope>
128+
<groupId>org.jenkins-ci.plugins</groupId>
129+
<artifactId>variant</artifactId>
133130
</dependency>
134131
<dependency>
135132
<groupId>org.jenkins-ci.plugins.workflow</groupId>
136-
<artifactId>workflow-job</artifactId>
137-
<scope>test</scope>
133+
<artifactId>workflow-multibranch</artifactId>
134+
<optional>true</optional>
135+
</dependency>
136+
<dependency>
137+
<groupId>org.jenkins-ci.plugins</groupId>
138+
<artifactId>branch-api</artifactId>
139+
<optional>true</optional>
138140
</dependency>
141+
142+
<!-- test only plugins -->
139143
<dependency>
140144
<groupId>org.jenkins-ci.plugins.workflow</groupId>
141-
<artifactId>workflow-basic-steps</artifactId>
145+
<artifactId>workflow-support</artifactId>
146+
<classifier>tests</classifier>
142147
<scope>test</scope>
143148
</dependency>
144149
<dependency>
145150
<groupId>org.jenkins-ci.plugins.workflow</groupId>
146-
<artifactId>workflow-durable-task-step</artifactId>
151+
<artifactId>workflow-basic-steps</artifactId>
147152
<scope>test</scope>
148153
</dependency>
149154
<dependency>
150155
<groupId>org.jenkins-ci.plugins.workflow</groupId>
151-
<artifactId>workflow-multibranch</artifactId>
156+
<artifactId>workflow-durable-task-step</artifactId>
152157
<scope>test</scope>
153158
</dependency>
154159
<dependency>
@@ -203,10 +208,5 @@
203208
<version>1.10.1</version>
204209
<scope>test</scope>
205210
</dependency>
206-
<dependency>
207-
<groupId>org.jenkins-ci.plugins</groupId>
208-
<artifactId>branch-api</artifactId>
209-
<scope>test</scope>
210-
</dependency>
211211
</dependencies>
212212
</project>

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.Collections;
1818
import java.util.List;
1919
import org.apache.commons.io.IOUtils;
20+
import org.apache.commons.lang.StringUtils;
2021

2122
public final class LibraryCachingConfiguration extends AbstractDescribableImpl<LibraryCachingConfiguration> {
2223
private int refreshTimeMinutes;
@@ -56,7 +57,19 @@ private List<String> getExcludedVersions() {
5657
}
5758

5859
public Boolean isExcluded(String version) {
59-
return getExcludedVersions().contains(version);
60+
// exit early if the version passed in is null or empty
61+
if (StringUtils.isBlank(version)) {
62+
return false;
63+
}
64+
for (String it : getExcludedVersions()) {
65+
// confirm that the excluded versions aren't null or empty
66+
// and if the version contains the exclusion thus it can be
67+
// anywhere in the string.
68+
if (StringUtils.isNotBlank(it) && version.contains(it)){
69+
return true;
70+
}
71+
}
72+
return false;
6073
}
6174

6275
@Override public String toString() {
@@ -96,4 +109,4 @@ public FormValidation doClearCache(@QueryParameter String name) throws Interrupt
96109
}
97110

98111
}
99-
}
112+
}

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import hudson.model.ItemGroup;
3838
import hudson.model.Run;
3939
import hudson.model.TaskListener;
40+
import hudson.scm.SCM;
4041
import hudson.security.AccessControlled;
4142
import java.io.File;
4243
import java.io.IOException;
@@ -60,6 +61,7 @@
6061
import edu.umd.cs.findbugs.annotations.NonNull;
6162
import javax.inject.Inject;
6263
import jenkins.model.Jenkins;
64+
import jenkins.scm.impl.SingleSCMSource;
6365
import org.codehaus.groovy.control.MultipleCompilationErrorsException;
6466
import org.codehaus.groovy.runtime.InvokerHelper;
6567
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.AbstractWhitelist;
@@ -192,6 +194,18 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
192194
} else if (version == null) {
193195
throw new AbortException("Must specify a version for library " + name);
194196
}
197+
// When a user specifies a non-null retriever, they may be using SCMVar in its configuration,
198+
// so we need to run MultibranchScmRevisionVerifier to prevent unsafe behavior.
199+
// SCMVar would typically be used with SCMRetriever, but it is also possible to use it with SCMSourceRetriever and SingleSCMSource.
200+
// There may be false-positive rejections if a Multibranch Pipeline for the repo of a Pipeline library
201+
// uses the library step with a non-null retriever to check out a static version of the library.
202+
// Fixing this would require us being able to detect usage of SCMVar precisely, which is not currently possible.
203+
else if (retriever instanceof SCMRetriever) {
204+
verifyRevision(((SCMRetriever) retriever).getScm(), name);
205+
} else if (retriever instanceof SCMSourceRetriever && ((SCMSourceRetriever) retriever).getScm() instanceof SingleSCMSource) {
206+
verifyRevision(((SingleSCMSource) ((SCMSourceRetriever) retriever).getScm()).getScm(), name);
207+
}
208+
195209
LibraryRecord record = new LibraryRecord(name, version, trusted, changelog, cachingConfiguration, source);
196210
LibrariesAction action = run.getAction(LibrariesAction.class);
197211
if (action == null) {
@@ -219,6 +233,12 @@ public static class Execution extends AbstractSynchronousNonBlockingStepExecutio
219233
return new LoadedClasses(name, record.getDirectoryName(), trusted, changelog, run);
220234
}
221235

236+
private void verifyRevision(SCM scm, String name) throws IOException, InterruptedException {
237+
for (LibraryStepRetrieverVerifier revisionVerifier : LibraryStepRetrieverVerifier.all()) {
238+
revisionVerifier.verify(this.run, listener, scm, name);
239+
}
240+
}
241+
222242
}
223243

224244
public static final class LoadedClasses extends GroovyObjectSupport implements Serializable {
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package org.jenkinsci.plugins.workflow.libs;
2+
3+
import hudson.ExtensionList;
4+
import hudson.ExtensionPoint;
5+
import hudson.model.Run;
6+
import hudson.model.TaskListener;
7+
import hudson.scm.SCM;
8+
import org.kohsuke.accmod.Restricted;
9+
import org.kohsuke.accmod.restrictions.NoExternalUse;
10+
11+
import java.io.IOException;
12+
13+
@Restricted(NoExternalUse.class)
14+
public interface LibraryStepRetrieverVerifier extends ExtensionPoint {
15+
16+
void verify(Run<?, ?> run, TaskListener listener, SCM scm, String name) throws IOException, InterruptedException;
17+
18+
static ExtensionList<LibraryStepRetrieverVerifier> all() {
19+
return ExtensionList.lookup(LibraryStepRetrieverVerifier.class);
20+
}
21+
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package org.jenkinsci.plugins.workflow.libs;
2+
3+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
4+
import hudson.AbortException;
5+
import hudson.model.Job;
6+
import hudson.model.Run;
7+
import hudson.model.TaskListener;
8+
import hudson.scm.SCM;
9+
import jenkins.branch.Branch;
10+
import jenkins.scm.api.SCMHead;
11+
import jenkins.scm.api.SCMRevision;
12+
import jenkins.scm.api.SCMRevisionAction;
13+
import jenkins.scm.api.SCMSource;
14+
import jenkins.scm.api.SCMSourceOwner;
15+
import org.jenkinsci.plugins.variant.OptionalExtension;
16+
import org.jenkinsci.plugins.workflow.multibranch.BranchJobProperty;
17+
18+
import java.io.IOException;
19+
20+
@OptionalExtension(requirePlugins={"workflow-multibranch"})
21+
public class MultibranchScmRevisionVerifier implements LibraryStepRetrieverVerifier {
22+
23+
@SuppressFBWarnings("MS_SHOULD_BE_FINAL") // For script console and tests.
24+
public static boolean DISABLED = Boolean.getBoolean(MultibranchScmRevisionVerifier.class.getName() + ".DISABLED");
25+
26+
/**
27+
* Abort library retrieval if the specified build is from a Multibranch Pipeline configured to build the library's SCM and the revision being built is untrusted.
28+
* Comparable to the defenses against untrusted users in {@code SCMBinder}, but here we care about the library rather than the Jenkinsfile.
29+
* @throws AbortException if the specified build is from a Multibranch Pipeline configured to build the library's SCM and the revision being built is untrusted
30+
*/
31+
@Override
32+
public void verify(Run<?, ?> run, TaskListener listener, SCM libraryScm, String name) throws IOException, InterruptedException {
33+
if (DISABLED) {
34+
return;
35+
}
36+
// Adapted from ReadTrustedStep
37+
Job<?, ?> job = run.getParent();
38+
BranchJobProperty property = job.getProperty(BranchJobProperty.class);
39+
if (property == null || !(job.getParent() instanceof SCMSourceOwner)) {
40+
// Not a multibranch project, so we do not care.
41+
// It is possible to use legacySCM(scm) from a non-multibranch Pipeline that uses CpsScmFlowDefinition,
42+
// but in that case we implicitly trust the changes because only a user with Item/Configure permission can select which branches to build.
43+
return;
44+
}
45+
Branch pipelineBranch = property.getBranch();
46+
SCMSource pipelineScmSource = ((SCMSourceOwner)job.getParent()).getSCMSource(pipelineBranch.getSourceId());
47+
if (pipelineScmSource == null) {
48+
throw new IllegalStateException(pipelineBranch.getSourceId() + " not found");
49+
}
50+
SCMHead head = pipelineBranch.getHead();
51+
SCMRevision headRevision;
52+
SCMRevisionAction action = run.getAction(SCMRevisionAction.class);
53+
if (action != null) {
54+
headRevision = action.getRevision();
55+
} else {
56+
headRevision = pipelineScmSource.fetch(head, listener);
57+
if (headRevision == null) {
58+
throw new AbortException("Could not determine exact tip revision of " + pipelineBranch.getName());
59+
}
60+
run.addAction(new SCMRevisionAction(pipelineScmSource, headRevision));
61+
}
62+
SCMRevision trustedRevision = pipelineScmSource.getTrustedRevision(headRevision, listener);
63+
if (!headRevision.equals(trustedRevision) && libraryScm.getKey().equals(pipelineScmSource.build(head, headRevision).getKey())) {
64+
throw new AbortException("Library '" + name + "' has been modified in an untrusted revision");
65+
}
66+
}
67+
}
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
<div>
2-
Space separated list of versions excluded from caching. Ex: "master release10 release9"
2+
Space separated list of versions to exclude from caching via substring search using .contains() method. Ex: "release/ master release10 release9".
33
</div>

src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public class LibraryCachingConfigurationTest {
5555
private LibraryCachingConfiguration nullVersionConfig;
5656
private LibraryCachingConfiguration oneVersionConfig;
5757
private LibraryCachingConfiguration multiVersionConfig;
58+
private LibraryCachingConfiguration substringVersionConfig;
5859

5960
private static int REFRESH_TIME_MINUTES = 23;
6061
private static int NO_REFRESH_TIME_MINUTES = 0;
@@ -66,18 +67,25 @@ public class LibraryCachingConfigurationTest {
6667
private static String MULTIPLE_EXCLUDED_VERSIONS_2 = "branch-2";
6768
private static String MULTIPLE_EXCLUDED_VERSIONS_3 = "branch-3";
6869

70+
private static String SUBSTRING_EXCLUDED_VERSIONS_1 = "feature/test-substring-exclude";
71+
private static String SUBSTRING_EXCLUDED_VERSIONS_2 = "test-other-substring-exclude";
72+
6973
private static String MULTIPLE_EXCLUDED_VERSIONS =
7074
MULTIPLE_EXCLUDED_VERSIONS_1 + " " +
7175
MULTIPLE_EXCLUDED_VERSIONS_2 + " " +
7276
MULTIPLE_EXCLUDED_VERSIONS_3;
7377

78+
private static String SUBSTRING_EXCLUDED_VERSIONS =
79+
"feature/ other-substring";
80+
7481
private static String NEVER_EXCLUDED_VERSION = "never-excluded-version";
7582

7683
@Before
7784
public void createCachingConfiguration() {
7885
nullVersionConfig = new LibraryCachingConfiguration(REFRESH_TIME_MINUTES, NULL_EXCLUDED_VERSION);
7986
oneVersionConfig = new LibraryCachingConfiguration(NO_REFRESH_TIME_MINUTES, ONE_EXCLUDED_VERSION);
8087
multiVersionConfig = new LibraryCachingConfiguration(REFRESH_TIME_MINUTES, MULTIPLE_EXCLUDED_VERSIONS);
88+
substringVersionConfig = new LibraryCachingConfiguration(REFRESH_TIME_MINUTES, SUBSTRING_EXCLUDED_VERSIONS);
8189
}
8290

8391
@Issue("JENKINS-66045") // NPE getting excluded versions
@@ -114,6 +122,7 @@ public void getExcludedVersionsStr() {
114122
assertThat(nullVersionConfig.getExcludedVersionsStr(), is(NULL_EXCLUDED_VERSION));
115123
assertThat(oneVersionConfig.getExcludedVersionsStr(), is(ONE_EXCLUDED_VERSION));
116124
assertThat(multiVersionConfig.getExcludedVersionsStr(), is(MULTIPLE_EXCLUDED_VERSIONS));
125+
assertThat(substringVersionConfig.getExcludedVersionsStr(), is(SUBSTRING_EXCLUDED_VERSIONS));
117126
}
118127

119128
@Test
@@ -128,17 +137,22 @@ public void isExcluded() {
128137
assertTrue(multiVersionConfig.isExcluded(MULTIPLE_EXCLUDED_VERSIONS_2));
129138
assertTrue(multiVersionConfig.isExcluded(MULTIPLE_EXCLUDED_VERSIONS_3));
130139

140+
assertTrue(substringVersionConfig.isExcluded(SUBSTRING_EXCLUDED_VERSIONS_1));
141+
assertTrue(substringVersionConfig.isExcluded(SUBSTRING_EXCLUDED_VERSIONS_2));
142+
131143
assertFalse(nullVersionConfig.isExcluded(NEVER_EXCLUDED_VERSION));
132144
assertFalse(oneVersionConfig.isExcluded(NEVER_EXCLUDED_VERSION));
133145
assertFalse(multiVersionConfig.isExcluded(NEVER_EXCLUDED_VERSION));
134146

135147
assertFalse(nullVersionConfig.isExcluded(""));
136148
assertFalse(oneVersionConfig.isExcluded(""));
137149
assertFalse(multiVersionConfig.isExcluded(""));
150+
assertFalse(substringVersionConfig.isExcluded(""));
138151

139152
assertFalse(nullVersionConfig.isExcluded(null));
140153
assertFalse(oneVersionConfig.isExcluded(null));
141154
assertFalse(multiVersionConfig.isExcluded(null));
155+
assertFalse(substringVersionConfig.isExcluded(null));
142156
}
143157

144158
@Test

src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryMemoryTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public static void register(Object o) {
8282
clearInvocationCaches.invoke(metaClass);
8383
}
8484
for (WeakReference<ClassLoader> loaderRef : LOADERS) {
85-
MemoryAssert.assertGC(loaderRef, false);
85+
MemoryAssert.assertGC(loaderRef);
8686
}
8787
}
8888

0 commit comments

Comments
 (0)