Skip to content

Address reported security issues in the LINCS module #652

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
vagisha merged 3 commits into from
Jun 29, 2026
Merged

Address reported security issues in the LINCS module #652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9b5048
Addresses reported security issues in the LINCS module
vagisha Jun 28, 2026
4267fd7
* manageClueCredentials.jsp: added <labkey:errors/> so the https-requ…
vagisha Jun 29, 2026
289226a
Container-scoped LincsManager.deleteLincsPspJobsForRun
vagisha 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
4 changes: 2 additions & 2 deletions lincs/src/org/labkey/lincs/DocImportListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ public void beforeRunDelete(ExpProtocol protocol, ExpRun run, User user)
return;
}

ITargetedMSRun tRun = TargetedMSService.get().getRunByFileName(run.getName(), run.getContainer());
ITargetedMSRun tRun = TargetedMSService.get().getRunByFileName(run.getName(), c);
if(tRun != null)
{
// Delete saved entries in lincs.lincspspjob table for this runId
LincsManager.get().deleteLincsPspJobsForRun(tRun.getId());
LincsManager.get().deleteLincsPspJobsForRun(tRun.getId(), c);
}

// Get the file root for the container
Expand Down
50 changes: 42 additions & 8 deletions lincs/src/org/labkey/lincs/LincsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.labkey.api.action.SimpleErrorView;
import org.labkey.api.action.SimpleViewAction;
import org.labkey.api.action.SpringActionController;
import org.labkey.api.audit.AuditLogService;
import org.labkey.api.audit.ClientApiAuditProvider;
import org.labkey.api.data.ActionButton;
import org.labkey.api.data.ButtonBar;
import org.labkey.api.data.Container;
Expand Down Expand Up @@ -108,6 +110,7 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -720,8 +723,22 @@ public ModelAndView getView(DownloadCustomGCTReportForm form, BindException erro
return new SimpleErrorView(errors, false);
}

Path gctDir = getGCTDir(getContainer());
Path downloadFile = gctDir.resolve(form.getFileName());
// isAllowedFileName returns null when the name is acceptable, an error message otherwise.
String fileNameError = FileUtil.isAllowedFileName(form.getFileName(), false);
if(fileNameError != null)
{
errors.reject(ERROR_MSG, "Invalid fileName '" + form.getFileName() + "': " + fileNameError);
return new SimpleErrorView(errors, false);
}

Path gctDir = getGCTDir(getContainer()).normalize();
Path downloadFile = gctDir.resolve(form.getFileName()).normalize();
// Ensure the resolved path is still inside the GCT directory
if(!downloadFile.startsWith(gctDir))
{
errors.reject(ERROR_MSG, "Invalid fileName '" + form.getFileName() + "'.");
return new SimpleErrorView(errors, false);
}
if(!Files.exists(downloadFile))
{
errors.reject(ERROR_MSG, "File does not exist '" + form.getFileName() + "'.");
Expand Down Expand Up @@ -1011,15 +1028,28 @@ private static boolean isOrHasAncestor(Container container, String name)
public static class ManageLincsClueCredentials extends FormViewAction<ClueCredentialsForm>
{
@Override
public void validateCommand(ClueCredentialsForm target, Errors errors) {}
public void validateCommand(ClueCredentialsForm form, Errors errors)
{
// The API key entered on this form will later be sent to this server URI as a request header,
// so reject a non-https URI at save time to keep the key out of clear text (CWE-319).
String uri = form.getServerUri();
if (StringUtils.isNotBlank(uri) && !uri.trim().toLowerCase(Locale.ROOT).startsWith("https://"))
{
errors.reject(ERROR_MSG, "Clue/PSP Server URI must use https so the API key is not transmitted in clear text.");
}
}

@Override
public boolean handlePost(ClueCredentialsForm form, BindException errors)
{
WritablePropertyMap map = PropertyManager.getEncryptedStore().getWritableProperties(getContainer(), LINCS_CLUE_CREDENTIALS, true);
map.put(CLUE_SERVER_URI, form.getServerUri());
map.put(CLUE_SERVER_URI, StringUtils.trim(form.getServerUri()));
map.put(CLUE_API_KEY, form.getApiKey());
map.save();

// Record an audit event noting the container and the user who changed the credentials.
AuditLogService.get().addEvent(getUser(),
new ClientApiAuditProvider.ClientApiAuditEvent(getContainer(), "LINCS Clue/PSP server credentials updated."));
return true;
}

Expand Down Expand Up @@ -1106,6 +1136,10 @@ public boolean handlePost(CromwellConfigForm form, BindException errors)
return false;
}
config.save(getContainer());

// Record an audit event noting the container and the user who changed the Cromwell configuration.
AuditLogService.get().addEvent(getUser(),
new ClientApiAuditProvider.ClientApiAuditEvent(getContainer(), "LINCS Cromwell configuration updated."));
return true;
}

