From cd3317b400e3d8ffab705f41f608771388207b91 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 29 Jan 2026 17:12:41 -0800 Subject: [PATCH 1/5] Support report-to CSP directive and Reporting-Endpoints header --- api/src/org/labkey/api/ApiModule.java | 3 +- .../org/labkey/api/action/BaseApiAction.java | 2 +- api/src/org/labkey/api/util/MimeMap.java | 3 +- .../filters/ContentSecurityPolicyFilter.java | 30 +++- .../labkey/core/admin/AdminController.java | 163 ++++++++++++------ 5 files changed, 146 insertions(+), 55 deletions(-) diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index cd6765adee2..4fe81fd31ac 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -316,7 +316,8 @@ private void addCSPFilter(ServletContext servletCtx, String parameterName, Strin { FilterRegistration registration = servletCtx.addFilter(filterName, new ContentSecurityPolicyFilter()); registration.addMappingForUrlPatterns(allOf(DispatcherType.class), false, "/*"); - registration.setInitParameters(Map.of("policy", policy, "disposition", disposition)); + String violation = servletCtx.getInitParameter("csp.violationEndpoint"); + registration.setInitParameters(Map.of("policy", policy, "disposition", disposition, "violationEndpoint", violation)); } } diff --git a/api/src/org/labkey/api/action/BaseApiAction.java b/api/src/org/labkey/api/action/BaseApiAction.java index e4a6809cede..d0944264647 100644 --- a/api/src/org/labkey/api/action/BaseApiAction.java +++ b/api/src/org/labkey/api/action/BaseApiAction.java @@ -312,7 +312,7 @@ private FormAndErrors
populateForm() throws Exception if (null != contentType) { if (MimeMap.DEFAULT.isJsonContentTypeHeader(contentType)) - { + { _reqFormat = ApiResponseWriter.Format.JSON; return populateJsonForm(); } diff --git a/api/src/org/labkey/api/util/MimeMap.java b/api/src/org/labkey/api/util/MimeMap.java index fc04a1f0c47..83e737464b4 100644 --- a/api/src/org/labkey/api/util/MimeMap.java +++ b/api/src/org/labkey/api/util/MimeMap.java @@ -126,12 +126,13 @@ public int hashCode() public static final MimeType JSON = new MimeType("application/json", false, true); public static final MimeType TEXT_JSON = new MimeType("text/json", false, true); public static final MimeType CSP = new MimeType("application/csp-report", false, true); + public static final MimeType CSP_REPORT = new MimeType("application/reports+json", false, true); } static { for (MimeType mt : Arrays.asList(MimeType.GIF, MimeType.JPEG, MimeType.PDF, MimeType.PNG, MimeType.SVG, MimeType.HTML, MimeType.PLAIN, MimeType.XML, - MimeType.TEXT_JSON, MimeType.JSON, MimeType.CSP)) + MimeType.TEXT_JSON, MimeType.JSON, MimeType.CSP, MimeType.CSP_REPORT)) { mimeTypeMap.put(mt.getContentType(), mt); } diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index ae58e441059..01fef2c6406 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -66,6 +66,7 @@ public class ContentSecurityPolicyFilter implements Filter private ContentSecurityPolicyType _type = ContentSecurityPolicyType.Enforce; private String _policyTemplate = null; private String _cspVersion = "Unknown"; + private String _reportingEndpoints = null; // Updated after every change to "allowed sources" private StringExpression _policyExpression = null; @@ -104,7 +105,7 @@ public String getHeaderName() public void init(FilterConfig filterConfig) throws ServletException { LogHelper.getLogger(ContentSecurityPolicyFilter.class, "CSP filter initialization").info("Initializing {}", filterConfig.getFilterName()); - + String violationEndpoint = null; Enumeration paramNames = filterConfig.getInitParameterNames(); while (paramNames.hasMoreElements()) { @@ -115,8 +116,7 @@ public void init(FilterConfig filterConfig) throws ServletException String s = filterPolicy(paramValue); // Replace REPORT_PARAMETER_SUBSTITUTION now since its value is static - s = StringExpressionFactory.create(s, false, NullValueBehavior.KeepSubstitution) - .eval(Map.of(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion()))); + s = substituteReportParams(s); _policyTemplate = s; @@ -130,18 +130,38 @@ else if ("disposition".equalsIgnoreCase(paramName)) if ("report".equalsIgnoreCase(s)) _type = ContentSecurityPolicyType.Report; } + else if ("violationEndpoint".equalsIgnoreCase(paramName)) + { + // We want to process this after we've extracted the CSP version from the policy, so stash for now + violationEndpoint = paramValue.trim(); + } else { throw new ServletException("ContentSecurityPolicyFilter is misconfigured, unexpected parameter name: " + paramName); } } + // Generate the Reporting-Endpoints header value now since its value is static + _reportingEndpoints = "csp-endpoint=\"" + substituteReportParams(violationEndpoint) + "\""; + if (CSP_FILTERS.put(_type, this) != null) throw new ServletException("ContentSecurityPolicyFilter is misconfigured, duplicate policies of type: " + _type); regeneratePolicyExpression(); } + private String substituteReportParams(String expression) + { + return StringExpressionFactory.create(expression, false, NullValueBehavior.KeepSubstitution) + .eval(new HashMap<>() + { + {{ + put(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion())); + put("CSP.VERSION", _cspVersion); + }} + }); + } + /** Filter out block comments and replace special characters in the provided policy */ public static String filterPolicy(String policy) { @@ -224,6 +244,10 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha Map map = Map.of(NONCE_SUBST, getScriptNonceHeader(req)); var csp = _policyExpression.eval(map); resp.setHeader(_type.getHeaderName(), csp); + + // report-to requires https. If http, omit the header so we fall back on report-uri. + if (request.isSecure()) + resp.setHeader("Reporting-Endpoints", _reportingEndpoints); } } chain.doFilter(request, response); diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 3d375a0b6da..88c53554eb5 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -44,6 +44,7 @@ import org.jfree.chart.JFreeChart; import org.jfree.chart.plot.PlotOrientation; import org.jfree.data.category.DefaultCategoryDataset; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.Assert; import org.junit.Test; @@ -190,7 +191,6 @@ import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.CSRF; import org.labkey.api.security.Directive; -import org.labkey.api.security.ElevatedUser; import org.labkey.api.security.Group; import org.labkey.api.security.GroupManager; import org.labkey.api.security.IgnoresTermsOfUse; @@ -11974,16 +11974,54 @@ public void addNavTrail(NavTree root) } } - private static final URI LABKEY_ORG_REPORT_ACTION; + private static final URI LABKEY_ORG_REPORT_URI_ACTION; + private static final URI LABKEY_ORG_REPORT_TO_ACTION; static { - LABKEY_ORG_REPORT_ACTION = URI.create("https://www.labkey.org/admin-contentSecurityPolicyReport.api"); + LABKEY_ORG_REPORT_URI_ACTION = URI.create("https://www.labkey.org/admin-contentSecurityPolicyReport.api"); + LABKEY_ORG_REPORT_TO_ACTION = URI.create("https://www.labkey.org/admin-contentSecurityPolicyReportTo.api"); + } + + // report-to endpoints get sent a JSON array of reports. Use Jackson to deserialize these into a List. + public static class ReportToJsonObjects extends ArrayList + { + } + + @RequiresNoPermission + @CSRF(CSRF.Method.NONE) + @Marshal(Marshaller.Jackson) + public static class ContentSecurityPolicyReportToAction extends BaseContentSecurityPolicyReportAction + { + @Override + public void handleReports(ReportToJsonObjects jsonObjects, HttpServletRequest request, String userAgent) throws IOException, InterruptedException + { + JSONArray reportsToForward = new JSONArray(); + + jsonObjects.forEach(jsonObject -> { + if (handleOneReport(jsonObject, request, userAgent, "body", "blockedURL", "documentURL")) + reportsToForward.put(jsonObject); + }); + + if (!reportsToForward.isEmpty()) + forwardReports(LABKEY_ORG_REPORT_TO_ACTION, request, reportsToForward.toString(2)); + } } @RequiresNoPermission @CSRF(CSRF.Method.NONE) - public static class ContentSecurityPolicyReportAction extends ReadOnlyApiAction + public static class ContentSecurityPolicyReportAction extends BaseContentSecurityPolicyReportAction + { + @Override + public void handleReports(SimpleApiJsonForm form, HttpServletRequest request, String userAgent) throws IOException, InterruptedException + { + JSONObject jsonObject = form.getJsonObject(); + if (handleOneReport(jsonObject, request, userAgent, "csp-report", "blocked-uri", "document-uri")) + forwardReports(LABKEY_ORG_REPORT_URI_ACTION, request, jsonObject.toString(2)); + } + } + + protected abstract static class BaseContentSecurityPolicyReportAction extends ReadOnlyApiAction { private static final Logger _log = LogHelper.getLogger(ContentSecurityPolicyReportAction.class, "CSP warnings"); @@ -11991,7 +12029,15 @@ public static class ContentSecurityPolicyReportAction extends ReadOnlyApiAction< private static final Map reports = Collections.synchronizedMap(new LRUMap<>(20)); @Override - public Object execute(SimpleApiJsonForm form, BindException errors) throws Exception + protected String getCommandClassMethodName() + { + return "handleReports"; + } + + abstract public void handleReports(FORM form, HttpServletRequest request, String userAgent) throws IOException, InterruptedException; + + @Override + public Object execute(FORM form, BindException errors) throws Exception { var ret = new JSONObject().put("success", true); @@ -12006,28 +12052,42 @@ public Object execute(SimpleApiJsonForm form, BindException errors) throws Excep if (PageFlowUtil.isRobotUserAgent(userAgent) && !_log.isDebugEnabled()) return ret; - // NOTE User may be "guest", and will always be guest if being relayed to labkey.org - var jsonObj = form.getJsonObject(); + handleReports(form, request, userAgent); + + return ret; + } + + // Returns true if the report should be forwarded + protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request, String userAgent, String bodyKey, String blockedUriKey, String documentUriKey) + { if (null != jsonObj) { - JSONObject cspReport = jsonObj.optJSONObject("csp-report"); + JSONObject cspReport = jsonObj.optJSONObject(bodyKey); if (cspReport != null) { - String blockedUri = cspReport.optString("blocked-uri", null); + String blockedUri = cspReport.optString(blockedUriKey, null); // Issue 52933 - suppress base-uri problems from a crawler or bot on labkey.org if (blockedUri != null && - blockedUri.startsWith("https://labkey.org%2C") && - blockedUri.endsWith("undefined") && - !_log.isDebugEnabled()) + blockedUri.startsWith("https://labkey.org%2C") && + blockedUri.endsWith("undefined") && + !_log.isDebugEnabled()) { - return ret; + return false; } - String urlString = cspReport.optString("document-uri", null); + String urlString = cspReport.optString(documentUriKey, null); if (urlString != null) { - URLHelper urlHelper = new URLHelper(urlString); + URLHelper urlHelper = null; + try + { + urlHelper = new URLHelper(urlString); + } + catch (URISyntaxException e) + { + throw new RuntimeException(e); + } // URL parameter that tells us to bypass suppression of redundant logging // Used to make sure that tests of CSP logging are deterministic and convenient boolean bypassCspDedupe = "true".equals(urlHelper.getParameter("bypassCspDedupe")); @@ -12042,7 +12102,7 @@ public Object execute(SimpleApiJsonForm form, BindException errors) throws Excep String email = null; // If the user is not logged in, we may still be able to snag the email address from our cookie if (user.isGuest()) - email = LoginController.getEmailFromCookie(getViewContext().getRequest()); + email = LoginController.getEmailFromCookie(request); if (null == email) email = user.getEmail(); jsonObj.put("user", email); @@ -12050,8 +12110,8 @@ public Object execute(SimpleApiJsonForm form, BindException errors) throws Excep if (ipAddress == null) ipAddress = request.getRemoteAddr(); jsonObj.put("ip", ipAddress); - if (isNotBlank(userAgent)) - jsonObj.put("user-agent", userAgent); + if (isNotBlank(userAgent) && !jsonObj.has("user_agent")) + jsonObj.put("user_agent", userAgent); String labkeyVersion = request.getParameter("labkeyVersion"); if (null != labkeyVersion) jsonObj.put("labkeyVersion", labkeyVersion); @@ -12063,48 +12123,53 @@ public Object execute(SimpleApiJsonForm form, BindException errors) throws Excep var jsonStr = jsonObj.toString(2); _log.warn("ContentSecurityPolicy warning on page: {}\n{}", urlString, jsonStr); - if (!forwarded && OptionalFeatureService.get().isFeatureEnabled(ContentSecurityPolicyFilter.FEATURE_FLAG_FORWARD_CSP_REPORTS)) - { + boolean shouldForward = !forwarded && OptionalFeatureService.get().isFeatureEnabled(ContentSecurityPolicyFilter.FEATURE_FLAG_FORWARD_CSP_REPORTS); + if (shouldForward) jsonObj.put("forwarded", true); - // Create an HttpClient - HttpClient client = HttpClient.newBuilder() - .connectTimeout(Duration.ofSeconds(10)) - .build(); + return shouldForward; + } + } + } + } - // Create the POST request - HttpRequest remoteRequest = HttpRequest.newBuilder() - .uri(LABKEY_ORG_REPORT_ACTION) - .header("Content-Type", request.getContentType()) // Use whatever the browser set - .POST(HttpRequest.BodyPublishers.ofString(jsonObj.toString(2))) - .build(); + return false; + } - // Send the request and get the response - HttpResponse response = client.send(remoteRequest, HttpResponse.BodyHandlers.ofString()); + protected void forwardReports(URI destination, HttpServletRequest request, String content) throws IOException, InterruptedException + { + // Create an HttpClient + try (HttpClient client = HttpClient.newBuilder() + .connectTimeout(Duration.ofSeconds(10)) + .build()) + { + // Create the POST request + HttpRequest remoteRequest = HttpRequest.newBuilder() + .uri(destination) + .header("Content-Type", request.getContentType()) // Use whatever the browser set + .POST(HttpRequest.BodyPublishers.ofString(content)) + .build(); - if (response.statusCode() != 200) - { - _log.error("ContentSecurityPolicy report forwarding to https://www.labkey.org failed: {}\n{}", response.statusCode(), response.body()); - } - else - { - JSONObject jsonResponse = new JSONObject(response.body()); - boolean success = jsonResponse.optBoolean("success", false); - if (!success) - { - _log.error("ContentSecurityPolicy report forwarding to https://www.labkey.org failed: {}", jsonResponse); - } - } - } - } + // Send the request and get the response + HttpResponse response = client.send(remoteRequest, HttpResponse.BodyHandlers.ofString()); + + if (response.statusCode() != 200) + { + _log.error("ContentSecurityPolicy report forwarding to https://www.labkey.org failed: {}\n{}", response.statusCode(), response.body()); + } + else + { + JSONObject jsonResponse = new JSONObject(response.body()); + boolean success = jsonResponse.optBoolean("success", false); + if (!success) + { + _log.error("ContentSecurityPolicy report forwarding to https://www.labkey.org failed: {}", jsonResponse); } } } - return ret; } } - public static class TestCase extends AbstractActionPermissionTest { @Override From e75455d50d370e5d48288e52a468c5c6df5d4a42 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Fri, 30 Jan 2026 13:38:51 -0800 Subject: [PATCH 2/5] Make report-to CSP directive conditional on https: being configured. Sync var names with report-to parameter names. --- api/src/org/labkey/api/ApiModule.java | 3 +- .../filters/ContentSecurityPolicyFilter.java | 34 +++++++++---------- .../labkey/core/admin/AdminController.java | 27 ++++++++------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 4fe81fd31ac..cd6765adee2 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -316,8 +316,7 @@ private void addCSPFilter(ServletContext servletCtx, String parameterName, Strin { FilterRegistration registration = servletCtx.addFilter(filterName, new ContentSecurityPolicyFilter()); registration.addMappingForUrlPatterns(allOf(DispatcherType.class), false, "/*"); - String violation = servletCtx.getInitParameter("csp.violationEndpoint"); - registration.setInitParameters(Map.of("policy", policy, "disposition", disposition, "violationEndpoint", violation)); + registration.setInitParameters(Map.of("policy", policy, "disposition", disposition)); } } diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index 01fef2c6406..b95e50bdda3 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -11,6 +11,7 @@ import org.apache.commons.collections4.SetValuedMap; import org.apache.commons.collections4.multimap.HashSetValuedHashMap; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Logger; import org.junit.Assert; import org.junit.Test; @@ -105,7 +106,6 @@ public String getHeaderName() public void init(FilterConfig filterConfig) throws ServletException { LogHelper.getLogger(ContentSecurityPolicyFilter.class, "CSP filter initialization").info("Initializing {}", filterConfig.getFilterName()); - String violationEndpoint = null; Enumeration paramNames = filterConfig.getInitParameterNames(); while (paramNames.hasMoreElements()) { @@ -130,19 +130,25 @@ else if ("disposition".equalsIgnoreCase(paramName)) if ("report".equalsIgnoreCase(s)) _type = ContentSecurityPolicyType.Report; } - else if ("violationEndpoint".equalsIgnoreCase(paramName)) - { - // We want to process this after we've extracted the CSP version from the policy, so stash for now - violationEndpoint = paramValue.trim(); - } else { throw new ServletException("ContentSecurityPolicyFilter is misconfigured, unexpected parameter name: " + paramName); } } - // Generate the Reporting-Endpoints header value now since its value is static - _reportingEndpoints = "csp-endpoint=\"" + substituteReportParams(violationEndpoint) + "\""; + String baseServerUrl = AppProps.getInstance().getBaseServerUrl(); + // 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://")) + { + // Generate the Reporting-Endpoints header value now since its value is static. Use an absolute URL so we + // always post reports to https:, even when the violating request happens to be http: + String violationEndpoint = substituteReportParams(baseServerUrl + "/admin-contentSecurityPolicyReportTo.api?${CSP.REPORT.PARAMS}"); + if (_cspVersion != null) + violationEndpoint += "&cspVersion=" + _cspVersion; + _reportingEndpoints = "csp-endpoint=\"" + violationEndpoint + "\""; + _policyTemplate = _policyTemplate + " report-to csp-endpoint ;"; + } if (CSP_FILTERS.put(_type, this) != null) throw new ServletException("ContentSecurityPolicyFilter is misconfigured, duplicate policies of type: " + _type); @@ -153,13 +159,7 @@ else if ("violationEndpoint".equalsIgnoreCase(paramName)) private String substituteReportParams(String expression) { return StringExpressionFactory.create(expression, false, NullValueBehavior.KeepSubstitution) - .eval(new HashMap<>() - { - {{ - put(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion())); - put("CSP.VERSION", _cspVersion); - }} - }); + .eval(Map.of(REPORT_PARAMETER_SUBSTITUTION, "labkeyVersion=" + PageFlowUtil.encodeURIComponent(AppProps.getInstance().getReleaseVersion()))); } /** Filter out block comments and replace special characters in the provided policy */ @@ -245,8 +245,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha var csp = _policyExpression.eval(map); resp.setHeader(_type.getHeaderName(), csp); - // report-to requires https. If http, omit the header so we fall back on report-uri. - if (request.isSecure()) + // non-null if https: is configured on this server + if (_reportingEndpoints != null) resp.setHeader("Reporting-Endpoints", _reportingEndpoints); } } diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 88c53554eb5..0f0fb9f3e95 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -12058,43 +12058,44 @@ public Object execute(FORM form, BindException errors) throws Exception } // Returns true if the report should be forwarded - protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request, String userAgent, String bodyKey, String blockedUriKey, String documentUriKey) + protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request, String userAgent, String bodyKey, String blockedUrlKey, String documentUrlKey) { if (null != jsonObj) { JSONObject cspReport = jsonObj.optJSONObject(bodyKey); if (cspReport != null) { - String blockedUri = cspReport.optString(blockedUriKey, null); + String blockedUrl = cspReport.optString(blockedUrlKey, null); // Issue 52933 - suppress base-uri problems from a crawler or bot on labkey.org - if (blockedUri != null && - blockedUri.startsWith("https://labkey.org%2C") && - blockedUri.endsWith("undefined") && + if (blockedUrl != null && + blockedUrl.startsWith("https://labkey.org%2C") && + blockedUrl.endsWith("undefined") && !_log.isDebugEnabled()) { return false; } - String urlString = cspReport.optString(documentUriKey, null); - if (urlString != null) + String documentUrl = cspReport.optString(documentUrlKey, null); + if (documentUrl != null) { - URLHelper urlHelper = null; + URLHelper documentUrlHelper; try { - urlHelper = new URLHelper(urlString); + documentUrlHelper = new URLHelper(documentUrl); } catch (URISyntaxException e) { throw new RuntimeException(e); } + // URL parameter that tells us to bypass suppression of redundant logging // Used to make sure that tests of CSP logging are deterministic and convenient - boolean bypassCspDedupe = "true".equals(urlHelper.getParameter("bypassCspDedupe")); - String path = urlHelper.deleteParameters().getURIString(); + boolean bypassCspDedupe = "true".equals(documentUrlHelper.getParameter("bypassCspDedupe")); + String path = documentUrlHelper.deleteParameters().getURIString(); if (null == reports.put(path, Boolean.TRUE) || _log.isDebugEnabled() || bypassCspDedupe) { - // Don't modify forwarded reports; they already have user, ip, user-agent, etc. from the forwarding server. + // Don't modify forwarded reports; they already have user, ip, user_agent, etc. from the forwarding server. boolean forwarded = jsonObj.optBoolean("forwarded", false); if (!forwarded) { @@ -12121,7 +12122,7 @@ protected boolean handleOneReport(JSONObject jsonObj, HttpServletRequest request } var jsonStr = jsonObj.toString(2); - _log.warn("ContentSecurityPolicy warning on page: {}\n{}", urlString, jsonStr); + _log.warn("ContentSecurityPolicy warning on page: {}\n{}", documentUrl, jsonStr); boolean shouldForward = !forwarded && OptionalFeatureService.get().isFeatureEnabled(ContentSecurityPolicyFilter.FEATURE_FLAG_FORWARD_CSP_REPORTS); if (shouldForward) From 2cf8eaa6e294eb755ad92e2fbb5e85e19bc1553d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 31 Jan 2026 09:06:44 -0800 Subject: [PATCH 3/5] Code review feedback --- api/src/org/labkey/api/util/MimeMap.java | 6 +++--- .../labkey/filters/ContentSecurityPolicyFilter.java | 10 ++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/api/src/org/labkey/api/util/MimeMap.java b/api/src/org/labkey/api/util/MimeMap.java index 83e737464b4..f0341ee6397 100644 --- a/api/src/org/labkey/api/util/MimeMap.java +++ b/api/src/org/labkey/api/util/MimeMap.java @@ -125,14 +125,14 @@ public int hashCode() public static final MimeType XML = new MimeType("text/xml"); public static final MimeType JSON = new MimeType("application/json", false, true); public static final MimeType TEXT_JSON = new MimeType("text/json", false, true); - public static final MimeType CSP = new MimeType("application/csp-report", false, true); - public static final MimeType CSP_REPORT = new MimeType("application/reports+json", false, true); + public static final MimeType CSP_REPORT_URI_JSON = new MimeType("application/csp-report", false, true); + public static final MimeType CSP_REPORT_TO_JSON = new MimeType("application/reports+json", false, true); } static { for (MimeType mt : Arrays.asList(MimeType.GIF, MimeType.JPEG, MimeType.PDF, MimeType.PNG, MimeType.SVG, MimeType.HTML, MimeType.PLAIN, MimeType.XML, - MimeType.TEXT_JSON, MimeType.JSON, MimeType.CSP, MimeType.CSP_REPORT)) + MimeType.TEXT_JSON, MimeType.JSON, MimeType.CSP_REPORT_URI_JSON, MimeType.CSP_REPORT_TO_JSON)) { mimeTypeMap.put(mt.getContentType(), mt); } diff --git a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java index b95e50bdda3..ef6bc8c1e5e 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -53,6 +53,7 @@ public class ContentSecurityPolicyFilter implements Filter private static final String REPORT_PARAMETER_SUBSTITUTION = "CSP.REPORT.PARAMS"; 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 CSP_ENDPOINT_NAME = "csp-endpoint"; private static final Map CSP_FILTERS = new CopyOnWriteHashMap<>(); @@ -143,11 +144,12 @@ else if ("disposition".equalsIgnoreCase(paramName)) { // Generate the Reporting-Endpoints header value now since its value is static. Use an absolute URL so we // always post reports to https:, even when the violating request happens to be http: - String violationEndpoint = substituteReportParams(baseServerUrl + "/admin-contentSecurityPolicyReportTo.api?${CSP.REPORT.PARAMS}"); + ActionURL violationUrl = new ActionURL("admin-contentSecurityPolicyReportTo.api"); + violationUrl = new ActionURL(substituteReportParams(violationUrl + "?${CSP.REPORT.PARAMS}")); if (_cspVersion != null) - violationEndpoint += "&cspVersion=" + _cspVersion; - _reportingEndpoints = "csp-endpoint=\"" + violationEndpoint + "\""; - _policyTemplate = _policyTemplate + " report-to csp-endpoint ;"; + violationUrl.addParameter("cspVersion", _cspVersion); + _reportingEndpoints = CSP_ENDPOINT_NAME + "=\"" + violationUrl.getURIString() + "\""; + _policyTemplate = _policyTemplate + " report-to " + CSP_ENDPOINT_NAME + " ;"; } if (CSP_FILTERS.put(_type, this) != null) From 34cb53ffb9c0e36fa43652d83fe67fe1ec054faa Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 31 Jan 2026 13:06:42 -0800 Subject: [PATCH 4/5] Handle base server URL changes --- api/src/org/labkey/api/admin/AdminUrls.java | 1 + .../filters/ContentSecurityPolicyFilter.java | 81 +++++++++++++------ .../labkey/core/admin/AdminController.java | 7 ++ 3 files changed, 65 insertions(+), 24 deletions(-) diff --git a/api/src/org/labkey/api/admin/AdminUrls.java b/api/src/org/labkey/api/admin/AdminUrls.java index f7a46e6ffff..4f2e30826d5 100644 --- a/api/src/org/labkey/api/admin/AdminUrls.java +++ b/api/src/org/labkey/api/admin/AdminUrls.java @@ -65,6 +65,7 @@ public interface AdminUrls extends UrlProvider ActionURL getSessionLoggingURL(); ActionURL getTrackedAllocationsViewerURL(); ActionURL getSystemMaintenanceURL(); + ActionURL getCspReportToURL(String cspVersion); /** * 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 ef6bc8c1e5e..71840b510a7 100644 --- a/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java +++ b/api/src/org/labkey/filters/ContentSecurityPolicyFilter.java @@ -13,8 +13,10 @@ 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.junit.Assert; 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; @@ -37,6 +39,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -53,7 +56,6 @@ public class ContentSecurityPolicyFilter implements Filter private static final String REPORT_PARAMETER_SUBSTITUTION = "CSP.REPORT.PARAMS"; 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 CSP_ENDPOINT_NAME = "csp-endpoint"; private static final Map CSP_FILTERS = new CopyOnWriteHashMap<>(); @@ -66,9 +68,14 @@ public class ContentSecurityPolicyFilter implements Filter // Per-filter-instance parameters that are set in init() and never changed private ContentSecurityPolicyType _type = ContentSecurityPolicyType.Enforce; - private String _policyTemplate = null; - private String _cspVersion = "Unknown"; - private String _reportingEndpoints = null; + private @NotNull String _cspVersion = "Unknown"; + private String _stashedTemplate = null; + private String _reportToEndpointName = null; + + // Per-filter-instance parameters that are set at first request and reset if base server URL changes + private volatile String _previousBaseServerUrl = null; + private volatile String _policyTemplate = null; + private volatile String _reportingEndpointsHeaderValue = null; // Updated after every change to "allowed sources" private StringExpression _policyExpression = null; @@ -119,7 +126,7 @@ public void init(FilterConfig filterConfig) throws ServletException // Replace REPORT_PARAMETER_SUBSTITUTION now since its value is static s = substituteReportParams(s); - _policyTemplate = s; + _policyTemplate = _stashedTemplate = s; extractCspVersion(s); } @@ -137,24 +144,12 @@ else if ("disposition".equalsIgnoreCase(paramName)) } } - String baseServerUrl = AppProps.getInstance().getBaseServerUrl(); - // 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://")) - { - // Generate the Reporting-Endpoints header value now since its value is static. Use an absolute URL so we - // always post reports to https:, even when the violating request happens to be http: - ActionURL violationUrl = new ActionURL("admin-contentSecurityPolicyReportTo.api"); - violationUrl = new ActionURL(substituteReportParams(violationUrl + "?${CSP.REPORT.PARAMS}")); - if (_cspVersion != null) - violationUrl.addParameter("cspVersion", _cspVersion); - _reportingEndpoints = CSP_ENDPOINT_NAME + "=\"" + violationUrl.getURIString() + "\""; - _policyTemplate = _policyTemplate + " report-to " + CSP_ENDPOINT_NAME + " ;"; - } - if (CSP_FILTERS.put(_type, this) != null) throw new ServletException("ContentSecurityPolicyFilter is misconfigured, duplicate policies of type: " + _type); + // configure a different endpoint for each type to convey the correct csp version (eXX vs. rXX) + _reportToEndpointName = "csp-" + _type.name().toLowerCase(); + regeneratePolicyExpression(); } @@ -221,7 +216,8 @@ private void extractCspVersion(String s) LOG.debug("CspVersion: {}", _cspVersion); } - // Make all the "allowed sources" substitutions at init() and whenever the allowed sources map changes. With this, + // Make all the "allowed sources" substitutions at init(), whenever the allowed sources map changes, or whenever the + // policy template changes (e.g., base server URL change that causes report-to to be added or removed). With this, // the only substitution needed on a per-request basis is the nonce value. private void regeneratePolicyExpression() { @@ -241,20 +237,57 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha { if (request instanceof HttpServletRequest req && response instanceof HttpServletResponse resp && null != _policyExpression) { + ensurePolicy(); + if (_type != ContentSecurityPolicyType.Enforce || !OptionalFeatureService.get().isFeatureEnabled(FEATURE_FLAG_DISABLE_ENFORCE_CSP)) { Map map = Map.of(NONCE_SUBST, getScriptNonceHeader(req)); var csp = _policyExpression.eval(map); resp.setHeader(_type.getHeaderName(), csp); - // non-null if https: is configured on this server - if (_reportingEndpoints != null) - resp.setHeader("Reporting-Endpoints", _reportingEndpoints); + // null if https: is not configured on this server + if (_reportingEndpointsHeaderValue != null) + resp.addHeader("Reporting-Endpoints", _reportingEndpointsHeaderValue); } } chain.doFilter(request, response); } + private void ensurePolicy() + { + String baseServerUrl = AppProps.getInstance().getBaseServerUrl(); + + // Reconsider "report-to" directive and "Reporting-Endpoints" header if base server URL has changed + if (!Objects.equals(baseServerUrl, _previousBaseServerUrl)) + { + synchronized (SUBSTITUTION_LOCK) + { + _previousBaseServerUrl = 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(_cspVersion); + // Use an absolute URL so we always post to https:, even if the violating request uses http: + _reportingEndpointsHeaderValue = _reportToEndpointName + "=\"" + substituteReportParams(violationUrl.getURIString() + "&${CSP.REPORT.PARAMS}") + "\""; + + // Add "report-to" directive to the policy + _policyTemplate = _stashedTemplate + " report-to " + _reportToEndpointName + " ;"; + } + else + { + _reportingEndpointsHeaderValue = null; + _policyTemplate = _stashedTemplate; + } + + regeneratePolicyExpression(); + } + } + } + public static String getScriptNonceHeader(HttpServletRequest request) { String nonce = (String)request.getAttribute(HEADER_NONCE); diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 0f0fb9f3e95..b2535b0b688 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -883,6 +883,13 @@ public ActionURL getSystemMaintenanceURL() return new ActionURL(ConfigureSystemMaintenanceAction.class, ContainerManager.getRoot()); } + @Override + public ActionURL getCspReportToURL(@NotNull String cspVersion) + { + return new ActionURL(ContentSecurityPolicyReportToAction.class, ContainerManager.getRoot()) + .addParameter("cspVersion", cspVersion); + } + public static ActionURL getDeprecatedFeaturesURL() { return new ActionURL(OptionalFeaturesAction.class, ContainerManager.getRoot()).addParameter("type", FeatureType.Deprecated.name()); From adbd1c16a5cfb894c559a0414871b03db1e333d9 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 31 Jan 2026 13:27:00 -0800 Subject: [PATCH 5/5] Limit ImpersonatingTroubleshooterRole permissions to root --- .../api/security/roles/ImpersonatingTroubleshooterRole.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/src/org/labkey/api/security/roles/ImpersonatingTroubleshooterRole.java b/api/src/org/labkey/api/security/roles/ImpersonatingTroubleshooterRole.java index b0a8d1ffddf..f6bdfcad095 100644 --- a/api/src/org/labkey/api/security/roles/ImpersonatingTroubleshooterRole.java +++ b/api/src/org/labkey/api/security/roles/ImpersonatingTroubleshooterRole.java @@ -26,4 +26,10 @@ public boolean isPrivileged() { return true; } + + @Override + public boolean isAvailableEverywhere() + { + return false; + } }