Skip to content

Commit 8255ec7

Browse files
committed
refactor: pin extracted-tree hash instead of archive hash
Rework the plugin lockfile to hash the extracted $id-$version/ directory (the code Nextflow actually loads and executes) rather than the downloaded archive. Hashing the unpacked tree lets a single, content-addressed check cover both integrity surfaces: - supply chain / registry compromise / silent drift (a tampered archive extracts to a different tree), and - cache poisoning of the executed code (a lower-trust user editing already -extracted files on a shared cache) — detected regardless of directory ownership or permissions, with no ownership heuristic and therefore no false-positive warnings on legitimate shared caches. This supersedes the archive-hash approach and the directory-ownership guard proposed in #7308: it is stronger (catches modifications an ownership check misses) and quieter (silent on any healthy cache; output only on a genuine mismatch). Verification runs once per plugin in load0(), before loading, on both cold and warm caches. The retained-archive machinery and the "archive absent" case are removed (the extracted tree is always present when a plugin loads). Full re-hash on every load is acceptable for now; the ADR documents a local fingerprint-cache optimisation (gated on a private, non-writable cache) as a measure-first follow-up for large plugins. Assisted-by: Claude Opus 4.8 (via Claude Code) Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
1 parent 51a8232 commit 8255ec7

6 files changed

Lines changed: 213 additions & 262 deletions

File tree

adr/20260727-plugin-lockfile-integrity.md

Lines changed: 77 additions & 114 deletions
Large diffs are not rendered by default.

docs/plugins/using-plugins.mdx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,21 +64,21 @@ The plugin cache is shared across pipelines and is not access-controlled. On mul
6464

6565
<AddedInVersion version="26.07" />
6666

67-
A `plugins.lock` file pins the exact plugin artifacts a pipeline expects. For each plugin it records the `sha512` checksum of the plugin archive, keyed by `id@version`. The file is meant to be committed to the pipeline repository so that everyone running the pipeline resolves the same plugin artifacts.
67+
A `plugins.lock` file pins the exact plugin code a pipeline expects. For each plugin it records a `sha512` hash of the **extracted plugin directory** — the code Nextflow actually loads and runs — keyed by `id@version`. The file is meant to be committed to the pipeline repository so that everyone running the pipeline executes the same plugin code.
6868

6969
The lockfile is populated automatically, like `go.sum` or `package-lock.json` — there is no separate command. To enable it, create an empty file in the pipeline directory and run the pipeline once:
7070

7171
```bash
7272
touch plugins.lock
7373
```
7474

75-
The first time each plugin is downloaded, its archive checksum is added to `plugins.lock`. Review the resulting file and commit it. On subsequent runs Nextflow verifies each plugin against the committed checksum. When no `plugins.lock` file is present, the feature is dormant and has no effect.
75+
The first time each plugin is loaded, its hash is added to `plugins.lock`. Review the resulting file and commit it. On subsequent runs Nextflow re-hashes each plugin's extracted directory and verifies it against the committed hash. When no `plugins.lock` file is present, the feature is dormant and has no effect.
7676

77-
Verification is fully offline: Nextflow re-computes the checksum of the plugin archive from a copy retained in the local cache and compares it to the lock entry, without contacting the plugin registry. An existing entry is never rewritten automatically — if a plugin archive legitimately changes, delete its entry and run again to re-pin it.
77+
Verification is fully offline — it re-hashes the files already in the local cache and never contacts the plugin registry. Because it hashes the extracted code rather than the download, it detects both a tampered or compromised download and a plugin directory that was modified after extraction (for example by another user on a shared cache), independently of file ownership or permissions. An existing entry is never rewritten automatically — if a plugin legitimately changes, delete its entry and run again to re-pin it.
7878

79-
Use [`NXF_PLUGINS_LOCK_MODE`][using-plugins-env-vars] to control what happens on a checksum mismatch: `warn` (default) logs a warning and continues, `strict` aborts the run, and `off` skips verification. A plugin whose retained archive is missing (for example, a cache populated before this feature existed) cannot be verified offline; it is reported but never aborts the run, and is never re-downloaded just to verify it.
79+
Use [`NXF_PLUGINS_LOCK_MODE`][using-plugins-env-vars] to control what happens on a mismatch: `warn` (default) logs a warning and continues, `strict` aborts the run, and `off` skips verification.
8080

