From 940a018ed79cc34bec45b66a1f39feac0af8a40f Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 11 Mar 2026 12:03:08 -0700 Subject: [PATCH 1/4] Remove CSP versions from headers and URLs --- api/src/org/labkey/api/admin/AdminUrls.java | 2 +- .../filters/ContentSecurityPolicyFilter.java | 81 ++++++++++++------- .../labkey/core/admin/AdminController.java | 6 +- 3 files changed, 55 insertions(+), 34 deletions(-) diff --git a/api/src/org/labkey/api/admin/AdminUrls.java b/api/src/org/labkey/api/admin/AdminUrls.java index 4f2e30826d5..7d99c11ca76 100644 --- a/api/src/org/labkey/api/admin/AdminUrls.java +++ b/api/src/org/labkey/api/admin/AdminUrls.java @@ -65,7 +65,7 @@ public interface AdminUrls extends UrlProvider ActionURL getSessionLoggingURL(); ActionURL getTrackedAllocationsViewerURL(); ActionURL getSystemMaintenanceURL(); - ActionURL getCspReportToURL(String cspVersion); + ActionURL getCspReportToURL(); /** * Simply adds an "Admin Console" link to nav trail if invoked in the root container. Otherwise, root is unchanged. diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index c28e8ebe84c..3546c728101 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -10,6 +10,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.collections4.SetValuedMap; 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; @@ -19,7 +20,6 @@ import org.junit.Test; import org.labkey.api.admin.AdminUrls; import org.labkey.api.collections.CopyOnWriteHashMap; -import org.labkey.api.collections.LabKeyCollectors; import org.labkey.api.security.Directive; import org.labkey.api.settings.AppProps; import org.labkey.api.settings.OptionalFeatureService; @@ -95,6 +95,11 @@ public String getHeaderName() { return _headerName; } + + private static @Nullable ContentSecurityPolicyType get(String disposition) + { + return EnumUtils.getEnumIgnoreCase(ContentSecurityPolicyType.class, disposition); + } } static @@ -119,8 +124,9 @@ public void init(FilterConfig filterConfig) throws ServletException String paramValue = filterConfig.getInitParameter(paramName); if ("policy".equalsIgnoreCase(paramName)) { + // Extract before filtering since CSP version is in a comment + extractCspVersion(paramValue); _stashedTemplate = filterPolicy(paramValue); - extractCspVersion(_stashedTemplate); } else if ("disposition".equalsIgnoreCase(paramName)) { @@ -144,7 +150,7 @@ else if ("disposition".equalsIgnoreCase(paramName)) } /** Filter out block comments and replace special characters in the provided policy */ - public static String filterPolicy(String policy) + private static String filterPolicy(String policy) { String s = policy.trim(); s = s.replace( '\n', ' ' ); @@ -164,40 +170,27 @@ public static String filterPolicy(String policy) return s; } + private static final String CSP_VERSION = "cspVersion="; + /** - * Extract the cspVersion parameter value from the report-uri directive, if possible. Otherwise, cspVersion is left - * as "Unknown". This value is reported as part of usage metrics. + * Extract the cspVersion parameter value from a comment in the CSP, if it exists. Otherwise, cspVersion is left as + * "Unknown". This value is reported as part of usage metrics and sent in reports. */ private void extractCspVersion(String s) { - // Simple parser that should be compliant with https://www.w3.org/TR/CSP3/#parse-serialized-policy - Map cspMap = Arrays.stream(s.split(";")) - .map(String::trim) - .filter(line -> !line.isEmpty()) - .map(line -> line.split("\\s+", 2)) - .filter(parts -> parts.length == 2) - .collect(LabKeyCollectors.toCaseInsensitiveLinkedMap(parts -> parts[0], parts -> parts[1])); - - String directive = "report-uri"; - String reportUri = cspMap.get(directive); - - if (reportUri != null) + int idx = s.indexOf(CSP_VERSION); + if (idx > -1) { - try - { - ActionURL reportUrl = new ActionURL(reportUri); - String cspVersion = reportUrl.getParameter("cspVersion"); - - if (null != cspVersion) - _cspVersion = cspVersion; - } - catch (IllegalArgumentException e) + int start = idx + CSP_VERSION.length(); + int end = s.indexOf(" ", start); + if (end > -1) { - LOG.warn("Unable to parse {} URI", directive, e); + _cspVersion = s.substring(start, end); + if (s.indexOf(CSP_VERSION, end) > -1) + LOG.warn("More than one " + CSP_VERSION + " assignment found; using the first one."); + LOG.debug("CspVersion: {}", getCspVersion()); } } - - LOG.debug("CspVersion: {}", getCspVersion()); } @Override @@ -277,7 +270,7 @@ private CspFilterSettings(ContentSecurityPolicyFilter filter, String baseServerU { // 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(filter.getCspVersion()); + 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() + "\""; @@ -406,6 +399,34 @@ public static boolean hasCsp(ContentSecurityPolicyType type) return CSP_FILTERS.get(type) != null; } + public static @NotNull String getCspVersion(@Nullable String disposition) + { + if (disposition != null) + { + ContentSecurityPolicyType type = ContentSecurityPolicyType.get(disposition); + + if (type != null) + { + var filter = CSP_FILTERS.get(type); + + if (null != filter) + { + return filter.getCspVersion(); + } + else + { + LOG.error("Disposition {} doesn't match a configured CSP filter", disposition); + } + } + else + { + LOG.error("Bad disposition: {}", disposition); + } + } + + return "Unknown"; + } + public static List getMissingSubstitutions(ContentSecurityPolicyType type) { ContentSecurityPolicyFilter filter = CSP_FILTERS.get(type); diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 2f892250a6f..73250a67052 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -886,10 +886,9 @@ public ActionURL getSystemMaintenanceURL() } @Override - public ActionURL getCspReportToURL(@NotNull String cspVersion) + public ActionURL getCspReportToURL() { - return new ActionURL(ContentSecurityPolicyReportToAction.class, ContainerManager.getRoot()) - .addParameter("cspVersion", cspVersion); + return new ActionURL(ContentSecurityPolicyReportToAction.class, ContainerManager.getRoot()); } public static ActionURL getDeprecatedFeaturesURL() @@ -12135,6 +12134,7 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request if (!forwarded) { jsonObj.put("labkeyVersion", AppProps.getInstance().getReleaseVersion()); + jsonObj.put("cspVersion", ContentSecurityPolicyFilter.getCspVersion(cspReport.optString("disposition", null))); User user = getUser(); String email = null; // If the user is not logged in, we may still be able to snag the email address from our cookie From c68eb69db7c6687aa068cdbf0aaa39aa9966e9ef Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 11 Mar 2026 12:12:09 -0700 Subject: [PATCH 2/4] Remove the old cspVersion put --- core/src/org/labkey/core/admin/AdminController.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 73250a67052..4ac26b86003 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -12149,9 +12149,6 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request jsonObj.put("ip", ipAddress); if (isNotBlank(userAgent) && !jsonObj.has("user_agent")) jsonObj.put("user_agent", userAgent); - String cspVersion = request.getParameter("cspVersion"); - if (null != cspVersion) - jsonObj.put("cspVersion", cspVersion); } var jsonStr = jsonObj.toString(2); From 2f5740400821021e07e3babb1639df8fcacbba22 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Mar 2026 09:15:57 -0700 Subject: [PATCH 3/4] Use regex. Unit test. --- .../filters/ContentSecurityPolicyFilter.java | 55 ++++++++++++++----- 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index 3546c728101..72cad29e109 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -42,6 +42,8 @@ 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; @@ -170,26 +172,23 @@ private static String filterPolicy(String policy) return s; } - private static final String CSP_VERSION = "cspVersion="; + private static final Pattern CSP_VERSION_PATTERN = Pattern.compile("cspVersion\\s*=\\s*(\\w+)"); /** - * Extract the cspVersion parameter value from a comment in the CSP, if it exists. Otherwise, cspVersion is left as - * "Unknown". This value is reported as part of usage metrics and sent in reports. + * Extract the cspVersion value from a comment in the CSP, if it exists. Otherwise, cspVersion is left as "Unknown". + * This value is reported as part of usage metrics and included in violation reports that are logged and forwarded. */ private void extractCspVersion(String s) { - int idx = s.indexOf(CSP_VERSION); - if (idx > -1) + Matcher matcher = CSP_VERSION_PATTERN.matcher(s); + if (matcher.find()) { - int start = idx + CSP_VERSION.length(); - int end = s.indexOf(" ", start); - if (end > -1) - { - _cspVersion = s.substring(start, end); - if (s.indexOf(CSP_VERSION, end) > -1) - LOG.warn("More than one " + CSP_VERSION + " assignment found; using the first one."); - LOG.debug("CspVersion: {}", getCspVersion()); - } + _cspVersion = matcher.group(1); + + if (matcher.find()) + LOG.warn("More than one cspVersion=XX assignment found; using the first one."); + + LOG.debug("CspVersion: {}", getCspVersion()); } } @@ -463,6 +462,32 @@ public static void registerMetricsProvider() public static class TestCase extends Assert { + @Test + public void testCspVersionExtraction() + { + testCspExtract("e14", "/* cspVersion=e14 */"); + testCspExtract("r14", "/*cspVersion=r14 */"); + testCspExtract("e15", "/* cspVersion = e15 */"); + testCspExtract("r15", "/* cspVersion=r15*/"); + testCspExtract("e15", "/* cspVersion = e15*/"); + testCspExtract("e15", "/* cspVersion = e15*/ /* cspVersion=XXX */"); + + testCspExtract("Unknown", ""); + testCspExtract("Unknown", " "); + testCspExtract("Unknown", "/* cspVersin=e14 */"); + testCspExtract("Unknown", "/* cspVersion */"); + testCspExtract("Unknown", "/* cspVersion= */"); + testCspExtract("Unknown", "/* cspVersion=*/"); + testCspExtract("Unknown", "/* cspVersion== */"); + } + + private void testCspExtract(String expected, String csp) + { + ContentSecurityPolicyFilter filter = new ContentSecurityPolicyFilter(); + filter.extractCspVersion(csp); + assertEquals(expected, filter.getCspVersion()); + } + @Test public void testPolicyFiltering() { @@ -482,7 +507,7 @@ public void testPolicyFiltering() report-uri /* Whoa! */ /admin-contentsecuritypolicyreport.api?${CSP.REPORT.PARAMS} https://*; """; - // Multi-line for readability, but notice that newlines are replaced before assignment + // Multi-line for readability, but notice that newlines are replaced when constructing the expected string String expected = """ default-src 'self' https: http: ; connect-src 'self' http://www.labkey.org localhost:* ws: ${LABKEY.ALLOWED.CONNECTIONS} ; From d4ad722497d0273a28febeef6e6f47c13fc270ef Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Mar 2026 09:22:54 -0700 Subject: [PATCH 4/4] Comment --- api/src/org/labkey/filters/ContentSecurityPolicyFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index 72cad29e109..90a9b1c5437 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -147,7 +147,7 @@ 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 to convey the correct csp version (eXX vs. rXX) + // 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(); }