Skip to content

Commit 1b5b431

Browse files
dwnusbaumdwnusbaum
committed
Fix library cache deletion after SECURITY-2586
1 parent e62a4eb commit 1b5b431

File tree

6 files changed

+194
-15
lines changed

6 files changed

+194
-15
lines changed

‎src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryAdder.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,14 +190,15 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
190190

191191
if(versionCacheDir.exists()) {
192192
listener.getLogger().println("Library " + name + "@" + version + " is cached. Copying from home.");
193-
lastReadFile.touch(System.currentTimeMillis());
194193
} else {
195194
listener.getLogger().println("Caching library " + name + "@" + version);
196195
versionCacheDir.mkdirs();
197196
retrieveLockFile.touch(System.currentTimeMillis());
198197
retriever.retrieve(name, version, changelog, versionCacheDir, run, listener);
199198
retrieveLockFile.delete();
200199
}
200+
lastReadFile.touch(System.currentTimeMillis());
201+
versionCacheDir.withSuffix("-name.txt").write(name, "UTF-8");
201202
versionCacheDir.copyRecursiveTo(libDir);
202203
} else {
203204
retriever.retrieve(name, version, changelog, libDir, run, listener);

‎src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,47 @@
88
import java.io.IOException;
99
import java.util.concurrent.TimeUnit;
1010

11+
import jenkins.util.SystemProperties;
12+
1113
@Extension public class LibraryCachingCleanup extends AsyncPeriodicWork {
12-
private final Long recurrencePeriod;
13-
private final Long unreadCacheClearTime;
14+
public static /* non-final for script console */ int EXPIRE_AFTER_READ_DAYS =
15+
SystemProperties.getInteger(LibraryCachingCleanup.class.getName() + ".CACHE_EXPIRY_DAYS", 7);
1416

1517
public LibraryCachingCleanup() {
1618
super("LibraryCachingCleanup");
17-
recurrencePeriod = Long.getLong("jenkins.workflow-libs.cacheCleanupPeriodDays", TimeUnit.DAYS.toMillis(7));
18-
unreadCacheClearTime = Long.getLong("jenkins.workflow-libs.unreadCacheClearDays", TimeUnit.DAYS.toMillis(7));
1919
}
2020

2121
@Override public long getRecurrencePeriod() {
22-
return recurrencePeriod;
22+
return TimeUnit.HOURS.toMillis(12);
2323
}
2424

2525
@Override protected void execute(TaskListener listener) throws IOException, InterruptedException {
2626
FilePath globalCacheDir = LibraryCachingConfiguration.getGlobalLibrariesCacheDir();
27-
for (FilePath library: globalCacheDir.list()) {
28-
for (FilePath version: library.list()) {
29-
final FilePath lastReadFile = new FilePath(version, LibraryCachingConfiguration.LAST_READ_FILE);
30-
if (lastReadFile.exists() && (lastReadFile.lastModified() + unreadCacheClearTime) < System.currentTimeMillis()) {
31-
version.deleteRecursive();
27+
for (FilePath library : globalCacheDir.list()) {
28+
if (!removeIfOldCacheDirectory(library, TimeUnit.DAYS.toMillis(EXPIRE_AFTER_READ_DAYS))) {
29+
// Prior to SECURITY-2586 security fixes, library caches had a two-level directory structure.
30+
// These caches will never be used again, so we delete any that we find.
31+
for (FilePath version: library.list()) {
32+
removeIfOldCacheDirectory(version, 0);
3233
}
3334
}
3435
}
3536
}
37+
38+
/**
39+
* Delete cache directories for the given library if they are outdated.
40+
* @return true if specified directory is a cache directory, regardless of whether it was outdated. Used to detect
41+
* whether the cache was created before or after the fix for SECURITY-2586.
42+
*/
43+
private boolean removeIfOldCacheDirectory(FilePath library, long maxDurationMillis) throws IOException, InterruptedException {
44+
final FilePath lastReadFile = new FilePath(library, LibraryCachingConfiguration.LAST_READ_FILE);
45+
if (lastReadFile.exists()) {
46+
if ((System.currentTimeMillis() - lastReadFile.lastModified()) > maxDurationMillis) {
47+
library.deleteRecursive();
48+
library.withSuffix("-name.txt").delete(); // Harmless if this is a pre-SECURITY-2586 cache directory.
49+
}
50+
return true;
51+
}
52+
return false;
53+
}
3654
}

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@
1111

1212
import java.io.File;
1313
import java.io.IOException;
14+
import java.io.InputStream;
15+
import java.nio.charset.StandardCharsets;
1416
import java.util.Arrays;
1517
import java.util.Collections;
1618
import java.util.List;
19+
import org.apache.commons.io.IOUtils;
1720

1821
public final class LibraryCachingConfiguration extends AbstractDescribableImpl<LibraryCachingConfiguration> {
1922
private int refreshTimeMinutes;
@@ -70,10 +73,21 @@ public static FilePath getGlobalLibrariesCacheDir() {
7073
public FormValidation doClearCache(@QueryParameter String name) throws InterruptedException {
7174
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
7275

73-
FilePath cacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), name);
7476
try {
75-
if (cacheDir.exists()) {
76-
cacheDir.deleteRecursive();
77+
if (LibraryCachingConfiguration.getGlobalLibrariesCacheDir().exists()) {
78+
for (FilePath libraryNamePath : LibraryCachingConfiguration.getGlobalLibrariesCacheDir().list("*-name.txt")) {
79+
// Libraries configured in distinct locations may have the same name. Since only admins are allowed here, this is not a huge issue, but it is probably unexpected.
80+
String cacheName;
81+
try (InputStream stream = libraryNamePath.read()) {
82+
cacheName = IOUtils.toString(stream, StandardCharsets.UTF_8);
83+
}
84+
if (libraryNamePath.readToString().equals(name)) {
85+
FilePath libraryCachePath = LibraryCachingConfiguration.getGlobalLibrariesCacheDir()
86+
.child(libraryNamePath.getName().replace("-name.txt", ""));
87+
libraryCachePath.deleteRecursive();
88+
libraryNamePath.delete();
89+
}
90+
}
7791
}
7892
} catch (IOException ex) {
7993
return FormValidation.error(ex, "The cache dir was not deleted successfully");

‎src/main/resources/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfiguration/config.jelly

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,7 @@ THE SOFTWARE.
3131
<f:entry title="${%Versions to exclude}" field="excludedVersionsStr">
3232
<f:textbox />
3333
</f:entry>
34-
<f:validateButton title="${%Clear cache}" progress="${%Clearing...}" method="clearCache" with="name" />
34+
<j:if test="${h.hasPermission(app.ADMINISTER)}">
35+
<f:validateButton title="${%Clear cache}" progress="${%Clearing...}" method="clearCache" with="name" />
36+
</j:if>
3537
</j:jelly>

‎src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanupTest.java

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
/*
2+
* The MIT License
3+
*
4+
* Copyright 2022 CloudBees, Inc.
5+
*
6+
* Permission is hereby granted, free of charge, to any person obtaining a copy
7+
* of this software and associated documentation files (the "Software"), to deal
8+
* in the Software without restriction, including without limitation the rights
9+
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
* copies of the Software, and to permit persons to whom the Software is
11+
* furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in
14+
* all copies or substantial portions of the Software.
15+
*
16+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
22+
* THE SOFTWARE.
23+
*/
24+
package org.jenkinsci.plugins.workflow.libs;
25+
26+
import hudson.ExtensionList;
27+
import hudson.FilePath;
28+
import hudson.util.StreamTaskListener;
29+
import java.io.File;
30+
import java.time.ZonedDateTime;
31+
import jenkins.plugins.git.GitSCMSource;
32+
import jenkins.plugins.git.GitSampleRepoRule;
33+
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
34+
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
35+
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
36+
import org.junit.Rule;
37+
import org.junit.Test;
38+
import org.jvnet.hudson.test.JenkinsRule;
39+
40+
import static org.hamcrest.MatcherAssert.assertThat;
41+
import static org.hamcrest.Matchers.not;
42+
import static org.hamcrest.io.FileMatchers.anExistingDirectory;
43+
import static org.hamcrest.io.FileMatchers.anExistingFile;
44+
45+
public class LibraryCachingCleanupTest {
46+
47+
@Rule
48+
public JenkinsRule r = new JenkinsRule();
49+
@Rule
50+
public GitSampleRepoRule sampleRepo = new GitSampleRepoRule();
51+
52+
@Test
53+
public void smokes() throws Throwable {
54+
sampleRepo.init();
55+
sampleRepo.write("vars/foo.groovy", "def call() { echo 'foo' }");
56+
sampleRepo.git("add", "vars");
57+
sampleRepo.git("commit", "--message=init");
58+
LibraryConfiguration config = new LibraryConfiguration("library",
59+
new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true)));
60+
config.setDefaultVersion("master");
61+
config.setImplicit(true);
62+
config.setCachingConfiguration(new LibraryCachingConfiguration(30, null));
63+
GlobalLibraries.get().getLibraries().add(config);
64+
// Run build and check that cache gets created.
65+
WorkflowJob p = r.createProject(WorkflowJob.class);
66+
p.setDefinition(new CpsFlowDefinition("foo()", true));
67+
WorkflowRun b = r.buildAndAssertSuccess(p);
68+
LibrariesAction action = b.getAction(LibrariesAction.class);
69+
LibraryRecord record = action.getLibraries().get(0);
70+
FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName());
71+
assertThat(new File(cache.getRemote()), anExistingDirectory());
72+
// Run LibraryCachingCleanup and show that cache is not deleted.
73+
ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr());
74+
assertThat(new File(cache.getRemote()), anExistingDirectory());
75+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
76+
// Run LibraryCachingCleanup after modifying LAST_READ_FILE to be an old date and and show that cache is deleted.
77+
long oldMillis = ZonedDateTime.now().minusDays(LibraryCachingCleanup.EXPIRE_AFTER_READ_DAYS + 1).toInstant().toEpochMilli();
78+
cache.child(LibraryCachingConfiguration.LAST_READ_FILE).touch(oldMillis);
79+
ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr());
80+
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
81+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingDirectory()));
82+
}
83+
84+
@Test
85+
public void preSecurity2586() throws Throwable {
86+
FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child("name").child("version");
87+
cache.mkdirs();
88+
cache.child(LibraryCachingConfiguration.LAST_READ_FILE).touch(System.currentTimeMillis());
89+
ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr());
90+
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
91+
}
92+
93+
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,34 @@
2424

