From 20bf52b39cb7aa5895b9a421a4d7188b09e38dc8 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Tue, 7 Jul 2026 10:10:15 -0600 Subject: [PATCH 1/6] Add Selenium regression test for list audit detail (#3048) Co-authored-by: Trey Chadick --- src/org/labkey/test/tests/list/ListTest.java | 94 ++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/src/org/labkey/test/tests/list/ListTest.java b/src/org/labkey/test/tests/list/ListTest.java index 672612ba83..f7cd933d2e 100644 --- a/src/org/labkey/test/tests/list/ListTest.java +++ b/src/org/labkey/test/tests/list/ListTest.java @@ -24,11 +24,15 @@ import org.junit.Test; import org.junit.experimental.categories.Category; import org.labkey.remoteapi.CommandException; +import org.labkey.remoteapi.Connection; import org.labkey.remoteapi.domain.Domain; import org.labkey.remoteapi.domain.DomainResponse; import org.labkey.remoteapi.domain.PropertyDescriptor; import org.labkey.remoteapi.domain.SaveDomainCommand; import org.labkey.remoteapi.query.Filter; +import org.labkey.remoteapi.query.SelectRowsCommand; +import org.labkey.remoteapi.query.SelectRowsResponse; +import org.labkey.remoteapi.query.Sort; import org.labkey.serverapi.reader.TabLoader; import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; @@ -54,6 +58,7 @@ import org.labkey.test.params.FieldDefinition.StringLookup; import org.labkey.test.params.FieldInfo; import org.labkey.test.params.FieldKey; +import org.labkey.test.params.list.IntListDefinition; import org.labkey.test.params.list.VarListDefinition; import org.labkey.test.tests.AuditLogTest; import org.labkey.test.util.AbstractDataRegionExportOrSignHelper.ColumnHeaderType; @@ -857,6 +862,95 @@ public void testCustomViews() crossContainerLookupTest(); } + /** + * CWE-639 (IDOR): a list audit event loaded by user-controlled rowId in + * ListItemDetailsAction must be tied back to the URL-requested listId before its + * old/new record maps are rendered. Without this, listId=X&rowId=N-belonging-to-Y + * would render List Y's audit payload inside List X's details page. + * + * Builds two lists in the same container, generates a modify-audit-event on the + * second one, then verifies the action refuses to render it when the URL names the + * first list. A positive control confirms the matched-listId path still works, so + * the test fails if the predicate is ever inverted/over-rejects. + */ + @Test + public void testAuditDetailRejectsRowIdFromOtherList() throws Exception + { + final String LIST_X = "IDOR_VICTIM_LIST"; // attacker claims to be viewing details for this list + final String LIST_Y = "IDOR_SOURCE_LIST"; // audit event actually belongs to this list + final String NAME_FIELD = "Name"; + final String LIST_Y_ROW_VALUE = "y-original"; + final String LIST_Y_ROW_EDITED = "y-modified-secret"; + + log("Set up two lists in the same container, each with a Name field"); + Connection connection = createDefaultConnection(); + new IntListDefinition(LIST_X, "Key") + .addField(new FieldDefinition(NAME_FIELD, ColumnType.String)) + .create(connection, getProjectName()); + new IntListDefinition(LIST_Y, "Key") + .addField(new FieldDefinition(NAME_FIELD, ColumnType.String)) + .create(connection, getProjectName()); + + log("Insert and then modify a row in List Y so it generates an audit event with old/new record maps"); + _listHelper.goToList(LIST_Y); + _listHelper.clickImportData() + .setText(NAME_FIELD + "\n" + LIST_Y_ROW_VALUE) + .submit(); + DataRegionTable yTable = new DataRegionTable("query", getDriver()); + yTable.clickEditRow(yTable.getRowIndex(LIST_Y_ROW_VALUE)); + setFormElement(Locator.name("quf_" + NAME_FIELD), LIST_Y_ROW_EDITED); + clickButton("Submit"); + + log("Discover List X's listId and the audit rowId for List Y's modification event"); + Connection cn = createDefaultConnection(); + int listXId = lookupListId(cn, LIST_X); + int listYId = lookupListId(cn, LIST_Y); + int listYAuditRowId = lookupListAuditRowId(cn, LIST_Y); + + log("Attack: URL says listId=" + LIST_X + " but rowId points at the audit event for " + LIST_Y); + String attackUrl = WebTestHelper.buildURL("list", getProjectName(), "listItemDetails", + Map.of("listId", String.valueOf(listXId), "rowId", String.valueOf(listYAuditRowId))); + beginAt(attackUrl); + assertEquals("Action should render normally (200), not throw", 200, getResponseCode()); + assertTextPresent("No details available for this event"); + assertTextNotPresent(LIST_Y_ROW_VALUE); // proof of non-disclosure: pre-edit value + assertTextNotPresent(LIST_Y_ROW_EDITED); // proof of non-disclosure: post-edit value + + log("Positive control: same rowId but with the matching listId must still render the audit changes"); + String matchedUrl = WebTestHelper.buildURL("list", getProjectName(), "listItemDetails", + Map.of("listId", String.valueOf(listYId), "rowId", String.valueOf(listYAuditRowId))); + beginAt(matchedUrl); + assertEquals(200, getResponseCode()); + assertTextPresent(LIST_Y_ROW_VALUE); + assertTextPresent(LIST_Y_ROW_EDITED); + assertTextNotPresent("No details available for this event"); + } + + private int lookupListId(Connection cn, String listName) throws Exception + { + SelectRowsCommand cmd = new SelectRowsCommand("exp", "Lists"); + cmd.setColumns(List.of("RowId", "Name")); + cmd.addFilter(new Filter("Name", listName, Filter.Operator.EQUAL)); + SelectRowsResponse rs = cmd.execute(cn, getProjectName()); + if (rs.getRows().isEmpty()) + throw new AssertionError("No exp.Lists row for " + listName); + return ((Number) rs.getRows().get(0).get("RowId")).intValue(); + } + + private int lookupListAuditRowId(Connection cn, String listName) throws Exception + { + // Most-recent ListAuditEvent for this list; the row-modify above will be it. + SelectRowsCommand cmd = new SelectRowsCommand("auditLog", "ListAuditEvent"); + cmd.setColumns(List.of("RowId", "ListName", "Comment")); + cmd.addFilter(new Filter("ListName", listName, Filter.Operator.EQUAL)); + cmd.setSorts(List.of(new Sort("RowId", Sort.Direction.DESCENDING))); + cmd.setMaxRows(1); + SelectRowsResponse rs = cmd.execute(cn, getProjectName()); + if (rs.getRows().isEmpty()) + throw new AssertionError("No ListAuditEvent for " + listName); + return ((Number) rs.getRows().get(0).get("RowId")).intValue(); + } + /* Issue 23487: add regression coverage for batch insert into list with multiple errors */ @Test From c4fb833fa83e4d508629c522abb58314b2de46a8 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 9 Jul 2026 08:51:22 -0700 Subject: [PATCH 2/6] Eliminate X-Frame-Options setting (#3094) #### Rationale This ` X-Frame-Options` setting has been removed. So has the `CSRF` setting (long ago). #### Related Pull Requests - https://github.com/LabKey/platform/pull/7831 --- .../pages/core/admin/CustomizeSitePage.java | 25 +------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java b/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java index 35cd9c0d9a..b1d4e4925a 100644 --- a/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java +++ b/src/org/labkey/test/pages/core/admin/CustomizeSitePage.java @@ -188,18 +188,6 @@ public CustomizeSitePage setAdminOnlyMessage(String value) return this; } - public CustomizeSitePage setCSRFCheck(CSRFCheck value) - { - elementCache().CSRFCheck.selectByValue(value.name()); - return this; - } - - public CustomizeSitePage setXFrameOption(XFrameOption value) - { - elementCache().XFrameOption.selectByValue(value.name()); - return this; - } - public CustomizeSitePage setEnableServerHttpHeader(boolean enable) { elementCache().enableServerHttpHeader.set(enable); @@ -271,8 +259,6 @@ protected RadioButton exceptionReportingLevel(ReportingLevel level) protected final Input adminOnlyMessage = Input(Locator.id("adminOnlyMessage"), getDriver()).findWhenNeeded(this); // HTTP Security Settings - protected final Select CSRFCheck = Select(Locator.id("CSRFCheck")).findWhenNeeded(this); - protected final Select XFrameOption = Select(Locator.id("XFrameOption")).findWhenNeeded(this); protected final Checkbox enableServerHttpHeader = Checkbox(Locator.id("includeServerHttpHeader")).findWhenNeeded(this); } @@ -281,17 +267,8 @@ public enum ReportingLevel NONE, LOW, MEDIUM, HIGH } - public enum CSRFCheck - { - POST, ADMINONLY - } - - public enum XFrameOption - { - SAMEORIGIN, ALLOW - } - private static final int SECONDS_PER_DAY = 60*60*24; + public enum KeyExpirationOptions implements OptionSelect.SelectOption { UNLIMITED(-1), From ce384807c356a8baffeaccfb9c4a21867f783387 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Thu, 9 Jul 2026 13:40:43 -0700 Subject: [PATCH 3/6] Backport to 26.3: PackageLockJsonTest allow "npm:" alias transitive dependencies (#2995) (#3095) ## Rationale Cherry-pick of #2995 to release26.3-SNAPSHOT. Packages that declare npm-aliased dependencies (e.g. `npm:react-is@^18.3.1`) are legitimate registry references, but the pre-#2995 version of the test flags them as untrusted sources. ## Changes - Allow transitive dependency version specs with an `npm:` alias prefix instead of the hard-coded allow-list; still flags URL, file, git, and workspace specs. - Error messages report lock-file paths relative to `server/modules`. Co-authored-by: Cory Nathe --- .../test/tests/PackageLockJsonTest.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index b9b040957b..14a255a10e 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -28,8 +28,7 @@ public class PackageLockJsonTest { private static final Set ALLOWED_DEPENDENCY_HOSTS = Set.of("registry.npmjs.org", "labkey.jfrog.io"); - // Allow-list of '@isaacs/cliui' dependencies - private static final Set ALLOWED_NONSTANDARD_VERSIONS = Set.of("npm:string-width@^4.2.0", "npm:strip-ansi@^6.0.1", "npm:wrap-ansi@^7.0.0"); + private static final File SERVER_MODULES_DIR = new File(TestFileUtils.getLabKeyRoot(), "server/modules"); private final List errors = new ArrayList<>(); private final File moduleDir; @@ -44,11 +43,10 @@ public static Collection data() { List allModules = new ArrayList<>(); - File modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules"); - File[] files = modulesDir.listFiles(); + File[] files = SERVER_MODULES_DIR.listFiles(); if (files == null) { - throw new RuntimeException("No files found in modules directory: " + modulesDir.getAbsolutePath()); + throw new RuntimeException("No files found in modules directory: " + SERVER_MODULES_DIR.getAbsolutePath()); } for (File file : files) { @@ -96,17 +94,19 @@ public void testPackageLock() throws Exception } } - Assert.assertTrue("Bad sources: " + errors, errors.isEmpty()); + Assert.assertTrue("Untrusted package sources:\n" + String.join("\n", errors), errors.isEmpty()); } /// Verify that a package reference in a package-lock.json file only resolves to known hosts and has a valid version /// Also checks sub-dependencies private void verifyPackage(String packageName, JSONObject packageJson, File packageLockFile) { + String relPath = SERVER_MODULES_DIR.toPath().relativize(packageLockFile.toPath()).toString(); + String resolved = packageJson.optString("resolved"); if (resolved.isBlank()) { - TestLogger.debug("Resolved field is blank for package " + packageName + " in " + packageLockFile.getAbsolutePath()); + TestLogger.debug("Resolved field is blank for package " + packageName + " in " + relPath); } else { @@ -116,14 +116,14 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack String host = resolvedURL.getHost(); if (!ALLOWED_DEPENDENCY_HOSTS.contains(host)) { - String message = "Package " + packageName + " resolved to unrecognized host [" + host + "] in " + packageLockFile.getAbsolutePath(); + String message = "Package " + packageName + " resolved to unrecognized host [" + host + "] in " + relPath; errors.add(message); TestLogger.error(message); } } catch (URISyntaxException e) { - String message = "Package " + packageName + " resolved to an invalid location [" + resolved + "] in " + packageLockFile.getAbsolutePath(); + String message = "Package " + packageName + " resolved to an invalid location [" + resolved + "] in " + relPath; errors.add(message); TestLogger.error(message); } @@ -132,7 +132,7 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack String version = packageJson.optString("version"); if (version.isBlank() || !CharUtils.isAsciiNumeric(version.charAt(0))) { - String message = "Package " + packageName + " has bad version [" + version + "] in " + packageLockFile.getAbsolutePath(); + String message = "Package " + packageName + " has bad version [" + version + "] in " + relPath; errors.add(message); TestLogger.error(message); } @@ -148,9 +148,9 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack else { String tVer = transitiveDeps.optString(tDep); - if (tVer == null || tVer.contains(":") && !ALLOWED_NONSTANDARD_VERSIONS.contains(tVer)) // URL, file, or workspace dependency + if (tVer == null || tVer.replaceAll("^npm:", "").contains(":")) // URL, file, or workspace dependency { - String message = "Package " + packageName + " has bad transitive dependency [" + tVer + "] in " + packageLockFile.getAbsolutePath(); + String message = "Package " + packageName + " has bad transitive dependency [" + tVer + "] in " + relPath; errors.add(message); TestLogger.error(message); } From 133e5b4a32fec947511e1461bcc3500a600dae1d Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Thu, 9 Jul 2026 17:05:08 -0700 Subject: [PATCH 4/6] Update expected class name for the API key display field (#3096) ## Rationale `ApiKeyDialog` didn't get the memo that this class name changed ## Related Pull Requests - https://github.com/LabKey/labkey-ui-components/pull/2032 --- src/org/labkey/test/components/core/ApiKeyDialog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/org/labkey/test/components/core/ApiKeyDialog.java b/src/org/labkey/test/components/core/ApiKeyDialog.java index 3841551a0d..4a61e18190 100644 --- a/src/org/labkey/test/components/core/ApiKeyDialog.java +++ b/src/org/labkey/test/components/core/ApiKeyDialog.java @@ -182,7 +182,7 @@ protected class ElementCache extends ModalDialog.ElementCache OptionSelect restrictionRoleSelect = new OptionSelect<>(Locator.tagWithId("select", "keyRole") .findWhenNeeded(this).withTimeout(2000)); WebElement generateApiKeyButton = Locator.tagWithText("button", "Generate API Key").findWhenNeeded(this); - Input inputField = Input.Input(Locator.tagWithClass("input", "api-key__input"), getDriver()).findWhenNeeded(this); + Input inputField = Input.Input(Locator.tagWithClass("input", "api-key__display"), getDriver()).findWhenNeeded(this); WebElement copyKeyButton = Locator.tagWithName("button", "copy_apikey_token").findWhenNeeded(this); WebElement doneButton = Locator.tagWithText("button", "Done").findWhenNeeded(this); } From 2b1a5eda52f3ed6ca0cd568d5ad7cf2d36e6c521 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Mon, 13 Jul 2026 16:53:05 -0700 Subject: [PATCH 5/6] GitHub Issue 1300: BulkUpgradeGroupAction scoping changes (#3102) ## Rationale BulkUpgradeGroupAction is inconsistent with other actions for its permission checks ## Related Pull Requests - https://github.com/LabKey/platform/pull/7836 ## Changes - Test coverage for other projects and adding users --- .../remoteapi/BulkUpdateGroupApiTest.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java b/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java index 3d4ac3ede5..0dc1489608 100644 --- a/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java +++ b/src/org/labkey/test/tests/remoteapi/BulkUpdateGroupApiTest.java @@ -30,6 +30,7 @@ import org.labkey.test.util.APIUserHelper; import org.labkey.test.util.ApiPermissionsHelper; import org.labkey.test.util.DataRegionTable; +import org.labkey.test.util.PermissionsHelper; import org.labkey.test.util.PermissionsHelper.PrincipalType; import java.util.ArrayList; @@ -62,11 +63,15 @@ public class BulkUpdateGroupApiTest extends BaseWebDriverTest private static Integer group1Id; private static Integer group2Id; private static final String siteGroup = "createdSiteGroup"; + private static final String OTHER_PROJECT = "BulkUpdateGroupApiTest Other Project"; + private static final String OTHER_GROUP = "otherProjectGroup"; + private static Integer otherProjectGroupId; @Override protected void doCleanup(boolean afterTest) throws TestTimeoutException { _containerHelper.deleteProject(getProjectName(), afterTest); + _containerHelper.deleteProject(OTHER_PROJECT, afterTest); deleteTestUsers(EMAIL_SUFFIX); _permissionsHelper.deleteGroup(siteGroup); } @@ -105,6 +110,9 @@ private void doSetup() user2Id = _userHelper.createUser(USER2).getUserId(); group1Id = _permissionsHelper.createProjectGroup(GROUP1, getProjectName()); group2Id = _permissionsHelper.createProjectGroup(GROUP2, getProjectName()); + + _containerHelper.createProject(OTHER_PROJECT, null); + otherProjectGroupId = _permissionsHelper.createProjectGroup(OTHER_GROUP, OTHER_PROJECT); } @Test @@ -503,6 +511,60 @@ public void testBulkUpdateAuditing() throws Exception AuditLogTest.verifyAuditEvent(this, AuditLogTest.GROUP_AUDIT_EVENT, "Comment", GROUP1, 2); } + // A project group may only be modified through the container it belongs to; targeting another project's group must be rejected. + @Test + public void testGroupFromDifferentContainerError() throws Exception + { + // otherProjectGroupId belongs to OTHER_PROJECT, but the command targets this test's project + BulkUpdateGroupCommand command = new BulkUpdateGroupCommand(otherProjectGroupId); + command.addMemberUser(USER1); + Connection connection = createDefaultConnection(); + + try + { + String message = command.execute(connection, getProjectName()).getText(); + fail("Expected CommandException modifying a group from another container\nResponse:\n" + message); + } + catch (CommandException e) + { + assertTrue("Expected cross-container error. Actual error: " + e.getMessage(), e.getMessage().contains("does not belong to this project")); + } + + _permissionsHelper.assertUserNotInGroup(USER1, OTHER_GROUP, OTHER_PROJECT, PrincipalType.USER); + } + + // Adding an existing user to a group requires only AdminPermission, but creating a brand-new user additionally requires AddUserPermission. + @Test + public void testCreateUserWithoutAddUserPermission() throws Exception + { + // A Folder Administrator has AdminPermission (so the action's @RequiresPermission passes) but lacks AddUserPermission + String folderAdmin = genTestEmail("folderadminnoadduser"); + String newUser = genTestEmail("shouldnotbecreated"); + _userHelper.createUser(folderAdmin); + _permissionsHelper.addMemberToRole(folderAdmin, PermissionsHelper.FOLDER_ADMIN_ROLE, PermissionsHelper.MemberType.user, getProjectName()); + + Connection connection = createDefaultConnection(); + connection.impersonate(folderAdmin, getProjectName()); + try + { + BulkUpdateGroupCommand command = new BulkUpdateGroupCommand(group1Id); + command.addMemberUser(USER1); // pre-existing user: should be added successfully + command.addMemberUser(newUser); // new user: folder admin lacks AddUserPermission + BulkUpdateGroupResponse response = command.execute(connection, getProjectName()); + + List errors = collectErrors(response); + assertEquals("Expected exactly one member error:\n" + String.join("\n", errors), 1, errors.size()); + assertTrue("Expected AddUserPermission error. Actual error: " + errors.get(0), errors.get(0).contains("do not have permission to create new users")); + } + finally + { + connection.stopImpersonating(); + } + + assertNull("New user was created despite the caller lacking AddUserPermission", _userHelper.getUserId(newUser)); + _permissionsHelper.assertUserInGroup(USER1, GROUP1, getProjectName(), PrincipalType.USER); + } + protected List collectErrors(BulkUpdateGroupResponse response) { Map errors = response.getErrors(); From abdbfa9e80926a7598ba41cbd6dbc8ed3a21c50d Mon Sep 17 00:00:00 2001 From: Dan Duffek Date: Mon, 13 Jul 2026 16:57:53 -0700 Subject: [PATCH 6/6] Fix Flakey Test Issues (#3097) --- src/org/labkey/test/util/AuditLogHelper.java | 24 ++++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/org/labkey/test/util/AuditLogHelper.java b/src/org/labkey/test/util/AuditLogHelper.java index 74c8a45114..7c99a3af18 100644 --- a/src/org/labkey/test/util/AuditLogHelper.java +++ b/src/org/labkey/test/util/AuditLogHelper.java @@ -261,19 +261,29 @@ public void checkAuditEventValuesForTransactionId(String containerPath, AuditEve public void checkAuditEventValuesForTransactionId(String containerPath, AuditEvent auditEventName, Integer transactionId, List> expectedValues) throws IOException, CommandException { - List columnNames = expectedValues.get(0).keySet().stream().map(Object::toString).toList(); + List columnNames = expectedValues.getFirst().keySet().stream().map(Object::toString).toList(); checkAuditEventValuesForTransactionId(containerPath, auditEventName, columnNames, transactionId, expectedValues); } public void checkAuditEventValuesForTransactionId(String containerPath, AuditEvent auditEventName, List columnNames, Integer transactionId, List> expectedValues) throws IOException, CommandException { List> events = getAuditLogsForTransactionId(containerPath, auditEventName, columnNames, transactionId, ContainerFilter.CurrentAndSubfolders); - assertEquals("Unexpected number of events for transactionId " + transactionId, expectedValues.size(), events.size()); - for (int i = 0; i < expectedValues.size(); i++) - { - for (String key : expectedValues.get(i).keySet()) - assertEquals("Event " + i + " value for " + key + " not as expected", expectedValues.get(i).get(key), events.get(i).get(key)); - } + + List> unmatched = expectedValues.stream() + .filter(expectedRow -> { + Set keysOfInterest = expectedRow.keySet(); + return events.stream().noneMatch(actualRow -> { + Map actualFiltered = actualRow.entrySet().stream() + .filter(e -> keysOfInterest.contains(e.getKey())) + .collect(HashMap::new, + (m, e) -> m.put(e.getKey(), e.getValue()), + HashMap::putAll); + return actualFiltered.equals(expectedRow); + }); + }) + .toList(); + + assertTrue("Expected rows with no match in actual: " + unmatched, unmatched.isEmpty()); } public Map getTransactionAuditLogDetails(Integer transactionAuditId)