Skip to content

Register filters & servlets later + CSP improvements #7550

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

Open
labkey-adam wants to merge 5 commits into
base: release26.3-SNAPSHOT
Choose a base branch
from
Open

Register filters & servlets later + CSP improvements #7550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28399d4
Register filters & servlets later + CSP improvements
labkey-adam Apr 4, 2026
4b75faf
Clarify comment
labkey-adam Apr 4, 2026
4b0632f
More clarity
labkey-adam Apr 4, 2026
758da39
Remove errant semicolon
labkey-adam Apr 4, 2026
07a278b
Set reporting endpoints header value later
labkey-adam Apr 4, 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
9 changes: 5 additions & 4 deletions api/src/org/labkey/api/mcp/McpService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -126,6 +126,7 @@ static void setInstance(McpService service)

boolean isReady();

// Register MCPs in Module.startup()
default void register(McpImpl mcp)
{
try
Expand Down
37 changes: 23 additions & 14 deletions api/src/org/labkey/api/module/ModuleLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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());
}

Expand Down
137 changes: 37 additions & 100 deletions api/src/org/labkey/filters/ContentSecurityPolicyFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import org.apache.commons.collections4.multimap.HashSetValuedHashMap;
import org.apache.commons.lang3.EnumUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Strings;
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -30,7 +29,6 @@
import org.labkey.api.util.StringExpressionFactory;
import org.labkey.api.util.StringExpressionFactory.AbstractStringExpression.NullValueBehavior;
import org.labkey.api.util.logging.LogHelper;
import org.labkey.api.view.ActionURL;

import java.io.IOException;
import java.security.SecureRandom;
Expand All @@ -40,13 +38,11 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;


/**
* Content Security Policies (CSPs) are loaded from the csp.enforce and csp.report properties in application.properties.
*/
Expand All @@ -57,6 +53,7 @@ public class ContentSecurityPolicyFilter implements Filter
private static final String NONCE_SUBST = "REQUEST.SCRIPT.NONCE";
private static final String UPGRADE_INSECURE_REQUESTS_SUBSTITUTION = "UPGRADE.INSECURE.REQUESTS";
private static final String HEADER_NONCE = "org.labkey.filters.ContentSecurityPolicyFilter#NONCE"; // needs to match PageConfig.HEADER_NONCE
private static final String REPORTING_ENDPOINTS_HEADER = "Reporting-Endpoints";

private static final Map<ContentSecurityPolicyType, ContentSecurityPolicyFilter> CSP_FILTERS = new CopyOnWriteHashMap<>();

Expand All @@ -76,11 +73,14 @@ public class ContentSecurityPolicyFilter implements Filter
private @NotNull String _cspVersion = "Unknown";
// These two are effectively @NotNull since they are set to non-null values in init() and never changed
private String _stashedTemplate = null;
private String _reportToEndpointName = null;

// Per-filter-instance settings are initialized on first request and reset when base server URL or allowed sources
// change. Don't reference this directly; always use ensureSettings().
private volatile @Nullable CspFilterSettings _settings = null;
// We can't set this statically because the class is referenced before URLProviders are available
@SuppressWarnings("DataFlowIssue")
private final String _reportingEndpointsHeaderValue = "csp-report=\"" + PageFlowUtil.urlProvider(AdminUrls.class).getCspReportToURL().getLocalURIString() + "\"";

// Initialized on first request and reset when allowed sources change. Don't reference this directly; always use
// ensurePolicyExpression().
private volatile StringExpression _policyExpression;

