Skip to content

Commit ca1e625

Browse files
jorgeepditommasobentsherman
authored
Fix remote modules when base dir is not the current working dir (#6932)
Co-authored-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-authored-by: Ben Sherman <bentshermann@gmail.com>
1 parent ea1f4ea commit ca1e625

12 files changed

Lines changed: 97 additions & 58 deletions

File tree

modules/nextflow/src/main/groovy/nextflow/exception/IllegalModulePath.groovy renamed to modules/nextflow/src/main/groovy/nextflow/exception/IllegalModulePathException.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@ import groovy.transform.InheritConstructors
2424
* @author Paolo Di Tommaso <paolo.ditommaso@gmail.com>
2525
*/
2626
@InheritConstructors
27-
class IllegalModulePath extends Exception {
27+
class IllegalModulePathException extends Exception {
2828
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import nextflow.Global
2222
import nextflow.config.ConfigBuilder
2323

2424
import nextflow.config.RegistryConfig
25-
import nextflow.exception.IllegalModulePath
25+
import nextflow.exception.IllegalModulePathException
2626
import nextflow.module.spi.RemoteModuleResolver
2727

2828
import java.nio.file.Path
@@ -45,8 +45,8 @@ import java.nio.file.Path
4545
class DefaultRemoteModuleResolver implements RemoteModuleResolver {
4646

4747
@Override
48-
Path resolve(String moduleName, Path baseDir) {
49-
48+
Path resolve(String moduleName, Path projectDir) {
49+
final baseDir = projectDir ?: Path.of('.').toAbsolutePath()
5050
final config = Global.config ?: new ConfigBuilder().setBaseDir(baseDir).build()
5151
final registryConfig = config.navigate('registry') as RegistryConfig
5252

@@ -65,7 +65,7 @@ class DefaultRemoteModuleResolver implements RemoteModuleResolver {
6565
log.debug "Module ${reference} resolved to ${mainFile}"
6666
return mainFile
6767
} catch (Exception e) {
68-
throw new IllegalModulePath("Failed to resolve remote module ${moduleName}: ${e.message}", e)
68+
throw new IllegalModulePathException("Failed to resolve remote module ${moduleName}: ${e.message}", e)
6969
}
7070
}
7171

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ class ModuleResolver {
161161
// Install to modules directory (will compute directory checksum for future integrity checks)
162162
InstalledModule installed = storage.installModule(reference, version, tempFile, downloadUrl)
163163

164-
log.info "Module ${reference}@${version} installed successfully at ${installed.mainFile.parent}"
164+
log.info "Module ${reference}@${version} installed successfully at ${installed.mainFile.parent.toAbsolutePath()}"
165165
return installed.mainFile
166166
}
167167
finally {

modules/nextflow/src/main/groovy/nextflow/script/IncludeDef.groovy

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import groovy.transform.PackageScope
2727
import groovy.util.logging.Slf4j
2828
import nextflow.NF
2929
import nextflow.Session
30-
import nextflow.exception.IllegalModulePath
30+
import nextflow.exception.IllegalModulePathException
3131
import nextflow.exception.ScriptCompilationException
3232
import nextflow.module.ModuleReference
3333
import nextflow.module.spi.RemoteModuleResolverProvider
@@ -167,7 +167,7 @@ class IncludeDef {
167167
final result = include as Path
168168
if( result.isAbsolute() ) {
169169
if( result.scheme == 'file' ) return result
170-
throw new IllegalModulePath("Cannot resolve module path: ${result.toUriString()}")
170+
throw new IllegalModulePathException("Cannot resolve module path: ${result.toUriString()}")
171171
}
172172
final str = include.toString()
173173
if( str.startsWith('./') || str.startsWith('../') ) {
@@ -212,10 +212,10 @@ class IncludeDef {
212212
@PackageScope
213213
void checkValidPath(path) {
214214
if( !path )
215-
throw new IllegalModulePath("Missing module path attribute")
215+
throw new IllegalModulePathException("Missing module path attribute")
216216

217217
if( path instanceof Path && path.scheme != 'file' )
218-
throw new IllegalModulePath("Remote modules are not allowed -- Offending module: ${path.toUriString()}")
218+
throw new IllegalModulePathException("Remote modules are not allowed -- Offending module: ${path.toUriString()}")
219219

220220
final str = path.toString()
221221
if( str.startsWith('/') || str.startsWith('./') || str.startsWith('../') || str.startsWith('plugin/') )
@@ -225,7 +225,7 @@ class IncludeDef {
225225
try {
226226
ModuleReference.parse(str)
227227
} catch( Exception e ) {
228-
throw new IllegalModulePath("Module path must start with '/', './', '../' or 'plugin/' prefix, or be a valid remote module reference (scope/name) -- Offending module: $str")
228+
throw new IllegalModulePathException("Module path must start with '/', './', '../' or 'plugin/' prefix, or be a valid remote module reference (scope/name) -- Offending module: $str")
229229
}
230230
}
231231

modules/nextflow/src/main/groovy/nextflow/script/parser/v2/ScriptCompiler.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,18 @@ public class ScriptCompiler {
8888

8989
private final CompilerConfiguration config;
9090
private final GroovyClassLoader loader;
91+
private final Path projectDir;
9192

9293
private Compiler compiler;
9394

94-
public ScriptCompiler(boolean debug, Path targetDirectory, ClassLoader parent) {
95-
this(getConfig(debug, targetDirectory), parent);
95+
public ScriptCompiler(boolean debug, Path targetDirectory, ClassLoader parent, Path projectDir) {
96+
this(getConfig(debug, targetDirectory), parent, projectDir);
9697
}
9798

98-
public ScriptCompiler(CompilerConfiguration config, ClassLoader parent) {
99+
public ScriptCompiler(CompilerConfiguration config, ClassLoader parent, Path projectDir) {
99100
this.config = config;
100101
this.loader = new GroovyClassLoader(parent, config);
102+
this.projectDir = projectDir;
101103
}
102104

103105
private static CompilerConfiguration getConfig(boolean debug, Path targetDirectory) {
@@ -269,7 +271,7 @@ public void addPhaseOperation(IPrimaryClassNodeOperation op, int phase) {
269271
private void analyze(SourceUnit source) {
270272
// on first pass, recursively add included modules to queue
271273
if( entry == null ) {
272-
modules = new ModuleResolver(compiler).resolve(source, uri -> createSourceUnit(uri));
274+
modules = new ModuleResolver(projectDir, compiler).resolve(source, uri -> createSourceUnit(uri));
273275
for( var su : modules )
274276
addSource(su);
275277
entry = source;
@@ -283,7 +285,7 @@ private void analyze(SourceUnit source) {
283285
var cn = source.getAST().getClasses().get(0);
284286

285287
// perform strict syntax checking
286-
var includeResolver = new ResolveIncludeVisitor(source, compiler);
288+
var includeResolver = new ResolveIncludeVisitor(source, projectDir, compiler);
287289
includeResolver.visit();
288290
for( var error : includeResolver.getErrors() )
289291
source.getErrorCollector().addErrorAndContinue(error);

modules/nextflow/src/main/groovy/nextflow/script/parser/v2/ScriptLoaderV2.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ class ScriptLoaderV2 implements ScriptLoader {
175175

176176
private ScriptCompiler getCompiler() {
177177
if( !compiler )
178-
compiler = new ScriptCompiler(session.debug, session.classesDir, session.getClassLoader())
178+
compiler = new ScriptCompiler(session.debug, session.classesDir, session.getClassLoader(), session.baseDir)
179179
return compiler
180180
}
181181

modules/nextflow/src/test/groovy/nextflow/script/IncludeDefTest.groovy

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import java.nio.file.NoSuchFileException
2424
import java.nio.file.Path
2525

2626
import nextflow.ast.NextflowDSL
27-
import nextflow.exception.IllegalModulePath
27+
import nextflow.exception.IllegalModulePathException
2828
import nextflow.file.FileHelper
2929
import org.codehaus.groovy.control.CompilerConfiguration
3030
import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer
@@ -51,7 +51,7 @@ class IncludeDefTest extends Specification {
5151
when:
5252
include.resolveModulePath('http://foo.com/bar')
5353
then:
54-
thrown(IllegalModulePath)
54+
thrown(IllegalModulePathException)
5555

5656
}
5757

@@ -147,17 +147,17 @@ class IncludeDefTest extends Specification {
147147
when:
148148
include.checkValidPath('invalid!')
149149
then:
150-
thrown(IllegalModulePath)
150+
thrown(IllegalModulePathException)
151151

152152
when:
153153
include.checkValidPath( 'http://foo.com/x.y')
154154
then:
155-
thrown(IllegalModulePath)
155+
thrown(IllegalModulePathException)
156156

157157
when:
158158
include.checkValidPath(FileHelper.asPath('http://foo.com/x/y/z'))
159159
then:
160-
thrown(IllegalModulePath)
160+
thrown(IllegalModulePathException)
161161

162162
}
163163

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@
3131
public class FallbackRemoteModuleResolver implements RemoteModuleResolver {
3232

3333
@Override
34-
public Path resolve(String moduleName, Path baseDir) {
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");
34+
public Path resolve(String moduleName, Path projectDir) {
35+
var baseDir = projectDir != null ? projectDir : Path.of(".").toAbsolutePath();
36+
var modulesDir = baseDir.resolve("modules").normalize();
37+
var resolved = modulesDir.resolve(moduleName).normalize();
38+
if( !resolved.startsWith(modulesDir) ) {
39+
throw new IllegalStateException("Invalid module name '" + moduleName + "' -- path escapes the modules directory");
3940
}
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");
41+
if( !Files.exists(resolved) ) {
42+
throw new IllegalStateException("Module '" + moduleName + "' not found in 'modules' directory -- use 'nextflow module install' to install module first");
4243
}
4344
return resolved.resolve("main.nf");
4445
}
@@ -47,4 +48,4 @@ public Path resolve(String moduleName, Path baseDir) {
4748
public int getPriority() {
4849
return Integer.MIN_VALUE; // Fallback has lowest possible priority
4950
}
50-
}
51+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ public interface RemoteModuleResolver {
4747
* </ol>
4848
*
4949
* @param moduleName The module reference string (e.g., '@scope/name' or '@scope/name@version')
50-
* @param baseDir The base directory for the project (used to locate the modules directory)
50+
* @param projectDir The base directory for the project (used to locate the modules directory)
5151
* @return Path to the resolved module's main.nf file
5252
* @throws IllegalArgumentException if the module reference is invalid or resolution fails
5353
*/
54-
Path resolve(String moduleName, Path baseDir);
54+
Path resolve(String moduleName, Path projectDir);
5555

5656
/**
5757
* Get the priority of this resolver. Higher priority resolvers are tried first.
@@ -65,4 +65,4 @@ public interface RemoteModuleResolver {
6565
default int getPriority() {
6666
return 0;
6767
}
68-
}
68+
}

modules/nf-lang/src/main/java/nextflow/script/control/ModuleResolver.java

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@
3737
public class ModuleResolver {
3838

3939
private Compiler compiler;
40+
private Path projectDir;
4041

41-
public ModuleResolver(Compiler compiler) {
42+
public ModuleResolver(Path projectDir, Compiler compiler) {
4243
this.compiler = compiler;
44+
this.projectDir = projectDir;
4345
}
4446

4547
/**
@@ -74,17 +76,8 @@ private SourceUnit resolveInclude(IncludeNode node, SourceUnit sourceUnit, Funct
7476
if( source.startsWith("plugin/") )
7577
return null;
7678

77-
var parent = Path.of(sourceUnit.getSource().getURI()).getParent();
78-
79-
// Resolve remote module paths (scope/name format, not starting with local prefixes)
80-
if( isRemoteModule(source) ) {
81-
var modules = Path.of("./modules");
82-
var resolver = RemoteModuleResolverProvider.getInstance();
83-
resolver.resolve(source, modules.getParent());
84-
parent = modules;
85-
}
86-
87-
var includeUri = getIncludeUri(parent, source);
79+
var uri = sourceUnit.getSource().getURI();
80+
var includeUri = getIncludeUri(uri, source);
8881
if( compiler.getSource(includeUri) != null )
8982
return null;
9083
if( !Files.exists(Path.of(includeUri)) )
@@ -97,20 +90,33 @@ private SourceUnit resolveInclude(IncludeNode node, SourceUnit sourceUnit, Funct
9790
return includeSource;
9891
}
9992

93+
private URI getIncludeUri(URI uri, String source) {
94+
if( isRemoteModule(source) ) {
95+
return RemoteModuleResolverProvider.getInstance()
96+
.resolve(source, projectDir)
97+
.normalize()
98+
.toUri();
99+
}
100+
else {
101+
var parent = Path.of(uri).getParent();
102+
return getLocalIncludeUri(parent, source);
103+
}
104+
}
105+
100106
/**
101107
* Module name pattern matching the canonical format used by ModuleReference.
102108
* Scope: lowercase alphanumeric with dots/underscores/hyphens.
103109
* Name: one or more slash-separated segments, each lowercase alphanumeric with dots/underscores/hyphens.
104110
*/
105-
static final String REMOTE_MODULE_PATTERN = "^[a-z0-9][a-z0-9._\\-]*/[a-z][a-z0-9._\\-]*(/[a-z][a-z0-9._\\-]*)*$";
111+
private static final String REMOTE_MODULE_PATTERN = "^[a-z0-9][a-z0-9._\\-]*/[a-z][a-z0-9._\\-]*(/[a-z][a-z0-9._\\-]*)*$";
106112

107113
static boolean isRemoteModule(String source) {
108114
if( source.startsWith("/") || source.startsWith("./") || source.startsWith("../") )
109115
return false;
110116
return source.matches(REMOTE_MODULE_PATTERN);
111117
}
112118

113-
private static URI getIncludeUri(Path parent, String source) {
119+
private static URI getLocalIncludeUri(Path parent, String source) {
114120
Path includePath = parent.resolve(source);
115121
if( Files.isDirectory(includePath) )
116122
includePath = includePath.resolve("main.nf");

0 commit comments

Comments
 (0)