Skip to content

nextflow security fixes #653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ankurjuneja merged 3 commits into from
Jun 29, 2026
Merged

nextflow security fixes #653

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5bf4d85
nextflow security fixes
ankurjuneja Jun 29, 2026
5fd01ed
self code review comment
ankurjuneja Jun 29, 2026
271b14c
code review suggestion: validate configfile is not blank
ankurjuneja Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions nextflow/src/org/labkey/nextflow/NextFlowController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.springframework.web.servlet.ModelAndView;

import java.io.File;
import java.nio.file.InvalidPathException;
import java.util.Arrays;
import java.util.List;

Expand Down Expand Up @@ -223,6 +224,10 @@ public ModelAndView getView(EnabledForm form, boolean reshow, BindException erro
@Override
public boolean handlePost(EnabledForm form, BindException errors)
{
if (!getUser().hasSiteAdminPermission())
{
throw new UnauthorizedException();
}
NextFlowManager.get().saveEnabledState(getContainer(), form.getEnabled());
return true;
}
Expand Down Expand Up @@ -257,6 +262,10 @@ public void validateCommand(AnalyzeForm o, Errors errors)
{
errors.reject(ERROR_MSG, "NextFlow is not enabled");
}
else if (NextFlowManager.get().getConfiguration() == null)
{
errors.reject(ERROR_MSG, "NextFlow has not been configured");
}
}

@Override
Expand All @@ -278,7 +287,7 @@ public ModelAndView getView(AnalyzeForm o, boolean b, BindException errors)
}

NextFlowConfiguration config = NextFlowManager.get().getConfiguration();
if (config.getNextFlowConfigFilePath() != null)
if (config != null && config.getNextFlowConfigFilePath() != null)
{
File configDir = new File(config.getNextFlowConfigFilePath());
if (configDir.isDirectory())
Expand Down Expand Up @@ -311,8 +320,28 @@ public boolean handlePost(AnalyzeForm form, BindException errors) throws Excepti
}

NextFlowConfiguration config = NextFlowManager.get().getConfiguration();
if (config == null || config.getNextFlowConfigFilePath() == null)
{
errors.reject(ERROR_MSG, "NextFlow has not been configured");
return false;
}
if (StringUtils.isBlank(form.getConfigFile()))
{
errors.reject(ERROR_MSG, "No config file specified");
return false;
}
File configDir = new File(config.getNextFlowConfigFilePath());
File configFile = FileUtil.appendPath(configDir, Path.parse(form.getConfigFile()));
File configFile;
try
{
// appendPath normalizes and enforces that the resolved path stays within configDir, rejecting traversal
configFile = FileUtil.appendPath(configDir, Path.parse(form.getConfigFile()));

@vagisha vagisha Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path.parse could throw a NPE if getConfigFile() returns null. Should we add a StringUtils.isBlank(form.getConfigFile()) guard before this try/catch?

}
catch (InvalidPathException e)
{
errors.reject(ERROR_MSG, "Invalid config file");
return false;
}
if (!configFile.exists())
{
errors.reject(ERROR_MSG, "Config file does not exist");
Expand Down