Skip to content

A couple more CSP enhancements #2530

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.

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

Merged
merged 6 commits into from
Jul 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions data/reports/knitr_no_scriptpad.rhtml
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,11 @@ end.rcode-->
<p>Well, everything seems to be working. Let's ask R what is the
value of &pi;? Of course it is <!--rinline pi -->.</p>

<span>Nonce check: <span id="nonce-check-result">FAIL</span></span>

<script>
document.getElementById('nonce-check-result').innerText = "SUCCESS";
</script>

</body>
</html>
6 changes: 6 additions & 0 deletions data/reports/knitr_no_scriptpad.rmd
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ library(knitr)
knit('knitr-minimal.Rmd')
```

<span>Nonce check: <span id="nonce-check-result">FAIL</span></span>

<script>
document.getElementById('nonce-check-result').innerText = "SUCCESS";
</script>

## Conclusion

Markdown is super easy to write. Go to **knitr** [homepage](http://yihui.name/knitr) for details.
15 changes: 15 additions & 0 deletions data/reports/nonce_check.rhtml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html>
<head>
<title>Test script nonce in Knitr HTML</title>
</head>
<body>

<span>Nonce check: <span id="nonce-check-result">FAIL</span></span>

<script>
document.getElementById('nonce-check-result').innerText = "SUCCESS";
</script>

</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,11 @@ end.rcode-->
<p>Well, everything seems to be working. Let's ask R what is the
value of &pi;? Of course it is <!--rinline pi -->.</p>

<span>Nonce check: <span id="nonce-check-result">FAIL</span></span>

<script>
document.getElementById('nonce-check-result').innerText = "SUCCESS";
</script>

</body>
</html>
6 changes: 6 additions & 0 deletions modules/scriptpad/resources/reports/schemas/script_rmd.rmd
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ library(knitr)
knit('knitr-minimal.Rmd')
```

<span>Nonce check: <span id="nonce-check-result">FAIL</span></span>

<script>
document.getElementById('nonce-check-result').innerText = "SUCCESS";
</script>

## Conclusion

