Merge remote-tracking branch 'origin/release26.3-SNAPSHOT' into 26.3_fb_tnprc_migration

Author: labkey-martyp

.github/pull_request_template.md

@@ -10,6 +10,6 @@
 <!-- list of standard tasks (remove this comment to enable)
 #### Tasks 📍
 - [ ] Manual Testing
-- [ ] Needs Automation
+- [ ] Test Automation
 - [ ] Verify Fix
--->
+-->

api/src/org/labkey/api/assay/AbstractAssayProvider.java

@@ -124,6 +124,7 @@
 import javax.script.ScriptEngine;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URI;
 import java.net.URL;
 import java.sql.ResultSet;
@@ -1274,9 +1275,9 @@ public Pair<ValidationException, Pair<String, String>> setValidationAndAnalysisS
                     if (!(engine instanceof ExternalScriptEngine && ((ExternalScriptEngine) engine).isBinary(scriptFile)))
                     {
                         String scriptText;
-                        try
+                        try (InputStream is = scriptFile.openInputStream())
                         {
-                            scriptText = IOUtils.toString(scriptFile.openInputStream(), StringUtilsLabKey.DEFAULT_CHARSET);
+                            scriptText = IOUtils.toString(is, StringUtilsLabKey.DEFAULT_CHARSET);
                         }
                         catch (IOException e)
                         {

api/src/org/labkey/api/exp/list/ListService.java

@@ -51,6 +51,7 @@ static void setInstance(ListService ls)
     boolean hasLists(Container container, boolean includeProjectAndShared);
     ListDefinition createList(Container container, String name, ListDefinition.KeyType keyType);
     ListDefinition createList(Container container, String name, ListDefinition.KeyType keyType, @Nullable TemplateInfo templateInfo, @Nullable ListDefinition.Category category);
+    void deleteLists(Container container, User user, @Nullable String auditUserComment);
     @Nullable ListDefinition getList(Container container, int listId);
     @Nullable ListDefinition getList(Container container, String name);
     @Nullable ListDefinition getList(Container container, String name, boolean includeProjectAndShared);

api/src/org/labkey/api/exp/property/AbstractDomainKind.java

@@ -206,7 +206,7 @@ protected boolean getBlanksSql(Domain domain, DomainProperty prop, SQLFragment b
         else
         {
             // Issue 29047
-            blankRowsSQL.appendIdentifier(columnId).append(" IS NOT NULL");
+            blankRowsSQL.appendIdentifier(columnId).append(" IS NULL");
         }
         if (prop.isMvEnabled())
         {

api/src/org/labkey/api/mcp/McpService.java

@@ -29,11 +29,11 @@
 /// ### MCP Development Guide
 /// `McpService` lets you expose functionality over the MCP protocol (only simple http for now). This allows external
 /// chat sessions to pull information from LabKey Server. Exposed functionality is also made available to chat sessions
-/// hosted by LabKey (see `AbstractAgentAction``).
+/// hosted by LabKey (see `AbstractAgentAction`).
 ///
 /// ### Adding a new MCP class
 /// 1. Create a new class that implements `McpImpl` (see below) in the appropriate module
-/// 2. Register that class in your module `init()` method: `McpService.get().register(new MyMcp())`
+/// 2. Register that class in your module's `startup()` method: `McpService.get().register(new MyMcp())`
 /// 3. Add tools and resources
 ///
 /// ### Adding a new MCP tool
@@ -44,13 +44,13 @@
 ///     permission annotation is required, otherwise your tool will not be registered.**
 /// 4.  Add `ToolContext` as the first parameter to the method
 /// 5.  Add additional required or optional parameters to the method signature, as needed. Note that "required" is the
-///     default. Again here, the parameter descriptions are very important. Provide examples.
+///     default. Again here, the parameter descriptions are very important. Provide examples of parameter values.
 /// 6.  Use the helper method `getContext(ToolContext)` to retrieve the current `Container` and `User`
 /// 7.  Use the helper method `getUser(ToolContext)` in the rare cases where you need just a `User`
 /// 8.  Perform additional permissions checking (beyond what the annotations offer), where appropriate
 /// 9.  Filter all results to the current container, of course
 /// 10. For any error conditions, throw exceptions with detailed information. These will get translated into appropriate
-///     failure responses and the LLM client will attempt to correct the problem.
+///     failure responses and the LLM client will attempt to correct any problems (hopefully).
 /// 11. For success cases, return a String with a message or JSON content, for example, `JSONObject.toString()`. Spring
 ///     has some limited ability to convert other objects into JSON strings, but we haven't experimented with that. See
 ///     `DefaultToolCallResultConverter` and the ability to provide a custom result converter via the `@Tool` annotation.
@@ -126,6 +126,7 @@ static void setInstance(McpService service)
 
     boolean isReady();
 
+    // Register MCPs in Module.startup()
     default void register(McpImpl mcp)
     {
         try

api/src/org/labkey/api/module/ModuleLoader.java

@@ -580,19 +580,6 @@ private void doInit(Execution execution) throws ServletException
             throw new IllegalStateException("Core module was not first or could not find the Core module. Ensure that Tomcat user can create directories under the <LABKEY_HOME>/modules directory.");
         setProjectRoot(coreModule);
 
-        for (Module module : modules)
-        {
-            module.registerFilters(_servletContext);
-        }
-        for (Module module : modules)
-        {
-            module.registerServlets(_servletContext);
-        }
-        for (Module module : modules)
-        {
-            module.registerFinalServlets(_servletContext);
-        }
-
         // Do this after we've checked to see if we can find the core module. See issue 22797.
         verifyProductionModeMatchesBuild();
 
@@ -790,12 +777,34 @@ public void addStaticWarnings(@NotNull Warnings warnings, boolean showAllWarning
         if (!modulesRequiringUpgrade.isEmpty() || !additionalSchemasRequiringUpgrade.isEmpty())
             setUpgradeState(UpgradeState.UpgradeRequired);
 
-        // Don't accept any requests if we're bootstrapping empty schemas or migrating from SQL Server
+        // Don't accept any requests if we're bootstrapping empty schemas or doing a database migration
         if (!shouldInsertData())
             execution = Execution.Synchronous;
 
         startNonCoreUpgradeAndStartup(execution, lockFile);
 
+        // Register filters and servlets at the last minute, just before Tomcat starts. At this point, the list of
+        // modules is final. We've had one case where the CSP filter was getting initialized before the core module
+        // was initialized, GitHub Issue 1008. We have no idea how that happened, but registering late won't hurt.
+
+        _log.info("Registering filters");
+
+        for (Module module : _modules)
+        {
+            module.registerFilters(_servletContext);
+        }
+
+        _log.info("Registering servlets");
+
+        for (Module module : _modules)
+        {
+            module.registerServlets(_servletContext);
+        }
+        for (Module module : _modules)
+        {
+            module.registerFinalServlets(_servletContext);
+        }
+
         _log.info("LabKey Server startup is complete; {}", execution.getLogMessage());
     }
 

api/src/org/labkey/api/query/QueryService.java

@@ -16,18 +16,38 @@
 
 package org.labkey.api.query;
 
+import jakarta.servlet.http.HttpSession;
 import org.apache.commons.collections4.SetValuedMap;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.json.JSONObject;
 import org.labkey.api.audit.AuditHandler;
 import org.labkey.api.audit.DetailedAuditTypeEvent;
+import org.labkey.api.data.ColumnHeaderType;
+import org.labkey.api.data.ColumnInfo;
+import org.labkey.api.data.CompareType;
+import org.labkey.api.data.Container;
+import org.labkey.api.data.ContainerFilter;
+import org.labkey.api.data.DbSchema;
+import org.labkey.api.data.DisplayColumn;
+import org.labkey.api.data.Filter;
+import org.labkey.api.data.JdbcType;
+import org.labkey.api.data.MethodInfo;
+import org.labkey.api.data.MutableColumnInfo;
+import org.labkey.api.data.ParameterDescription;
+import org.labkey.api.data.ParameterDescriptionImpl;
+import org.labkey.api.data.QueryLogging;
+import org.labkey.api.data.Results;
+import org.labkey.api.data.SQLFragment;
+import org.labkey.api.data.Sort;
+import org.labkey.api.data.SqlSelector;
+import org.labkey.api.data.TableInfo;
+import org.labkey.api.data.TableSelector;
+import org.labkey.api.data.dialect.SqlDialect;
 import org.labkey.api.gwt.client.model.GWTPropertyDescriptor;
+import org.labkey.api.module.Module;
 import org.labkey.api.pipeline.PipelineJob;
 import org.labkey.api.query.column.ColumnInfoTransformer;
-import org.labkey.api.data.*;
-import org.labkey.api.data.dialect.SqlDialect;
-import org.labkey.api.module.Module;
 import org.labkey.api.query.column.ConceptURIColumnInfoTransformer;
 import org.labkey.api.query.snapshot.QuerySnapshotDefinition;
 import org.labkey.api.security.User;
@@ -41,7 +61,6 @@
 import org.labkey.data.xml.TableType;
 import org.springframework.web.servlet.mvc.Controller;
 
-import jakarta.servlet.http.HttpSession;
 import java.io.IOException;
 import java.sql.ResultSet;
 import java.sql.SQLException;
@@ -653,6 +672,13 @@ default MutableColumnInfo applyColumnTransformer(MutableColumnInfo col)
     @Nullable
     ContainerFilter getContainerFilterForLookups(Container container, User user);
 
+    /**
+     * Provides the configured ContainerFilter to utilize when requesting data that is being read
+     * within a folder context. Equivalent to the client side function in @labkey/components
+     */
+    @Nullable
+    ContainerFilter getContainerFilterForFolder(Container container, User user);
+
 
     interface SelectBuilder
     {

api/src/org/labkey/api/study/publish/StudyPublishService.java

@@ -156,7 +156,7 @@ ActionURL publishData(User user, Container sourceContainer, @Nullable Container
 
     String checkForLockedLinks(Dataset def, @Nullable List<Long> rowIds);
 
-    void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection<Pair<String,Long>> datasetRowLsidAndSourceRowIds);
+    void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection<Long> rowIds);
 
     /**
      * Adds columns to an assay data table, providing a link to any datasets that have

api/src/org/labkey/api/util/XmlBeansUtil.java

@@ -132,6 +132,7 @@ public static void addComment(XmlTokenSource doc, String comment)
     public static final SAXParserFactory SAX_PARSER_FACTORY_ALLOWING_DOCTYPE;
     public static final XMLInputFactory XML_INPUT_FACTORY;
     public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY;
+    public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE;
 
     static
     {
@@ -145,16 +146,9 @@ public static void addComment(XmlTokenSource doc, String comment)
             SAX_PARSER_FACTORY = saxParserFactory(false);
             SAX_PARSER_FACTORY_ALLOWING_DOCTYPE = saxParserFactory(true);
 
-            //noinspection XMLInputFactory
-            DOCUMENT_BUILDER_FACTORY = DocumentBuilderFactory.newInstance();
-            DOCUMENT_BUILDER_FACTORY.setNamespaceAware(true);
-            DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-            DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-general-entities", false);
-            DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-            DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
-            DOCUMENT_BUILDER_FACTORY.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
-            DOCUMENT_BUILDER_FACTORY.setXIncludeAware(false);
-            DOCUMENT_BUILDER_FACTORY.setExpandEntityReferences(false);
+            DOCUMENT_BUILDER_FACTORY = documentBuilderFactory(false);
+            // Use the ALLOWING_DOCTYPE variant when parsing XML that contains a <!DOCTYPE> declaration (e.g. NCBI's eSummary responses)
+            DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE = documentBuilderFactory(true);
         }
         catch (ParserConfigurationException | SAXException e)
         {
@@ -181,4 +175,26 @@ private static SAXParserFactory saxParserFactory(boolean allowDocType) throws SA
         result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
         return result;
     }
+
+    private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocType) throws ParserConfigurationException
+    {
+        //noinspection XMLInputFactory
+        DocumentBuilderFactory result = DocumentBuilderFactory.newInstance();
+        result.setNamespaceAware(true);
+
+        // Disable features that could lead to XXE or other vulnerabilities.
+        // When allowDocType is true the DOCTYPE declaration is permitted. External entity
+        // resolution remains disabled, so XXE protection is still in effect.
+        if (!allowDocType)
+        {
+            result.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        }
+        result.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        result.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        result.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        result.setXIncludeAware(false);
+        result.setExpandEntityReferences(false);
+        return result;
+    }
 }