Skip to content

Commit 8560315

Browse files
pditommasoclaude
andcommitted
Fix path traversal, resource leak, and race condition 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>
1 parent 34426ea commit 8560315

3 files changed

Lines changed: 15 additions & 5 deletions

File tree

modules/nextflow/src/main/groovy/nextflow/cli/module/CmdModuleInstall.groovy

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import nextflow.exception.AbortOperationException
2828
import nextflow.module.ModuleReference
2929
import nextflow.module.ModuleRegistryClient
3030
import nextflow.module.ModuleResolver
31+
import nextflow.module.ModuleSpec
3132

3233
import nextflow.util.TestOnly
3334

@@ -85,7 +86,8 @@ class CmdModuleInstall extends CmdBase {
8586

8687
try {
8788
def installedMainFile = resolver.installModule(reference, version, force)
88-
def installedVersion = version ?: resolver.resolveVersion(reference)
89+
// Read the installed version from meta.yml to avoid a redundant registry call
90+
def installedVersion = ModuleSpec.load(installedMainFile.parent.resolve('meta.yml')).version
8991

9092
println "Module ${reference}@${installedVersion} installed and configured successfully"
9193
}

modules/nextflow/src/main/groovy/nextflow/module/ModuleSpec.groovy

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@ class ModuleSpec {
5454

5555
try {
5656
def yaml = new Yaml()
57-
def data = yaml.load(Files.newInputStream(metaYamlPath)) as Map<String, Object>
57+
Map<String, Object> data
58+
try( def stream = Files.newInputStream(metaYamlPath) ) {
59+
data = yaml.load(stream) as Map<String, Object>
60+
}
5861

5962
def spec = new ModuleSpec()
6063
spec.name = data.name as String

modules/nf-lang/src/main/java/nextflow/module/spi/FallbackRemoteModuleResolver.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,15 @@ public class FallbackRemoteModuleResolver implements RemoteModuleResolver {
3232

3333
@Override
3434
public Path resolve(String moduleName, Path baseDir) {
35-
if (!Files.exists(baseDir.resolve(moduleName))) {
36-
throw new IllegalStateException("Module '" + moduleName + "' not locally found at 'modules' folder - use 'nextflow install' to download module files");
35+
final var resolved = baseDir.resolve(moduleName).normalize();
36+
// Prevent path traversal outside the base directory
37+
if (!resolved.startsWith(baseDir.normalize())) {
38+
throw new IllegalStateException("Invalid module name '" + moduleName + "' - path escapes the modules directory");
3739
}
38-
return baseDir.resolve(moduleName).resolve("main.nf");
40+
if (!Files.exists(resolved)) {
41+
throw new IllegalStateException("Module '" + moduleName + "' not locally found at 'modules' folder - use 'nextflow module install' to download module files");
42+
}
43+
return resolved.resolve("main.nf");
3944
}
4045

4146
@Override

0 commit comments

Comments
 (0)