Fix security issues flagged by Claude

OpenLdapSync/src/org/labkey/openldapsync/ldap/LdapSyncRunner.java

@@ -269,16 +269,16 @@ private void setUserActive(User u, boolean active, String reason)
         try
         {
             log("Changing active state of user: " + u.getEmail() + " to " + active + (reason == null ? "" : ", reason: " + reason));
-            _usersInactivated++;
 
             if (!_previewOnly)
             {
                 UserManager.setUserActive(_settings.getLabKeyAdminUser(), u, active);
             }
+            _usersInactivated++;
         }
         catch (SecurityManager.UserManagementException e)
         {
-            _log.error("Unable to deactive user: " + u.getEmail());
+            _log.error("Unable to deactivate user: " + u.getEmail(), e);
         }
     }
 

SequenceAnalysis/api-src/org/labkey/api/sequenceanalysis/SequenceOutputFile.java

@@ -16,6 +16,7 @@
 package org.labkey.api.sequenceanalysis;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
+import org.apache.logging.log4j.Logger;
 import org.json.JSONObject;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
@@ -24,6 +25,11 @@
 import org.labkey.api.exp.api.ExpData;
 import org.labkey.api.exp.api.ExperimentService;
 import org.labkey.api.pipeline.PipelineJobService;
+import org.labkey.api.security.User;
+import org.labkey.api.security.permissions.Permission;
+import org.labkey.api.security.permissions.ReadPermission;
+import org.labkey.api.util.logging.LogHelper;
+import org.labkey.api.view.UnauthorizedException;
 
 import java.io.File;
 import java.io.Serializable;
@@ -34,6 +40,8 @@
  */
 public class SequenceOutputFile implements Serializable
 {
+    private static final Logger _log = LogHelper.getLogger(SequenceOutputFile.class, "Messages related to SequenceOutputFile");
+
     private Integer _rowid;
     private String _name;
     private String _description;
@@ -211,6 +219,28 @@ public void setModified(Date modified)
         _modified = modified;
     }
 
