From 51a82323f4c023f87033fabb7abf06816718950f Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Mon, 27 Jul 2026 21:58:25 +0200 Subject: [PATCH 1/2] feat: add plugin lockfile integrity (plugins.lock) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an opt-in, offline-verifiable integrity check for the local plugin cache, complementing the extracted-directory guard proposed in #7308. A committed plugins.lock file pins the sha512 hash of each plugin archive, computed over the downloaded bytes and keyed by id@version. Following the go.sum / package-lock.json model, the file is populated automatically on first download (trust-on-first-use) — there is no dedicated command; enable it by creating an empty plugins.lock and running the pipeline once. An existing entry is never rewritten silently: a downloaded archive whose hash differs from a committed entry is a verification failure, gated by NXF_PLUGINS_LOCK_MODE (warn default, strict, off). Verification is network-free: it re-hashes the archive retained alongside the extracted plugin and never re-fetches from the registry as a remedy. A locked plugin whose retained archive is unavailable (e.g. a cache created before this feature) is reported but never aborts the run — integrity of the extracted code that executes remains the job of the #7308 directory guard. The trust anchor lives in the pipeline repo under version control, so it survives registry compromise and silent drift with no runtime dependency on the registry. See adr/20260727-plugin-lockfile-integrity.md. Assisted-by: Claude Opus 4.8 (via Claude Code) Signed-off-by: Paolo Di Tommaso --- adr/20260727-plugin-lockfile-integrity.md | 210 +++++++++++++ docs/plugins/using-plugins.mdx | 25 ++ docs/reference/env-vars.mdx | 6 + .../nextflow/plugin/PluginLockFile.groovy | 177 +++++++++++ .../nextflow/plugin/PluginLockVerifier.groovy | 182 ++++++++++++ .../main/nextflow/plugin/PluginUpdater.groovy | 48 ++- .../nextflow/plugin/PluginLockFileTest.groovy | 151 ++++++++++ .../plugin/PluginLockVerifierTest.groovy | 275 ++++++++++++++++++ 8 files changed, 1072 insertions(+), 2 deletions(-) create mode 100644 adr/20260727-plugin-lockfile-integrity.md create mode 100644 modules/nf-commons/src/main/nextflow/plugin/PluginLockFile.groovy create mode 100644 modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy create mode 100644 modules/nf-commons/src/test/nextflow/plugin/PluginLockFileTest.groovy create mode 100644 modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy diff --git a/adr/20260727-plugin-lockfile-integrity.md b/adr/20260727-plugin-lockfile-integrity.md new file mode 100644 index 0000000000..9d045018c5 --- /dev/null +++ b/adr/20260727-plugin-lockfile-integrity.md @@ -0,0 +1,210 @@ +# Plugin Lockfile Integrity + +- Authors: Claude +- Status: draft +- Date: 2026-07-27 +- Tags: plugins, security, registry, integrity + +## Summary + +Introduce a committed lockfile (`plugins.lock`) that pins the SHA-512 hash of each resolved plugin **archive**, computed over the actual archive bytes. The lockfile is populated automatically as a side effect of the normal plugin download — the first time a coordinate is fetched it is pinned (trust-on-first-use) — exactly like `go.sum` / `package-lock.json`, with **no dedicated command**. The trusted hash lives in the pipeline repository under version control and is not re-fetched from the registry on every run. During plugin resolution Nextflow re-hashes the downloaded — or retained — archive and verifies it against the locked hash. + +Verification is network-free **whenever the archive being verified is present locally** (always true on a cold download; true on a warm cache only if the archive was retained). This ADR is explicit about the one case where an archive is not present — a cache extracted before this feature existed — and specifies an offline-safe behavior for it that never triggers a download (see *Migration and first adoption* and *Verification flow*). + +## Problem Statement + +Nextflow resolves plugins from a registry (or mirror), downloads a zip archive, and extracts it into the local plugin cache under `$id-$version/`. Today there is no persistent, user-owned record of what a plugin archive is *supposed* to hash to. The registry returns a `sha512sum` during resolution, but that value is fetched fresh on every run and is only ever compared against the download in transit — so it can detect corruption on the wire, never a compromise of the source of truth itself. + +**What is already covered today.** In-transit archive integrity is *not* an open gap: `HttpPluginRepository` wires pf4j's `CompoundVerifier` (`HttpPluginRepository.groovy:134-135`), which checks the downloaded archive against the registry-supplied `sha512sum` (`:222`) at download time. A corrupted or MITM-mangled download that no longer matches the registry's own hash is already rejected. The lockfile does **not** claim novelty there. + +The genuine, non-redundant gaps the lockfile closes are: + +- **Runtime trust in the registry's own hash**: today the *expected* value is whatever the registry asserts on this run. A compromised registry (or mirror) that serves a tampered archive together with a matching tampered hash passes the existing `CompoundVerifier` check — because both sides of the comparison come from the same untrusted source. The lockfile removes the registry from the trust path at run time. +- **Silent version drift**: a coordinate that resolves to a different artifact than it did last week produces no signal today. +- **Reproducibility / committed baseline for teams that vendor or mirror plugins**: there is no local, authoritative, VCS-reviewed record of what each coordinate must hash to, independent of the registry. + +The core issue is that the *trusted* hash must be owned by the pipeline and committed to version control, so verification depends on the repository — which the team controls and reviews — rather than on the registry being honest at run time. + +## Goals or Decision Drivers + +- **Local, network-free verification when the archive is present**: once the lockfile exists, checking a locally-present archive against the locked hash requires no network access. Network is used only to *fetch* a plugin that is not yet cached, exactly as today. This ADR does **not** claim verification is unconditionally offline — see the migration case below. +- **Content-addressed trust anchor**: the pinned hash is computed over the downloaded archive bytes at first download, so the baseline is a genuine content hash (the `go.sum` / `package-lock.json` model), not a re-recording of a registry claim. +- **User-owned trust anchor**: the authoritative hash is committed to the pipeline repository and reviewed through normal VCS workflows (pull requests, code review, blame). +- **Reproducibility**: a given commit of a pipeline resolves to exactly the same plugin archives, or fails loudly. +- **Zero breakage by default**: pipelines without a lockfile behave exactly as they do today. The feature is opt-in by the mere presence of the file, and its default enforcement level mirrors PR #7308's warn-first rollout philosophy (see *Modes and rollout*). +- **Never turn verification into a network dependency**: the verification path must never re-fetch from the registry as a remedy — that would re-introduce the exact runtime dependency this design rejects in Option A. +- **Honest scope**: the mechanism must defend what it can actually defend (the archive, and only as a re-extraction gate on a warm cache) and not overclaim protection it does not provide (the extracted tree that actually runs). + +## Non-goals + +- **Extracted-tree hashing**: hashing or re-verifying the contents of the extracted `$id-$version/` directory is out of scope. That surface is covered by the `PluginSecurity` directory guard in PR #7308 (see *Threat model* and *Known residual* below). +- **Transitive / dependency lock resolution**: only the coordinates declared in the pipeline config are locked. There is no dependency-graph resolution or locking of plugins pulled in indirectly. +- **Registry-server changes**: this ADR is entirely client-side. The registry contract is unchanged. +- **Signature or provenance verification**: the lockfile pins a hash, not a cryptographic signature or attestation. Provenance metadata (the `url`) is recorded for auditability only. +- **Catching in-transit download corruption**: already handled by pf4j's `CompoundVerifier` against the registry-supplied `sha512sum`. Not a job the lockfile duplicates. + +## Considered Options + +### Option A: Load-time checksum comparison against the registry + +Fetch the plugin, then re-query the registry for the expected `sha512sum` at load time and compare. No committed lockfile. + +- Good, because there is nothing new to commit or maintain in the pipeline repo. +- Bad, because it makes the **registry a runtime dependency** of every plugin load — breaking air-gapped and offline execution, the exact environments where integrity matters most. +- Bad, because the registry becomes both the source of the artifact *and* the source of the trusted hash: a compromised registry can serve a tampered archive with a matching tampered hash and defeat the check entirely (this is exactly the `CompoundVerifier` behavior that already exists today). +- Bad, because it provides no reproducibility anchor — the "expected" value can change out from under a pipeline between runs with no committed record. + +### Option B: Extracted directory tree-hash + +Compute a hash over the extracted `$id-$version/` directory tree and pin that. + +- Good, because it would detect edits to already-extracted files (cache poisoning of the unpacked plugin) — the surface the archive hash cannot see. +- Bad, because tree hashing is heavier: it must walk and hash the full extracted tree, and is sensitive to extraction non-determinism (file ordering, timestamps, permissions, symlinks) across platforms and unzip implementations. +- Bad, because it duplicates the protection the `PluginSecurity` directory guard in PR #7308 already provides for the extracted tree. +- Bad, because it would tempt an implementation to re-extract and re-hash on every run, adding cost to the warm-cache hot path. +- Deferred: the extracted-tree surface is real and is what actually executes, but it is owned by the #7308 guard, not by the lockfile. (Note: a variant of Option B — deriving the lock hash from the extracted tree — is the only way to make verification offline on a pre-existing cache with no retained archive; that is called out where relevant below but remains deferred.) + +### Option C: Committed archive-hash lockfile with download-gate and retain-and-re-verify (adopted) + +Commit a `plugins.lock` file pinning the SHA-512 of each resolved plugin **archive**, hashed from the archive bytes at generation time. On a cold cache, gate the download against the locked hash. Retain the verified archive in the cache and re-verify it on warm-cache runs. + +- Good, because the trusted hash is owned by the pipeline and committed to VCS — the registry is not trusted at verification time. +- Good, because the pinned hash is content-addressed (computed over the bytes), so it survives registry compromise on all runs *after* the lock was honestly generated (trust-on-first-use). +- Good, because it adds a reproducibility / drift anchor the registry cannot silently move. +- Good, because verification of a locally-present archive is network-free. +- Good, because archive hashing is deterministic and cheap (one hash of one zip), with no extraction-ordering pitfalls. +- Bad (accepted), because it requires **retaining** the archive in the cache — a new persisted artifact with size and lifecycle cost (see *Tradeoff: retaining the archive*), and one an attacker can simply ignore. +- Bad (accepted), because on a warm cache the thing that actually executes is the extracted tree, not the retained zip — so re-hashing the zip protects only a *future* re-extraction, not the current run (see *Threat model* and *Known residual*). Warm-run integrity of the executed code rests on the #7308 guard. +- Bad (accepted), because a cache extracted *before* this feature has no retained archive, so its archive cannot be verified offline; the design degrades safely rather than re-downloading (see *Migration and first adoption*). + +## Solution or decision outcome + +**Option C — committed archive-hash lockfile with download-gate and retain-and-re-verify** — is the recommended approach. It places a content-addressed trust anchor in the pipeline repository, keeps verification network-free whenever the archive is locally present, never re-fetches from the registry as a verification remedy, and composes cleanly with the extracted-directory guard from PR #7308 rather than duplicating it. + +## Rationale & discussion + +### Threat model + +The lockfile's genuine contribution (beyond what pf4j's `CompoundVerifier` already does at download time) is: + +- **Removing runtime trust in the registry-supplied hash** — a compromised registry/mirror that serves a tampered archive with a matching tampered hash passes today's `CompoundVerifier`, but fails against the committed content hash. This holds on every run after the lock was honestly generated. +- **Silent version drift** — a coordinate resolving to a different artifact fails against the committed hash. +- **A committed reproducibility baseline** for teams that vendor or mirror plugins. + +What the lockfile does **not** defend: + +- **In-transit corruption** — already caught by `CompoundVerifier`; not a lockfile novelty. +- **A poisoned *extracted* cache** — this is the important honesty point. On a warm cache Nextflow loads and executes the extracted `$id-$version/` tree; the retained zip is not what runs. An attacker who can write to the plugin cache will simply modify the extracted tree and leave the retained zip untouched, and the lockfile check passes while poisoned code executes. Re-hashing the archive therefore provides **essentially zero protection for what actually runs on a warm cache**; it only detects tampering of the archive itself, which matters solely if that archive is later re-extracted. Warm-run integrity of the executed plugin rests **entirely** on the `PluginSecurity` directory guard in **PR #7308**, which is a required complement, not optional. + +The two features compose: + +- the **lockfile** guarantees the *archive* you obtained (and retained) is the archive you committed to, and gates the cold-cache download and any future re-extraction; +- the **#7308 directory guard** guarantees the *extracted tree* — the code that actually executes — has not been altered after extraction. + +This split is stated plainly: the lockfile must not be marketed as "surviving cache poisoning." It survives archive tampering; the directory guard survives extracted-file tampering. + +### Lockfile format and location + +- **File name**: `plugins.lock`, in the pipeline project root next to `nextflow.config`, committed to VCS. +- **Format**: JSON, chosen for diff-friendliness under code review. +- **Keying**: by resolved `id@version`. + +```json +{ + "version": 1, + "plugins": { + "nf-amazon@2.0.0": { + "sha512": "cbc4..." + } + } +} +``` + +- `sha512` is the hash of the plugin **archive** (the zip), **computed over the downloaded archive bytes**. It is a content hash the pipeline owns, not a re-recording of the registry's `sha512sum`. (No `url` field is stored: it is not available at the point the archive is retained, and it would be provenance-only — never a trust input.) + +### Generation — automatic on first download (trust-on-first-use) + +There is **no dedicated `lock` command**. The lockfile is populated as a side effect of the normal plugin download, exactly as `go` writes `go.sum` and `npm` writes `package-lock.json` on first install: + +- The feature is dormant unless a `plugins.lock` file is present. To start, create an empty one (`touch plugins.lock`) and run the pipeline once. +- On a **cold-cache download** the archive is being fetched anyway; Nextflow hashes the retained bytes. If the coordinate is **missing** from the lock, its computed hash is **appended** (trust-on-first-use). Reviewing the resulting diff and committing it establishes the baseline. +- Because the pinned value is computed from the bytes actually received (not copied from registry metadata), it is a genuine content hash — the `go.sum` model — which is what makes the "survives registry compromise" property honest. +- Trust model: TOFU. Pinning trusts the registry at the moment a coordinate is first seen; every run thereafter is anchored to the committed content hash and no longer trusts the registry. This is why the pinned lockfile is meant to be reviewed and committed to VCS. + +Auto-pinning only ever **adds a missing coordinate**. An entry already present is **never silently rewritten** — a downloaded archive whose hash differs from a committed entry is a verification *failure* (mode-gated below), not a silent update, exactly as `go.sum` refuses to quietly change a recorded hash. Re-pinning a legitimately changed plugin is an explicit action: delete the stale entry and re-run. + +### Verification flow (in `PluginUpdater`) + +Verification happens during plugin resolution in `PluginUpdater`. **No branch of this flow ever re-fetches from the registry as a remedy** — fetching happens only to obtain a plugin that is genuinely absent from the cache, exactly as today. + +- **Cold cache (download path)**: the archive is downloaded to obtain the plugin regardless of the lockfile. After fetching the zip, compute its SHA-512. If the coordinate is **already locked**, compare against the committed entry — the **lock**, not the registry-supplied `sha512sum`, is authoritative. If the coordinate is **not yet locked**, append it (trust-on-first-use). Retain the zip in the cache (`$id-$version.zip` next to the extracted `$id-$version/` directory) so later runs can re-verify it offline. +- **Warm cache, archive retained (extracted dir + retained zip present)**: re-hash the retained zip against the lock, offline. This gates a *future* re-extraction only; see the threat model for why it does not protect the currently-executing extracted tree. +- **Warm cache, archive absent (extracted dir present, no retained zip)** — the universal state for every cache extracted before this feature (see *Migration and first adoption*): the plugin is already present and functional, so **do not download anything**. Verification of the archive is simply not possible offline for this cache. Behavior is: + - `strict` / `warn`: emit a one-time notice that the archive is unavailable for lockfile verification and that a fresh download (or re-vendoring the archive) is needed to establish an offline-verifiable baseline. **Do not abort** and **do not re-download** — a present, functioning plugin is never failed solely because its archive is missing and cannot be fetched offline. Integrity of the extracted tree for this run is provided by the #7308 directory guard. + - `off`: skip silently. +- **Coordinate not in the lock**: on a cold download it is **auto-pinned** (trust-on-first-use, see *Generation*); on a warm cache with no retained archive there are no bytes to pin, so it is a silent no-op. + +Verification of a **locally-present** archive requires no network. The one case where the archive is not present (a pre-feature cache) is handled without any network access, by design — it never falls back to a download. + +#### Known residual (documented, not overclaimed) + +Re-hashing the retained zip proves the **archive** is intact. On a warm run it does **not** protect the code that actually executes: Nextflow runs the already-extracted `$id-$version/` tree, and re-hashing the zip verifies an artifact that is not the thing being run. For a warm run the archive re-verify is therefore effectively inert for the executed code — its only value is gating a subsequent re-extraction. Detection of extracted-file tampering — the integrity of what actually runs — is the responsibility of the `PluginSecurity` directory guard in PR #7308. This residual is expected and by design. + +#### Tradeoff: retaining the archive + +Today Nextflow unzips the archive and immediately deletes it (`PluginUpdater.groovy:267-269`); a warm cache returns the extracted dir with no archive (`:259-262`). This ADR changes that for locked plugins by keeping `$id-$version.zip` alongside the extracted directory. Costs and caveats: + +- **Cache size**: roughly doubles on-disk footprint per plugin (compressed archive + extracted tree). Acceptable for the reproducibility/offline benefit; could be scoped to locked plugins only. +- **Lifecycle**: the retained zip must be cleaned up with the plugin directory and re-written on re-download. +- **Attacker can ignore it**: retaining the zip adds no protection for the running code — an attacker edits the extracted tree and leaves the zip untouched, and the check still passes. This is precisely why the #7308 directory guard is a required complement. + +### Migration and first adoption + +Because current code deletes the archive right after extraction (`PluginUpdater.groovy:267-269`) and warm-cache resolution returns the extracted directory with no archive (`:259-262`), **every plugin cache that predates this feature has no retained archive**. This is the *universal initial state* on first adoption, not an edge case. + +Consequences, stated plainly: + +- On the first lockfile-enabled run against a pre-existing cache — including an air-gapped one — plugins are already extracted and functional. The archive-absent branch above applies: Nextflow does **not** re-download, does **not** abort in strict mode, and simply notes that an offline-verifiable archive baseline has not yet been established. No network is required and no false abort occurs. +- An offline-verifiable archive baseline is established the next time each plugin is downloaded through the gate (cold cache) once a `plugins.lock` file exists, at which point the archive is retained and the coordinate pinned. +- **Air-gapped teams that vendor or mirror plugins** typically ship the extracted `$id-$version/` directories, not the `.zip` archives. To get *archive-level* offline verification in such an environment, the archives must be vendored too — i.e. the mirror/vendor step must include the retained `$id-$version.zip` files (produced by running the pipeline once against the registry in a connected environment, then committing/shipping the archives alongside the lockfile). Absent that, air-gapped runs fall into the archive-absent branch and rely on the #7308 directory guard for the extracted tree — which is safe and network-free, but is not archive verification. The only alternative that would make archive-free caches offline-verifiable is deriving the lock hash from the extracted tree (deferred Option B). + +### Modes and rollout + +- **Opt-in by presence**: if no `plugins.lock` exists, the feature is dormant and behavior is unchanged — zero breakage for existing pipelines. +- When the file exists, behavior is controlled by `NXF_PLUGINS_LOCK_MODE`, reusing the `warn`/`strict`/`off` tri-state plumbing introduced by PR #7308's `NXF_PLUGINS_STRICT_MODE`, but **independent** from it (the two knobs can be set separately). + +Mode gates only the **hash-mismatch** outcome. A coordinate *missing* from the lock is auto-pinned (see *Generation*), not gated; an *absent* archive is never gated (never aborts). + +| Mode | Hash mismatch (locked entry) | Coordinate missing from lock | Archive absent (pre-feature cache) | +|------|------------------------------|------------------------------|-------------------------------------| +| `strict` | **Abort** with a re-pin hint | Auto-pin (cold) / no-op (warm) | Notice only, proceed (never abort) | +| `warn` (default when the file is present) | Log a warning once and proceed | Auto-pin (cold) / no-op (warm) | Notice only, proceed | +| `off` | Skip verification | Auto-pin (cold) / no-op (warm) | Skip silently | + +**Default is `warn` when a lockfile is present**, deliberately mirroring PR #7308's warn-first rollout philosophy (`NXF_PLUGINS_STRICT_MODE` defaults to `warn`). This was a considered choice: an earlier draft proposed `strict`-when-present on the reasoning that a committed lockfile expresses intent to enforce. That was rejected because it diverges from the #7308 rollout philosophy this feature otherwise mirrors, and because it produces a surprising hard failure in the one gated case — a plugin whose committed lock entry is legitimately stale (e.g. the archive was re-released) would hard-abort until the entry is deleted and re-pinned. `warn`-by-default surfaces the drift without breaking the run; teams that want enforcement opt into `strict` explicitly (a staged `warn` → `strict` rollout), exactly as with #7308. (Note the auto-pin model already removes the most common friction: a *new or bumped* coordinate is pinned on first download rather than aborting, even in `strict`.) + +Independently of mode, a **present and functioning plugin is never aborted solely because its archive is missing** and cannot be fetched offline (the archive-absent column above) — this avoids a false-positive failure on precisely the air-gapped caches the feature is meant to serve. + +### Reused and new components + +| Component | Module | Change | +|-----------|--------|--------| +| `PluginLockFile` (new) | nf-commons | Read/write/round-trip of `plugins.lock`; blank/malformed-file handling | +| `PluginLockVerifier` (new) | nf-commons | Auto-pin on first download (TOFU); re-verify retained archive; archive-absent handling; mode gating | +| `PluginUpdater` | nf-commons | Cold-cache: retain zip + verify/pin; warm-cache: re-verify retained zip. No dedicated command | +| `HttpPluginRepository` | nf-commons | No change — `CompoundVerifier` still checks downloads against the registry `sha512sum` at fetch time | +| `NXF_PLUGINS_LOCK_MODE` | nf-commons | New env var, tri-state (`strict`/`warn`/`off`), default `warn`, independent of `NXF_PLUGINS_STRICT_MODE` | + +### Relationship to PR #7308 + +PR #7308 introduces the `PluginSecurity` directory guard, which protects the **extracted** plugin directory — the code that actually executes — and the `NXF_PLUGINS_STRICT_MODE` tri-state plumbing (default `warn`). This ADR's lockfile is the **sibling** feature protecting the **archive**, reusing the same mode-plumbing pattern and the same warn-first default under a separate, independent switch. Together they cover both integrity surfaces — archive and extracted tree — without either overclaiming the other's protection. Critically, warm-run integrity of executing code depends on the #7308 guard; the lockfile does not substitute for it. + +## Testing + +- **`PluginLockFile`**: read, write, and round-trip of `plugins.lock`; blank/`touch`ed file parses as empty; malformed file throws. +- **Auto-pin (TOFU)**: an enabled but empty lock, given a downloaded archive for an unlocked coordinate, appends the **byte-computed** hash to the file on disk; a dormant (no file) verifier never pins. +- **Verification**: + - matching hash passes; + - mismatch **aborts** in `strict`, **warns once** in `warn`, is **ignored** in `off`; + - **archive-absent (pre-feature cache)**: with a locked coordinate but no retained zip, the run proceeds in `strict` with a notice, performs **no download**, and never aborts. +- **No-network guarantee**: verification and pinning operate purely on locally-present bytes; no branch re-fetches from the registry as a remedy. diff --git a/docs/plugins/using-plugins.mdx b/docs/plugins/using-plugins.mdx index 82a370749c..89972d1294 100644 --- a/docs/plugins/using-plugins.mdx +++ b/docs/plugins/using-plugins.mdx @@ -56,6 +56,30 @@ Plugin declarations in Nextflow configuration files are ignored when specifying When Nextflow downloads plugins, it caches them in the directory specified by `NXF_PLUGINS_DIR` (`$HOME/.nextflow/plugins` by default). +:::note +The plugin cache is shared across pipelines and is not access-controlled. On multi-tenant or shared systems, use a private cache directory per user (set `NXF_PLUGINS_DIR` to a location only you can write) to avoid loading plugin artifacts populated by another user. +::: + +### Lockfile + + + +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. + +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: + +```bash +touch plugins.lock +``` + +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. + +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. + +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. + +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. + ## Offline usage When running Nextflow in an offline environment, any required plugins must be downloaded and moved into the offline environment prior to any runs. @@ -75,6 +99,7 @@ To use Nextflow plugins in an offline environment: ::: [install-standalone]: ../install#standalone-distribution +[using-plugins-env-vars]: ../reference/env-vars#nxf_plugins_lock_mode [using-plugins-config]: ./using-plugins#configuration [using-plugins-identifiers]: ./using-plugins#identifiers diff --git a/docs/reference/env-vars.mdx b/docs/reference/env-vars.mdx index 277615e3af..58ed715376 100644 --- a/docs/reference/env-vars.mdx +++ b/docs/reference/env-vars.mdx @@ -210,6 +210,12 @@ Whether to use the default plugins when no plugins are specified in the Nextflow The path where the plugin archives are loaded and stored (default: `$NXF_HOME/plugins`). +##### `NXF_PLUGINS_LOCK_MODE` + + + +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. + ##### `NXF_PLUGINS_REGISTRY_URL` diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginLockFile.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginLockFile.groovy new file mode 100644 index 0000000000..5041047755 --- /dev/null +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginLockFile.groovy @@ -0,0 +1,177 @@ +/* + * Copyright 2013-2026, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nextflow.plugin + +import java.nio.file.Files +import java.nio.file.Path + +import com.google.gson.Gson +import com.google.gson.GsonBuilder +import com.google.gson.JsonSyntaxException +import groovy.transform.CompileStatic +import groovy.transform.EqualsAndHashCode +import groovy.transform.ToString +import groovy.util.logging.Slf4j + +/** + * Model the {@code plugins.lock} file. It holds a format version number and a map + * of plugin fully-qualified ids (ie. {@code id@version}) to the corresponding + * {@link Entry} carrying the {@code sha512} checksum of the plugin archive. + * + * The file is serialised as pretty-printed JSON with a stable (sorted) key order to + * keep diffs deterministic. + * + * @author Paolo Di Tommaso + */ +@Slf4j +@CompileStatic +@EqualsAndHashCode(includeFields = true) +@ToString(includeNames = true, includeFields = true) +class PluginLockFile { + + /** + * The current lock file format version + */ + static final int CURRENT_VERSION = 1 + + /** + * Represent a single locked plugin entry + */ + @EqualsAndHashCode + @ToString(includeNames = true) + static class Entry { + String sha512 + + Entry() {} + + Entry(String sha512) { + this.sha512 = sha512 + } + } + + private int version = CURRENT_VERSION + + private Map plugins = new TreeMap() + + int getVersion() { version } + + void setVersion(int value) { this.version = value } + + /** + * @return An immutable view of the locked plugin entries, keyed by fully-qualified id + */ + Map getEntries() { + return Collections.unmodifiableMap(plugins) + } + + /** + * Lookup a locked entry by its fully-qualified id ie. {@code id@version}. + * + * @param fqid The plugin fully-qualified id + * @return The corresponding {@link Entry} or {@code null} if not present + */ + Entry getEntry(String fqid) { + return plugins.get(fqid) + } + + /** + * Add or update a locked entry. + * + * @param fqid The plugin fully-qualified id ie. {@code id@version} + * @param entry The {@link Entry} to associate with the given id + * @return The object itself to enable method chaining + */ + PluginLockFile addEntry(String fqid, Entry entry) { + if( !fqid ) + throw new IllegalArgumentException("Plugin lock entry id cannot be empty") + if( entry == null ) + throw new IllegalArgumentException("Plugin lock entry cannot be null") + plugins.put(fqid, entry) + return this + } + + /** + * @return {@code true} when no plugin entries are held + */ + boolean isEmpty() { + return plugins.isEmpty() + } + + /** + * Serialise this lock file as pretty-printed JSON with a stable key order. + * + * @param path The target file path + */ + void write(Path path) { + final json = gson0().toJson(toModel()) + Files.write(path, json.getBytes('UTF-8')) + } + + private Map toModel() { + // use a plain map so that only 'version' and 'plugins' are emitted, with + // plugins held in a TreeMap to guarantee a stable, sorted key order + final result = new LinkedHashMap() + result.put('version', version) + result.put('plugins', new TreeMap(plugins)) + return result + } + + private static Gson gson0() { + return new GsonBuilder().setPrettyPrinting().create() + } + + /** + * Read and parse a {@code plugins.lock} file. + * + * @param path The lock file path + * @return A {@link PluginLockFile}; an empty (dormant) instance when the file does not exist + * @throws IllegalStateException when the file content cannot be parsed + */ + static PluginLockFile read(Path path) { + if( path == null || !Files.exists(path) ) { + log.debug "Plugins lock file does not exist: $path - returning an empty lock" + return new PluginLockFile() + } + + final text = new String(Files.readAllBytes(path), 'UTF-8') + // a blank or freshly `touch`ed file is a valid, empty lock (bootstrap case) + if( !text.trim() ) + return new PluginLockFile() + try { + final model = gson0().fromJson(text, ModelBean) + if( model == null ) + throw new IllegalStateException("Invalid plugins lock file - empty content: $path") + final result = new PluginLockFile() + result.version = model.version + if( model.plugins ) + result.plugins.putAll(model.plugins) + return result + } + catch( JsonSyntaxException e ) { + throw new IllegalStateException("Invalid plugins lock file - malformed JSON: $path", e) + } + } + + /** + * Deserialization bean matching the on-disk JSON structure + */ + static class ModelBean { + int version + Map plugins + } + +} diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy new file mode 100644 index 0000000000..25a9c48695 --- /dev/null +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy @@ -0,0 +1,182 @@ +/* + * Copyright 2013-2026, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nextflow.plugin + +import java.nio.file.Files +import java.nio.file.Path +import java.security.MessageDigest + +import groovy.transform.CompileStatic +import groovy.util.logging.Slf4j +import nextflow.SysEnv +import nextflow.exception.AbortOperationException + +/** + * Verifies plugin archives against the {@code plugins.lock} file. + * + * The feature is opt-in by the presence of the lock file: it is dormant (a no-op) when no + * {@code plugins.lock} exists. When the file is present, the sha512 of the retained plugin archive + * is re-computed locally (no network) and compared to the committed lock entry. + * + * Following the {@code go.sum} / {@code package-lock.json} model, the lock is populated + * automatically: the first time a coordinate is downloaded and it is missing from the lock, its + * archive checksum is appended (trust-on-first-use). An existing entry is never rewritten silently + * — a mismatch against a committed entry is a verification failure. + * + * The behaviour on a checksum mismatch is gated by the {@code NXF_PLUGINS_LOCK_MODE} environment + * variable: + *
    + *
  • {@code strict} - abort with an {@link AbortOperationException}
    • + *
  • {@code warn} (default) - log a warning once per coordinate and proceed
    • + *
  • {@code off} - skip verification silently
    • + *