81-
The lockfile complements, but does not replace, the private-cache guidance above: the cache isolation prevents untrusted artifacts from being loaded, while the lockfile ensures the artifacts that are loaded match what the pipeline pinned.
81+
The lockfile complements the private-cache guidance above: keeping the cache private prevents untrusted code from being written in the first place, while the lockfile detects any change to the plugin code that is actually loaded.
8282

8383
## Offline usage
8484

docs/reference/env-vars.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ The path where the plugin archives are loaded and stored (default: `$NXF_HOME/pl
214214

215215
<AddedInVersion version="26.07" />
216216

217-
Controls how Nextflow reacts when a downloaded plugin artifact does not match the entry recorded in the `plugins.lock` file: `warn` logs a warning once per plugin and continues, `strict` aborts the run, and `off` skips verification silently (default: `warn`). Verification is dormant when no `plugins.lock` file is present.
217+
Controls how Nextflow reacts when a plugin's extracted directory does not match the hash recorded in the `plugins.lock` file: `warn` logs a warning once per plugin and continues, `strict` aborts the run, and `off` skips verification silently (default: `warn`). Verification is dormant when no `plugins.lock` file is present.
218218

219219
##### `NXF_PLUGINS_REGISTRY_URL`
220220

modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy

Lines changed: 55 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616

1717
package nextflow.plugin
1818

19+
import java.nio.file.FileVisitResult
1920
import java.nio.file.Files
2021
import java.nio.file.Path
22+
import java.nio.file.SimpleFileVisitor
23+
import java.nio.file.attribute.BasicFileAttributes
2124
import java.security.MessageDigest
2225

2326
import groovy.transform.CompileStatic
@@ -26,30 +29,26 @@ import nextflow.SysEnv
2629
import nextflow.exception.AbortOperationException
2730

2831
/**
29-
* Verifies plugin archives against the {@code plugins.lock} file.
32+
* Verifies an extracted plugin directory against the {@code plugins.lock} file.
3033
*
3134
* The feature is opt-in <em>by the presence of the lock file</em>: it is dormant (a no-op) when no
32-
* {@code plugins.lock} exists. When the file is present, the sha512 of the retained plugin archive
33-
* is re-computed locally (no network) and compared to the committed lock entry.
35+
* {@code plugins.lock} exists. When the file is present, a canonical hash of the extracted
36+
* {@code $id-$version/} directory - the code that Nextflow actually loads and executes - is
37+
* re-computed locally (no network) and compared to the committed lock entry. Because it hashes the
38+
* unpacked tree rather than the archive, it detects both a tampered/compromised download and a
39+
* poisoned cache directory (a lower-trust user editing already-extracted files on a shared cache).
3440
*
35-
* Following the {@code go.sum} / {@code package-lock.json} model, the lock is populated
36-
* automatically: the first time a coordinate is downloaded and it is missing from the lock, its
37-
* archive checksum is appended (trust-on-first-use). An existing entry is never rewritten silently
38-
* — a mismatch against a committed entry is a verification failure.
39-
*
40-
* The behaviour on a checksum mismatch is gated by the {@code NXF_PLUGINS_LOCK_MODE} environment
41-
* variable:
41+
* Following the {@code go.sum} / {@code package-lock.json} model the lock is populated
42+
* automatically: the first time a coordinate is seen and it is missing from the lock, its tree
43+
* hash is appended (trust-on-first-use). An existing entry is never rewritten silently — a
44+
* mismatch against a committed entry is a verification failure, gated by
45+
* {@code NXF_PLUGINS_LOCK_MODE}:
4246
* <ul>
4347
* <li>{@code strict} - abort with an {@link AbortOperationException}</li>
4448
* <li>{@code warn} (default) - log a warning once per coordinate and proceed</li>
4549
* <li>{@code off} - skip verification silently</li>
4650
* </ul>
4751
*
48-
* A locked plugin whose archive is not available locally (e.g. a cache extracted before this
49-
* feature existed) is never aborted: it cannot be verified offline and its archive is never
50-
* re-downloaded just to verify it. Integrity of the extracted code that actually runs is the
51-
* responsibility of the plugin directory guard, not of this archive check.
52-
*
5352
* @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
5453
*/
5554
@Slf4j
@@ -103,46 +102,39 @@ class PluginLockVerifier {
103102
}
104103

105104
/**
106-
* Verify - and, on first sight, pin - a plugin archive against the lock.
105+
* Verify - and, on first sight, pin - an extracted plugin directory against the lock.
107106
*
108107
* @param fqid The plugin fully-qualified id ie. {@code id@version}
109-
* @param zip The retained plugin archive; may be {@code null} or missing
108+
* @param pluginDir The extracted plugin directory ({@code $id-$version/})
110109
*/
111-
void verify(String fqid, Path zip) {
110+
void verify(String fqid, Path pluginDir) {
112111
if( !enabled )
113112
return
114-
final entry = lock.getEntry(fqid)
115-
final present = zip != null && Files.exists(zip)
116-
117-
// coordinate not yet locked: pin it on first download (trust-on-first-use), never fail
118-
if( entry == null ) {
119-
if( present )
120-
pin(fqid, sha512(zip))
121-
else
122-
log.debug "Plugin '$fqid' is not in the plugins lock file and its archive is not available to pin"
113+
if( pluginDir == null || !Files.isDirectory(pluginDir) ) {
114+
log.debug "Cannot verify plugin '$fqid' against the plugins lock file - directory not available: $pluginDir"
123115
return
124116
}
125117

126-
// locked, but the archive is not available (e.g. a cache created before this feature):
127-
// it cannot be verified offline - never abort and never re-download to verify
128-
if( !present ) {
129-
if( getMode() != Mode.OFF && notified.add(fqid) )
130-
log.warn "Cannot verify plugin '$fqid' against the plugins lock file - its archive is not available in the cache"
118+
final actual = sha512Tree(pluginDir)
119+
final entry = lock.getEntry(fqid)
120+
121+
// coordinate not yet locked: pin it on first sight (trust-on-first-use), never fail
122+
if( entry == null ) {
123+
pin(fqid, actual)
131124
return
132125
}
133-
134-
// verify the retained archive against the committed checksum
135-
final actual = sha512(zip)
126+
// matches the committed hash
136127
if( actual == entry.sha512 )
137128
return
138129

139-
final reason = "Plugin '$fqid' checksum does not match the plugins lock file\n- expected: ${entry.sha512}\n- actual : ${actual}"
130+
// mismatch: the extracted plugin differs from what the lock pinned
131+
final reason = "Plugin '$fqid' does not match the plugins lock file\n- expected: ${entry.sha512}\n- actual : ${actual}"
140132
switch( getMode() ) {
141133
case Mode.OFF:
142134
log.debug "Plugins lock verification failed (ignored, mode=off) - $reason"
143135
break
144136
case Mode.STRICT:
145-
throw new AbortOperationException("$reason\n- delete the entry from the plugins lock file and re-run to re-pin, or restore the expected plugin archive")
137+
throw new AbortOperationException("$reason\n- delete the entry from the plugins lock file and re-run to re-pin, or restore the expected plugin")
146138
case Mode.WARN:
147139
// warn only once per coordinate to avoid log spam
148140
if( notified.add(fqid) )
@@ -154,7 +146,7 @@ class PluginLockVerifier {
154146
/**
155147
* Append a new entry to the lock and persist it (trust-on-first-use). Existing entries are
156148
* never overwritten by this path — {@link #verify} routes an already-locked coordinate through
157-
* the checksum comparison instead.
149+
* the hash comparison instead.
158150
*/
159151
private synchronized void pin(String fqid, String sha512) {
160152
lock.addEntry(fqid, new PluginLockFile.Entry(sha512))
@@ -164,18 +156,35 @@ class PluginLockVerifier {
164156
}
165157

166158
/**
167-
* Compute the sha512 hex digest of the given file.
159+
* Compute a canonical sha512 digest over the content of a directory tree. The digest covers,
160+
* for every regular file (visited in sorted relative-path order for determinism), its relative
161+
* path and its bytes - not timestamps or permissions - so it is stable across extractions and
162+
* platforms while still detecting any change to the files that will be executed.
168163
*
169-
* @param file The file to hash
164+
* @param dir The directory to hash
170165
* @return The lowercase hex-encoded sha512 digest (128 chars)
171166
*/
172-
static String sha512(Path file) {
167+
static String sha512Tree(Path dir) {
173168
final md = MessageDigest.getInstance('SHA-512')
174-
try (InputStream is = Files.newInputStream(file)) {
175-
final buffer = new byte[8192]
176-
int read
177-
while( (read = is.read(buffer)) != -1 )
178-
md.update(buffer, 0, read)
169+
// TreeMap keyed by relative path -> deterministic, sorted iteration order
170+
final files = new TreeMap<String, Path>()
171+
Files.walkFileTree(dir, new SimpleFileVisitor<Path>() {
172+
@Override
173+
FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
174+
files.put(dir.relativize(file).toString().replace('\\', '/'), file)
175+
return FileVisitResult.CONTINUE
176+
}
177+
})
178+
final buffer = new byte[8192]
179+
for( Map.Entry<String, Path> it : files.entrySet() ) {
180+
md.update(it.key.getBytes('UTF-8'))
181+
md.update((byte) 0)
182+
try (InputStream is = Files.newInputStream(it.value)) {
183+
int read
184+
while( (read = is.read(buffer)) != -1 )
185+
md.update(buffer, 0, read)
186+
}
187+
md.update((byte) 0)
179188
}
180189
return HexFormat.of().formatHex(md.digest())
181190
}

modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ class PluginUpdater extends UpdateManager {
253253
}
254254

255255
/**
256-
* The {@code plugins.lock} file location used to verify downloaded plugin artifacts.
256+
* The {@code plugins.lock} file location used to verify plugins against their pinned hash.
257257
* Defaults to a {@code plugins.lock} file in the current working directory.
258258
*/
259259
protected Path lockFilePath() {
@@ -269,13 +269,6 @@ class PluginUpdater extends UpdateManager {
269269
return lockVerifier
270270
}
271271

272-
/**
273-
* @return the path to the retained plugin zip artifact used for lock verification
274-
*/
275-
private Path retainedZip(String id, String version) {
276-
return pluginsStore.resolve("${id}-${version}.zip")
277-
}
278-
279272
private Path download0(String id, String version) {
280273
// 0. check if version is specified
281274
if( !version )
@@ -291,21 +284,9 @@ class PluginUpdater extends UpdateManager {
291284
// 2. download to temporary location
292285
Path downloaded = safeDownloadPlugin(id, version);
293286

294-
// 3. unzip the content
287+
// 3. unzip the content and delete downloaded file
295288
Path dir = FileUtils.expandIfZip(downloaded)
296-
297-
// 3.1 when the plugins lock is enabled retain the downloaded zip next to the extracted
298-
// dir and verify (or pin) its checksum against the lock (cold cache). Otherwise delete it.
299-
if( getLockVerifier().isEnabled() ) {
300-
final retained = retainedZip(id, version)
301-
FileHelper.deletePath(retained)
302-
FileHelper.copyPath(downloaded, retained)
303-
FileHelper.deletePath(downloaded)
304-
getLockVerifier().verify("${id}@${version}", retained)
305-
}
306-
else {
307-
FileHelper.deletePath(downloaded)
308-
}
289+
FileHelper.deletePath(downloaded)
309290

310291
// 4. move the final destination the plugin directory
311292
assert pluginPath.getFileName() == dir.getFileName()
@@ -434,17 +415,16 @@ class PluginUpdater extends UpdateManager {
434415
if( !FilesEx.exists(pluginPath) ) {
435416
pluginPath = safeDownload(id, version)
436417
}
437-
else {
438-
// warm cache: re-hash the retained artifact and verify it against the plugins lock
439-
// (missing retained zip is treated as a lock-miss by the verifier)
440-
getLockVerifier().verify("${id}@${version}", retainedZip(id, version))
441-
}
442418

443419
// verify the plugin install path contains the expected manifest path
444420
if( !FilesEx.exists(pluginPath.resolve('classes/META-INF/MANIFEST.MF')) ) {
445421
log.warn("Plugin '${pluginPath.getFileName()}' installation looks corrupted - Delete the following directory and run nextflow again: $pluginPath")
446422
}
447423

424+
// verify (or pin) the extracted plugin directory against the plugins lock, before loading
425+
// and executing its code. This covers both a fresh download and a reused (warm) cache.
426+
getLockVerifier().verify("${id}@${version}", pluginPath)
427+
448428
// load the plugin from the file system
449429
PluginWrapper wrapper = pluginManager.loadPluginFromPath(pluginPath)
450430

0 commit comments

Comments
 (0)