Skip to content

Fix path traversal, resource leak, and race condition in module system#6929

Merged
bentsherman merged 87 commits intomasterfrom
fix/module-system-code-issues
Mar 17, 2026
Merged

Fix path traversal, resource leak, and race condition in module system#6929
bentsherman merged 87 commits intomasterfrom
fix/module-system-code-issues

Conversation

@pditommaso
Copy link
Copy Markdown
Member

@pditommaso pditommaso commented Mar 17, 2026

Summary

Fixes three coding issues found during code review of the module system feature:

  • Path traversal vulnerability in FallbackRemoteModuleResolver: The resolve() method used moduleName directly in baseDir.resolve(moduleName) without validation. A crafted module name like ../../etc could escape the modules directory. Now normalizes the resolved path and verifies it stays within baseDir. Also fixed the error message to reference the correct command (nextflow module install).

  • InputStream leak in ModuleSpec.load(): Files.newInputStream() was passed directly to the YAML parser without being closed. If parsing threw an exception, the stream would leak. Wrapped in try-with-resources so the stream is always closed.

  • Race condition in CmdModuleInstall: After installModule() (which internally resolves the latest version and installs it), the code called resolveVersion() again -- a second HTTP request to the registry. If a new version was published between the two calls, the displayed version would differ from what was actually installed. Now reads the version from the just-installed module meta.yml instead.

Test plan

  • CmdModuleInstallTest passes
  • ModuleSpecTest passes
  • Full compilation succeeds

pditommaso and others added 30 commits November 17, 2025 15:09
@pditommaso @claude
Add Nextflow Development Constitution
59983c1 
Introduce comprehensive development constitution documenting core principles
and practices for Nextflow development including modular architecture,
test-driven quality assurance, dataflow programming model, licensing
compliance, DCO requirements, semantic versioning, and Groovy code standards.

The constitution codifies existing best practices from CLAUDE.md and
CONTRIBUTING.md to provide clear governance and quality standards for
the project.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Module adr v1
fccfc74 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Module adr v2
20020a7 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
ADR: Nextflow remote modules system v2.1
226d4ee 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Merge master into 251117-module-system [ci skip]
513fa6a 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Merge branch 'master' into 251117-module-system
beaaa08
@pditommaso
Add detailed CLI command descriptions to module system ADR [ci skip]
8e681e9 
- Add comprehensive documentation for all module CLI commands
- Add `nextflow module run` command for standalone module execution
- Remove `module update` command to simplify the design
- Use single-dash prefix for Nextflow options, double-dash for module inputs
- Remove @ prefix from scope in CLI commands (keep only in DSL syntax)

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Merge branch 'master' into 251117-module-system
c6ca725
@pditommaso @claude
Simplify module spec: unify dependencies under requires
5df12cd 
- Remove separate `dependencies` and `components` fields
- Expand `requires` to include:
  - `nextflow`: version constraint (unchanged)
  - `plugins`: array with name@constraint syntax
  - `modules`: array of module dependencies
  - `workflows`: array of workflow dependencies
- Unified version constraint syntax: `[scope/]name[@constraint]`
- Mark `components` as deprecated (use requires.modules)
- Update all examples in ADR and schema

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Add structured tool arguments to module spec schema
01c777a 
- Add `args` property to tools section for type-safe argument configuration
- Define toolArgSpec with flag, type, description, default, enum, required
- Support implicit variable `tools.<toolname>.args.<argname>` returning
  formatted flag+value (e.g., "-K 100000000")
- Support `tools.<toolname>.args` to return all args concatenated
- Document deprecation of ext.args/ext.args2/ext.args3 pattern
- Update ADR with Tool Arguments Configuration section and appendix