public enum ContentSecurityPolicyType
{
Expand Down Expand Up @@ -118,7 +118,7 @@ public String getHeaderName()
@Override
public void init(FilterConfig filterConfig) throws ServletException
{
LogHelper.getLogger(ContentSecurityPolicyFilter.class, "CSP filter initialization").info("Initializing {}", filterConfig.getFilterName());
LOG.info("Initializing {}", filterConfig.getFilterName());
Enumeration<String> paramNames = filterConfig.getInitParameterNames();
while (paramNames.hasMoreElements())
{
Expand Down Expand Up @@ -146,9 +146,6 @@ else if ("disposition".equalsIgnoreCase(paramName))

if (CSP_FILTERS.put(getType(), this) != null)
throw new ServletException("ContentSecurityPolicyFilter is misconfigured, duplicate policies of type: " + getType());

// configure a different endpoint for each type. TODO: We only need one CSP violation reporting endpoint now, so one header would do
_reportToEndpointName = "csp-" + getType().name().toLowerCase();
}

/** Filter out block comments and replace special characters in the provided policy */
Expand Down Expand Up @@ -197,18 +194,21 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
{
if (request instanceof HttpServletRequest req && response instanceof HttpServletResponse resp)
{
CspFilterSettings settings = ensureSettings();
StringExpression expression = ensurePolicyExpression();

if (getType() != ContentSecurityPolicyType.Enforce || !OptionalFeatureService.get().isFeatureEnabled(FEATURE_FLAG_DISABLE_ENFORCE_CSP))
{
Map<String, String> map = Map.of(NONCE_SUBST, getScriptNonceHeader(req));
var csp = settings.getPolicyExpression().eval(map);
resp.setHeader(getType().getHeaderName(), csp);
String csp = expression.eval(map);

if ("https".equals(req.getScheme()))
{
if (resp.getHeader(REPORTING_ENDPOINTS_HEADER) == null)
resp.addHeader(REPORTING_ENDPOINTS_HEADER, _reportingEndpointsHeaderValue);
csp = csp + " report-to csp-report ;";
}

// null if https: is not configured on this server
String reportingEndpointsHeaderValue = settings.getReportingEndpointsHeaderValue();
if (reportingEndpointsHeaderValue != null)
resp.addHeader("Reporting-Endpoints", reportingEndpointsHeaderValue);
resp.setHeader(getType().getHeaderName(), csp);
}
}
chain.doFilter(request, response);
Expand All @@ -229,91 +229,26 @@ public String getStashedTemplate()
return _stashedTemplate;
}

public String getReportToEndpointName()
private void clearPolicyExpression()
{
return _reportToEndpointName;
_policyExpression = null;
}

private void clearSettings()
private @NotNull StringExpression ensurePolicyExpression()
{
_settings = null;
}
StringExpression expression = _policyExpression;

private @NotNull CspFilterSettings ensureSettings()
{
String baseServerUrl = AppProps.getInstance().getBaseServerUrl();
CspFilterSettings settings = _settings; // Stash a local copy to ensure consistency in the checks below

// Reset settings if null or if base server URL has changed
if (null == settings || !Objects.equals(baseServerUrl, settings.getPreviousBaseServerUrl()))
if (expression == null)
{
settings = _settings = new CspFilterSettings(this, baseServerUrl);
}

return settings;
}

// Hold all the mutable per-filter settings in a single object so they can be set atomically
private static class CspFilterSettings
{
private final String _policyTemplate;
private final String _reportingEndpointsHeaderValue;
private final String _previousBaseServerUrl;
private final StringExpression _policyExpression;

private CspFilterSettings(ContentSecurityPolicyFilter filter, String baseServerUrl)
{
// Add "Reporting-Endpoints" header and "report-to" directive only if https: is configured on this
// server. This ensures that browsers fall-back on report-uri if https: isn't configured.
if (Strings.CI.startsWith(baseServerUrl, "https://"))
{
// Each filter adds its own "Reporting-Endpoints" header since we want to convey the correct version (eXX vs. rXX)
@SuppressWarnings("DataFlowIssue")
ActionURL violationUrl = PageFlowUtil.urlProvider(AdminUrls.class).getCspReportToURL();
// Use an absolute URL so we always post to https:, even if the violating request uses http:
_reportingEndpointsHeaderValue = filter.getReportToEndpointName() + "=\"" + violationUrl.getURIString() + "\"";

// Add "report-to" directive to the policy
_policyTemplate = filter.getStashedTemplate() + " report-to " + filter.getReportToEndpointName() + " ;";
}
else
{
_policyTemplate = filter.getStashedTemplate();
_reportingEndpointsHeaderValue = null;
}

_previousBaseServerUrl = baseServerUrl;

final String substitutedPolicy;

synchronized (SUBSTITUTION_LOCK)
{
substitutedPolicy = StringExpressionFactory.create(_policyTemplate, false, NullValueBehavior.KeepSubstitution)
var substitutedPolicy = StringExpressionFactory.create(getStashedTemplate(), false, NullValueBehavior.KeepSubstitution)
.eval(SUBSTITUTION_MAP);
expression = _policyExpression = StringExpressionFactory.create(substitutedPolicy, false, NullValueBehavior.ReplaceNullAndMissingWithBlank);
}

_policyExpression = StringExpressionFactory.create(substitutedPolicy, false, NullValueBehavior.ReplaceNullAndMissingWithBlank);
}

public String getPolicyTemplate()
{
return _policyTemplate;
}

public String getReportingEndpointsHeaderValue()
{
return _reportingEndpointsHeaderValue;
}

public String getPreviousBaseServerUrl()
{
return _previousBaseServerUrl;
}

public StringExpression getPolicyExpression()
{
return _policyExpression;
}
return expression;
}

public static String getScriptNonceHeader(HttpServletRequest request)
Expand Down Expand Up @@ -362,7 +297,7 @@ public static void unregisterAllowedSources(String key, Directive directive)

/**
* Regenerate the substitution map on every register/unregister. The policy expression will be regenerated on the
* next request (see {@link #ensureSettings()}).
* next request (see {@link #ensurePolicyExpression()}).
*/
public static void regenerateSubstitutionMap()
{
Expand All @@ -389,7 +324,7 @@ public static void regenerateSubstitutionMap()

// Tell each registered ContentSecurityPolicyFilter to clear its settings so the next request recreates them
// using the new substitution map
CSP_FILTERS.values().forEach(ContentSecurityPolicyFilter::clearSettings);
CSP_FILTERS.values().forEach(ContentSecurityPolicyFilter::clearPolicyExpression);
}
}

Expand Down Expand Up @@ -436,7 +371,7 @@ public static List<String> getMissingSubstitutions(ContentSecurityPolicyType typ
}
else
{
String template = filter.ensureSettings().getPolicyTemplate();
String template = filter.getStashedTemplate();
ret = Arrays.stream(Directive.values())
.map(dir -> "${" + dir.getSubstitutionKey() + "}")
.filter(key -> !template.contains(key))
Expand All @@ -451,13 +386,15 @@ public static void registerMetricsProvider()
UsageMetricsService.get().registerUsageMetrics("API", () -> Map.of("cspFilters", CSP_FILTERS.values().stream()
.collect(Collectors.toMap(ContentSecurityPolicyFilter::getType,
filter -> {
CspFilterSettings settings = filter.ensureSettings();
StringExpression expression = filter.ensurePolicyExpression();
return Map.of(
"version", filter.getCspVersion(),
"csp", settings.getPolicyTemplate(),
"cspSubstituted", settings.getPolicyExpression().getSource()
"csp", filter.getStashedTemplate(),
"cspSubstituted", expression.getSource()
);
}))));
}))
)
);
}

