Skip to content

Remove CSP versions from headers and URLs #7487

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 2 commits into
base: release26.3-SNAPSHOT
Choose a base branch
from
+55 −37
Open

Remove CSP versions from headers and URLs #7487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
940a018
Remove CSP versions from headers and URLs
labkey-adam Mar 11, 2026
c68eb69
Remove the old cspVersion put
labkey-adam Mar 11, 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
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/admin/AdminUrls.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
81 changes: 51 additions & 30 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 @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -95,6 +95,11 @@ public String getHeaderName()
{
return _headerName;
}

private static @Nullable ContentSecurityPolicyType get(String disposition)
{
return EnumUtils.getEnumIgnoreCase(ContentSecurityPolicyType.class, disposition);
}
}

static
Expand All @@ -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))
{
Expand All @@ -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', ' ' );
Expand All @@ -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)
Copy link
Contributor

@labkey-jeckels labkey-jeckels Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good candidate for a new, targeted unit test.

{
// Simple parser that should be compliant with https://www.w3.org/TR/CSP3/#parse-serialized-policy
Map<String, String> 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);
Copy link
Contributor

@labkey-jeckels labkey-jeckels Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we guaranteed to always have a space afterwards? If that's the required convention, consider error logging if we don't.

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
Expand Down Expand Up @@ -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() + "\"";

Expand Down Expand Up @@ -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<String> getMissingSubstitutions(ContentSecurityPolicyType type)
{
ContentSecurityPolicyFilter filter = CSP_FILTERS.get(type);
Expand Down
9 changes: 3 additions & 6 deletions core/src/org/labkey/core/admin/AdminController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
Loading