[ci skip]

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @jorgee
Update adr/20251114-module-system.md [ci skip]
7e12f3a 
Co-authored-by: Jorge Ejarque <jorgee@users.noreply.github.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @jorgee
Update adr/20251114-module-system.md [ci skip]
5a01d00 
Co-authored-by: Jorge Ejarque <jorgee@users.noreply.github.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @jorgee
Update adr/20251114-module-system.md [ci skip]
4779d50 
Co-authored-by: Jorge Ejarque <jorgee@users.noreply.github.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Update adr/20251114-module-system.md [ci skip]
5b1d4e5 
Expand deprecation notice to cover all ext.* custom directives

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Fix inconsistencies in module system ADR [ci skip]
a06ab7a 
- Use consistent module path format with version: modules/@scope/name@version/
- Fix directory structure example: samtools-view -> samtools/view
- Standardize on 'license' spelling (American English)
- Fix author -> authors (plural array format)

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Fix registry API path in comparison table [ci skip]
f7dab8d 
Remove @ prefix from scope in API path to match API definition

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Fix checksum format: sha256- -> sha256: [ci skip]
0242a4a 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Update module API to match registry implementation [ci skip]
5461f5b 
- Update API endpoints to match seqeralabs/nextflow-registry#266
- Use /api/modules base path (no v1 prefix)
- Use single {name} parameter with namespace (e.g., "nf-core/fastqc")
- Add separate /releases endpoint
- Simplify publish to single POST endpoint

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Update adr [ci skip]
926052c 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Simplify module storage and verification model [ci skip]
ec187fd 
- Remove checksums from nextflow.config (registry is source of truth)
- Use single version per module locally (no version in directory path)
- Add .checksum file in module directory for integrity verification
- Simplify freeze command to only pin transitive dependency versions
- Checksum mismatch reports warning instead of automatic re-download
- Remove resolved open question about checksum verification

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @jorgee
Update adr/20251114-module-system.md [ci skip]
46c6060 
Co-authored-by: Jorge Ejarque <jorgee@users.noreply.github.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Clarify module resolution rules for version changes vs local modifica…
1b0ab1a 
…tions [ci skip]

- Added Resolution Rules table clearly specifying behavior for each scenario:
  - Version change (local unmodified): automatically replace with declared version
  - Local modification (checksum mismatch): warn and protect local changes
  - Use -force flag to override locally modified modules
- Updated install command behavior to reflect checksum verification
- Updated Technical Details with expanded resolution flow

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
ADR v2.3: Fix resolution rules consistency and add version entry [ci …
7a1e62e 
…skip]

- Fixed Resolution Rules table: locally modified modules will NOT be
  replaced unless -force is used (was incorrectly saying "will be replaced")
- Added Version 2.3 changelog entry documenting:
  - Resolution Rules table with clear behavior matrix
  - Local modification protection with -force flag
  - Simplified storage model (single version per module)
  - .checksum file for fast integrity verification

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Fix Version 2.1 date typo: 2025 -> 2024 [ci skip]
c6eaabb 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Merge branch 'master' into 251117-module-system [ci skip]
daf01be 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Add module system client specification [ci skip]
c8bc50d 
Specification for Nextflow module system client implementation based on
ADR 20251114-module-system.md. Covers:

P1 (Core):
- Install and use registry modules via @scope/name syntax
- Run modules directly from CLI without wrapper workflow
- Structured tool arguments replacing ext.args pattern

P2 (Important):
- Module version management and freeze command
- Module integrity protection with checksum validation

P3 (Nice to have):
- Remove module command
- Search and discover modules
- Publish module to registry