Markdown is super easy to write. Go to **knitr** [homepage](http://yihui.name/knitr) for details.
6 changes: 3 additions & 3 deletions src/org/labkey/test/TestFileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public static String getFileContents(Path path)
{
try
{
return new String(Files.readAllBytes(path), StandardCharsets.UTF_8);
return Files.readString(path);
}
catch (IOException fail)
{
Expand Down Expand Up @@ -306,7 +306,7 @@ public static Set<File> getSampleDataDirs()
if (sampledataDirsFile.exists())
{
String path = getFileContents(sampledataDirsFile);
_sampledataDirs.addAll(Arrays.stream(path.split(";")).map(File::new).collect(Collectors.toList()));
_sampledataDirs.addAll(Arrays.stream(path.split(";")).map(File::new).toList());
}
else
{
Expand All @@ -317,7 +317,7 @@ public static Set<File> getSampleDataDirs()
// We know where the modules live; no reason to insist that sampledata.dirs exists.
Files.walkFileTree(modulesDir, Collections.emptySet(), 2, new SimpleFileVisitor<>(){
@Override
public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException
public @NotNull FileVisitResult preVisitDirectory(@NotNull Path dir, @NotNull BasicFileAttributes attrs)
{
if (dir.equals(modulesDir))
{
Expand Down
1 change: 1 addition & 0 deletions src/org/labkey/test/pages/admin/ExternalSourcesPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ public enum Directive implements OptionSelect.SelectOption
Frame("frame-src"),
Image("image-src"),
Style("style-src"),
Object("object-src"),
;

private final String directiveId;
Expand Down
20 changes: 20 additions & 0 deletions src/org/labkey/test/pages/reports/ScriptReportPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@
import org.labkey.test.pages.LabKeyPage;
import org.labkey.test.util.CodeMirrorHelper;
import org.labkey.test.util.Ext4Helper;
import org.labkey.test.util.PipelineStatusTable;
import org.labkey.test.util.TestLogger;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.support.ui.ExpectedConditions;

import java.util.Map;
import java.util.Objects;
import java.util.Optional;

import static org.labkey.test.components.ext4.Checkbox.Ext4Checkbox;
Expand Down Expand Up @@ -79,13 +81,19 @@ public CodeMirrorHelper getEditor()

public String saveReport(String name, boolean isSaveAs, int wait)
{
String reportIdBeforeSave = getReportId();
WebElement saveButton = Ext4Helper.Locators.ext4Button(isSaveAs ? "Save As" : "Save").findElement(getDriver());
scrollIntoView(saveButton, true);
clickAndWait(saveButton, wait);
if (null != name)
{
saveReportWithName(name, isSaveAs);
}
return Objects.requireNonNullElse(getReportId(), reportIdBeforeSave);
}

public String getReportId()
{
return getUrlParam("reportId", true);
}

Expand Down Expand Up @@ -141,6 +149,7 @@ public void clearOption(ReportOption option)

private void _selectOption(ReportOption option, boolean checked)
{
clickSourceTab();
ensureFieldSetExpanded(option.getSection());
Checkbox checkbox;
if (option.isCheckbox())
Expand Down Expand Up @@ -202,6 +211,17 @@ public WebElement findReportElement()
return Locator.byClass("reportView").findElement(getDriver());
}

public void startPipelineJobAndWait()
{
clickReportTab();

waitAndClick(Locator.lkButton("Start Job"));
waitAndClickAndWait(Locator.linkWithText("click here"));
new PipelineStatusTable(this)
.clickStatusLink(0)
.waitForComplete();
}

@Override
protected ElementCache newElementCache()
{
Expand Down
38 changes: 38 additions & 0 deletions src/org/labkey/test/pages/wiki/EditPage.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,48 @@ public EditPage setTitle(String title)

public EditPage setBody(String body)
{
switchWikiToSourceView();
elementCache().bodyTextArea.set(body);
return this;
}

/**
* Switches the wiki edit page to source view when the format type is HTML.
*/
public void switchWikiToSourceView()
{
String curFormat = executeScript("return LABKEY._wiki.getProps().rendererType;", String.class);
if (curFormat.equalsIgnoreCase("HTML"))
{
if (isElementPresent(Locator.css("#wiki-tab-source.labkey-tab-inactive")))
{
Locator tab = Locator.css("#wiki-tab-source > a");
waitForElementToBeVisible(tab);
click(tab);
waitForElement(Locator.css("#wiki-tab-source.labkey-tab-active"));
}
}
}

public void switchWikiToVisualView()
{
String curFormat = (String) executeScript("return LABKEY._wiki.getProps().rendererType;");
if (curFormat.equalsIgnoreCase("HTML"))
{
if (isElementPresent(Locator.css("#wiki-tab-visual.labkey-tab-inactive")))
{
Locator tab = Locator.css("#wiki-tab-visual > a");
waitForElementToBeVisible(tab);
click(tab);

Locator yesButton = Locator.tagWithText("span","Yes");
waitForElementToBeVisible(yesButton);
waitAndClick(yesButton);
waitForElement(Locator.css("#wiki-tab-visual.labkey-tab-active"));
}
}
}

public EditPage setShouldIndex(boolean shouldIndex)
{
elementCache().shouldIndexCheckbox.set(shouldIndex);
Expand Down
79 changes: 77 additions & 2 deletions src/org/labkey/test/tests/AbstractKnitrReportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.junit.Assume;
import org.junit.BeforeClass;
import org.junit.Test;
import org.labkey.remoteapi.CommandException;
import org.labkey.test.BaseWebDriverTest;
import org.labkey.test.Locator;
Expand All @@ -25,12 +26,16 @@
import org.labkey.test.pages.admin.ExternalSourcesPage;
import org.labkey.test.pages.core.admin.logger.ManagerPage;
import org.labkey.test.pages.reports.ManageViewsPage;
import org.labkey.test.pages.reports.ScriptReportPage;
import org.labkey.test.pages.reports.ScriptReportPage.StandardReportOption;
import org.labkey.test.util.CodeMirrorHelper;
import org.labkey.test.util.CspLogUtil;
import org.labkey.test.util.Log4jUtils;
import org.labkey.test.util.LogMethod;
import org.labkey.test.util.LoggedParam;
import org.labkey.test.util.PortalHelper;
import org.labkey.test.util.RReportHelper;
import org.labkey.test.util.WikiHelper;
import org.labkey.test.util.core.admin.CspConfigHelper;
import org.openqa.selenium.WebElement;

Expand All @@ -54,6 +59,10 @@ public abstract class AbstractKnitrReportTest extends BaseWebDriverTest
protected static final Path rmdReport_no_scriptpad = TestFileUtils.getSampleData("reports/knitr_no_scriptpad.rmd").toPath();
private static final Path rhtmlReport = scriptpadReports.resolve("script_rhtml.rhtml");
private static final Path rhtmlReport_no_scriptpad = TestFileUtils.getSampleData("reports/knitr_no_scriptpad.rhtml").toPath();
private static final Path rhtmlNonceCheck = TestFileUtils.getSampleData("reports/nonce_check.rhtml").toPath();
private static final Locator.XPathLocator nonceCheckLoc = Locator.id("nonce-check-result");
private static final Locator.XPathLocator nonceCheckSuccessLoc = nonceCheckLoc.withText("SUCCESS");

protected final RReportHelper _rReportHelper = new RReportHelper(this);

private static String readReport(final Path reportFile)
Expand Down Expand Up @@ -81,6 +90,7 @@ protected void setupProject()
try
{
new CspConfigHelper(this).setAllowedHosts(Map.of(
ExternalSourcesPage.Directive.Object, List.of("'self'"), // Issue 53226: reports-streamFile is blocked by object-src CSP directive
ExternalSourcesPage.Directive.Style, List.of("https://cdn.datatables.net"),
ExternalSourcesPage.Directive.Font, List.of("https://mathjax.rstudio.com")));
}
Expand Down Expand Up @@ -166,7 +176,9 @@ protected void htmlFormat()
Locator.tag("pre").containing("## \"1\",249318596,\"2008-05-17\",86,36,129,76,64"),
Locator.tag("pre").withText("## knitr says hello to HTML!"),
Locator.tag("pre").startsWith("## Error").containing(": non-numeric argument to binary operator"),
Locator.tag("p").startsWith("Well, everything seems to be working. Let's ask R what is the value of \u03C0? Of course it is 3.141")};
Locator.tag("p").startsWith("Well, everything seems to be working. Let's ask R what is the value of \u03C0? Of course it is 3.141"),
nonceCheckSuccessLoc // Inline script should run
};
String[] reportNotContains = {"<html>", // Uninterpreted html
"<!--", // ditto
"A minimal knitr example in HTML", // report title element
Expand Down Expand Up @@ -195,7 +207,8 @@ protected void markdownV2()
Locator.tag("h2").withText("R code chunks"),
Locator.tag("code").containing("set.seed(123)"), // Echoed R code
Locator.css("p").containing("2 x pi = 6.283"),
Locator.tag("sup").withText("write") //should not contain the hat markdown v2 closing tag
Locator.tag("sup").withText("write"), //should not contain the hat markdown v2 closing tag
nonceCheckSuccessLoc // Inline script should run
};

String[] reportNotContains = {"```", // Markdown for R code chunks
Expand Down Expand Up @@ -224,4 +237,66 @@ protected void moduleReportDependencies()
_ext4Helper.waitForMaskToDisappear(3 * BaseWebDriverTest.WAIT_FOR_JAVASCRIPT);
waitForElement(Locator.id("mtcars_table"));
}

/**
* Issue 53211: CSP reports when an R/Plotly graph is displayed in Reports web part, same thing wrapped in a wiki works fine with strict csp
*/
@Test
public void testEmbeddedReportNonce()
{
CspConfigHelper.debugCspWarnings();
new CspConfigHelper(this).setEnforceCsp(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

So we're still disabling the enforce CSP and checking for logged warnings?

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose we don't have to now that you mention it.
I'll remove these in a separate PR after this is merged. I don't want to touch this branch since it is merging to 25.7 but TeamCity is running it against develop.


String name = "rhtml nonce check";
Locator[] reportContains = {nonceCheckSuccessLoc};

createAndVerifyKnitrReport(rhtmlNonceCheck, RReportHelper.ReportOption.knitrHtml, reportContains,
null, true, name);
assertNonceSuccess();

log("Create wiki with embedded report");
new WikiHelper(this).createNewWikiPage()
.setName(name)
.setBody("""
${labkey.webPart(partName='Report',
reportName='%s',
showFrame='false'
)}
""".formatted(name))
.saveAndClose();
clickAndWait(Locator.linkWithText(name));
assertNonceSuccess();

log("Add report webpart");
new PortalHelper(this).doInAdminMode(ph -> {
ph.addTab(name + " tab"); // Use a separate tab to ensure report isn't run accidentally
ph.addReportWebPart(name);
assertNonceSuccess();
});

log("Re-verify with report run as pipeline job");
goToManageViews();
waitAndClickAndWait(Locator.linkWithText(name));
ScriptReportPage reportPage = _rReportHelper.getReportPage();
reportPage.selectOption(StandardReportOption.runInPipeline);
String reportId = reportPage.saveReport(null, false, WAIT_FOR_PAGE);
goBack();
reportPage.startPipelineJobAndWait();

ScriptReportPage.beginAtReport(this, getCurrentContainerPath(), reportId);
assertNonceSuccess();

clickTab(name + " tab");
assertNonceSuccess();

goToModule("Wiki");
clickAndWait(Locator.linkWithText(name));
assertNonceSuccess();
}

private void assertNonceSuccess()
{
assertEquals("Nonce check result", "SUCCESS", getText(nonceCheckLoc));
CspLogUtil.checkNewCspWarnings(getArtifactCollector());
}
}
2 changes: 1 addition & 1 deletion src/org/labkey/test/tests/TabTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private void doTabManagementTests()
navigateToFolder(getProjectName(), FOLDER_NAME);

// Move tabs
portalHelper.enableTabEditMode();
portalHelper.enterAdminMode();
portalHelper.moveTab("Tab 1", PortalHelper.Direction.LEFT); // Nothing should happen.
portalHelper.moveTab("Tab 1", PortalHelper.Direction.RIGHT);
List<PortalTab> tabs = PortalTab.findTabs(getDriver());
Expand Down
4 changes: 2 additions & 2 deletions src/org/labkey/test/util/CspLogUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;

Expand All @@ -28,8 +29,7 @@ public class CspLogUtil
"/_rstudio/",
"/_rstudioReport/"
);
// Issue 53226: reports-streamFile is blocked by object-src CSP directive
private static final Set<String> ignoredDirectives = Set.of("object-src");
private static final Set<String> ignoredDirectives = Collections.emptySet();

private static final String logName = "csp-report.log";
private static final File logFile = new File(TestFileUtils.getServerLogDir(), logName);
Expand Down
Loading