From ad72e21a3444b6a52cb1f6340f8b41801ba99537 Mon Sep 17 00:00:00 2001 From: Oussama El Azizi Date: Wed, 8 Jul 2026 10:53:18 +0200 Subject: [PATCH 1/6] Add plugin directory trust guard to harden shared plugin caches Nextflow loads and executes plugin code from the local plugin cache ($id-$version) without re-verifying its origin when the directory already exists. On a shared, writable plugin cache (NXF_PLUGINS_DIR or $NXF_HOME/plugins), a lower-trust local user can pre-populate an official pinned plugin coordinate with attacker-controlled code that then runs in the victim's Nextflow process. The plugin cache is expected to be a per-user, private trust boundary (default $HOME/.nextflow/plugins). This adds a defense-in-depth guard that verifies a plugin directory is trusted before loading it: the directory must be owned by the current user or root AND not writable by group or others. This accepts the two legitimate shapes (private user cache; an admin-managed read-only shared cache) while rejecting an attacker-owned or group/world-writable directory. Behaviour is controlled by NXF_PLUGINS_STRICT_MODE: warn (default) logs a warning and continues, strict aborts the run, off disables the check. The check is a no-op on filesystems without POSIX permissions and in dev mode (where plugins load from the source tree, not the cache). Co-Authored-By: Claude Opus 4.8 Signed-off-by: Oussama El Azizi --- docs/plugins/using-plugins.mdx | 6 + docs/reference/env-vars.mdx | 6 + .../nextflow/plugin/PluginSecurity.groovy | 116 ++++++++++++++ .../main/nextflow/plugin/PluginUpdater.groovy | 4 + .../main/nextflow/plugin/PluginsFacade.groovy | 6 + .../nextflow/plugin/PluginSecurityTest.groovy | 141 ++++++++++++++++++ 6 files changed, 279 insertions(+) create mode 100644 modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy create mode 100644 modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy diff --git a/docs/plugins/using-plugins.mdx b/docs/plugins/using-plugins.mdx index 82a370749c..ec9af0f597 100644 --- a/docs/plugins/using-plugins.mdx +++ b/docs/plugins/using-plugins.mdx @@ -56,6 +56,12 @@ 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). +:::warning +The plugin cache should be a private, per-user directory that is not writable by other users. Nextflow loads and executes cached plugin code without re-verifying its origin, so a shared plugin cache that is writable by other users (or owned by another user) allows a lower-trust user to replace cached plugin code with their own, which would then run inside your Nextflow process. + +Keep `NXF_PLUGINS_DIR` private (for example `chmod 700`) and owned by you. If a shared cache is required, use an administrator-owned, **read-only** directory that regular users cannot modify. Use [`NXF_PLUGINS_STRICT_MODE`](../reference/env-vars.mdx) to control whether Nextflow warns about or refuses to load plugins from an untrusted directory. +::: + ## 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. diff --git a/docs/reference/env-vars.mdx b/docs/reference/env-vars.mdx index a4b4b1a3c3..a77d18ba9b 100644 --- a/docs/reference/env-vars.mdx +++ b/docs/reference/env-vars.mdx @@ -216,6 +216,12 @@ The path where the plugin archives are loaded and stored (default: `$NXF_HOME/pl Specifies the URL of the plugin registry used to download and resolve plugins. This allows using custom or private plugin registries instead of the default public registry. +##### `NXF_PLUGINS_STRICT_MODE` + + + +Controls how Nextflow reacts when the plugins directory (`NXF_PLUGINS_DIR`) or a cached plugin directory is not a trusted location — i.e. it is owned by another user or writable by group or others. Such a directory could be modified by other users, allowing untrusted plugin code to run in your Nextflow process. Allowed values are `warn` (default, logs a warning and continues), `strict` (aborts the run) and `off` (disables the check). The check is skipped on filesystems that do not support POSIX permissions. + ##### `NXF_PLUGINS_TEST_REPOSITORY` diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy new file mode 100644 index 0000000000..6b80808595 --- /dev/null +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy @@ -0,0 +1,116 @@ +/* + * 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.nio.file.attribute.PosixFileAttributeView +import java.nio.file.attribute.PosixFileAttributes +import java.nio.file.attribute.PosixFilePermission +import java.nio.file.attribute.PosixFilePermissions + +import groovy.transform.CompileStatic +import groovy.util.logging.Slf4j +import nextflow.SysEnv +import nextflow.exception.AbortOperationException +/** + * Validate that a plugin directory can be trusted before its content is loaded and + * executed inside the Nextflow process. + * + * The plugin cache ({@code NXF_PLUGINS_DIR}, {@code $NXF_HOME/plugins}) is expected to be a + * per-user, private trust boundary. When it is shared and writable by other users, a lower-trust + * user can pre-populate an official pinned plugin coordinate (e.g. {@code nf-amazon-3.10.0}) with + * attacker-controlled code that Nextflow would then load from the cache without any origin check. + * + * A directory is considered trusted only when it is owned by the current user (or root) AND it is + * not writable by group or others. This accepts the two legitimate deployment shapes - a private + * user cache (e.g. {@code 0700}) and an admin-managed read-only shared cache (e.g. {@code root:root + * 0755}) - while rejecting an attacker-owned directory or a group/world-writable cache. + * + * The behaviour is controlled by the {@code NXF_PLUGINS_STRICT_MODE} environment variable: + * {@code warn} (default) logs a warning and continues, {@code strict} aborts the run, and + * {@code off} disables the check. On filesystems that do not support POSIX attributes the check is + * a no-op. + * + * @author Claude + */ +@Slf4j +@CompileStatic +class PluginSecurity { + + static final String MODE_WARN = 'warn' + static final String MODE_STRICT = 'strict' + static final String MODE_OFF = 'off' + + /** + * Resolve the strict mode from the {@code NXF_PLUGINS_STRICT_MODE} environment variable. + * + * @return one of {@code warn} (default), {@code strict} or {@code off} + */ + static String getMode() { + return SysEnv.get('NXF_PLUGINS_STRICT_MODE', MODE_WARN) + } + + /** + * Verify that the given plugin directory can be trusted, using the mode resolved from the + * environment. + * + * @param dir The plugin directory (the plugins root or a {@code $id-$version} directory) + */ + static void checkTrustedDir(Path dir) { + checkTrustedDir(dir, getMode()) + } + + /** + * Verify that the given plugin directory can be trusted. + * + * @param dir The plugin directory (the plugins root or a {@code $id-$version} directory) + * @param mode The strict mode: {@code warn}, {@code strict} or {@code off} + */ + static void checkTrustedDir(Path dir, String mode) { + if( mode == MODE_OFF || dir == null ) + return + try { + final view = Files.getFileAttributeView(dir, PosixFileAttributeView) + // non-POSIX filesystem (e.g. Windows, some object stores) - nothing to check + if( view == null ) + return + final PosixFileAttributes attrs = view.readAttributes() + final perms = attrs.permissions() + final owner = attrs.owner().name + final me = System.getProperty('user.name') + + final writableByOthers = perms.contains(PosixFilePermission.GROUP_WRITE) || perms.contains(PosixFilePermission.OTHERS_WRITE) + final untrustedOwner = owner != me && owner != 'root' + + if( writableByOthers || untrustedOwner ) { + final msg = "Plugin directory '${dir}' is not secure (owner=${owner}, mode=${PosixFilePermissions.toString(perms)}) " + + "- it may be modified by other users, which could allow untrusted plugin code to run in your Nextflow process" + if( mode == MODE_STRICT ) + throw new AbortOperationException("${msg} -- refusing to load plugins. Use a private directory (chmod 700) owned by you, or set NXF_PLUGINS_STRICT_MODE=warn to override") + log.warn "${msg} -- set NXF_PLUGINS_STRICT_MODE=strict to refuse loading, or use a private plugins directory owned by you" + } + } + catch( AbortOperationException e ) { + throw e + } + catch( Exception e ) { + // never let a permission-inspection failure break a run + log.debug "Unable to verify plugin directory permissions for '${dir}' - ${e.message}" + } + } +} diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy index e261563ed1..3a05cb12ed 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginUpdater.groovy @@ -401,6 +401,10 @@ class PluginUpdater extends UpdateManager { log.warn("Plugin '${pluginPath.getFileName()}' installation looks corrupted - Delete the following directory and run nextflow again: $pluginPath") } + // verify the plugin dir is a trusted (private, non-shared) location before loading it, + // to avoid executing attacker-controlled content from a shared/writable plugin cache + PluginSecurity.checkTrustedDir(pluginPath) + // load the plugin from the file system PluginWrapper wrapper = pluginManager.loadPluginFromPath(pluginPath) diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginsFacade.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginsFacade.groovy index f5a99c7c59..af9dde2bc3 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginsFacade.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginsFacade.groovy @@ -275,6 +275,9 @@ class PluginsFacade implements PluginStateListener { // make sure plugins dir exists if( mode!=DEV_MODE && !FilesEx.exists(root) && !FilesEx.mkdirs(root) ) throw new IOException("Unable to create plugins dir: $root") + // verify the plugins dir is a trusted (private, non-shared) location + if( mode!=DEV_MODE ) + PluginSecurity.checkTrustedDir(root) this.manager = createManager(root, embedded) this.updater = createUpdater(root, manager) @@ -295,6 +298,9 @@ class PluginsFacade implements PluginStateListener { this.manager.addPluginStateListener(this) // setup the updater this.updater = createUpdater(root, manager) + // verify the plugins dir is a trusted (private, non-shared) location + if( mode!=DEV_MODE ) + PluginSecurity.checkTrustedDir(root) // load plugins manager.loadPlugins() if( embedded ) { diff --git a/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy new file mode 100644 index 0000000000..310952c66a --- /dev/null +++ b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy @@ -0,0 +1,141 @@ +/* + * 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.nio.file.attribute.PosixFilePermissions + +import nextflow.SysEnv +import nextflow.exception.AbortOperationException +import spock.lang.IgnoreIf +import spock.lang.Specification +import spock.lang.Unroll +/** + * + * @author Claude + */ +@IgnoreIf({ os.windows }) +class PluginSecurityTest extends Specification { + + private Path tempDirWithPerms(String perms) { + final dir = Files.createTempDirectory('test-plugin-sec') + Files.setPosixFilePermissions(dir, PosixFilePermissions.fromString(perms)) + return dir + } + + @Unroll + def 'should accept a private dir owned by current user - mode=#MODE perms=#PERMS' () { + given: + def dir = tempDirWithPerms(PERMS) + + when: + PluginSecurity.checkTrustedDir(dir, MODE) + then: + noExceptionThrown() + + cleanup: + dir?.deleteDir() + + where: + MODE | PERMS + 'strict' | 'rwx------' + 'warn' | 'rwx------' + 'strict' | 'rwxr-xr-x' + 'warn' | 'rwxr-xr-x' + } + + @Unroll + def 'should reject a group/world-writable dir in strict mode - perms=#PERMS' () { + given: + def dir = tempDirWithPerms(PERMS) + + when: + PluginSecurity.checkTrustedDir(dir, 'strict') + then: + def e = thrown(AbortOperationException) + e.message.contains('is not secure') + + cleanup: + dir?.deleteDir() + + where: + PERMS << ['rwxrwx---', 'rwxrwxrwx', 'rwx-w----', 'rwx----w-'] + } + + @Unroll + def 'should only warn (not throw) for a group/world-writable dir in warn mode - perms=#PERMS' () { + given: + def dir = tempDirWithPerms(PERMS) + + when: + PluginSecurity.checkTrustedDir(dir, 'warn') + then: + noExceptionThrown() + + cleanup: + dir?.deleteDir() + + where: + PERMS << ['rwxrwx---', 'rwxrwxrwx'] + } + + def 'should skip the check entirely when mode is off' () { + given: + def dir = tempDirWithPerms('rwxrwxrwx') + + when: + PluginSecurity.checkTrustedDir(dir, 'off') + then: + noExceptionThrown() + + cleanup: + dir?.deleteDir() + } + + def 'should be a no-op for a null dir' () { + when: + PluginSecurity.checkTrustedDir(null, 'strict') + then: + noExceptionThrown() + } + + def 'should default to warn mode from the environment' () { + given: + SysEnv.push([:]) + expect: + PluginSecurity.getMode() == 'warn' + + cleanup: + SysEnv.pop() + } + + @Unroll + def 'should resolve mode #VALUE from the environment' () { + given: + SysEnv.push([NXF_PLUGINS_STRICT_MODE: VALUE]) + expect: + PluginSecurity.getMode() == VALUE + + cleanup: + SysEnv.pop() + + where: + VALUE << ['warn', 'strict', 'off'] + } + +} From df2e3cff9c34f25499929b989e1848ff3e1cbb90 Mon Sep 17 00:00:00 2001 From: Oussama El Azizi Date: Wed, 8 Jul 2026 15:32:38 +0200 Subject: [PATCH 2/6] Validate and normalise NXF_PLUGINS_STRICT_MODE value Address review feedback: the mode was read verbatim, so typos, different case or a blank value would silently behave as 'warn'. Normalise the value (lower-case and trim) and validate it against the known modes, logging a warning on an unrecognised value instead of ignoring it silently. Add test coverage for normalisation and the invalid/blank fallback. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Oussama El Azizi --- .../nextflow/plugin/PluginSecurity.groovy | 16 +++++++++- .../nextflow/plugin/PluginSecurityTest.groovy | 32 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy index 6b80808595..8babc4268f 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy @@ -56,13 +56,27 @@ class PluginSecurity { static final String MODE_STRICT = 'strict' static final String MODE_OFF = 'off' + static final Set MODES = [MODE_WARN, MODE_STRICT, MODE_OFF] as Set + /** * Resolve the strict mode from the {@code NXF_PLUGINS_STRICT_MODE} environment variable. * + * The value is normalised (lower-cased and trimmed) and validated against the known modes. + * An unset or blank value falls back to the default silently, while an unrecognised value + * (e.g. a typo) falls back to the default and logs a warning so it is not silently ignored. + * * @return one of {@code warn} (default), {@code strict} or {@code off} */ static String getMode() { - return SysEnv.get('NXF_PLUGINS_STRICT_MODE', MODE_WARN) + final raw = SysEnv.get('NXF_PLUGINS_STRICT_MODE') + final mode = raw?.toLowerCase()?.trim() + if( !mode ) + return MODE_WARN + if( !MODES.contains(mode) ) { + log.warn "Invalid NXF_PLUGINS_STRICT_MODE value '${raw}' - expected one of ${MODES.join(', ')}; using default '${MODE_WARN}'" + return MODE_WARN + } + return mode } /** diff --git a/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy index 310952c66a..794fb6b94c 100644 --- a/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy +++ b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy @@ -138,4 +138,36 @@ class PluginSecurityTest extends Specification { VALUE << ['warn', 'strict', 'off'] } + @Unroll + def 'should normalise the env value #VALUE to #EXPECTED' () { + given: + SysEnv.push([NXF_PLUGINS_STRICT_MODE: VALUE]) + expect: + PluginSecurity.getMode() == EXPECTED + + cleanup: + SysEnv.pop() + + where: + VALUE | EXPECTED + 'STRICT' | 'strict' + 'Warn' | 'warn' + ' off ' | 'off' + ' Strict ' | 'strict' + } + + @Unroll + def 'should fall back to warn for an unrecognised or blank env value #VALUE' () { + given: + SysEnv.push([NXF_PLUGINS_STRICT_MODE: VALUE]) + expect: + PluginSecurity.getMode() == 'warn' + + cleanup: + SysEnv.pop() + + where: + VALUE << ['bogus', 'strictly', '', ' '] + } + } From 31118ade6364053459b912c92a21fcc021f8aeb7 Mon Sep 17 00:00:00 2001 From: Oussama El Azizi Date: Wed, 8 Jul 2026 15:39:58 +0200 Subject: [PATCH 3/6] Add test coverage for the untrusted-owner branch Address review feedback: existing tests only exercised the permission branch. Extract the trust decision into a pure isUntrusted(perms, owner, currentUser) helper so the owner branch can be tested deterministically - a temp dir in a unit test is always owned by the current user (and would be root-owned under a root CI runner), so it cannot reach that branch via real files. Add a data-driven test covering owned-by-me, root-owned, untrusted-owner (the reported scenario) and group/world-writable cases. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Oussama El Azizi --- .../nextflow/plugin/PluginSecurity.groovy | 22 +++++++++++++++---- .../nextflow/plugin/PluginSecurityTest.groovy | 19 ++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy index 8babc4268f..ea69741522 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy @@ -79,6 +79,23 @@ class PluginSecurity { return mode } + /** + * Determine whether a plugin directory should be considered untrusted. + * + * A directory is untrusted when it is writable by group or others, or when it is owned by a + * principal other than the current user or root. + * + * @param perms The POSIX permissions of the directory + * @param owner The name of the directory owner + * @param currentUser The name of the user running Nextflow + * @return {@code true} if the directory cannot be trusted + */ + static boolean isUntrusted(Set perms, String owner, String currentUser) { + final writableByOthers = perms.contains(PosixFilePermission.GROUP_WRITE) || perms.contains(PosixFilePermission.OTHERS_WRITE) + final untrustedOwner = owner != currentUser && owner != 'root' + return writableByOthers || untrustedOwner + } + /** * Verify that the given plugin directory can be trusted, using the mode resolved from the * environment. @@ -108,10 +125,7 @@ class PluginSecurity { final owner = attrs.owner().name final me = System.getProperty('user.name') - final writableByOthers = perms.contains(PosixFilePermission.GROUP_WRITE) || perms.contains(PosixFilePermission.OTHERS_WRITE) - final untrustedOwner = owner != me && owner != 'root' - - if( writableByOthers || untrustedOwner ) { + if( isUntrusted(perms, owner, me) ) { final msg = "Plugin directory '${dir}' is not secure (owner=${owner}, mode=${PosixFilePermissions.toString(perms)}) " + "- it may be modified by other users, which could allow untrusted plugin code to run in your Nextflow process" if( mode == MODE_STRICT ) diff --git a/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy index 794fb6b94c..fcfd7ac4d9 100644 --- a/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy +++ b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy @@ -59,6 +59,25 @@ class PluginSecurityTest extends Specification { 'warn' | 'rwxr-xr-x' } + @Unroll + def 'should classify trust by owner and perms - owner=#OWNER user=#USER perms=#PERMS untrusted=#UNTRUSTED' () { + expect: + PluginSecurity.isUntrusted(PosixFilePermissions.fromString(PERMS), OWNER, USER) == UNTRUSTED + + where: + OWNER | USER | PERMS | UNTRUSTED + // trusted: private dir owned by the current user + 'alice' | 'alice' | 'rwx------' | false + // trusted: admin-managed read-only shared cache (root-owned, not group/other writable) + 'root' | 'alice' | 'rwxr-xr-x' | false + // untrusted owner: dir owned by another user, even with safe permissions (the PoC case) + 'attacker' | 'victim' | 'rwx------' | true + 'attacker' | 'victim' | 'rwxr-xr-x' | true + // untrusted perms: group/world writable even when owned by the current user + 'alice' | 'alice' | 'rwxrwx---' | true + 'alice' | 'alice' | 'rwxrwxrwx' | true + } + @Unroll def 'should reject a group/world-writable dir in strict mode - perms=#PERMS' () { given: From 9c9459caf6b6fbbe22317a2f72d208b27172e306 Mon Sep 17 00:00:00 2001 From: Oussama El Azizi Date: Wed, 8 Jul 2026 17:39:11 +0200 Subject: [PATCH 4/6] Harden plugin-dir trust predicate to avoid false positives Address review feedback: the trust predicate flagged legitimate mainstream deployments. Rework it to flag a directory only when a non-trusted principal can write to it: - Drop the group-write check: a self-owned, group-writable dir (rwxrwxr-x created under the common umask 002 with a user-private group) is now trusted. Only world-writable (others-write) remains a risk for a trusted owner. Fixes spurious warnings/aborts on every run. - Compare owner by UID instead of name, so containers/OpenShift/ arbitrary-UID/NFS/LDAP (where user.name is '?') no longer flag the user's own cache. - When running as root (uid 0), trust any owner but still flag a world-writable cache (fixes sudo against a user-owned 0700 cache). - Add NXF_PLUGINS_TRUSTED_OWNERS (UIDs or names) so an admin-managed read-only cache owned by a service account is trusted. - Warn at most once per offending directory, and emit the invalid-mode warning once, so a multi-plugin run no longer floods duplicates. - Fail open (skip the check) when owner/current UID cannot be determined. World-writable directories and foreign-owned directories are still refused, so the original PoC (root-owned 777 root + attacker-owned 755 subdir) remains caught. Update docs and tests accordingly. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Oussama El Azizi --- docs/plugins/using-plugins.mdx | 2 +- docs/reference/env-vars.mdx | 8 +- .../nextflow/plugin/PluginSecurity.groovy | 141 +++++++++++++++--- .../nextflow/plugin/PluginSecurityTest.groovy | 82 ++++++---- 4 files changed, 180 insertions(+), 53 deletions(-) diff --git a/docs/plugins/using-plugins.mdx b/docs/plugins/using-plugins.mdx index ec9af0f597..3f2b98d104 100644 --- a/docs/plugins/using-plugins.mdx +++ b/docs/plugins/using-plugins.mdx @@ -59,7 +59,7 @@ When Nextflow downloads plugins, it caches them in the directory specified by `N :::warning The plugin cache should be a private, per-user directory that is not writable by other users. Nextflow loads and executes cached plugin code without re-verifying its origin, so a shared plugin cache that is writable by other users (or owned by another user) allows a lower-trust user to replace cached plugin code with their own, which would then run inside your Nextflow process. -Keep `NXF_PLUGINS_DIR` private (for example `chmod 700`) and owned by you. If a shared cache is required, use an administrator-owned, **read-only** directory that regular users cannot modify. Use [`NXF_PLUGINS_STRICT_MODE`](../reference/env-vars.mdx) to control whether Nextflow warns about or refuses to load plugins from an untrusted directory. +Keep `NXF_PLUGINS_DIR` owned by you and not world-writable (a group-writable directory owned by you, as created under the common umask 002, is fine). If a shared cache is required, use a directory owned by an administrator or a dedicated service account that regular users cannot modify, and add that owner to [`NXF_PLUGINS_TRUSTED_OWNERS`](../reference/env-vars.mdx). Use [`NXF_PLUGINS_STRICT_MODE`](../reference/env-vars.mdx) to control whether Nextflow warns about or refuses to load plugins from an untrusted directory. ::: ## Offline usage diff --git a/docs/reference/env-vars.mdx b/docs/reference/env-vars.mdx index a77d18ba9b..7d7aba6106 100644 --- a/docs/reference/env-vars.mdx +++ b/docs/reference/env-vars.mdx @@ -220,7 +220,13 @@ Specifies the URL of the plugin registry used to download and resolve plugins. T -Controls how Nextflow reacts when the plugins directory (`NXF_PLUGINS_DIR`) or a cached plugin directory is not a trusted location — i.e. it is owned by another user or writable by group or others. Such a directory could be modified by other users, allowing untrusted plugin code to run in your Nextflow process. Allowed values are `warn` (default, logs a warning and continues), `strict` (aborts the run) and `off` (disables the check). The check is skipped on filesystems that do not support POSIX permissions. +Controls how Nextflow reacts when the plugins directory (`NXF_PLUGINS_DIR`) or a cached plugin directory is not a trusted location — i.e. it is owned by a user other than the one running Nextflow (compared by UID; `root` and any owner listed in `NXF_PLUGINS_TRUSTED_OWNERS` are trusted) or it is world-writable. Such a directory could be modified by other users, allowing untrusted plugin code to run in your Nextflow process. Allowed values are `warn` (default, logs a warning and continues), `strict` (aborts the run) and `off` (disables the check). The check is skipped on filesystems that do not support POSIX permissions, and when running as `root` only a world-writable directory is flagged. + +##### `NXF_PLUGINS_TRUSTED_OWNERS` + + + +Comma-separated list of additional owners (numeric UIDs or user names) that are trusted to own the plugins directory or cached plugin directories, beyond the current user and `root`. Use this to allow an admin-managed shared plugin cache owned by a service account (e.g. `NXF_PLUGINS_TRUSTED_OWNERS=nextflow`) without triggering [`NXF_PLUGINS_STRICT_MODE`](#nxf_plugins_strict_mode). ##### `NXF_PLUGINS_TEST_REPOSITORY` diff --git a/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy index ea69741522..90ee0c7c77 100644 --- a/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy +++ b/modules/nf-commons/src/main/nextflow/plugin/PluginSecurity.groovy @@ -22,6 +22,8 @@ import java.nio.file.attribute.PosixFileAttributeView import java.nio.file.attribute.PosixFileAttributes import java.nio.file.attribute.PosixFilePermission import java.nio.file.attribute.PosixFilePermissions +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicBoolean import groovy.transform.CompileStatic import groovy.util.logging.Slf4j @@ -36,15 +38,29 @@ import nextflow.exception.AbortOperationException * user can pre-populate an official pinned plugin coordinate (e.g. {@code nf-amazon-3.10.0}) with * attacker-controlled code that Nextflow would then load from the cache without any origin check. * - * A directory is considered trusted only when it is owned by the current user (or root) AND it is - * not writable by group or others. This accepts the two legitimate deployment shapes - a private - * user cache (e.g. {@code 0700}) and an admin-managed read-only shared cache (e.g. {@code root:root - * 0755}) - while rejecting an attacker-owned directory or a group/world-writable cache. + * A directory is considered untrusted when a principal other than the current user can write to + * it, that is: + *
    + *
  • it is owned by a user other than the one running Nextflow (compared by UID), unless the + * owner is {@code root} or listed in {@code NXF_PLUGINS_TRUSTED_OWNERS}; or
    • + *
  • it is world-writable (the {@code others-write} bit is set).
    • + *