2525
package org.jenkinsci.plugins.workflow.libs;
2626

27+
import hudson.ExtensionList;
28+
import hudson.FilePath;
29+
import java.io.File;
30+
import jenkins.plugins.git.GitSCMSource;
31+
import jenkins.plugins.git.GitSampleRepoRule;
32+
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
33+
import org.jenkinsci.plugins.workflow.job.WorkflowJob;
34+
import org.jenkinsci.plugins.workflow.job.WorkflowRun;
2735
import org.junit.Before;
36+
import org.junit.Rule;
2837
import org.junit.Test;
2938
import org.jvnet.hudson.test.Issue;
39+
import org.jvnet.hudson.test.JenkinsRule;
40+
import org.jvnet.hudson.test.WithoutJenkins;
3041

3142
import static org.hamcrest.MatcherAssert.*;
3243
import static org.hamcrest.Matchers.*;
44+
import static org.hamcrest.io.FileMatchers.anExistingDirectory;
45+
import static org.hamcrest.io.FileMatchers.anExistingFile;
3346
import static org.junit.Assert.assertFalse;
3447
import static org.junit.Assert.assertTrue;
3548

3649
public class LibraryCachingConfigurationTest {
50+
@Rule
51+
public JenkinsRule r = new JenkinsRule();
52+
@Rule
53+
public GitSampleRepoRule sampleRepo = new GitSampleRepoRule();
54+
3755
private LibraryCachingConfiguration nullVersionConfig;
3856
private LibraryCachingConfiguration oneVersionConfig;
3957
private LibraryCachingConfiguration multiVersionConfig;
@@ -64,36 +82,42 @@ public void createCachingConfiguration() {
6482

6583
@Issue("JENKINS-66045") // NPE getting excluded versions
6684
@Test
85+
@WithoutJenkins
6786
public void npeGetExcludedVersions() {
6887
assertFalse(nullVersionConfig.isExcluded(NEVER_EXCLUDED_VERSION));
6988
}
7089

7190
@Test
91+
@WithoutJenkins
7292
public void getRefreshTimeMinutes() {
7393
assertThat(nullVersionConfig.getRefreshTimeMinutes(), is(REFRESH_TIME_MINUTES));
7494
assertThat(oneVersionConfig.getRefreshTimeMinutes(), is(NO_REFRESH_TIME_MINUTES));
7595
}
7696

7797
@Test
98+
@WithoutJenkins
7899
public void getRefreshTimeMilliseconds() {
79100
assertThat(nullVersionConfig.getRefreshTimeMilliseconds(), is(60 * 1000L * REFRESH_TIME_MINUTES));
80101
assertThat(oneVersionConfig.getRefreshTimeMilliseconds(), is(60 * 1000L * NO_REFRESH_TIME_MINUTES));
81102
}
82103

83104
@Test
105+
@WithoutJenkins
84106
public void isRefreshEnabled() {
85107
assertTrue(nullVersionConfig.isRefreshEnabled());
86108
assertFalse(oneVersionConfig.isRefreshEnabled());
87109
}
88110

89111
@Test
112+
@WithoutJenkins
90113
public void getExcludedVersionsStr() {
91114
assertThat(nullVersionConfig.getExcludedVersionsStr(), is(NULL_EXCLUDED_VERSION));
92115
assertThat(oneVersionConfig.getExcludedVersionsStr(), is(ONE_EXCLUDED_VERSION));
93116
assertThat(multiVersionConfig.getExcludedVersionsStr(), is(MULTIPLE_EXCLUDED_VERSIONS));
94117
}
95118

96119
@Test
120+
@WithoutJenkins
97121
public void isExcluded() {
98122
assertFalse(nullVersionConfig.isExcluded(NULL_EXCLUDED_VERSION));
99123
assertFalse(nullVersionConfig.isExcluded(""));
@@ -117,4 +141,31 @@ public void isExcluded() {
117141
assertFalse(multiVersionConfig.isExcluded(null));
118142
}
119143

144+
@Test
145+
public void clearCache() throws Exception {
146+
sampleRepo.init();
147+
sampleRepo.write("vars/foo.groovy", "def call() { echo 'foo' }");
148+
sampleRepo.git("add", "vars");
149+
sampleRepo.git("commit", "--message=init");
150+
LibraryConfiguration config = new LibraryConfiguration("library",
151+
new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true)));
152+
config.setDefaultVersion("master");
153+
config.setImplicit(true);
154+
config.setCachingConfiguration(new LibraryCachingConfiguration(30, null));
155+
GlobalLibraries.get().getLibraries().add(config);
156+
// Run build and check that cache gets created.
157+
WorkflowJob p = r.createProject(WorkflowJob.class);
158+
p.setDefinition(new CpsFlowDefinition("foo()", true));
159+
WorkflowRun b = r.buildAndAssertSuccess(p);
160+
LibrariesAction action = b.getAction(LibrariesAction.class);
161+
LibraryRecord record = action.getLibraries().get(0);
162+
FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName());
163+
assertThat(new File(cache.getRemote()), anExistingDirectory());
164+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
165+
// Clear the cache. TODO: Would be more realistic to set up security and use WebClient.
166+
ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library");
167+
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
168+
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
169+
}
170+
120171
}

0 commit comments

Comments
 (0)