Expand Down Expand Up @@ -1188,7 +1222,7 @@ public ModelAndView getView(LincsPspJobForm form, BindException errors)
{
int runId = form.getRunId();

LincsPspJob pspJob = LincsManager.get().getLincsPspJobForRun(runId);
LincsPspJob pspJob = LincsManager.get().getLincsPspJobForRun(runId, getContainer());
if(pspJob == null)
{
errors.addError(new LabKeyError("Could not find a PSP job for runId: " + runId));
Expand Down Expand Up @@ -1333,7 +1367,7 @@ public ModelAndView getView(LincsPspJobForm form, BindException errors)
{
int jobId = form.getJobId();

LincsPspJob pspJob = LincsManager.get().getLincsPspJob(jobId);
LincsPspJob pspJob = LincsManager.get().getLincsPspJob(jobId, getContainer());
if(pspJob == null)
{
errors.addError(new LabKeyError("Could not find a PSP job for id: " + jobId));
Expand Down Expand Up @@ -1411,7 +1445,7 @@ public boolean handlePost(LincsPspJobForm form, BindException errors)
int jobId = form.getJobId();
_runId = form.getRunId();

LincsPspJob pspJob = LincsManager.get().getLincsPspJob(jobId);
LincsPspJob pspJob = LincsManager.get().getLincsPspJob(jobId, getContainer());
if(pspJob == null)
{
errors.reject(ERROR_MSG, "Could not find a PSP job for id: " + jobId);
Expand Down Expand Up @@ -1537,7 +1571,7 @@ public boolean handlePost(LincsPspJobForm form, BindException errors)

LincsManager lincsManager = LincsManager.get();

LincsPspJob oldPspJob = lincsManager.getLincsPspJobForRun(runId);
LincsPspJob oldPspJob = lincsManager.getLincsPspJobForRun(runId, container);
LincsPspJob newPspJob = lincsManager.saveNewLincsPspJob(skylineRun, getUser());

ViewBackgroundInfo info = new ViewBackgroundInfo(container, getUser(), null);
Expand Down
9 changes: 5 additions & 4 deletions lincs/src/org/labkey/lincs/LincsDataTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.jetbrains.annotations.Nullable;
import org.labkey.api.analytics.AnalyticsService;
import org.labkey.api.data.ColumnInfo;
import org.labkey.api.data.Container;
import org.labkey.api.data.DataColumn;
import org.labkey.api.data.RenderContext;
import org.labkey.api.data.TableInfo;
Expand Down Expand Up @@ -154,7 +155,7 @@ public void renderGridCellContents(RenderContext ctx, HtmlWriter out)
out.write("NO_RUN_ID");
return;
}
LincsPspJob pspJob = LincsManager.get().getLincsPspJobForRun(runId);
LincsPspJob pspJob = LincsManager.get().getLincsPspJobForRun(runId, getContainer());
if(pspJob == null)
{
out.write("PSP job not found for runId: " + runId);
Expand Down Expand Up @@ -374,7 +375,7 @@ public void renderGridCellContents(RenderContext ctx, HtmlWriter out)
String downloadFileName = getBaseName(fileName);
String extension = LincsModule.getExt(getLevel());
downloadFileName = downloadFileName + extension;
if(!fileAvailable(runId, downloadFileName))
if(!fileAvailable(runId, downloadFileName, ctx.getContainer()))
{
out.write("NOT AVAILABLE");
return;
Expand Down Expand Up @@ -403,9 +404,9 @@ private void renderGridCell(HtmlWriter out, String analyticsScript, String downl
).appendTo(out);
}

private boolean fileAvailable(Integer runId, String downloadFileName)
private boolean fileAvailable(Integer runId, String downloadFileName, Container container)
{
LincsPspJob job = LincsManager.get().getLincsPspJobForRun(runId);
LincsPspJob job = LincsManager.get().getLincsPspJobForRun(runId, container);
if(job != null)
{
if(getLevel() == LincsModule.LincsLevel.Two && job.isLevel2Done())
Expand Down
18 changes: 12 additions & 6 deletions lincs/src/org/labkey/lincs/LincsManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,24 +164,30 @@ public void updatePipelineJobId(LincsPspJob job)
job.getPipelineJobId(), job.getId());
}

public LincsPspJob getLincsPspJobForRun(int runId)
public LincsPspJob getLincsPspJobForRun(int runId, Container container)
{
// Get the most recent PSP job details
Sort sort = new Sort();
sort.appendSortColumn(FieldKey.fromParts("Modified"), Sort.SortDirection.DESC, true);
TableSelector ts = new TableSelector(getTableInfoLincsPspJob(), new SimpleFilter(FieldKey.fromParts("RunId"), runId), sort);
SimpleFilter filter = SimpleFilter.createContainerFilter(container);
filter.addCondition(FieldKey.fromParts("RunId"), runId);
TableSelector ts = new TableSelector(getTableInfoLincsPspJob(), filter, sort);
ts.setMaxRows(1);
return ts.getObject(LincsPspJob.class);
}

public LincsPspJob getLincsPspJob(int id)
public LincsPspJob getLincsPspJob(int id, Container container)
{
return new TableSelector(getTableInfoLincsPspJob(), new SimpleFilter(FieldKey.fromParts("Id"), id), null).getObject(LincsPspJob.class);
SimpleFilter filter = SimpleFilter.createContainerFilter(container);
filter.addCondition(FieldKey.fromParts("Id"), id);
return new TableSelector(getTableInfoLincsPspJob(), filter, null).getObject(LincsPspJob.class);
}

public void deleteLincsPspJobsForRun(long runId)
public void deleteLincsPspJobsForRun(long runId, Container container)
{
Table.delete(getTableInfoLincsPspJob(), new SimpleFilter(FieldKey.fromParts("runId"), runId));
SimpleFilter filter = SimpleFilter.createContainerFilter(container);
filter.addCondition(FieldKey.fromParts("runId"), runId);
Table.delete(getTableInfoLincsPspJob(), filter);
}

public void deleteLincsPspJob(LincsPspJob job)
Expand Down
7 changes: 7 additions & 0 deletions lincs/src/org/labkey/lincs/psp/LincsPspUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.net.HttpURLConnection;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.Locale;

public class LincsPspUtil
{
Expand Down Expand Up @@ -55,6 +56,12 @@ public static PspEndpoint getPspEndpoint(Container container) throws LincsPspExc
{
throw new LincsPspException("Could not find PSP API Key in the saved properties.");
}
// The API key retrieved here is later sent to the endpoint URL as a request header by the callers.
// Refuse to return a non-https endpoint so the key is never transmitted in clear text (CWE-319).
if(!pspUrl.trim().toLowerCase(Locale.ROOT).startsWith("https://"))
{
throw new LincsPspException("PSP endpoint URL must use https so the API key is not transmitted in clear text.");
}
return new PspEndpoint(pspUrl, pspApiKey);
}

Expand Down
1 change: 1 addition & 0 deletions lincs/src/org/labkey/lincs/view/manageClueCredentials.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
LincsController.ClueCredentialsForm form = ((JspView<LincsController.ClueCredentialsForm>) HttpView.currentView()).getModelBean();
%>

<labkey:errors/>
<labkey:form method="post">
<table>
<tr>
Expand Down