public static class TestCase extends Assert
Expand Down Expand Up @@ -630,7 +567,7 @@ public void testSubstitutionMap()
private void verifySubstitutionInPolicyExpressions(String value, int expectedCount)
{
List<String> failures = CSP_FILTERS.values().stream()
.map(filter -> filter.ensureSettings().getPolicyExpression().eval(Map.of()))
.map(filter -> filter.ensurePolicyExpression().eval(Map.of()))
.filter(policy -> StringUtils.countMatches(policy, value) != expectedCount)
.toList();

Expand Down
3 changes: 2 additions & 1 deletion core/src/org/labkey/core/CoreModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,6 @@ public QuerySchema createSchema(DefaultSchema schema, Module module)

ScriptEngineManagerImpl.registerEncryptionMigrationHandler();

McpService.get().register(new CoreMcp());
PostgreSqlService.setInstance(PostgreSqlDialectFactory::getLatestSupportedDialect);

deleteTempFiles();
Expand Down Expand Up @@ -1283,6 +1282,8 @@ public void moduleStartupComplete(ServletContext servletContext)
UserManager.addUserListener(new EmailPreferenceUserListener());

Encryption.checkMigration();

McpService.get().register(new CoreMcp());
}

// Issue 7527: Auto-detect missing SQL views and attempt to recreate
Expand Down
Loading
Loading