+ * + * A locked plugin whose archive is not available locally (e.g. a cache extracted before this + * feature existed) is never aborted: it cannot be verified offline and its archive is never + * re-downloaded just to verify it. Integrity of the extracted code that actually runs is the + * responsibility of the plugin directory guard, not of this archive check. + * + * @author Paolo Di Tommaso + */ +@Slf4j +@CompileStatic +class PluginLockVerifier { + + static enum Mode { STRICT, WARN, OFF } + + private final Path lockPath + + private final PluginLockFile lock + + private final boolean enabled + + private final Set notified = Collections.synchronizedSet(new HashSet()) + + /** + * @param lockPath The {@code plugins.lock} path; the feature is enabled only when this file + * exists. A {@code null} or missing path leaves the verifier dormant. + */ + PluginLockVerifier(Path lockPath) { + this.lockPath = lockPath + this.enabled = lockPath != null && Files.exists(lockPath) + this.lock = PluginLockFile.read(lockPath) + } + + /** + * @return {@code true} when a lock file is present ie. verification/pinning is enabled + */ + boolean isEnabled() { + return enabled + } + + /** + * Resolve the verification mode from the {@code NXF_PLUGINS_LOCK_MODE} environment variable. + * + * @return The resolved {@link Mode}; {@link Mode#WARN} when unset or unrecognised + */ + static Mode getMode() { + final value = SysEnv.get('NXF_PLUGINS_LOCK_MODE') + if( !value ) + return Mode.WARN + switch( value.toLowerCase() ) { + case 'strict': return Mode.STRICT + case 'warn': return Mode.WARN + case 'off': return Mode.OFF + default: + log.warn "Invalid NXF_PLUGINS_LOCK_MODE value: '$value' - using default 'warn'" + return Mode.WARN + } + } + + /** + * Verify - and, on first sight, pin - a plugin archive against the lock. + * + * @param fqid The plugin fully-qualified id ie. {@code id@version} + * @param zip The retained plugin archive; may be {@code null} or missing + */ + void verify(String fqid, Path zip) { + if( !enabled ) + return + final entry = lock.getEntry(fqid) + final present = zip != null && Files.exists(zip) + + // coordinate not yet locked: pin it on first download (trust-on-first-use), never fail + if( entry == null ) { + if( present ) + pin(fqid, sha512(zip)) + else + log.debug "Plugin '$fqid' is not in the plugins lock file and its archive is not available to pin" + return + } + + // locked, but the archive is not available (e.g. a cache created before this feature): + // it cannot be verified offline - never abort and never re-download to verify + if( !present ) { + if( getMode() != Mode.OFF && notified.add(fqid) ) + log.warn "Cannot verify plugin '$fqid' against the plugins lock file - its archive is not available in the cache" + return + } + + // verify the retained archive against the committed checksum + final actual = sha512(zip) + if( actual == entry.sha512 ) + return + + final reason = "Plugin '$fqid' checksum does not match the plugins lock file\n- expected: ${entry.sha512}\n- actual : ${actual}" + switch( getMode() ) { + case Mode.OFF: + log.debug "Plugins lock verification failed (ignored, mode=off) - $reason" + break + case Mode.STRICT: + 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") + case Mode.WARN: + // warn only once per coordinate to avoid log spam + if( notified.add(fqid) ) + log.warn "$reason\n- delete the entry from the plugins lock file and re-run to re-pin" + break + } + } + + /** + * Append a new entry to the lock and persist it (trust-on-first-use). Existing entries are + * never overwritten by this path — {@link #verify} routes an already-locked coordinate through + * the checksum comparison instead. + */ + private synchronized void pin(String fqid, String sha512) { + lock.addEntry(fqid, new PluginLockFile.Entry(sha512)) + if( lockPath != null ) + lock.write(lockPath) + log.info "Added plugin '$fqid' to the plugins lock file" + } + + /** + * Compute the sha512 hex digest of the given file. + * + * @param file The file to hash + * @return The lowercase hex-encoded sha512 digest (128 chars) + */ + static String sha512(Path file) { + final md = MessageDigest.getInstance('SHA-512') + try (InputStream is = Files.newInputStream(file)) { + final buffer = new byte[8192] + int read + while( (read = is.read(buffer)) != -1 ) + md.update(buffer, 0, read) + } + return HexFormat.of().formatHex(md.digest()) + } +} diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy index e261563ed1..34cf6813fc 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy @@ -20,6 +20,7 @@ import static java.nio.file.StandardCopyOption.* import java.nio.file.Files import java.nio.file.Path +import java.nio.file.Paths import java.util.regex.Pattern import com.github.zafarkhaja.semver.Version @@ -70,6 +71,8 @@ class PluginUpdater extends UpdateManager { private DefaultPlugins defaultPlugins = DefaultPlugins.INSTANCE + private PluginLockVerifier lockVerifier + protected PluginUpdater(CustomPluginManager pluginManager) { super(pluginManager) this.pluginManager = pluginManager @@ -249,6 +252,30 @@ class PluginUpdater extends UpdateManager { return load0(id, version) } + /** + * The {@code plugins.lock} file location used to verify downloaded plugin artifacts. + * Defaults to a {@code plugins.lock} file in the current working directory. + */ + protected Path lockFilePath() { + return Paths.get('plugins.lock') + } + + /** + * Lazily create the {@link PluginLockVerifier}. The lock file is read once and cached. + */ + protected synchronized PluginLockVerifier getLockVerifier() { + if( lockVerifier == null ) + lockVerifier = new PluginLockVerifier(lockFilePath()) + return lockVerifier + } + + /** + * @return the path to the retained plugin zip artifact used for lock verification + */ + private Path retainedZip(String id, String version) { + return pluginsStore.resolve("${id}-${version}.zip") + } + private Path download0(String id, String version) { // 0. check if version is specified if( !version ) @@ -264,9 +291,21 @@ class PluginUpdater extends UpdateManager { // 2. download to temporary location Path downloaded = safeDownloadPlugin(id, version); - // 3. unzip the content and delete downloaded file + // 3. unzip the content Path dir = FileUtils.expandIfZip(downloaded) - FileHelper.deletePath(downloaded) + + // 3.1 when the plugins lock is enabled retain the downloaded zip next to the extracted + // dir and verify (or pin) its checksum against the lock (cold cache). Otherwise delete it. + if( getLockVerifier().isEnabled() ) { + final retained = retainedZip(id, version) + FileHelper.deletePath(retained) + FileHelper.copyPath(downloaded, retained) + FileHelper.deletePath(downloaded) + getLockVerifier().verify("${id}@${version}", retained) + } + else { + FileHelper.deletePath(downloaded) + } // 4. move the final destination the plugin directory assert pluginPath.getFileName() == dir.getFileName() @@ -395,6 +434,11 @@ class PluginUpdater extends UpdateManager { if( !FilesEx.exists(pluginPath) ) { pluginPath = safeDownload(id, version) } + else { + // warm cache: re-hash the retained artifact and verify it against the plugins lock + // (missing retained zip is treated as a lock-miss by the verifier) + getLockVerifier().verify("${id}@${version}", retainedZip(id, version)) + } // verify the plugin install path contains the expected manifest path if( !FilesEx.exists(pluginPath.resolve('classes/META-INF/MANIFEST.MF')) ) { diff --git a/modules/nf-commons/src/test/nextflow/plugin/PluginLockFileTest.groovy b/modules/nf-commons/src/test/nextflow/plugin/PluginLockFileTest.groovy new file mode 100644 index 0000000000..84740ff179 --- /dev/null +++ b/modules/nf-commons/src/test/nextflow/plugin/PluginLockFileTest.groovy @@ -0,0 +1,151 @@ +/* + * Copyright 2013-2026, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nextflow.plugin + +import java.nio.file.Files +import java.nio.file.Path + +import spock.lang.Specification + +/** + * + * @author Paolo Di Tommaso + */ +class PluginLockFileTest extends Specification { + + def 'should round-trip write and read' () { + given: + def path = Files.createTempFile('plugins', '.lock') + and: + final lock = new PluginLockFile() + lock.version = 1 + lock.addEntry('nf-amazon@2.0.0', new PluginLockFile.Entry('sha512-aaa')) + lock.addEntry('nf-google@1.5.0', new PluginLockFile.Entry('sha512-ggg')) + + when: + lock.write(path) + and: + final copy = PluginLockFile.read(path) + + then: + copy.version == 1 + copy.getEntry('nf-amazon@2.0.0') == new PluginLockFile.Entry('sha512-aaa') + copy.getEntry('nf-google@1.5.0') == new PluginLockFile.Entry('sha512-ggg') + copy.entries.size() == 2 + and: + copy == lock + + cleanup: + Files.deleteIfExists(path) + } + + def 'should write pretty json with stable key order' () { + given: + def path = Files.createTempFile('plugins', '.lock') + and: + final lock = new PluginLockFile() + lock.version = 1 + // add out of order + lock.addEntry('nf-zeta@1.0.0', new PluginLockFile.Entry('sha512-z')) + lock.addEntry('nf-alpha@1.0.0', new PluginLockFile.Entry('sha512-a')) + + when: + lock.write(path) + final text = path.text + + then: + // pretty printed (contains newlines and indentation) + text.contains('\n') + // keys are sorted: alpha appears before zeta + text.indexOf('nf-alpha@1.0.0') < text.indexOf('nf-zeta@1.0.0') + + cleanup: + Files.deleteIfExists(path) + } + + def 'should return empty for missing file' () { + given: + final path = Path.of('/no/such/plugins.lock') + + when: + final lock = PluginLockFile.read(path) + + then: + lock != null + lock.isEmpty() + lock.entries.isEmpty() + } + + def 'should return empty for a blank (touched) file' () { + given: + def path = Files.createTempFile('plugins', '.lock') + path.text = ' \n' + + when: + final lock = PluginLockFile.read(path) + + then: + lock != null + lock.isEmpty() + + cleanup: + Files.deleteIfExists(path) + } + + def 'should throw for malformed file' () { + given: + def path = Files.createTempFile('plugins', '.lock') + path.text = '{ this is not valid json ]' + + when: + PluginLockFile.read(path) + + then: + thrown(IllegalStateException) + + cleanup: + Files.deleteIfExists(path) + } + + def 'should lookup an entry by id and version' () { + given: + final lock = new PluginLockFile() + lock.addEntry('nf-amazon@2.0.0', new PluginLockFile.Entry('sha512-aaa')) + + expect: + lock.getEntry('nf-amazon@2.0.0') == new PluginLockFile.Entry('sha512-aaa') + lock.getEntry('nf-unknown@9.9.9') == null + } + + def 'should add and update an entry' () { + given: + final lock = new PluginLockFile() + + when: + lock.addEntry('nf-amazon@2.0.0', new PluginLockFile.Entry('sha512-old')) + then: + lock.getEntry('nf-amazon@2.0.0').sha512 == 'sha512-old' + lock.entries.size() == 1 + + when: + lock.addEntry('nf-amazon@2.0.0', new PluginLockFile.Entry('sha512-new')) + then: + lock.getEntry('nf-amazon@2.0.0').sha512 == 'sha512-new' + lock.entries.size() == 1 + } + +} diff --git a/modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy b/modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy new file mode 100644 index 0000000000..8cca290629 --- /dev/null +++ b/modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy @@ -0,0 +1,275 @@ +/* + * Copyright 2013-2026, Seqera Labs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package nextflow.plugin + +import java.nio.file.Files +import java.nio.file.Path + +import nextflow.SysEnv +import nextflow.exception.AbortOperationException +import spock.lang.Specification +import spock.lang.Unroll + +/** + * + * @author Paolo Di Tommaso + */ +class PluginLockVerifierTest extends Specification { + + private Path zipWith(String content) { + final zip = Files.createTempFile('plugin', '.zip') + Files.write(zip, content.getBytes('UTF-8')) + return zip + } + + /** Write a lock file holding the given fqid->sha512 entries and return its path */ + private Path lockFileWith(Map entries) { + final path = Files.createTempFile('plugins', '.lock') + final lock = new PluginLockFile() + entries.each { k, v -> lock.addEntry(k, new PluginLockFile.Entry(v)) } + lock.write(path) + return path + } + + @Unroll + def 'should resolve lock mode from env [#VALUE]' () { + given: + SysEnv.push(VALUE != null ? [NXF_PLUGINS_LOCK_MODE: VALUE] : [:]) + + expect: + PluginLockVerifier.getMode() == EXPECTED + + cleanup: + SysEnv.pop() + + where: + VALUE | EXPECTED + null | PluginLockVerifier.Mode.WARN + 'warn' | PluginLockVerifier.Mode.WARN + 'WARN' | PluginLockVerifier.Mode.WARN + 'strict' | PluginLockVerifier.Mode.STRICT + 'STRICT' | PluginLockVerifier.Mode.STRICT + 'off' | PluginLockVerifier.Mode.OFF + 'OFF' | PluginLockVerifier.Mode.OFF + 'nonsense' | PluginLockVerifier.Mode.WARN + } + + // --------------------------------------------------------------------------- + // dormant / enabled + + def 'should be dormant when no lock file exists' () { + given: + SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) + and: + def zip = zipWith('hello plugin') + def verifier = new PluginLockVerifier(Path.of('/no/such/plugins.lock')) + + expect: + !verifier.isEnabled() + + when: + verifier.verify('nf-foo@1.0.0', zip) + then: + noExceptionThrown() + + cleanup: + SysEnv.pop() + Files.deleteIfExists(zip) + } + + def 'should be enabled when an (even empty) lock file exists' () { + given: + def path = Files.createTempFile('plugins', '.lock') + path.text = '' + + expect: + new PluginLockVerifier(path).isEnabled() + + cleanup: + Files.deleteIfExists(path) + } + + // --------------------------------------------------------------------------- + // trust-on-first-use pinning + + def 'should pin a coordinate missing from the lock on first sight (TOFU)' () { + given: + SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) + and: + def zip = zipWith('hello plugin') + final sha = PluginLockVerifier.sha512(zip) + and: + // an empty but present lock file -> feature enabled, nothing pinned yet + def lockPath = lockFileWith([:]) + def verifier = new PluginLockVerifier(lockPath) + + when: + verifier.verify('nf-foo@1.0.0', zip) + + then: + noExceptionThrown() + and: + // the entry was appended to the lock file on disk + def written = PluginLockFile.read(lockPath) + written.getEntry('nf-foo@1.0.0').sha512 == sha + + cleanup: + SysEnv.pop() + Files.deleteIfExists(zip) + Files.deleteIfExists(lockPath) + } + + def 'should not pin when dormant' () { + given: + def zip = zipWith('hello plugin') + def verifier = new PluginLockVerifier(Path.of('/no/such/plugins.lock')) + + when: + verifier.verify('nf-foo@1.0.0', zip) + + then: + !verifier.isEnabled() + noExceptionThrown() + + cleanup: + Files.deleteIfExists(zip) + } + + // --------------------------------------------------------------------------- + // verification of an already-locked coordinate + + def 'should pass verification when the artifact matches the lock' () { + given: + SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) + and: + def zip = zipWith('hello plugin') + final sha = PluginLockVerifier.sha512(zip) + def lockPath = lockFileWith(['nf-foo@1.0.0': sha]) + def verifier = new PluginLockVerifier(lockPath) + + when: + verifier.verify('nf-foo@1.0.0', zip) + + then: + noExceptionThrown() + + cleanup: + SysEnv.pop() + Files.deleteIfExists(zip) + Files.deleteIfExists(lockPath) + } + + def 'strict mode should abort on a checksum mismatch' () { + given: + SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) + and: + def zip = zipWith('hello plugin') + def lockPath = lockFileWith(['nf-foo@1.0.0': 'deadbeef']) + def verifier = new PluginLockVerifier(lockPath) + + when: + verifier.verify('nf-foo@1.0.0', zip) + + then: + def e = thrown(AbortOperationException) + e.message.contains('checksum does not match') + + cleanup: + SysEnv.pop() + Files.deleteIfExists(zip) + Files.deleteIfExists(lockPath) + } + + def 'warn mode should not abort on a checksum mismatch' () { + given: + SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'warn']) + and: + def zip = zipWith('hello plugin') + def lockPath = lockFileWith(['nf-foo@1.0.0': 'deadbeef']) + def verifier = new PluginLockVerifier(lockPath) + + when: + verifier.verify('nf-foo@1.0.0', zip) + // second call to exercise the "log once" path + verifier.verify('nf-foo@1.0.0', zip) + + then: + noExceptionThrown() + + cleanup: + SysEnv.pop() + Files.deleteIfExists(zip) + Files.deleteIfExists(lockPath) + } + + def 'off mode should ignore a checksum mismatch' () { + given: + SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'off']) + and: + def zip = zipWith('hello plugin') + def lockPath = lockFileWith(['nf-foo@1.0.0': 'deadbeef']) + def verifier = new PluginLockVerifier(lockPath) + + when: + verifier.verify('nf-foo@1.0.0', zip) + + then: + noExceptionThrown() + + cleanup: + SysEnv.pop() + Files.deleteIfExists(zip) + Files.deleteIfExists(lockPath) + } + + // --------------------------------------------------------------------------- + // archive-absent (e.g. a cache extracted before this feature): never abort + + def 'strict mode should NOT abort when a locked artifact is unavailable' () { + given: + SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) + and: + final missing = Path.of('/no/such/nf-foo-1.0.0.zip') + def lockPath = lockFileWith(['nf-foo@1.0.0': 'abc']) + def verifier = new PluginLockVerifier(lockPath) + + when: + verifier.verify('nf-foo@1.0.0', missing) + + then: + noExceptionThrown() + + cleanup: + SysEnv.pop() + Files.deleteIfExists(lockPath) + } + + def 'sha512 should compute a 128-char lowercase hex digest' () { + given: + def zip = zipWith('some bytes') + + when: + final sha = PluginLockVerifier.sha512(zip) + + then: + sha.length() == 128 + sha ==~ /[0-9a-f]{128}/ + + cleanup: + Files.deleteIfExists(zip) + } +} From 8255ec7d902cb67003b36b252f4815ac35d1e176 Mon Sep 17 00:00:00 2001 From: Paolo Di Tommaso Date: Tue, 28 Jul 2026 10:06:15 +0200 Subject: [PATCH 2/2] refactor: pin extracted-tree hash instead of archive hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- adr/20260727-plugin-lockfile-integrity.md | 191 +++++++----------- docs/plugins/using-plugins.mdx | 10 +- docs/reference/env-vars.mdx | 2 +- .../nextflow/plugin/PluginLockVerifier.groovy | 101 ++++----- .../main/nextflow/plugin/PluginUpdater.groovy | 34 +--- .../plugin/PluginLockVerifierTest.groovy | 137 +++++++------ 6 files changed, 213 insertions(+), 262 deletions(-) diff --git a/adr/20260727-plugin-lockfile-integrity.md b/adr/20260727-plugin-lockfile-integrity.md index 9d045018c5..f06423ddd4 100644 --- a/adr/20260727-plugin-lockfile-integrity.md +++ b/adr/20260727-plugin-lockfile-integrity.md @@ -7,41 +7,37 @@ ## Summary -Introduce a committed lockfile (`plugins.lock`) that pins the SHA-512 hash of each resolved plugin **archive**, computed over the actual archive bytes. The lockfile is populated automatically as a side effect of the normal plugin download — the first time a coordinate is fetched it is pinned (trust-on-first-use) — exactly like `go.sum` / `package-lock.json`, with **no dedicated command**. The trusted hash lives in the pipeline repository under version control and is not re-fetched from the registry on every run. During plugin resolution Nextflow re-hashes the downloaded — or retained — archive and verifies it against the locked hash. +Introduce a committed lockfile (`plugins.lock`) that pins a canonical SHA-512 hash of each plugin's **extracted directory** — the unpacked `$id-$version/` tree that Nextflow actually loads and executes. The lockfile is populated automatically the first time a plugin is seen (trust-on-first-use), exactly like `go.sum` / `package-lock.json`, with **no dedicated command**. The trusted hash lives in the pipeline repository under version control and is not re-fetched from the registry on every run. Before loading a plugin, Nextflow re-hashes its extracted directory and verifies it against the committed hash. -Verification is network-free **whenever the archive being verified is present locally** (always true on a cold download; true on a warm cache only if the archive was retained). This ADR is explicit about the one case where an archive is not present — a cache extracted before this feature existed — and specifies an offline-safe behavior for it that never triggers a download (see *Migration and first adoption* and *Verification flow*). +Because it hashes the code that runs rather than the archive it came from, a single mechanism covers **both** integrity surfaces: a tampered/compromised download or registry, **and** a poisoned cache directory (a lower-trust user editing already-extracted files on a shared filesystem). Verification is fully offline, and it uses no ownership or permission heuristics, so it does not produce false-positive warnings on legitimate shared caches. ## Problem Statement -Nextflow resolves plugins from a registry (or mirror), downloads a zip archive, and extracts it into the local plugin cache under `$id-$version/`. Today there is no persistent, user-owned record of what a plugin archive is *supposed* to hash to. The registry returns a `sha512sum` during resolution, but that value is fetched fresh on every run and is only ever compared against the download in transit — so it can detect corruption on the wire, never a compromise of the source of truth itself. +Nextflow resolves plugins from a registry (or mirror), downloads a zip archive, extracts it into the local plugin cache under `$id-$version/`, and loads the extracted code. Two integrity gaps exist: -**What is already covered today.** In-transit archive integrity is *not* an open gap: `HttpPluginRepository` wires pf4j's `CompoundVerifier` (`HttpPluginRepository.groovy:134-135`), which checks the downloaded archive against the registry-supplied `sha512sum` (`:222`) at download time. A corrupted or MITM-mangled download that no longer matches the registry's own hash is already rejected. The lockfile does **not** claim novelty there. +1. **Cache poisoning (the executed code).** When a `$id-$version/` directory already exists, `PluginUpdater.load0()` short-circuits the download entirely and loads the extracted tree as-is. On a shared, writable plugin cache (`NXF_PLUGINS_DIR` pointed at a group-writable or world-writable location), a lower-trust local user can pre-populate or edit an official pinned coordinate (e.g. `nf-amazon-3.10.0/`) with attacker-controlled class/jar files that then execute inside the victim's Nextflow process. +2. **Runtime trust in the registry.** The registry returns a `sha512sum` during resolution, but that value is fetched fresh every run and only ever compared against the download in transit (pf4j's `CompoundVerifier`, `HttpPluginRepository.groovy:134-135` / `:222`). A compromised registry/mirror that serves a tampered archive with a matching tampered hash passes that check, and silent version drift produces no signal. There is no persistent, user-owned record of what a plugin is *supposed* to be. -The genuine, non-redundant gaps the lockfile closes are: +**What is already covered today.** In-transit corruption of a download is caught by `CompoundVerifier` against the registry-supplied hash. The lockfile does not duplicate that. Its job is the *post-download* surface: the extracted code, plus removing runtime trust in the registry's own hash. -- **Runtime trust in the registry's own hash**: today the *expected* value is whatever the registry asserts on this run. A compromised registry (or mirror) that serves a tampered archive together with a matching tampered hash passes the existing `CompoundVerifier` check — because both sides of the comparison come from the same untrusted source. The lockfile removes the registry from the trust path at run time. -- **Silent version drift**: a coordinate that resolves to a different artifact than it did last week produces no signal today. -- **Reproducibility / committed baseline for teams that vendor or mirror plugins**: there is no local, authoritative, VCS-reviewed record of what each coordinate must hash to, independent of the registry. - -The core issue is that the *trusted* hash must be owned by the pipeline and committed to version control, so verification depends on the repository — which the team controls and reviews — rather than on the registry being honest at run time. +The core issue is that the *trusted* baseline must be owned by the pipeline and committed to version control, and it must describe the artifact that actually executes — the extracted tree — so verification depends on the repository (which the team controls and reviews) rather than on the registry being honest at run time or on the cache being un-tampered. ## Goals or Decision Drivers -- **Local, network-free verification when the archive is present**: once the lockfile exists, checking a locally-present archive against the locked hash requires no network access. Network is used only to *fetch* a plugin that is not yet cached, exactly as today. This ADR does **not** claim verification is unconditionally offline — see the migration case below. -- **Content-addressed trust anchor**: the pinned hash is computed over the downloaded archive bytes at first download, so the baseline is a genuine content hash (the `go.sum` / `package-lock.json` model), not a re-recording of a registry claim. -- **User-owned trust anchor**: the authoritative hash is committed to the pipeline repository and reviewed through normal VCS workflows (pull requests, code review, blame). -- **Reproducibility**: a given commit of a pipeline resolves to exactly the same plugin archives, or fails loudly. -- **Zero breakage by default**: pipelines without a lockfile behave exactly as they do today. The feature is opt-in by the mere presence of the file, and its default enforcement level mirrors PR #7308's warn-first rollout philosophy (see *Modes and rollout*). -- **Never turn verification into a network dependency**: the verification path must never re-fetch from the registry as a remedy — that would re-introduce the exact runtime dependency this design rejects in Option A. -- **Honest scope**: the mechanism must defend what it can actually defend (the archive, and only as a re-extraction gate on a warm cache) and not overclaim protection it does not provide (the extracted tree that actually runs). +- **Cover the executed code.** The check must protect the extracted `$id-$version/` tree that Nextflow loads, not merely the archive it was unpacked from. +- **One mechanism, both surfaces.** Supply-chain/registry integrity and cache-poisoning integrity should be answered by the same content hash, not two features. +- **No false positives, no noise.** Verification must not rely on directory ownership or permission heuristics; a legitimate shared or admin-managed cache must be silent. Output happens only on a genuine mismatch. +- **Local, network-free verification.** Once the lockfile exists, checking a plugin requires no network access; the registry is never a runtime dependency of verification. +- **Content-addressed, user-owned trust anchor.** The pinned hash is computed over the actual files and committed to the pipeline repository, reviewed through normal VCS workflows. +- **Zero breakage by default.** Pipelines without a lockfile behave exactly as today; the feature is opt-in by the mere presence of the file, defaulting to `warn` on mismatch (mirroring PR #7308's warn-first rollout). ## Non-goals -- **Extracted-tree hashing**: hashing or re-verifying the contents of the extracted `$id-$version/` directory is out of scope. That surface is covered by the `PluginSecurity` directory guard in PR #7308 (see *Threat model* and *Known residual* below). -- **Transitive / dependency lock resolution**: only the coordinates declared in the pipeline config are locked. There is no dependency-graph resolution or locking of plugins pulled in indirectly. -- **Registry-server changes**: this ADR is entirely client-side. The registry contract is unchanged. -- **Signature or provenance verification**: the lockfile pins a hash, not a cryptographic signature or attestation. Provenance metadata (the `url`) is recorded for auditability only. -- **Catching in-transit download corruption**: already handled by pf4j's `CompoundVerifier` against the registry-supplied `sha512sum`. Not a job the lockfile duplicates. +- **A dedicated `lock` command.** The lockfile is populated automatically on first load; there is no `nextflow plugins lock` subcommand to run or maintain. +- **Transitive / dependency lock resolution.** Only coordinates that are actually loaded are pinned; there is no dependency-graph resolution. +- **Registry-server changes.** Entirely client-side; the registry contract is unchanged. +- **Signature / provenance verification.** The lockfile pins a content hash, not a cryptographic signature or attestation. +- **Catching in-transit download corruption.** Already handled by pf4j's `CompoundVerifier`. ## Considered Options @@ -49,64 +45,47 @@ The core issue is that the *trusted* hash must be owned by the pipeline and comm Fetch the plugin, then re-query the registry for the expected `sha512sum` at load time and compare. No committed lockfile. -- Good, because there is nothing new to commit or maintain in the pipeline repo. -- Bad, because it makes the **registry a runtime dependency** of every plugin load — breaking air-gapped and offline execution, the exact environments where integrity matters most. -- Bad, because the registry becomes both the source of the artifact *and* the source of the trusted hash: a compromised registry can serve a tampered archive with a matching tampered hash and defeat the check entirely (this is exactly the `CompoundVerifier` behavior that already exists today). -- Bad, because it provides no reproducibility anchor — the "expected" value can change out from under a pipeline between runs with no committed record. +- Bad, because it makes the **registry a runtime dependency** of every plugin load — breaking air-gapped and offline execution, the environments where integrity matters most. +- Bad, because the registry becomes both the source of the artifact *and* the source of the trusted hash: a compromised registry defeats the check entirely. +- Bad, because it provides no reproducibility anchor and does nothing for cache poisoning. -### Option B: Extracted directory tree-hash +### Option B: Committed archive-hash lockfile -Compute a hash over the extracted `$id-$version/` directory tree and pin that. +Commit a `plugins.lock` pinning the SHA-512 of each plugin **archive** (the zip), verified on download and (by retaining the zip) on warm-cache runs. -- Good, because it would detect edits to already-extracted files (cache poisoning of the unpacked plugin) — the surface the archive hash cannot see. -- Bad, because tree hashing is heavier: it must walk and hash the full extracted tree, and is sensitive to extraction non-determinism (file ordering, timestamps, permissions, symlinks) across platforms and unzip implementations. -- Bad, because it duplicates the protection the `PluginSecurity` directory guard in PR #7308 already provides for the extracted tree. -- Bad, because it would tempt an implementation to re-extract and re-hash on every run, adding cost to the warm-cache hot path. -- Deferred: the extracted-tree surface is real and is what actually executes, but it is owned by the #7308 guard, not by the lockfile. (Note: a variant of Option B — deriving the lock hash from the extracted tree — is the only way to make verification offline on a pre-existing cache with no retained archive; that is called out where relevant below but remains deferred.) +- Good, because it is a committed, offline, content-addressed anchor for supply-chain integrity and drift. +- Bad, because on a warm cache **the archive is not what executes** — Nextflow runs the extracted tree. Re-hashing a retained zip cannot detect edits to already-unpacked files, so it does **not** cover cache poisoning (the executed code) at all. It would still need PR #7308's directory guard as a complement. +- Bad, because it requires **retaining the archive** next to the extracted tree — roughly doubling cache footprint — and introduces an "archive absent" case (every pre-feature cache, and admin caches that ship only extracted dirs) that produces notices without adding protection. -### Option C: Committed archive-hash lockfile with download-gate and retain-and-re-verify (adopted) +### Option C: Committed extracted-tree-hash lockfile (adopted) -Commit a `plugins.lock` file pinning the SHA-512 of each resolved plugin **archive**, hashed from the archive bytes at generation time. On a cold cache, gate the download against the locked hash. Retain the verified archive in the cache and re-verify it on warm-cache runs. +Commit a `plugins.lock` pinning a canonical SHA-512 over the **extracted `$id-$version/` directory contents**. Before loading a plugin, re-hash its directory and compare to the committed entry. -- Good, because the trusted hash is owned by the pipeline and committed to VCS — the registry is not trusted at verification time. -- Good, because the pinned hash is content-addressed (computed over the bytes), so it survives registry compromise on all runs *after* the lock was honestly generated (trust-on-first-use). -- Good, because it adds a reproducibility / drift anchor the registry cannot silently move. -- Good, because verification of a locally-present archive is network-free. -- Good, because archive hashing is deterministic and cheap (one hash of one zip), with no extraction-ordering pitfalls. -- Bad (accepted), because it requires **retaining** the archive in the cache — a new persisted artifact with size and lifecycle cost (see *Tradeoff: retaining the archive*), and one an attacker can simply ignore. -- Bad (accepted), because on a warm cache the thing that actually executes is the extracted tree, not the retained zip — so re-hashing the zip protects only a *future* re-extraction, not the current run (see *Threat model* and *Known residual*). Warm-run integrity of the executed code rests on the #7308 guard. -- Bad (accepted), because a cache extracted *before* this feature has no retained archive, so its archive cannot be verified offline; the design degrades safely rather than re-downloading (see *Migration and first adoption*). +- Good, because it hashes exactly the bytes that get executed, so a **single** hash covers supply-chain/drift **and** cache poisoning. +- Good, because it uses **no ownership/permission heuristic** — it is silent on any legitimate cache (private, shared read-only, admin service-account) and speaks only on a real content mismatch. This eliminates the false-positive/warning-noise problem of a directory-ownership guard. +- Good, because it needs **no retained archive** (no doubled footprint) and has **no "archive absent" case** — the extracted tree is always present when a plugin loads. +- Good, because verification of a locally-present directory is network-free. +- Bad (accepted), because it must re-hash the extracted tree; for large cloud plugins this cost is non-trivial and motivates the caching optimisation described below. +- Bad (accepted), because, like any lockfile, the first pin is trust-on-first-use — it trusts the artifact present when the coordinate is first seen. ## Solution or decision outcome -**Option C — committed archive-hash lockfile with download-gate and retain-and-re-verify** — is the recommended approach. It places a content-addressed trust anchor in the pipeline repository, keeps verification network-free whenever the archive is locally present, never re-fetches from the registry as a verification remedy, and composes cleanly with the extracted-directory guard from PR #7308 rather than duplicating it. +**Option C — committed extracted-tree-hash lockfile** — is adopted. It places a content-addressed, user-owned trust anchor over the code that actually runs, keeps verification network-free, produces no ownership-based false positives, and with one mechanism covers both the supply-chain surface (Option B's goal) and the cache-poisoning surface (PR #7308's goal). It therefore **supersedes** both the archive-hash approach and the directory-ownership guard. ## Rationale & discussion ### Threat model -The lockfile's genuine contribution (beyond what pf4j's `CompoundVerifier` already does at download time) is: - -- **Removing runtime trust in the registry-supplied hash** — a compromised registry/mirror that serves a tampered archive with a matching tampered hash passes today's `CompoundVerifier`, but fails against the committed content hash. This holds on every run after the lock was honestly generated. -- **Silent version drift** — a coordinate resolving to a different artifact fails against the committed hash. -- **A committed reproducibility baseline** for teams that vendor or mirror plugins. - -What the lockfile does **not** defend: - -- **In-transit corruption** — already caught by `CompoundVerifier`; not a lockfile novelty. -- **A poisoned *extracted* cache** — this is the important honesty point. On a warm cache Nextflow loads and executes the extracted `$id-$version/` tree; the retained zip is not what runs. An attacker who can write to the plugin cache will simply modify the extracted tree and leave the retained zip untouched, and the lockfile check passes while poisoned code executes. Re-hashing the archive therefore provides **essentially zero protection for what actually runs on a warm cache**; it only detects tampering of the archive itself, which matters solely if that archive is later re-extracted. Warm-run integrity of the executed plugin rests **entirely** on the `PluginSecurity` directory guard in **PR #7308**, which is a required complement, not optional. +- **Cache poisoning of the executed code.** A lower-trust user edits the extracted `$id-$version/` tree on a shared/writable cache. Re-hashing the tree at load detects any change to the files, **independently of who owns the directory or its permissions** — so it catches cases an ownership heuristic misses (e.g. a group-writable cache owned by a trusted service account) and never mis-fires on a legitimate read-only shared cache. +- **Registry/mirror compromise and drift.** A tampered archive extracts to a different tree, so its hash no longer matches the committed entry. This holds on every run after the lock was honestly generated. +- **In-transit corruption.** Already caught by `CompoundVerifier`; not a lockfile novelty. -The two features compose: - -- the **lockfile** guarantees the *archive* you obtained (and retained) is the archive you committed to, and gates the cold-cache download and any future re-extraction; -- the **#7308 directory guard** guarantees the *extracted tree* — the code that actually executes — has not been altered after extraction. - -This split is stated plainly: the lockfile must not be marketed as "surviving cache poisoning." It survives archive tampering; the directory guard survives extracted-file tampering. +The first pin of each coordinate is **trust-on-first-use**: it trusts the tree present when first seen (which, on a cold download, was just fetched and checked in transit against the registry hash). Once committed to VCS, every later run is anchored to that reviewed baseline and no longer trusts the registry or the cache. ### Lockfile format and location - **File name**: `plugins.lock`, in the pipeline project root next to `nextflow.config`, committed to VCS. -- **Format**: JSON, chosen for diff-friendliness under code review. +- **Format**: JSON, chosen for diff-friendliness under code review, with a stable (sorted) key order. - **Keying**: by resolved `id@version`. ```json @@ -120,91 +99,75 @@ This split is stated plainly: the lockfile must not be marketed as "surviving ca } ``` -- `sha512` is the hash of the plugin **archive** (the zip), **computed over the downloaded archive bytes**. It is a content hash the pipeline owns, not a re-recording of the registry's `sha512sum`. (No `url` field is stored: it is not available at the point the archive is retained, and it would be provenance-only — never a trust input.) +- `sha512` is a canonical hash of the **extracted directory tree**: for every regular file, in sorted relative-path order, the digest absorbs the relative path and the file bytes (not timestamps or permissions), so it is stable across extractions and platforms while detecting any change to executable content. -### Generation — automatic on first download (trust-on-first-use) +### Generation — automatic on first load (trust-on-first-use) -There is **no dedicated `lock` command**. The lockfile is populated as a side effect of the normal plugin download, exactly as `go` writes `go.sum` and `npm` writes `package-lock.json` on first install: +There is **no dedicated command**. The lockfile is populated as a side effect of loading a plugin, as `go` writes `go.sum` and `npm` writes `package-lock.json`: - The feature is dormant unless a `plugins.lock` file is present. To start, create an empty one (`touch plugins.lock`) and run the pipeline once. -- On a **cold-cache download** the archive is being fetched anyway; Nextflow hashes the retained bytes. If the coordinate is **missing** from the lock, its computed hash is **appended** (trust-on-first-use). Reviewing the resulting diff and committing it establishes the baseline. -- Because the pinned value is computed from the bytes actually received (not copied from registry metadata), it is a genuine content hash — the `go.sum` model — which is what makes the "survives registry compromise" property honest. -- Trust model: TOFU. Pinning trusts the registry at the moment a coordinate is first seen; every run thereafter is anchored to the committed content hash and no longer trusts the registry. This is why the pinned lockfile is meant to be reviewed and committed to VCS. +- The first time a coordinate is loaded and it is **missing** from the lock, its extracted-tree hash is **appended**. Reviewing the resulting diff and committing it establishes the baseline. +- The pinned value is computed from the files on disk, so it is a genuine content hash. -Auto-pinning only ever **adds a missing coordinate**. An entry already present is **never silently rewritten** — a downloaded archive whose hash differs from a committed entry is a verification *failure* (mode-gated below), not a silent update, exactly as `go.sum` refuses to quietly change a recorded hash. Re-pinning a legitimately changed plugin is an explicit action: delete the stale entry and re-run. +Auto-pinning only ever **adds a missing coordinate**. An entry already present is **never silently rewritten** — a plugin whose tree differs from a committed entry is a verification *failure* (mode-gated below), not a silent update, exactly as `go.sum` refuses to quietly change a recorded hash. Re-pinning a legitimately changed plugin is explicit: delete the stale entry and re-run. ### Verification flow (in `PluginUpdater`) -Verification happens during plugin resolution in `PluginUpdater`. **No branch of this flow ever re-fetches from the registry as a remedy** — fetching happens only to obtain a plugin that is genuinely absent from the cache, exactly as today. - -- **Cold cache (download path)**: the archive is downloaded to obtain the plugin regardless of the lockfile. After fetching the zip, compute its SHA-512. If the coordinate is **already locked**, compare against the committed entry — the **lock**, not the registry-supplied `sha512sum`, is authoritative. If the coordinate is **not yet locked**, append it (trust-on-first-use). Retain the zip in the cache (`$id-$version.zip` next to the extracted `$id-$version/` directory) so later runs can re-verify it offline. -- **Warm cache, archive retained (extracted dir + retained zip present)**: re-hash the retained zip against the lock, offline. This gates a *future* re-extraction only; see the threat model for why it does not protect the currently-executing extracted tree. -- **Warm cache, archive absent (extracted dir present, no retained zip)** — the universal state for every cache extracted before this feature (see *Migration and first adoption*): the plugin is already present and functional, so **do not download anything**. Verification of the archive is simply not possible offline for this cache. Behavior is: - - `strict` / `warn`: emit a one-time notice that the archive is unavailable for lockfile verification and that a fresh download (or re-vendoring the archive) is needed to establish an offline-verifiable baseline. **Do not abort** and **do not re-download** — a present, functioning plugin is never failed solely because its archive is missing and cannot be fetched offline. Integrity of the extracted tree for this run is provided by the #7308 directory guard. - - `off`: skip silently. -- **Coordinate not in the lock**: on a cold download it is **auto-pinned** (trust-on-first-use, see *Generation*); on a warm cache with no retained archive there are no bytes to pin, so it is a silent no-op. +Verification happens once per plugin in `load0()`, immediately before `loadPluginFromPath()`, covering both a fresh download and a reused (warm) cache with the same code path. **No branch ever re-fetches from the registry as a remedy** — network access happens only to obtain a plugin genuinely absent from the cache, exactly as today. -Verification of a **locally-present** archive requires no network. The one case where the archive is not present (a pre-feature cache) is handled without any network access, by design — it never falls back to a download. +- Compute the canonical hash of the extracted `$id-$version/` directory. +- If the coordinate is **not in the lock** → append it (trust-on-first-use). +- If it **matches** the committed entry → proceed. +- If it **differs** → mode-gated (below). -#### Known residual (documented, not overclaimed) +Verification of a locally-present directory requires no network. There is no "artifact absent" case: if a plugin is being loaded, its extracted directory exists by definition. -Re-hashing the retained zip proves the **archive** is intact. On a warm run it does **not** protect the code that actually executes: Nextflow runs the already-extracted `$id-$version/` tree, and re-hashing the zip verifies an artifact that is not the thing being run. For a warm run the archive re-verify is therefore effectively inert for the executed code — its only value is gating a subsequent re-extraction. Detection of extracted-file tampering — the integrity of what actually runs — is the responsibility of the `PluginSecurity` directory guard in PR #7308. This residual is expected and by design. +### Performance and the re-hash cost -#### Tradeoff: retaining the archive +Verification re-hashes the extracted directory on every load when a lockfile is present. For small plugins this is negligible next to JVM and Nextflow start-up; for large cloud plugins (bundled SDKs of tens to hundreds of MB) it is material. -Today Nextflow unzips the archive and immediately deletes it (`PluginUpdater.groovy:267-269`); a warm cache returns the extracted dir with no archive (`:259-262`). This ADR changes that for locked plugins by keeping `$id-$version.zip` alongside the extracted directory. Costs and caveats: +The initial implementation performs the full hash each time and does **not** pre-optimise. A follow-up optimisation, added only once the cost is shown to matter, avoids the re-hash when nothing changed: -- **Cache size**: roughly doubles on-disk footprint per plugin (compressed archive + extracted tree). Acceptable for the reproducibility/offline benefit; could be scoped to locked plugins only. -- **Lifecycle**: the retained zip must be cleaned up with the plugin directory and re-written on re-download. -- **Attacker can ignore it**: retaining the zip adds no protection for the running code — an attacker edits the extracted tree and leaves the zip untouched, and the check still passes. This is precisely why the #7308 directory guard is a required complement. +- Keep a **local, non-committed** sidecar (e.g. under `$NXF_PLUGINS_DIR`) recording, per plugin, the last verified tree hash and a cheap fingerprint of the tree — the set of `(relative-path, size, mtime)`. +- On load, stat the files (no reads) and rebuild the fingerprint; if it is unchanged and the lock entry is unchanged, reuse the last verified result and skip the full hash. +- **Safety of the shortcut:** `mtime`/`size` are forgeable by anyone who can write the files, so the fingerprint shortcut is trusted **only when the cache cannot change under you** — i.e. a directory you own that is not writable by group or others. On a shared/writable cache the fingerprint is not trusted and the full hash always runs. Ownership is thus used to select the *strategy* (fast vs full), never to emit a warning — so the noise problem of an ownership guard does not reappear. -### Migration and first adoption - -Because current code deletes the archive right after extraction (`PluginUpdater.groovy:267-269`) and warm-cache resolution returns the extracted directory with no archive (`:259-262`), **every plugin cache that predates this feature has no retained archive**. This is the *universal initial state* on first adoption, not an edge case. - -Consequences, stated plainly: - -- On the first lockfile-enabled run against a pre-existing cache — including an air-gapped one — plugins are already extracted and functional. The archive-absent branch above applies: Nextflow does **not** re-download, does **not** abort in strict mode, and simply notes that an offline-verifiable archive baseline has not yet been established. No network is required and no false abort occurs. -- An offline-verifiable archive baseline is established the next time each plugin is downloaded through the gate (cold cache) once a `plugins.lock` file exists, at which point the archive is retained and the coordinate pinned. -- **Air-gapped teams that vendor or mirror plugins** typically ship the extracted `$id-$version/` directories, not the `.zip` archives. To get *archive-level* offline verification in such an environment, the archives must be vendored too — i.e. the mirror/vendor step must include the retained `$id-$version.zip` files (produced by running the pipeline once against the registry in a connected environment, then committing/shipping the archives alongside the lockfile). Absent that, air-gapped runs fall into the archive-absent branch and rely on the #7308 directory guard for the extracted tree — which is safe and network-free, but is not archive verification. The only alternative that would make archive-free caches offline-verifiable is deriving the lock hash from the extracted tree (deferred Option B). +This keeps steady-state launches on a private cache nearly free, runs the full hash exactly where tampering is possible, and stays silent in all cases except a genuine mismatch. ### Modes and rollout -- **Opt-in by presence**: if no `plugins.lock` exists, the feature is dormant and behavior is unchanged — zero breakage for existing pipelines. -- When the file exists, behavior is controlled by `NXF_PLUGINS_LOCK_MODE`, reusing the `warn`/`strict`/`off` tri-state plumbing introduced by PR #7308's `NXF_PLUGINS_STRICT_MODE`, but **independent** from it (the two knobs can be set separately). +- **Opt-in by presence**: no `plugins.lock` → dormant, behaviour unchanged, zero breakage. +- When the file exists, mismatch behaviour is controlled by `NXF_PLUGINS_LOCK_MODE`, reusing the `warn`/`strict`/`off` tri-state plumbing pattern from PR #7308's `NXF_PLUGINS_STRICT_MODE`, but independent from it. -Mode gates only the **hash-mismatch** outcome. A coordinate *missing* from the lock is auto-pinned (see *Generation*), not gated; an *absent* archive is never gated (never aborts). +| Mode | Tree hash mismatch (locked entry) | Coordinate missing from lock | +|------|-----------------------------------|------------------------------| +| `strict` | **Abort** with a re-pin hint | Auto-pin (trust-on-first-use) | +| `warn` (default when the file is present) | Log a warning once and proceed | Auto-pin | +| `off` | Skip verification | Auto-pin | -| Mode | Hash mismatch (locked entry) | Coordinate missing from lock | Archive absent (pre-feature cache) | -|------|------------------------------|------------------------------|-------------------------------------| -| `strict` | **Abort** with a re-pin hint | Auto-pin (cold) / no-op (warm) | Notice only, proceed (never abort) | -| `warn` (default when the file is present) | Log a warning once and proceed | Auto-pin (cold) / no-op (warm) | Notice only, proceed | -| `off` | Skip verification | Auto-pin (cold) / no-op (warm) | Skip silently | +Mode gates only the **mismatch** outcome. A coordinate missing from the lock is auto-pinned, not gated. Default is `warn` for a friendly rollout; `strict` is opt-in for teams that want a fail-closed guarantee. Unlike a directory-ownership guard, **either default is silent in normal operation** — a mismatch is a real, rare, actionable event, not per-run noise on healthy caches. -**Default is `warn` when a lockfile is present**, deliberately mirroring PR #7308's warn-first rollout philosophy (`NXF_PLUGINS_STRICT_MODE` defaults to `warn`). This was a considered choice: an earlier draft proposed `strict`-when-present on the reasoning that a committed lockfile expresses intent to enforce. That was rejected because it diverges from the #7308 rollout philosophy this feature otherwise mirrors, and because it produces a surprising hard failure in the one gated case — a plugin whose committed lock entry is legitimately stale (e.g. the archive was re-released) would hard-abort until the entry is deleted and re-pinned. `warn`-by-default surfaces the drift without breaking the run; teams that want enforcement opt into `strict` explicitly (a staged `warn` → `strict` rollout), exactly as with #7308. (Note the auto-pin model already removes the most common friction: a *new or bumped* coordinate is pinned on first download rather than aborting, even in `strict`.) +### Relationship to PR #7308 -Independently of mode, a **present and functioning plugin is never aborted solely because its archive is missing** and cannot be fetched offline (the archive-absent column above) — this avoids a false-positive failure on precisely the air-gapped caches the feature is meant to serve. +PR #7308 proposed a `PluginSecurity` directory guard that flags a plugin directory as untrusted when it is foreign-owned or world-writable, to defend the extracted tree on shared caches. This ADR's extracted-tree hash defends the **same** surface by content instead of by ownership, which is both stronger (it catches any modification regardless of ownership, including the group-writable-trusted-owner case the heuristic misses) and quieter (it never warns on a legitimate shared/read-only cache). It also covers the supply-chain surface the guard did not. This mechanism therefore **supersedes PR #7308**, which can be closed in its favour. The zero-config baseline for the extracted-tree surface remains the operational guidance to keep the plugin cache private (per-user); the lockfile is the committed, enforceable layer on top for teams that want it. ### Reused and new components | Component | Module | Change | |-----------|--------|--------| | `PluginLockFile` (new) | nf-commons | Read/write/round-trip of `plugins.lock`; blank/malformed-file handling | -| `PluginLockVerifier` (new) | nf-commons | Auto-pin on first download (TOFU); re-verify retained archive; archive-absent handling; mode gating | -| `PluginUpdater` | nf-commons | Cold-cache: retain zip + verify/pin; warm-cache: re-verify retained zip. No dedicated command | +| `PluginLockVerifier` (new) | nf-commons | Canonical extracted-tree hash; auto-pin on first load (TOFU); mismatch mode gating | +| `PluginUpdater` | nf-commons | Verify/pin the extracted directory in `load0()` before loading. No retained archive, no dedicated command | | `HttpPluginRepository` | nf-commons | No change — `CompoundVerifier` still checks downloads against the registry `sha512sum` at fetch time | | `NXF_PLUGINS_LOCK_MODE` | nf-commons | New env var, tri-state (`strict`/`warn`/`off`), default `warn`, independent of `NXF_PLUGINS_STRICT_MODE` | -### Relationship to PR #7308 - -PR #7308 introduces the `PluginSecurity` directory guard, which protects the **extracted** plugin directory — the code that actually executes — and the `NXF_PLUGINS_STRICT_MODE` tri-state plumbing (default `warn`). This ADR's lockfile is the **sibling** feature protecting the **archive**, reusing the same mode-plumbing pattern and the same warn-first default under a separate, independent switch. Together they cover both integrity surfaces — archive and extracted tree — without either overclaiming the other's protection. Critically, warm-run integrity of executing code depends on the #7308 guard; the lockfile does not substitute for it. - ## Testing -- **`PluginLockFile`**: read, write, and round-trip of `plugins.lock`; blank/`touch`ed file parses as empty; malformed file throws. -- **Auto-pin (TOFU)**: an enabled but empty lock, given a downloaded archive for an unlocked coordinate, appends the **byte-computed** hash to the file on disk; a dormant (no file) verifier never pins. +- **`PluginLockFile`**: read, write, round-trip; blank/`touch`ed file parses as empty; malformed file throws. +- **`sha512Tree`**: identical trees in different locations hash equal; any file-content change changes the hash; output is a 128-char lowercase hex digest. +- **Auto-pin (TOFU)**: an enabled but empty lock, given an extracted directory for an unlocked coordinate, appends the tree hash to the file on disk; a dormant (no file) verifier never pins. - **Verification**: - - matching hash passes; + - matching tree passes; - mismatch **aborts** in `strict`, **warns once** in `warn`, is **ignored** in `off`; - - **archive-absent (pre-feature cache)**: with a locked coordinate but no retained zip, the run proceeds in `strict` with a notice, performs **no download**, and never aborts. -- **No-network guarantee**: verification and pinning operate purely on locally-present bytes; no branch re-fetches from the registry as a remedy. + - **cache poisoning**: pinning a good tree then editing an extracted file in place is detected as a mismatch — regardless of directory ownership. +- **No-network guarantee**: verification and pinning operate purely on local files; no branch re-fetches from the registry. diff --git a/docs/plugins/using-plugins.mdx b/docs/plugins/using-plugins.mdx index 89972d1294..fe8d04c888 100644 --- a/docs/plugins/using-plugins.mdx +++ b/docs/plugins/using-plugins.mdx @@ -64,7 +64,7 @@ The plugin cache is shared across pipelines and is not access-controlled. On mul -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. +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. 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: @@ -72,13 +72,13 @@ The lockfile is populated automatically, like `go.sum` or `package-lock.json` touch plugins.lock ``` -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. +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. -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. +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. -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. +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. -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. +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. ## Offline usage diff --git a/docs/reference/env-vars.mdx b/docs/reference/env-vars.mdx index 58ed715376..da625abe5f 100644 --- a/docs/reference/env-vars.mdx +++ b/docs/reference/env-vars.mdx @@ -214,7 +214,7 @@ The path where the plugin archives are loaded and stored (default: `$NXF_HOME/pl -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. +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. ##### `NXF_PLUGINS_REGISTRY_URL` diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy index 25a9c48695..28fa532801 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginLockVerifier.groovy @@ -16,8 +16,11 @@ package nextflow.plugin +import java.nio.file.FileVisitResult import java.nio.file.Files import java.nio.file.Path +import java.nio.file.SimpleFileVisitor +import java.nio.file.attribute.BasicFileAttributes import java.security.MessageDigest import groovy.transform.CompileStatic @@ -26,30 +29,26 @@ import nextflow.SysEnv import nextflow.exception.AbortOperationException /** - * Verifies plugin archives against the {@code plugins.lock} file. + * Verifies an extracted plugin directory against the {@code plugins.lock} file. * * The feature is opt-in by the presence of the lock file: it is dormant (a no-op) when no - * {@code plugins.lock} exists. When the file is present, the sha512 of the retained plugin archive - * is re-computed locally (no network) and compared to the committed lock entry. + * {@code plugins.lock} exists. When the file is present, a canonical hash of the extracted + * {@code $id-$version/} directory - the code that Nextflow actually loads and executes - is + * re-computed locally (no network) and compared to the committed lock entry. Because it hashes the + * unpacked tree rather than the archive, it detects both a tampered/compromised download and a + * poisoned cache directory (a lower-trust user editing already-extracted files on a shared cache). * - * Following the {@code go.sum} / {@code package-lock.json} model, the lock is populated - * automatically: the first time a coordinate is downloaded and it is missing from the lock, its - * archive checksum is appended (trust-on-first-use). An existing entry is never rewritten silently - * — a mismatch against a committed entry is a verification failure. - * - * The behaviour on a checksum mismatch is gated by the {@code NXF_PLUGINS_LOCK_MODE} environment - * variable: + * Following the {@code go.sum} / {@code package-lock.json} model the lock is populated + * automatically: the first time a coordinate is seen and it is missing from the lock, its tree + * hash is appended (trust-on-first-use). An existing entry is never rewritten silently — a + * mismatch against a committed entry is a verification failure, gated by + * {@code NXF_PLUGINS_LOCK_MODE}: *
    *
  • {@code strict} - abort with an {@link AbortOperationException}
    • *
  • {@code warn} (default) - log a warning once per coordinate and proceed
    • *
  • {@code off} - skip verification silently
    • *
* - * A locked plugin whose archive is not available locally (e.g. a cache extracted before this - * feature existed) is never aborted: it cannot be verified offline and its archive is never - * re-downloaded just to verify it. Integrity of the extracted code that actually runs is the - * responsibility of the plugin directory guard, not of this archive check. - * * @author Paolo Di Tommaso */ @Slf4j @@ -103,46 +102,39 @@ class PluginLockVerifier { } /** - * Verify - and, on first sight, pin - a plugin archive against the lock. + * Verify - and, on first sight, pin - an extracted plugin directory against the lock. * * @param fqid The plugin fully-qualified id ie. {@code id@version} - * @param zip The retained plugin archive; may be {@code null} or missing + * @param pluginDir The extracted plugin directory ({@code $id-$version/}) */ - void verify(String fqid, Path zip) { + void verify(String fqid, Path pluginDir) { if( !enabled ) return - final entry = lock.getEntry(fqid) - final present = zip != null && Files.exists(zip) - - // coordinate not yet locked: pin it on first download (trust-on-first-use), never fail - if( entry == null ) { - if( present ) - pin(fqid, sha512(zip)) - else - log.debug "Plugin '$fqid' is not in the plugins lock file and its archive is not available to pin" + if( pluginDir == null || !Files.isDirectory(pluginDir) ) { + log.debug "Cannot verify plugin '$fqid' against the plugins lock file - directory not available: $pluginDir" return } - // locked, but the archive is not available (e.g. a cache created before this feature): - // it cannot be verified offline - never abort and never re-download to verify - if( !present ) { - if( getMode() != Mode.OFF && notified.add(fqid) ) - log.warn "Cannot verify plugin '$fqid' against the plugins lock file - its archive is not available in the cache" + final actual = sha512Tree(pluginDir) + final entry = lock.getEntry(fqid) + + // coordinate not yet locked: pin it on first sight (trust-on-first-use), never fail + if( entry == null ) { + pin(fqid, actual) return } - - // verify the retained archive against the committed checksum - final actual = sha512(zip) + // matches the committed hash if( actual == entry.sha512 ) return - final reason = "Plugin '$fqid' checksum does not match the plugins lock file\n- expected: ${entry.sha512}\n- actual : ${actual}" + // mismatch: the extracted plugin differs from what the lock pinned + final reason = "Plugin '$fqid' does not match the plugins lock file\n- expected: ${entry.sha512}\n- actual : ${actual}" switch( getMode() ) { case Mode.OFF: log.debug "Plugins lock verification failed (ignored, mode=off) - $reason" break case Mode.STRICT: - 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") + throw new AbortOperationException("$reason\n- delete the entry from the plugins lock file and re-run to re-pin, or restore the expected plugin") case Mode.WARN: // warn only once per coordinate to avoid log spam if( notified.add(fqid) ) @@ -154,7 +146,7 @@ class PluginLockVerifier { /** * Append a new entry to the lock and persist it (trust-on-first-use). Existing entries are * never overwritten by this path — {@link #verify} routes an already-locked coordinate through - * the checksum comparison instead. + * the hash comparison instead. */ private synchronized void pin(String fqid, String sha512) { lock.addEntry(fqid, new PluginLockFile.Entry(sha512)) @@ -164,18 +156,35 @@ class PluginLockVerifier { } /** - * Compute the sha512 hex digest of the given file. + * Compute a canonical sha512 digest over the content of a directory tree. The digest covers, + * for every regular file (visited in sorted relative-path order for determinism), its relative + * path and its bytes - not timestamps or permissions - so it is stable across extractions and + * platforms while still detecting any change to the files that will be executed. * - * @param file The file to hash + * @param dir The directory to hash * @return The lowercase hex-encoded sha512 digest (128 chars) */ - static String sha512(Path file) { + static String sha512Tree(Path dir) { final md = MessageDigest.getInstance('SHA-512') - try (InputStream is = Files.newInputStream(file)) { - final buffer = new byte[8192] - int read - while( (read = is.read(buffer)) != -1 ) - md.update(buffer, 0, read) + // TreeMap keyed by relative path -> deterministic, sorted iteration order + final files = new TreeMap() + Files.walkFileTree(dir, new SimpleFileVisitor() { + @Override + FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { + files.put(dir.relativize(file).toString().replace('\\', '/'), file) + return FileVisitResult.CONTINUE + } + }) + final buffer = new byte[8192] + for( Map.Entry it : files.entrySet() ) { + md.update(it.key.getBytes('UTF-8')) + md.update((byte) 0) + try (InputStream is = Files.newInputStream(it.value)) { + int read + while( (read = is.read(buffer)) != -1 ) + md.update(buffer, 0, read) + } + md.update((byte) 0) } return HexFormat.of().formatHex(md.digest()) } diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy index 34cf6813fc..73482782c4 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy @@ -253,7 +253,7 @@ class PluginUpdater extends UpdateManager { } /** - * The {@code plugins.lock} file location used to verify downloaded plugin artifacts. + * The {@code plugins.lock} file location used to verify plugins against their pinned hash. * Defaults to a {@code plugins.lock} file in the current working directory. */ protected Path lockFilePath() { @@ -269,13 +269,6 @@ class PluginUpdater extends UpdateManager { return lockVerifier } - /** - * @return the path to the retained plugin zip artifact used for lock verification - */ - private Path retainedZip(String id, String version) { - return pluginsStore.resolve("${id}-${version}.zip") - } - private Path download0(String id, String version) { // 0. check if version is specified if( !version ) @@ -291,21 +284,9 @@ class PluginUpdater extends UpdateManager { // 2. download to temporary location Path downloaded = safeDownloadPlugin(id, version); - // 3. unzip the content + // 3. unzip the content and delete downloaded file Path dir = FileUtils.expandIfZip(downloaded) - - // 3.1 when the plugins lock is enabled retain the downloaded zip next to the extracted - // dir and verify (or pin) its checksum against the lock (cold cache). Otherwise delete it. - if( getLockVerifier().isEnabled() ) { - final retained = retainedZip(id, version) - FileHelper.deletePath(retained) - FileHelper.copyPath(downloaded, retained) - FileHelper.deletePath(downloaded) - getLockVerifier().verify("${id}@${version}", retained) - } - else { - FileHelper.deletePath(downloaded) - } + FileHelper.deletePath(downloaded) // 4. move the final destination the plugin directory assert pluginPath.getFileName() == dir.getFileName() @@ -434,17 +415,16 @@ class PluginUpdater extends UpdateManager { if( !FilesEx.exists(pluginPath) ) { pluginPath = safeDownload(id, version) } - else { - // warm cache: re-hash the retained artifact and verify it against the plugins lock - // (missing retained zip is treated as a lock-miss by the verifier) - getLockVerifier().verify("${id}@${version}", retainedZip(id, version)) - } // verify the plugin install path contains the expected manifest path if( !FilesEx.exists(pluginPath.resolve('classes/META-INF/MANIFEST.MF')) ) { log.warn("Plugin '${pluginPath.getFileName()}' installation looks corrupted - Delete the following directory and run nextflow again: $pluginPath") } + // verify (or pin) the extracted plugin directory against the plugins lock, before loading + // and executing its code. This covers both a fresh download and a reused (warm) cache. + getLockVerifier().verify("${id}@${version}", pluginPath) + // load the plugin from the file system PluginWrapper wrapper = pluginManager.loadPluginFromPath(pluginPath) diff --git a/modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy b/modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy index 8cca290629..3312210e70 100644 --- a/modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy +++ b/modules/nf-commons/src/test/nextflow/plugin/PluginLockVerifierTest.groovy @@ -30,10 +30,15 @@ import spock.lang.Unroll */ class PluginLockVerifierTest extends Specification { - private Path zipWith(String content) { - final zip = Files.createTempFile('plugin', '.zip') - Files.write(zip, content.getBytes('UTF-8')) - return zip + /** Create an extracted-plugin-like directory tree with the given relative-path->content files */ + private Path pluginDir(Map files) { + final dir = Files.createTempDirectory('plugin') + files.each { rel, content -> + final f = dir.resolve(rel) + Files.createDirectories(f.parent) + Files.write(f, content.getBytes('UTF-8')) + } + return dir } /** Write a lock file holding the given fqid->sha512 entries and return its path */ @@ -68,6 +73,28 @@ class PluginLockVerifierTest extends Specification { 'nonsense' | PluginLockVerifier.Mode.WARN } + // --------------------------------------------------------------------------- + // tree hashing + + def 'sha512Tree should be deterministic and content-sensitive' () { + given: + def a = pluginDir(['classes/A.class': 'aaa', 'META-INF/MANIFEST.MF': 'mmm']) + def b = pluginDir(['classes/A.class': 'aaa', 'META-INF/MANIFEST.MF': 'mmm']) + def c = pluginDir(['classes/A.class': 'aaa', 'META-INF/MANIFEST.MF': 'CHANGED']) + + expect: + // identical content in different dirs -> identical hash + PluginLockVerifier.sha512Tree(a) == PluginLockVerifier.sha512Tree(b) + // any content change -> different hash + PluginLockVerifier.sha512Tree(a) != PluginLockVerifier.sha512Tree(c) + and: + PluginLockVerifier.sha512Tree(a).length() == 128 + PluginLockVerifier.sha512Tree(a) ==~ /[0-9a-f]{128}/ + + cleanup: + [a, b, c].each { it.deleteDir() } + } + // --------------------------------------------------------------------------- // dormant / enabled @@ -75,20 +102,20 @@ class PluginLockVerifierTest extends Specification { given: SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) and: - def zip = zipWith('hello plugin') + def dir = pluginDir(['classes/A.class': 'aaa']) def verifier = new PluginLockVerifier(Path.of('/no/such/plugins.lock')) expect: !verifier.isEnabled() when: - verifier.verify('nf-foo@1.0.0', zip) + verifier.verify('nf-foo@1.0.0', dir) then: noExceptionThrown() cleanup: SysEnv.pop() - Files.deleteIfExists(zip) + dir.deleteDir() } def 'should be enabled when an (even empty) lock file exists' () { @@ -110,15 +137,15 @@ class PluginLockVerifierTest extends Specification { given: SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) and: - def zip = zipWith('hello plugin') - final sha = PluginLockVerifier.sha512(zip) + def dir = pluginDir(['classes/A.class': 'hello']) + final sha = PluginLockVerifier.sha512Tree(dir) and: // an empty but present lock file -> feature enabled, nothing pinned yet def lockPath = lockFileWith([:]) def verifier = new PluginLockVerifier(lockPath) when: - verifier.verify('nf-foo@1.0.0', zip) + verifier.verify('nf-foo@1.0.0', dir) then: noExceptionThrown() @@ -129,147 +156,119 @@ class PluginLockVerifierTest extends Specification { cleanup: SysEnv.pop() - Files.deleteIfExists(zip) + dir.deleteDir() Files.deleteIfExists(lockPath) } - def 'should not pin when dormant' () { - given: - def zip = zipWith('hello plugin') - def verifier = new PluginLockVerifier(Path.of('/no/such/plugins.lock')) - - when: - verifier.verify('nf-foo@1.0.0', zip) - - then: - !verifier.isEnabled() - noExceptionThrown() - - cleanup: - Files.deleteIfExists(zip) - } - // --------------------------------------------------------------------------- // verification of an already-locked coordinate - def 'should pass verification when the artifact matches the lock' () { + def 'should pass verification when the plugin matches the lock' () { given: SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) and: - def zip = zipWith('hello plugin') - final sha = PluginLockVerifier.sha512(zip) + def dir = pluginDir(['classes/A.class': 'hello']) + final sha = PluginLockVerifier.sha512Tree(dir) def lockPath = lockFileWith(['nf-foo@1.0.0': sha]) def verifier = new PluginLockVerifier(lockPath) when: - verifier.verify('nf-foo@1.0.0', zip) + verifier.verify('nf-foo@1.0.0', dir) then: noExceptionThrown() cleanup: SysEnv.pop() - Files.deleteIfExists(zip) + dir.deleteDir() Files.deleteIfExists(lockPath) } - def 'strict mode should abort on a checksum mismatch' () { + def 'strict mode should abort when the extracted plugin does not match' () { given: SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) and: - def zip = zipWith('hello plugin') + def dir = pluginDir(['classes/A.class': 'poisoned']) def lockPath = lockFileWith(['nf-foo@1.0.0': 'deadbeef']) def verifier = new PluginLockVerifier(lockPath) when: - verifier.verify('nf-foo@1.0.0', zip) + verifier.verify('nf-foo@1.0.0', dir) then: def e = thrown(AbortOperationException) - e.message.contains('checksum does not match') + e.message.contains('does not match') cleanup: SysEnv.pop() - Files.deleteIfExists(zip) + dir.deleteDir() Files.deleteIfExists(lockPath) } - def 'warn mode should not abort on a checksum mismatch' () { + def 'warn mode should not abort on a mismatch' () { given: SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'warn']) and: - def zip = zipWith('hello plugin') + def dir = pluginDir(['classes/A.class': 'poisoned']) def lockPath = lockFileWith(['nf-foo@1.0.0': 'deadbeef']) def verifier = new PluginLockVerifier(lockPath) when: - verifier.verify('nf-foo@1.0.0', zip) + verifier.verify('nf-foo@1.0.0', dir) // second call to exercise the "log once" path - verifier.verify('nf-foo@1.0.0', zip) + verifier.verify('nf-foo@1.0.0', dir) then: noExceptionThrown() cleanup: SysEnv.pop() - Files.deleteIfExists(zip) + dir.deleteDir() Files.deleteIfExists(lockPath) } - def 'off mode should ignore a checksum mismatch' () { + def 'off mode should ignore a mismatch' () { given: SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'off']) and: - def zip = zipWith('hello plugin') + def dir = pluginDir(['classes/A.class': 'poisoned']) def lockPath = lockFileWith(['nf-foo@1.0.0': 'deadbeef']) def verifier = new PluginLockVerifier(lockPath) when: - verifier.verify('nf-foo@1.0.0', zip) + verifier.verify('nf-foo@1.0.0', dir) then: noExceptionThrown() cleanup: SysEnv.pop() - Files.deleteIfExists(zip) + dir.deleteDir() Files.deleteIfExists(lockPath) } - // --------------------------------------------------------------------------- - // archive-absent (e.g. a cache extracted before this feature): never abort - - def 'strict mode should NOT abort when a locked artifact is unavailable' () { + def 'should catch cache poisoning regardless of directory ownership' () { given: + // pin the good tree, then tamper an extracted file in place (the shared-cache attack) SysEnv.push([NXF_PLUGINS_LOCK_MODE: 'strict']) and: - final missing = Path.of('/no/such/nf-foo-1.0.0.zip') - def lockPath = lockFileWith(['nf-foo@1.0.0': 'abc']) + def dir = pluginDir(['classes/A.class': 'good', 'lib/x.jar': 'jar']) + final good = PluginLockVerifier.sha512Tree(dir) + def lockPath = lockFileWith(['nf-foo@1.0.0': good]) def verifier = new PluginLockVerifier(lockPath) + and: + // attacker edits an already-extracted file + Files.write(dir.resolve('classes/A.class'), 'evil'.getBytes('UTF-8')) when: - verifier.verify('nf-foo@1.0.0', missing) + verifier.verify('nf-foo@1.0.0', dir) then: - noExceptionThrown() + thrown(AbortOperationException) cleanup: SysEnv.pop() + dir.deleteDir() Files.deleteIfExists(lockPath) } - - def 'sha512 should compute a 128-char lowercase hex digest' () { - given: - def zip = zipWith('some bytes') - - when: - final sha = PluginLockVerifier.sha512(zip) - - then: - sha.length() == 128 - sha ==~ /[0-9a-f]{128}/ - - cleanup: - Files.deleteIfExists(zip) - } }