Fix library cache deletion after SECURITY-2586 #148
Conversation
| } else { | ||
| listener.getLogger().println("Caching library " + name + "@" + version); | ||
| versionCacheDir.mkdirs(); | ||
| retrieveLockFile.touch(System.currentTimeMillis()); | ||
| retriever.retrieve(name, version, changelog, versionCacheDir, run, listener); | ||
| retrieveLockFile.delete(); | ||
| } | ||
| lastReadFile.touch(System.currentTimeMillis()); |
There was a problem hiding this comment.
Now we write this even when creating the cache directory for the first time, so that it will eventually be deleted even if it was only used once.
| } else { | ||
| listener.getLogger().println("Caching library " + name + "@" + version); | ||
| versionCacheDir.mkdirs(); | ||
| retrieveLockFile.touch(System.currentTimeMillis()); | ||
| retriever.retrieve(name, version, changelog, versionCacheDir, run, listener); | ||
| retrieveLockFile.delete(); | ||
| } | ||
| lastReadFile.touch(System.currentTimeMillis()); | ||
| versionCacheDir.withSuffix("-name.txt").write(name, "UTF-8"); |
There was a problem hiding this comment.
We write this every time rather than just when the cache is first created to handle caches created after the security fix but before this PR.
src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingCleanup.java
Outdated
Show resolved
Hide resolved
| recurrencePeriod = Long.getLong("jenkins.workflow-libs.cacheCleanupPeriodDays", TimeUnit.DAYS.toMillis(7)); | ||
| unreadCacheClearTime = Long.getLong("jenkins.workflow-libs.unreadCacheClearDays", TimeUnit.DAYS.toMillis(7)); |
There was a problem hiding this comment.
These did not follow standard Jenkins system property naming conventions, and I did not see any documentation related to them, so I created a new EXPIRE_AFTER_READ_DAYS property that follows the normal Jenkins style.
| } | ||
|
|
||
| @Override public long getRecurrencePeriod() { | ||
| return recurrencePeriod; | ||
| return TimeUnit.HOURS.toMillis(12); |
There was a problem hiding this comment.
Previously this was 7 days, which combined with unreadCacheClearTime being 7 days meant that in the worst case a library could go almost 2 weeks without being cleaned up, and if Jenkins restarts in that time frame, the timer would start all over again.
Since the check is not that expensive, I think it is fine to run it twice a day.
| <j:if test="${h.hasPermission(app.ADMINISTER)}"> | ||
| <f:validateButton title="${%Clear cache}" progress="${%Clearing...}" method="clearCache" with="name" /> | ||
| </j:if> |
There was a problem hiding this comment.
The backend has always required ADMINISTER permission, but we used to show this button on folder-level libraries to anyone with Job/Configure permission, which is just confusing.
|
Any idea when this will get merged? |
The security fixes related to the cache feature in ace0de3 broke
LibraryCachingCleanup(cleans up unused cache directories) andLibraryCachingConfiguration.doClearCache(allows admins to manually delete all cached versions of a specific library). This went unnoticed because neither piece of functionality had any test coverage.This PR fixes the issues with library cache deletion after SECURITY-2586 and adds relevant test coverage.