+    public static SequenceOutputFile getForId(Integer rowId, User u)
+    {
+        return getForId(rowId, u, ReadPermission.class);
+    }
+
+    public static SequenceOutputFile getForId(Integer rowId, User u, Class<? extends Permission> perm)
+    {
+        SequenceOutputFile so = getForId(rowId);
+        if (so.getContainerObj() == null)
+        {
+            _log.error("SequenceOutputFile lacks a valid container: " + rowId);
+            return null;
+        }
+
+        if (!so.getContainerObj().hasPermission(u, perm))
+        {
+            throw new UnauthorizedException("Insufficient permissions: " + rowId);
+        }
+
+        return so;
+    }
+
     public static SequenceOutputFile getForId(Integer rowId)
     {
         if (PipelineJobService.get().getLocationType() != PipelineJobService.LocationType.WebServer)

SequenceAnalysis/resources/queries/sequenceanalysis/distinctAnalysisSets.sql

@@ -0,0 +1,3 @@
+SELECT
+    DISTINCT rowid, name
+FROM sequenceanalysis.analysisSets

SequenceAnalysis/resources/web/SequenceAnalysis/window/AddFileSetsWindow.js

@@ -50,7 +50,8 @@ Ext4.define('SequenceAnalysis.window.AddFileSetsWindow', {
                         type: 'labkey-store',
                         containerPath: Laboratory.Utils.getQueryContainerPath(),
                         schemaName: 'laboratory',
-                        sql: 'SELECT DISTINCT rowid, name FROM sequenceanalysis.analysisSets',
+                        queryName: 'distinctAnalysisSets',
+                        columns: 'rowid,name',
                         autoLoad: true
                     },
                     valueField: 'rowid',

SequenceAnalysis/test/src/org/labkey/test/tests/external/labModules/SequenceTest.java

@@ -230,10 +230,15 @@ private void importReadsetMetadata()
         waitForElement(Locator.tagContainingText("a", "SRA2"));
 
         dr.checkAllOnPage();
+        Assert.assertEquals("Incorrect checked row count", 3, dr.getCheckedCount());
         dr.clickHeaderButtonAndWait("Delete");
         clickButton("OK");
 
         _readsetCt -= 3;
+
+        log("verifying readset count correct");
+        goToProjectHome();
+        waitForElement(LabModuleHelper.getNavPanelItem("Readsets:", _readsetCt.toString()));
     }
 
     /**
@@ -417,7 +422,7 @@ private void importIlluminaTest() throws Exception
     }
 
     /**
-     * This method has several puposes.  It will verify that the records from illuminaImportTest() were
+     * This method has several purposes.  It will verify that the records from illuminaImportTest() were
      * created properly.  It also exercises various features associated with the readset grid, including
      * the FASTQC report and downloading of results
      *

Studies/api-src/org/labkey/api/studies/study/StudyDefinition.java

@@ -2,12 +2,12 @@
 
 import com.fasterxml.jackson.annotation.JsonGetter;
 import com.fasterxml.jackson.annotation.JsonIgnore;
-import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonSetter;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectWriter;
 import org.json.JSONObject;
+import org.labkey.api.security.User;
 
 import java.util.Date;
 import java.util.List;
@@ -660,9 +660,9 @@ public String toJson() throws JsonProcessingException
         return ow.writeValueAsString(this);
     }
 
-    public static StudyDefinition getForId(int studyId)
+    public static StudyDefinition getForId(int studyId, User u)
     {
-        // TODO: implement this. This should query the DB and return a populated StudyDefinition
+        // TODO: implement this. This should query the DB and return a populated StudyDefinition. It should make sure the passed user has ReadPermission on that container
 
         return null;
     }

Studies/src/org/labkey/studies/StudiesManager.java

@@ -63,15 +63,33 @@ public StudyDefinition insertOrUpdateStudyDefinition(StudyDefinition sd, Contain
         DbScope scope   = schema.getScope();
 
         UserSchema us = QueryService.get().getUserSchema(u, c, StudiesSchema.NAME);
-
         TableInfo tblStudies       = us.getTable(StudiesSchema.TABLE_STUDIES);
         TableInfo tblCohorts       = us.getTable(StudiesSchema.TABLE_COHORTS);
         TableInfo tblAnchorEvents  = us.getTable(StudiesSchema.TABLE_ANCHOR_EVENTS);
         TableInfo tblTimepoints    = us.getTable(StudiesSchema.TABLE_EXPECTED_TIMEPOINTS);
 
         try (DbScope.Transaction tx = scope.ensureTransaction())
         {
-            sd.setContainer(c.getEntityId().toString());
+            if (sd.getRowId() != null)
+            {
+                TableSelector ts = new TableSelector(tblStudies, PageFlowUtil.set("container"), new SimpleFilter(FieldKey.fromString("rowId"), sd.getRowId()), null);
+                if (!ts.exists())
+                {
+                    throw new IllegalArgumentException("Unable to find existing study with rowId: " + sd.getRowId());
+                }
+
+                String existingContainerId = ts.getObject(String.class);
+                Container existingContainer = ContainerManager.getForId(existingContainerId);
+                if (!c.equals(existingContainer))
+                {
+                    throw new IllegalArgumentException("The study is from the wrong container: " + sd.getRowId());
+                }
+            }
+            else
+            {
+                sd.setContainer(c.getEntityId().toString());
+            }
+
             sd = upsertStudy(sd, tblStudies, c, u);
 
             upsertChildRecords(
@@ -198,7 +216,9 @@ private <T> void upsertChildRecords(int studyRowId,
         {
             List<Map<String,Object>> ret = qus.insertRows(u, c, inserts, bve, null, null);
             for (int i = 0; i < ret.size(); i++)
+            {
                 setRowId.set(insertBeans.get(i), asInteger(ret.get(i).get("rowId")));
+            }
         }
 
         if (!updates.isEmpty())

blast/src/org/labkey/blast/BLASTController.java

@@ -262,7 +262,6 @@ public String getResponse(RunBlastForm form, Map<String, Pair<FileLike, String>>
                 if (files.isEmpty())
                 {
                     //save query to string
-                    AssayFileWriter writer = new AssayFileWriter();
                     try
                     {
                         FileLike targetDirectory = AssayFileWriter.ensureUploadDirectory(getContainer());
@@ -280,24 +279,20 @@ public String getResponse(RunBlastForm form, Map<String, Pair<FileLike, String>>
                     }
                 }
 
-                if (!inputFiles.isEmpty())
+                for (FileLike input : inputFiles)
                 {
-                    for (FileLike input : inputFiles)
-                    {
-                        String jobId = BLASTManager.get().runBLASTN(getContainer(), getUser(), form.getDatabase(), input.toNioPathForRead().toFile(), form.getTask(), form.getTitle(), form.getSaveResults(), params, true);
-                        resp.put("jobId", jobId);
-                    }
+                    String jobId = BLASTManager.get().runBLASTN(getContainer(), getUser(), form.getDatabase(), input.toNioPathForRead().toFile(), form.getTask(), form.getTitle(), form.getSaveResults(), params, true);
+                    resp.put("jobId", jobId);
                 }
 
                 resp.put("success", true);
             }
             catch (Exception e)
             {
-                ExceptionUtil.logExceptionToMothership(getViewContext().getRequest(), e);
                 getViewContext().getResponse().setStatus(HttpServletResponse.SC_BAD_REQUEST);
                 logger.error(e.getMessage(), e);
                 resp.put("success", false);
-                resp.put("exception", e.getMessage());
+                resp.put("exception", "Error running BLASTN");
             }
 
             return resp.toString();

blast/src/org/labkey/blast/BLASTManager.java

@@ -22,6 +22,7 @@
 import org.json.JSONObject;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
+import org.labkey.api.data.ContainerType;
 import org.labkey.api.data.PropertyManager;
 import org.labkey.api.data.PropertyManager.WritablePropertyMap;
 import org.labkey.api.data.SimpleFilter;
@@ -139,7 +140,7 @@ public Container getContainerForDatabase(String databaseId)
     public void createDatabase(Container c, User u, Integer libraryId) throws IllegalArgumentException, IOException
     {
         //only create once per library
-        TableInfo databases = BLASTSchema.getInstance().getSchema().getTable(BLASTSchema.TABLE_DATABASES);
+        TableInfo databases = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), BLASTSchema.NAME).getTable(BLASTSchema.TABLE_DATABASES);
         TableSelector dbTs = new TableSelector(databases, PageFlowUtil.set("objectid"), new SimpleFilter(FieldKey.fromString("libraryid"), libraryId), null);
         if (dbTs.exists())
         {
@@ -159,14 +160,14 @@ public void createDatabase(Container c, User u, Integer libraryId) throws Illega
 
     public String runBLASTN(Container c, User u, String blastDb, File input, String task, String title, boolean saveResults, Map<String, String> params, boolean async) throws IllegalArgumentException, IOException
     {
-        TableInfo databases = BLASTSchema.getInstance().getSchema().getTable(BLASTSchema.TABLE_DATABASES);
+        TableInfo databases = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), BLASTSchema.NAME).getTable(BLASTSchema.TABLE_DATABASES);
         TableSelector ts = new TableSelector(databases, new SimpleFilter(FieldKey.fromString("objectid"), blastDb), null);
         if (!ts.exists())
         {
             throw new IllegalArgumentException("Unable to find BLAST DB: " + blastDb);
         }
 
-        TableInfo jobs = BLASTSchema.getInstance().getSchema().getTable(BLASTSchema.TABLE_BLAST_JOBS);
+        TableInfo jobs = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), BLASTSchema.NAME).getTable(BLASTSchema.TABLE_BLAST_JOBS);
         BlastJob databaseRecord = new BlastJob();
         databaseRecord.setDatabaseId(blastDb);
         databaseRecord.setTitle(title);

cluster/src/org/labkey/cluster/ClusterController.java

@@ -330,6 +330,12 @@ public boolean handlePost(JobIdsForm form, BindException errors) throws Exceptio
                     return false;
                 }
 
+                if (!sf.lookupContainer().hasPermission(getUser(), AdminPermission.class))
+                {
+                    errors.reject(ERROR_MSG, "Insufficient permissions to update: " + id);
+                    return false;
+                }
+
                 sfs.add(sf);
             }
 
@@ -422,6 +428,12 @@ public boolean handlePost(JobIdsForm form, BindException errors) throws Exceptio
                     return false;
                 }
 
+                if (!sf.lookupContainer().hasPermission(getUser(), AdminPermission.class))
+                {
+                    errors.reject(ERROR_MSG, "Insufficient permissions to update: " + id);
+                    return false;
+                }
+
                 sfs.add(sf);
             }
 

cluster/src/org/labkey/cluster/pipeline/AbstractClusterExecutionEngine.java

@@ -27,6 +27,7 @@
 import org.labkey.api.query.FieldKey;
 import org.labkey.api.security.User;
 import org.labkey.api.util.FileUtil;
+import org.labkey.api.util.LabKeyProcessBuilder;
 import org.labkey.api.util.NetworkDrive;
 import org.labkey.api.util.PageFlowUtil;
 import org.labkey.api.util.Pair;
@@ -785,7 +786,7 @@ protected List<String> execute(String command, @Nullable File workDir)
 
         try
         {
-            Process p = Runtime.getRuntime().exec(command, null, workDir);
+            Process p = new LabKeyProcessBuilder(command).directory(workDir).start();
             try
             {
                 String output = IOUtils.toString(p.getInputStream(), StringUtilsLabKey.DEFAULT_CHARSET);