Registry backend is out of scope (assumed implemented).

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @claude
Remove transitive dependency resolution from module system [ci skip]
68b202f 
- Remove `freeze` command from CLI
- Remove transitive dependency install behavior
- Remove orphaned transitive dependency removal from `remove` command
- Update rationale and consequences sections
- Simplify dependency resolution flow
- Update ADR to version 2.4

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @jorgee
Update adr/20251114-module-system.md [ci skip]
d118d5a 
Co-authored-by: Jorge Ejarque <jorgee@users.noreply.github.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @ewels
Update adr/20251114-module-system.md [ci skip]
ffe1116 
Co-authored-by: Phil Ewels <phil.ewels@seqera.io>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Update modules plan [ci skip]
806a41f 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
pditommaso and others added 19 commits March 3, 2026 11:09
@pditommaso @claude
Add logStreamId to Tower task record (#6877)
9a3c847 
Propagate the log stream identifier from TraceRecord to the Tower
task record so that Seqera Platform can link tasks to their cloud
log streams.

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@bentsherman @pditommaso
Don't include container in task trace when containers are not enabled (…
4bf4b75 
…#6824)
@bentsherman @pditommaso
Propagate errors from workflow outputs (#6876)
3137069
@bentsherman @pditommaso
Fix type detection of CLI params in v2 config parser (#6765)
8644993
@pditommaso @claude
Add NVMe disk allocation and diskMountPath support (#6879) [ci fast]
7d772ea 
* Add NVMe disk allocation and diskMountPath support for Seqera executor

For task/node allocation the default is /tmp; for nvme allocation
the default is the server-configured NVMe mount path (typically /scratch).


Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@jorgee
Merge remote-tracking branch 'origin/master' into 251117-module-syste…
b0feed3 
…m [ci skip]

Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@bentsherman
Version 2.7: rename .checksum to .module-info, remove @ prefix …
f145979 
…from module scopes, remove version pinning from `nextflow.config`

Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@jorgee
Module System Implementation (#6768)
583285c 
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@bentsherman
minor edits
b2fa0f8 
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@bentsherman
Cleanup module run output
9b647ef 
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@bentsherman
Fix meta.yaml -> meta.yml, module manifest -> module spec
00824f3 
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@bentsherman
Merge branch 'master' into 251117-module-system
24e14ee
@bentsherman
Fix failing test
2e183b5 
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@jorgee
fix tests
0a7ef22 
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@jorgee
modify module list to skip subdirectories of directories that already…
6c15bc8 
… contains the .module-info. This folders can't be a module

Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@bentsherman
Update docs
3be2f02 
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
@bentsherman
Merge branch 'master' into 251117-module-system
512b27d
@pditommaso
Apply suggestion from @pditommaso [ci skip]
34426ea 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso @claude
Fix path traversal, resource leak, and race condition in module system
8560315 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso pditommaso requested a review from a team as a code owner March 17, 2026 08:51
@pditommaso pditommaso mentioned this pull request Mar 17, 2026
Merged
6 tasks
@pditommaso pditommaso requested a review from jorgee March 17, 2026 09:00
Base automatically changed from 251117-module-system to master March 17, 2026 09:34
@pditommaso pditommaso requested a review from a team as a code owner March 17, 2026 09:34
@jorgee
Merge branch 'master' into fix/module-system-code-issues
8aefdb2 
Signed-off-by: Jorge Ejarque <jorgee@users.noreply.github.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 17, 2026 •
edited
Loading

✅ Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 946454b
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs-staging/deploys/69b952efdd5a6100087e3af0

@pditommaso @claude @jorgee
Fix trailing slash, response leak, NPE, and minor issues in module sy…
946454b 
…stem (#6930)

* Fix additional code issues in module system

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

* Use shared RegistryClient from npr-client for module commands (#6931)

* Use shared RegistryClient from npr-client for module commands

Replace ModuleRegistryClient with RegistryClient from the new
io.seqera:npr-client library. This moves the registry HTTP client
and checksum algorithm to a shared library used by both Nextflow
and the plugin-registry server.

Changes:
- ModuleChecksum now delegates to io.seqera.npr.client.ModuleChecksum
- ModuleRegistryClient removed, replaced by RegistryClientFactory
  that creates RegistryClient instances from RegistryConfig
- All CLI commands and ModuleResolver use RegistryClient directly
- Tests updated to use RegistryClient type for mocks/stubs
- Exception contract changed from AbortOperationException to
  RegistryException for registry operations

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

* fix import sort

Signed-off-by: jorgee <jorge.ejarque@seqera.io>

---------

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: jorgee <jorge.ejarque@seqera.io>

---------

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Jorge Ejarque <jorgee@users.noreply.github.com>
Co-authored-by: jorgee <jorge.ejarque@seqera.io>
@bentsherman bentsherman merged commit 4d36d22 into master Mar 17, 2026
23 checks passed
@bentsherman bentsherman deleted the fix/module-system-code-issues branch March 17, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorgee jorgee jorgee approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.
⚡