+ * + * This accepts the mainstream deployment shapes - a private user cache (including the + * group-writable {@code rwxrwxr-x} created under the common umask 002 with a user-private group), + * an admin-managed read-only cache owned by {@code root} or a configured service account - while + * rejecting an attacker-owned directory or a world-writable cache. When running as {@code root} + * (UID 0) the owner is always trusted (the admin's responsibility) but a world-writable cache is + * still refused. + * + * Note: a self-owned cache that is group-writable to a shared primary group is treated as + * trusted; this residual risk is accepted because user-private-group is the modern default and + * flagging it would produce false positives on almost every run. World-writable is always refused. * * The behaviour is controlled by the {@code NXF_PLUGINS_STRICT_MODE} environment variable: * {@code warn} (default) logs a warning and continues, {@code strict} aborts the run, and - * {@code off} disables the check. On filesystems that do not support POSIX attributes the check is - * a no-op. + * {@code off} disables the check. On filesystems that do not support POSIX attributes, or when the + * owner/current UID cannot be determined, the check is a no-op (fails open). * * @author Claude */ @@ -58,12 +74,21 @@ class PluginSecurity { static final Set MODES = [MODE_WARN, MODE_STRICT, MODE_OFF] as Set + private static final long ROOT_UID = 0 + + // track dirs already reported so a multi-plugin run emits at most one warning per dir + private static final Set reported = ConcurrentHashMap.newKeySet() + + // emit the invalid-mode warning at most once per run + private static final AtomicBoolean invalidModeWarned = new AtomicBoolean() + /** * Resolve the strict mode from the {@code NXF_PLUGINS_STRICT_MODE} environment variable. * * The value is normalised (lower-cased and trimmed) and validated against the known modes. * An unset or blank value falls back to the default silently, while an unrecognised value - * (e.g. a typo) falls back to the default and logs a warning so it is not silently ignored. + * (e.g. a typo) falls back to the default and logs a warning (once) so it is not silently + * ignored. * * @return one of {@code warn} (default), {@code strict} or {@code off} */ @@ -73,27 +98,46 @@ class PluginSecurity { if( !mode ) return MODE_WARN if( !MODES.contains(mode) ) { - log.warn "Invalid NXF_PLUGINS_STRICT_MODE value '${raw}' - expected one of ${MODES.join(', ')}; using default '${MODE_WARN}'" + if( invalidModeWarned.compareAndSet(false, true) ) + log.warn "Invalid NXF_PLUGINS_STRICT_MODE value '${raw}' - expected one of ${MODES.join(', ')}; using default '${MODE_WARN}'" return MODE_WARN } return mode } /** - * Determine whether a plugin directory should be considered untrusted. + * The set of owner identities trusted as plugin cache owners, in addition to the current user + * and root, parsed from the {@code NXF_PLUGINS_TRUSTED_OWNERS} environment variable. Each + * comma-separated entry is either a numeric UID or a user name. * - * A directory is untrusted when it is writable by group or others, or when it is owned by a - * principal other than the current user or root. + * @return the trimmed, non-empty trusted-owner tokens (may be empty, never {@code null}) + */ + static Set getTrustedOwners() { + final list = SysEnv.get('NXF_PLUGINS_TRUSTED_OWNERS') + if( !list ) + return Collections.emptySet() + return list.tokenize(',')*.trim().findAll { it } as Set + } + + /** + * Determine whether a plugin directory should be considered untrusted. * * @param perms The POSIX permissions of the directory - * @param owner The name of the directory owner - * @param currentUser The name of the user running Nextflow + * @param ownerUid The UID of the directory owner + * @param currentUid The UID of the user running Nextflow + * @param trustedUids Additional owner UIDs trusted beyond the current user and root * @return {@code true} if the directory cannot be trusted */ - static boolean isUntrusted(Set perms, String owner, String currentUser) { - final writableByOthers = perms.contains(PosixFilePermission.GROUP_WRITE) || perms.contains(PosixFilePermission.OTHERS_WRITE) - final untrustedOwner = owner != currentUser && owner != 'root' - return writableByOthers || untrustedOwner + static boolean isUntrusted(Set perms, long ownerUid, long currentUid, Set trustedUids) { + final worldWritable = perms.contains(PosixFilePermission.OTHERS_WRITE) + // running as root: trust any owner (admin's responsibility) but still flag a world-writable cache + if( currentUid == ROOT_UID ) + return worldWritable + final ownerTrusted = ownerUid == currentUid || ownerUid == ROOT_UID || trustedUids.contains(ownerUid) + if( !ownerTrusted ) + return true + // owner is trusted; group-write is fine (user-private-group / umask 002), only world-write is a risk + return worldWritable } /** @@ -122,15 +166,23 @@ class PluginSecurity { return final PosixFileAttributes attrs = view.readAttributes() final perms = attrs.permissions() - final owner = attrs.owner().name - final me = System.getProperty('user.name') - if( isUntrusted(perms, owner, me) ) { + // resolve owner and current UID; if either is unavailable, fail open to avoid false positives + final ownerUid = ownerUid(dir) + final currentUid = currentUid() + if( ownerUid == null || currentUid == null ) + return + + final trustedUids = resolveTrustedUids(attrs.owner().name, ownerUid) + if( isUntrusted(perms, ownerUid, currentUid, trustedUids) ) { + final owner = attrs.owner().name final msg = "Plugin directory '${dir}' is not secure (owner=${owner}, mode=${PosixFilePermissions.toString(perms)}) " + "- it may be modified by other users, which could allow untrusted plugin code to run in your Nextflow process" if( mode == MODE_STRICT ) - throw new AbortOperationException("${msg} -- refusing to load plugins. Use a private directory (chmod 700) owned by you, or set NXF_PLUGINS_STRICT_MODE=warn to override") - log.warn "${msg} -- set NXF_PLUGINS_STRICT_MODE=strict to refuse loading, or use a private plugins directory owned by you" + throw new AbortOperationException("${msg} -- refusing to load plugins. Use a private directory (chmod 700) owned by you, add the owner to NXF_PLUGINS_TRUSTED_OWNERS, or set NXF_PLUGINS_STRICT_MODE=warn to override") + // warn at most once per offending directory + if( reported.add(dir.toString()) ) + log.warn "${msg} -- set NXF_PLUGINS_STRICT_MODE=strict to refuse loading, add the owner to NXF_PLUGINS_TRUSTED_OWNERS, or use a private plugins directory owned by you" } } catch( AbortOperationException e ) { @@ -141,4 +193,49 @@ class PluginSecurity { log.debug "Unable to verify plugin directory permissions for '${dir}' - ${e.message}" } } + + /** + * Resolve the trusted owner UIDs from {@code NXF_PLUGINS_TRUSTED_OWNERS}. Numeric entries are + * matched by UID; a name entry matching the directory owner name resolves to the owner UID. + */ + private static Set resolveTrustedUids(String ownerName, long ownerUid) { + final tokens = getTrustedOwners() + if( !tokens ) + return Collections.emptySet() + final result = new HashSet() + for( String tok : tokens ) { + if( tok.isLong() ) + result.add(tok.toLong()) + else if( tok == ownerName ) + result.add(ownerUid) + } + return result + } + + /** + * @return the owner UID of the given path, or {@code null} if it cannot be determined + */ + private static Long ownerUid(Path dir) { + try { + final uid = Files.getAttribute(dir, 'unix:uid') + return uid != null ? ((Number) uid).longValue() : null + } + catch( Exception e ) { + log.debug "Unable to read owner UID for '${dir}' - ${e.message}" + return null + } + } + + /** + * @return the UID of the user running Nextflow, or {@code null} if it cannot be determined + */ + private static Long currentUid() { + try { + return new com.sun.security.auth.module.UnixSystem().getUid() + } + catch( Throwable e ) { + log.debug "Unable to determine current user UID - ${e.message}" + return null + } + } } diff --git a/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy index fcfd7ac4d9..5b38a1a8a4 100644 --- a/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy +++ b/modules/nf-commons/src/test/nextflow/plugin/PluginSecurityTest.groovy @@ -39,7 +39,34 @@ class PluginSecurityTest extends Specification { } @Unroll - def 'should accept a private dir owned by current user - mode=#MODE perms=#PERMS' () { + def 'should classify trust by uid and perms - owner=#OWNER current=#CUR perms=#PERMS trusted=#TRUSTED untrusted=#UNTRUSTED' () { + expect: + PluginSecurity.isUntrusted(PosixFilePermissions.fromString(PERMS), OWNER, CUR, TRUSTED) == UNTRUSTED + + where: + OWNER | CUR | PERMS | TRUSTED | UNTRUSTED + // self-owned: private, read-only, or umask-002 group-writable -> trusted + 1000L | 1000L | 'rwx------' | ([] as Set) | false + 1000L | 1000L | 'rwxr-xr-x' | ([] as Set) | false + 1000L | 1000L | 'rwxrwxr-x' | ([] as Set) | false // umask 002 default - the key regression + 1000L | 1000L | 'rwxrwx---' | ([] as Set) | false + // self-owned but world-writable -> untrusted + 1000L | 1000L | 'rwxrwxrwx' | ([] as Set) | true + // root-owned admin cache -> trusted; root-owned world-writable -> untrusted + 0L | 1000L | 'rwxr-xr-x' | ([] as Set) | false + 0L | 1000L | 'rwxrwxrwx' | ([] as Set) | true + // foreign owner -> untrusted even with safe perms (the PoC case) + 1201L | 1202L | 'rwx------' | ([] as Set) | true + 1201L | 1202L | 'rwxr-xr-x' | ([] as Set) | true + // foreign owner listed as trusted (service-account cache) -> trusted + 1201L | 1202L | 'rwx------' | ([1201L] as Set) | false + // running as root: trust any owner, but still flag world-writable + 1201L | 0L | 'rwx------' | ([] as Set) | false + 1201L | 0L | 'rwxrwxrwx' | ([] as Set) | true + } + + @Unroll + def 'should accept a self-owned dir that is not world-writable - mode=#MODE perms=#PERMS' () { given: def dir = tempDirWithPerms(PERMS) @@ -56,30 +83,12 @@ class PluginSecurityTest extends Specification { 'strict' | 'rwx------' 'warn' | 'rwx------' 'strict' | 'rwxr-xr-x' - 'warn' | 'rwxr-xr-x' - } - - @Unroll - def 'should classify trust by owner and perms - owner=#OWNER user=#USER perms=#PERMS untrusted=#UNTRUSTED' () { - expect: - PluginSecurity.isUntrusted(PosixFilePermissions.fromString(PERMS), OWNER, USER) == UNTRUSTED - - where: - OWNER | USER | PERMS | UNTRUSTED - // trusted: private dir owned by the current user - 'alice' | 'alice' | 'rwx------' | false - // trusted: admin-managed read-only shared cache (root-owned, not group/other writable) - 'root' | 'alice' | 'rwxr-xr-x' | false - // untrusted owner: dir owned by another user, even with safe permissions (the PoC case) - 'attacker' | 'victim' | 'rwx------' | true - 'attacker' | 'victim' | 'rwxr-xr-x' | true - // untrusted perms: group/world writable even when owned by the current user - 'alice' | 'alice' | 'rwxrwx---' | true - 'alice' | 'alice' | 'rwxrwxrwx' | true + 'strict' | 'rwxrwxr-x' // umask-002 group-writable, owned by me + 'warn' | 'rwxrwx---' } @Unroll - def 'should reject a group/world-writable dir in strict mode - perms=#PERMS' () { + def 'should reject a world-writable dir in strict mode - perms=#PERMS' () { given: def dir = tempDirWithPerms(PERMS) @@ -93,13 +102,12 @@ class PluginSecurityTest extends Specification { dir?.deleteDir() where: - PERMS << ['rwxrwx---', 'rwxrwxrwx', 'rwx-w----', 'rwx----w-'] + PERMS << ['rwxrwxrwx', 'rwx----w-'] } - @Unroll - def 'should only warn (not throw) for a group/world-writable dir in warn mode - perms=#PERMS' () { + def 'should only warn (not throw) for a world-writable dir in warn mode' () { given: - def dir = tempDirWithPerms(PERMS) + def dir = tempDirWithPerms('rwxrwxrwx') when: PluginSecurity.checkTrustedDir(dir, 'warn') @@ -108,9 +116,6 @@ class PluginSecurityTest extends Specification { cleanup: dir?.deleteDir() - - where: - PERMS << ['rwxrwx---', 'rwxrwxrwx'] } def 'should skip the check entirely when mode is off' () { @@ -189,4 +194,23 @@ class PluginSecurityTest extends Specification { VALUE << ['bogus', 'strictly', '', ' '] } + @Unroll + def 'should parse NXF_PLUGINS_TRUSTED_OWNERS #VALUE' () { + given: + SysEnv.push(VALUE != null ? [NXF_PLUGINS_TRUSTED_OWNERS: VALUE] : [:]) + expect: + PluginSecurity.getTrustedOwners() == EXPECTED + + cleanup: + SysEnv.pop() + + where: + VALUE | EXPECTED + null | ([] as Set) + '' | ([] as Set) + '1000' | (['1000'] as Set) + 'nextflow' | (['nextflow'] as Set) + '1000, nextflow ,' | (['1000', 'nextflow'] as Set) + } + } From 71ff9316c9229dddbc3f88a6299e9efaab655459 Mon Sep 17 00:00:00 2001 From: aNewUser Date: Mon, 13 Jul 2026 09:59:47 +0200 Subject: [PATCH 5/6] Update docs/plugins/using-plugins.mdx Co-authored-by: Chris Hakkaart Signed-off-by: aNewUser --- docs/plugins/using-plugins.mdx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plugins/using-plugins.mdx b/docs/plugins/using-plugins.mdx index 3f2b98d104..3b4c789657 100644 --- a/docs/plugins/using-plugins.mdx +++ b/docs/plugins/using-plugins.mdx @@ -57,9 +57,9 @@ 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). :::warning -The plugin cache should be a private, per-user directory that is not writable by other users. Nextflow loads and executes cached plugin code without re-verifying its origin, so a shared plugin cache that is writable by other users (or owned by another user) allows a lower-trust user to replace cached plugin code with their own, which would then run inside your Nextflow process. +The plugin cache should be a private, per-user directory that other users cannot write to. Nextflow loads and executes cached plugin code without re-verifying its origin. If the cache is writable by other users (or owned by another user), a lower-trust user can replace the cached code with their own, and that code then runs inside your Nextflow process. -Keep `NXF_PLUGINS_DIR` owned by you and not world-writable (a group-writable directory owned by you, as created under the common umask 002, is fine). If a shared cache is required, use a directory owned by an administrator or a dedicated service account that regular users cannot modify, and add that owner to [`NXF_PLUGINS_TRUSTED_OWNERS`](../reference/env-vars.mdx). Use [`NXF_PLUGINS_STRICT_MODE`](../reference/env-vars.mdx) to control whether Nextflow warns about or refuses to load plugins from an untrusted directory. +Keep `NXF_PLUGINS_DIR` owned by you and not world-writable. A group-writable directory owned by you, as created under the common umask 002, is fine. If you need a shared cache, use a directory owned by an administrator or a dedicated service account that regular users cannot modify, and add that owner to [`NXF_PLUGINS_TRUSTED_OWNERS`][env-vars]. Use [`NXF_PLUGINS_STRICT_MODE`][env-vars] to control whether Nextflow warns about or refuses to load plugins from an untrusted directory. ::: ## Offline usage From 54b91cb2937e588de021bf07a9e026984b6ea7dc Mon Sep 17 00:00:00 2001 From: aNewUser Date: Mon, 13 Jul 2026 09:59:55 +0200 Subject: [PATCH 6/6] Update docs/plugins/using-plugins.mdx Co-authored-by: Chris Hakkaart Signed-off-by: aNewUser --- docs/plugins/using-plugins.mdx | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/plugins/using-plugins.mdx b/docs/plugins/using-plugins.mdx index 3b4c789657..be4460d4ce 100644 --- a/docs/plugins/using-plugins.mdx +++ b/docs/plugins/using-plugins.mdx @@ -80,6 +80,7 @@ To use Nextflow plugins in an offline environment: Nextflow will attempt to download newer versions of plugins if their versions are not set. See [Identifiers][using-plugins-identifiers] for more information. ::: +[env-vars]: ../reference/env-vars [install-standalone]: ../install#standalone-distribution [using-plugins-config]: ./using-plugins#configuration [using-plugins-identifiers]: ./using-plugins#identifiers