From 514552b01f6defd92755882b5d0309de8827d160 Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Thu, 6 Nov 2025 14:27:29 -0800 Subject: [PATCH 1/9] Framework for package lock scan --- .../test/tests/PackageLockJsonTest.java | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 src/org/labkey/test/tests/PackageLockJsonTest.java diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java new file mode 100644 index 0000000000..6d8e55d26b --- /dev/null +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -0,0 +1,113 @@ +package org.labkey.test.tests; + +import org.json.JSONException; +import org.json.JSONObject; +import org.json.JSONTokener; +import org.junit.Assert; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.labkey.serverapi.reader.Readers; +import org.labkey.test.TestFileUtils; +import org.labkey.test.util.TestLogger; + +import java.io.File; +import java.io.Reader; +import java.net.URL; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; + +@Category({}) +public class PackageLockJsonTest +{ + private static final Set ALLOWED_SOURCES = Set.of("registry.npmjs.org", "labkey.jfrog.io"); + private final Map depCounts = new HashMap<>(); + private final List badSources = new ArrayList<>(); + + @Test + public void test() throws Exception + { + File modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules"); + File[] files = modulesDir.listFiles(); + if (files == null) + { + throw new RuntimeException("No files found in modules directory: " + modulesDir.getAbsolutePath()); + } + for (File file : files) + { + if (file.isDirectory()) + { + if (new File(file, "module.properties").exists()) + testModule(file); + else + testModuleContainer(file); + } + } + TestLogger.log("Package lock JSON dependencies:" + depCounts); + Assert.assertTrue("Bad sources: " + badSources, badSources.isEmpty()); + } + + private void testModuleContainer(File moduleContainer) throws Exception + { + File[] modules = moduleContainer.listFiles(); + if (modules == null) + { + throw new RuntimeException("No files found in modules directory: " + moduleContainer.getAbsolutePath()); + } + for (File module : modules) + { + if (module.isDirectory() && new File(module, "module.properties").isFile()) + { + testModule(module); + } + } + } + + private void testModule(File module) throws Exception + { + File packageLock = new File(module, "package-lock.json"); + if (packageLock.isFile()) + { + TestLogger.log("Testing module: " + module.getAbsolutePath()); + + JSONObject packages; + try (Reader reader = Readers.getReader(packageLock)) + { + JSONObject jsonObject = new JSONObject(new JSONTokener(reader)); + packages = jsonObject.optJSONObject("packages"); + if (packages == null) + packages = jsonObject.getJSONObject("dependencies"); // old lockfile version + } + catch (JSONException e) + { + TestLogger.error("Testing module: " + module.getName() + " failed to parse package-lock.json: " + e.getMessage()); + return; + } + for (String packageName : packages.keySet()) + { + if (!packageName.isBlank()) + { + JSONObject packageJson = packages.getJSONObject(packageName); + String resolved = packageJson.optString("resolved"); + if (resolved.isBlank()) + { + TestLogger.warn("Resolved field is blank for package " + packageName + " in " + packageLock.getAbsolutePath()); + continue; + } + URL resolvedURL = new URL(resolved); + String host = resolvedURL.getHost(); + if (!ALLOWED_SOURCES.contains(host)) + { + String message = "Package " + packageName + " resolved to unrecognized host " + host + " in " + packageLock.getAbsolutePath(); + badSources.add(message); + TestLogger.error(message); + } + depCounts.computeIfAbsent(host, k -> new AtomicInteger()).incrementAndGet(); + } + } + } + } +} From 06e1b49d464da1efab50ee6bcf0463543b3792c6 Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Mon, 19 Jan 2026 15:24:45 -0800 Subject: [PATCH 2/9] Scan all package.json files for URL dependencies --- .../test/tests/PackageLockJsonTest.java | 68 ++++++++++++++++++- 1 file changed, 67 insertions(+), 1 deletion(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index 6d8e55d26b..b9f6b6ece0 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -11,24 +11,90 @@ import org.labkey.test.util.TestLogger; import java.io.File; +import java.io.IOException; +import java.io.InputStream; import java.io.Reader; import java.net.URL; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.PathMatcher; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.regex.Pattern; +import java.util.stream.Stream; @Category({}) public class PackageLockJsonTest { private static final Set ALLOWED_SOURCES = Set.of("registry.npmjs.org", "labkey.jfrog.io"); + private static final Pattern STARTS_WITH_ALPHA = Pattern.compile("^[a-zA-Z]"); private final Map depCounts = new HashMap<>(); private final List badSources = new ArrayList<>(); @Test - public void test() throws Exception + public void packageJsonTest() throws Exception + { + Path modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules").toPath(); + PathMatcher packageJsonMatcher = FileSystems.getDefault().getPathMatcher("glob:**/package.json"); + + try (Stream paths = Files.walk(modulesDir)) + { + paths + .filter(path -> !path.toString().contains("/.gradle/")) + .filter(packageJsonMatcher::matches) + .forEach(this::scanPackageJson); + } + Assert.assertTrue("Bad sources: " + String.join("\n", badSources), badSources.isEmpty()); + } + + public void scanPackageJson(Path packageJson) + { + TestLogger.log("Scanning " + packageJson); + + try (InputStream is = Files.newInputStream(packageJson)) + { + JSONObject jsonObject = new JSONObject(new JSONTokener(is)); + JSONObject dependencies = jsonObject.optJSONObject("dependencies"); + JSONObject devDependencies = jsonObject.optJSONObject("devDependencies"); + if (dependencies != null) + { + dependencies.keySet().forEach(key -> { + String val = dependencies.getString(key); + if (STARTS_WITH_ALPHA.matcher(val).matches()) + { + badSources.add(key + " = " + val + " - " + packageJson); + } + }); + } + if (devDependencies != null) + { + devDependencies.keySet().forEach(key -> { + String val = devDependencies.getString(key); + if (STARTS_WITH_ALPHA.matcher(val).matches()) + { + badSources.add(key + " = " + val + " - " + packageJson); + } + }); + } + } + catch (JSONException ex) + { + TestLogger.log(" JSONException: " + ex.getMessage()); + } + catch (IOException e) + { + throw new RuntimeException("Error reading " + packageJson, e); + } + + } + + @Test + public void testPackageLockJson() throws Exception { File modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules"); File[] files = modulesDir.listFiles(); From c1baf6055448d4caff007659f77b7ed077bb648f Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Wed, 28 Jan 2026 08:36:59 -0800 Subject: [PATCH 3/9] Improve package-lock check --- .../test/tests/PackageLockJsonTest.java | 89 +++++++++++++------ 1 file changed, 63 insertions(+), 26 deletions(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index b9f6b6ece0..93970932df 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -1,9 +1,11 @@ package org.labkey.test.tests; +import org.apache.commons.lang3.CharUtils; import org.json.JSONException; import org.json.JSONObject; import org.json.JSONTokener; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; import org.labkey.serverapi.reader.Readers; @@ -14,7 +16,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.Reader; -import java.net.URL; +import java.net.URI; +import java.net.URISyntaxException; import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; @@ -25,18 +28,16 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; -import java.util.regex.Pattern; import java.util.stream.Stream; @Category({}) public class PackageLockJsonTest { private static final Set ALLOWED_SOURCES = Set.of("registry.npmjs.org", "labkey.jfrog.io"); - private static final Pattern STARTS_WITH_ALPHA = Pattern.compile("^[a-zA-Z]"); private final Map depCounts = new HashMap<>(); - private final List badSources = new ArrayList<>(); + private final List errors = new ArrayList<>(); - @Test + @Test @Ignore("package-lock check seems sufficient") public void packageJsonTest() throws Exception { Path modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules").toPath(); @@ -49,7 +50,7 @@ public void packageJsonTest() throws Exception .filter(packageJsonMatcher::matches) .forEach(this::scanPackageJson); } - Assert.assertTrue("Bad sources: " + String.join("\n", badSources), badSources.isEmpty()); + Assert.assertTrue("Bad sources: " + String.join("\n", errors), errors.isEmpty()); } public void scanPackageJson(Path packageJson) @@ -65,9 +66,9 @@ public void scanPackageJson(Path packageJson) { dependencies.keySet().forEach(key -> { String val = dependencies.getString(key); - if (STARTS_WITH_ALPHA.matcher(val).matches()) + if (val.contains(":")) // URL, file, or workspace dependency { - badSources.add(key + " = " + val + " - " + packageJson); + errors.add(key + " = " + val + " - " + packageJson); } }); } @@ -75,9 +76,9 @@ public void scanPackageJson(Path packageJson) { devDependencies.keySet().forEach(key -> { String val = devDependencies.getString(key); - if (STARTS_WITH_ALPHA.matcher(val).matches()) + if (val.contains(":")) // URL, file, or workspace dependency { - badSources.add(key + " = " + val + " - " + packageJson); + errors.add(key + " = " + val + " - " + packageJson); } }); } @@ -113,7 +114,7 @@ public void testPackageLockJson() throws Exception } } TestLogger.log("Package lock JSON dependencies:" + depCounts); - Assert.assertTrue("Bad sources: " + badSources, badSources.isEmpty()); + Assert.assertTrue("Bad sources: " + errors, errors.isEmpty()); } private void testModuleContainer(File moduleContainer) throws Exception @@ -157,21 +158,57 @@ private void testModule(File module) throws Exception if (!packageName.isBlank()) { JSONObject packageJson = packages.getJSONObject(packageName); - String resolved = packageJson.optString("resolved"); - if (resolved.isBlank()) - { - TestLogger.warn("Resolved field is blank for package " + packageName + " in " + packageLock.getAbsolutePath()); - continue; - } - URL resolvedURL = new URL(resolved); - String host = resolvedURL.getHost(); - if (!ALLOWED_SOURCES.contains(host)) - { - String message = "Package " + packageName + " resolved to unrecognized host " + host + " in " + packageLock.getAbsolutePath(); - badSources.add(message); - TestLogger.error(message); - } - depCounts.computeIfAbsent(host, k -> new AtomicInteger()).incrementAndGet(); + verifyPackage(packageName, packageJson, packageLock); + } + } + } + } + + private void verifyPackage(String packageName, JSONObject packageJson, File packageLock) throws URISyntaxException + { + String resolved = packageJson.optString("resolved"); + if (resolved.isBlank()) + { + TestLogger.debug("Resolved field is blank for package " + packageName + " in " + packageLock.getAbsolutePath()); + + } + else + { + URI resolvedURL = new URI(resolved); + String host = resolvedURL.getHost(); + if (!ALLOWED_SOURCES.contains(host)) + { + String message = "Package " + packageName + " resolved to unrecognized host " + host + " in " + packageLock.getAbsolutePath(); + errors.add(message); + TestLogger.error(message); + } + depCounts.computeIfAbsent(host, k -> new AtomicInteger()).incrementAndGet(); + } + + String version = packageJson.optString("version"); + if (version.isBlank() || !CharUtils.isAsciiNumeric(version.charAt(0))) + { + String message = "Package " + packageName + " has bad version [" + version + "] in " + packageLock.getAbsolutePath(); + errors.add(message); + TestLogger.error(message); + } + + JSONObject transitiveDeps = packageJson.optJSONObject("dependencies", new JSONObject()); + for (String tDep : transitiveDeps.keySet()) + { + JSONObject packageJsonDep = transitiveDeps.optJSONObject(tDep); + if (packageJsonDep != null) + { + verifyPackage(tDep, packageJsonDep, packageLock); + } + else + { + String tVer = transitiveDeps.optString(tDep); + if (tVer == null || tVer.contains(":") && !tVer.startsWith("npm:")) // URL, file, or workspace dependency + { + String message = "Package " + packageName + " has bad transitive dependency [" + tVer + "] in " + packageLock.getAbsolutePath(); + errors.add(message); + TestLogger.error(message); } } } From 9901db0c0b164f094293d2de093cfd0c8007f616 Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Wed, 28 Jan 2026 12:10:58 -0800 Subject: [PATCH 4/9] Update PackageLockJsonTest to be parameterized --- .../test/tests/PackageLockJsonTest.java | 158 ++++++------------ 1 file changed, 53 insertions(+), 105 deletions(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index 93970932df..0825afe736 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -5,98 +5,49 @@ import org.json.JSONObject; import org.json.JSONTokener; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.labkey.serverapi.reader.Readers; import org.labkey.test.TestFileUtils; import org.labkey.test.util.TestLogger; import java.io.File; -import java.io.IOException; -import java.io.InputStream; import java.io.Reader; import java.net.URI; import java.net.URISyntaxException; -import java.nio.file.FileSystems; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.PathMatcher; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.Stream; @Category({}) +@RunWith(Parameterized.class) public class PackageLockJsonTest { private static final Set ALLOWED_SOURCES = 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 final Map depCounts = new HashMap<>(); private final List errors = new ArrayList<>(); - @Test @Ignore("package-lock check seems sufficient") - public void packageJsonTest() throws Exception - { - Path modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules").toPath(); - PathMatcher packageJsonMatcher = FileSystems.getDefault().getPathMatcher("glob:**/package.json"); - - try (Stream paths = Files.walk(modulesDir)) - { - paths - .filter(path -> !path.toString().contains("/.gradle/")) - .filter(packageJsonMatcher::matches) - .forEach(this::scanPackageJson); - } - Assert.assertTrue("Bad sources: " + String.join("\n", errors), errors.isEmpty()); - } + private final File moduleDir; - public void scanPackageJson(Path packageJson) + public PackageLockJsonTest(File moduleDir, String name) { - TestLogger.log("Scanning " + packageJson); - - try (InputStream is = Files.newInputStream(packageJson)) - { - JSONObject jsonObject = new JSONObject(new JSONTokener(is)); - JSONObject dependencies = jsonObject.optJSONObject("dependencies"); - JSONObject devDependencies = jsonObject.optJSONObject("devDependencies"); - if (dependencies != null) - { - dependencies.keySet().forEach(key -> { - String val = dependencies.getString(key); - if (val.contains(":")) // URL, file, or workspace dependency - { - errors.add(key + " = " + val + " - " + packageJson); - } - }); - } - if (devDependencies != null) - { - devDependencies.keySet().forEach(key -> { - String val = devDependencies.getString(key); - if (val.contains(":")) // URL, file, or workspace dependency - { - errors.add(key + " = " + val + " - " + packageJson); - } - }); - } - } - catch (JSONException ex) - { - TestLogger.log(" JSONException: " + ex.getMessage()); - } - catch (IOException e) - { - throw new RuntimeException("Error reading " + packageJson, e); - } - + this.moduleDir = moduleDir; } - @Test - public void testPackageLockJson() throws Exception - { + @Parameterized.Parameters(name = "{1}") + public static Collection data() { + List allModules = new ArrayList<>(); + File modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules"); File[] files = modulesDir.listFiles(); if (files == null) @@ -108,58 +59,55 @@ public void testPackageLockJson() throws Exception if (file.isDirectory()) { if (new File(file, "module.properties").exists()) - testModule(file); + { + allModules.add(file); + } else - testModuleContainer(file); + { + allModules.addAll(Arrays.stream(Objects.requireNonNull(file.listFiles(), () -> "No files found in " + file.getAbsolutePath())) + .filter(f -> f.isDirectory() && new File(f, "module.properties").isFile()).toList()); + } } } - TestLogger.log("Package lock JSON dependencies:" + depCounts); - Assert.assertTrue("Bad sources: " + errors, errors.isEmpty()); + + return allModules.stream().filter(file -> new File(file, "package-lock.json").exists()) + .map(f -> new Object[]{f, f.getName()}).toList(); } - private void testModuleContainer(File moduleContainer) throws Exception + @Test + public void testPackageLock() throws Exception { - File[] modules = moduleContainer.listFiles(); - if (modules == null) - { - throw new RuntimeException("No files found in modules directory: " + moduleContainer.getAbsolutePath()); - } - for (File module : modules) - { - if (module.isDirectory() && new File(module, "module.properties").isFile()) - { - testModule(module); - } - } + testModule(moduleDir); + Assert.assertTrue("Bad sources: " + errors, errors.isEmpty()); } private void testModule(File module) throws Exception { File packageLock = new File(module, "package-lock.json"); - if (packageLock.isFile()) + Assert.assertTrue("No package-lock.json found in module: " + module.getAbsolutePath(), packageLock.isFile()); // Should be unfailable + + TestLogger.log("Testing module: " + module.getAbsolutePath()); + + JSONObject packages; + try (Reader reader = Readers.getReader(packageLock)) + { + JSONObject jsonObject = new JSONObject(new JSONTokener(reader)); + packages = jsonObject.optJSONObject("packages"); + if (packages == null) + packages = jsonObject.getJSONObject("dependencies"); // old lockfile version + } + catch (JSONException e) { - TestLogger.log("Testing module: " + module.getAbsolutePath()); + TestLogger.error("Testing module: " + module.getName() + " failed to parse package-lock.json: " + e.getMessage()); + return; + } - JSONObject packages; - try (Reader reader = Readers.getReader(packageLock)) - { - JSONObject jsonObject = new JSONObject(new JSONTokener(reader)); - packages = jsonObject.optJSONObject("packages"); - if (packages == null) - packages = jsonObject.getJSONObject("dependencies"); // old lockfile version - } - catch (JSONException e) - { - TestLogger.error("Testing module: " + module.getName() + " failed to parse package-lock.json: " + e.getMessage()); - return; - } - for (String packageName : packages.keySet()) + for (String packageName : packages.keySet()) + { + if (!packageName.isBlank()) { - if (!packageName.isBlank()) - { - JSONObject packageJson = packages.getJSONObject(packageName); - verifyPackage(packageName, packageJson, packageLock); - } + JSONObject packageJson = packages.getJSONObject(packageName); + verifyPackage(packageName, packageJson, packageLock); } } } @@ -182,7 +130,7 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack errors.add(message); TestLogger.error(message); } - depCounts.computeIfAbsent(host, k -> new AtomicInteger()).incrementAndGet(); + depCounts.computeIfAbsent(host, _ -> new AtomicInteger()).incrementAndGet(); } String version = packageJson.optString("version"); @@ -199,12 +147,12 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack JSONObject packageJsonDep = transitiveDeps.optJSONObject(tDep); if (packageJsonDep != null) { - verifyPackage(tDep, packageJsonDep, packageLock); + verifyPackage(tDep, packageJsonDep, packageLock); // belt and suspenders } else { String tVer = transitiveDeps.optString(tDep); - if (tVer == null || tVer.contains(":") && !tVer.startsWith("npm:")) // URL, file, or workspace dependency + if (tVer == null || tVer.contains(":") && !ALLOWED_NONSTANDARD_VERSIONS.contains(tVer)) // URL, file, or workspace dependency { String message = "Package " + packageName + " has bad transitive dependency [" + tVer + "] in " + packageLock.getAbsolutePath(); errors.add(message); From 91b54519245d16ae1ccf161c841e7032247d504c Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Wed, 28 Jan 2026 12:12:12 -0800 Subject: [PATCH 5/9] Formatting cleanup --- .../labkey/test/tests/PackageLockJsonTest.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index 0825afe736..fcc28e5297 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -34,9 +34,9 @@ public class PackageLockJsonTest private static final Set ALLOWED_SOURCES = 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 final Map depCounts = new HashMap<>(); private final List errors = new ArrayList<>(); - private final File moduleDir; public PackageLockJsonTest(File moduleDir, String name) @@ -112,13 +112,12 @@ private void testModule(File module) throws Exception } } - private void verifyPackage(String packageName, JSONObject packageJson, File packageLock) throws URISyntaxException + private void verifyPackage(String packageName, JSONObject packageJson, File packageLockFile) throws URISyntaxException { String resolved = packageJson.optString("resolved"); if (resolved.isBlank()) { - TestLogger.debug("Resolved field is blank for package " + packageName + " in " + packageLock.getAbsolutePath()); - + TestLogger.debug("Resolved field is blank for package " + packageName + " in " + packageLockFile.getAbsolutePath()); } else { @@ -126,7 +125,7 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack String host = resolvedURL.getHost(); if (!ALLOWED_SOURCES.contains(host)) { - String message = "Package " + packageName + " resolved to unrecognized host " + host + " in " + packageLock.getAbsolutePath(); + String message = "Package " + packageName + " resolved to unrecognized host " + host + " in " + packageLockFile.getAbsolutePath(); errors.add(message); TestLogger.error(message); } @@ -136,7 +135,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 " + packageLock.getAbsolutePath(); + String message = "Package " + packageName + " has bad version [" + version + "] in " + packageLockFile.getAbsolutePath(); errors.add(message); TestLogger.error(message); } @@ -147,14 +146,14 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack JSONObject packageJsonDep = transitiveDeps.optJSONObject(tDep); if (packageJsonDep != null) { - verifyPackage(tDep, packageJsonDep, packageLock); // belt and suspenders + verifyPackage(tDep, packageJsonDep, packageLockFile); // belt and suspenders } else { String tVer = transitiveDeps.optString(tDep); if (tVer == null || tVer.contains(":") && !ALLOWED_NONSTANDARD_VERSIONS.contains(tVer)) // URL, file, or workspace dependency { - String message = "Package " + packageName + " has bad transitive dependency [" + tVer + "] in " + packageLock.getAbsolutePath(); + String message = "Package " + packageName + " has bad transitive dependency [" + tVer + "] in " + packageLockFile.getAbsolutePath(); errors.add(message); TestLogger.error(message); } From 1b88bd7111d8850c0d4856bba6fab204c5ee407b Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Wed, 28 Jan 2026 12:12:39 -0800 Subject: [PATCH 6/9] Formatting cleanup --- src/org/labkey/test/tests/PackageLockJsonTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index fcc28e5297..85698ad780 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -45,7 +45,8 @@ public PackageLockJsonTest(File moduleDir, String name) } @Parameterized.Parameters(name = "{1}") - public static Collection data() { + public static Collection data() + { List allModules = new ArrayList<>(); File modulesDir = new File(TestFileUtils.getLabKeyRoot(), "server/modules"); From 35edf71d63787b0a000146083a768d2ed816a29a Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Wed, 28 Jan 2026 12:59:39 -0800 Subject: [PATCH 7/9] Minor cleanup --- .../test/tests/PackageLockJsonTest.java | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index 85698ad780..df7494e95a 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -1,7 +1,6 @@ package org.labkey.test.tests; import org.apache.commons.lang3.CharUtils; -import org.json.JSONException; import org.json.JSONObject; import org.json.JSONTokener; import org.junit.Assert; @@ -20,12 +19,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; @Category({}) @RunWith(Parameterized.class) @@ -35,7 +31,6 @@ public class PackageLockJsonTest // 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 final Map depCounts = new HashMap<>(); private final List errors = new ArrayList<>(); private final File moduleDir; @@ -78,16 +73,10 @@ public static Collection data() @Test public void testPackageLock() throws Exception { - testModule(moduleDir); - Assert.assertTrue("Bad sources: " + errors, errors.isEmpty()); - } - - private void testModule(File module) throws Exception - { - File packageLock = new File(module, "package-lock.json"); - Assert.assertTrue("No package-lock.json found in module: " + module.getAbsolutePath(), packageLock.isFile()); // Should be unfailable + File packageLock = new File(moduleDir, "package-lock.json"); + Assert.assertTrue("No package-lock.json found in module: " + moduleDir.getAbsolutePath(), packageLock.isFile()); - TestLogger.log("Testing module: " + module.getAbsolutePath()); + TestLogger.log("Testing module: " + moduleDir.getAbsolutePath()); JSONObject packages; try (Reader reader = Readers.getReader(packageLock)) @@ -97,11 +86,6 @@ private void testModule(File module) throws Exception if (packages == null) packages = jsonObject.getJSONObject("dependencies"); // old lockfile version } - catch (JSONException e) - { - TestLogger.error("Testing module: " + module.getName() + " failed to parse package-lock.json: " + e.getMessage()); - return; - } for (String packageName : packages.keySet()) { @@ -111,9 +95,13 @@ private void testModule(File module) throws Exception verifyPackage(packageName, packageJson, packageLock); } } + + Assert.assertTrue("Bad sources: " + errors, errors.isEmpty()); } - private void verifyPackage(String packageName, JSONObject packageJson, File packageLockFile) throws URISyntaxException + /// 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 resolved = packageJson.optString("resolved"); if (resolved.isBlank()) @@ -122,15 +110,23 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack } else { - URI resolvedURL = new URI(resolved); - String host = resolvedURL.getHost(); - if (!ALLOWED_SOURCES.contains(host)) + try + { + URI resolvedURL = new URI(resolved); + String host = resolvedURL.getHost(); + if (!ALLOWED_SOURCES.contains(host)) + { + String message = "Package " + packageName + " resolved to unrecognized host [" + host + "] in " + packageLockFile.getAbsolutePath(); + errors.add(message); + TestLogger.error(message); + } + } + catch (URISyntaxException e) { - String message = "Package " + packageName + " resolved to unrecognized host " + host + " in " + packageLockFile.getAbsolutePath(); + String message = "Package " + packageName + " resolved to an invalid location [" + resolved + "] in " + packageLockFile.getAbsolutePath(); errors.add(message); TestLogger.error(message); } - depCounts.computeIfAbsent(host, _ -> new AtomicInteger()).incrementAndGet(); } String version = packageJson.optString("version"); From b818ed849403c93934b9467ee07f560112c43d33 Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Wed, 28 Jan 2026 13:31:15 -0800 Subject: [PATCH 8/9] Better name --- src/org/labkey/test/tests/PackageLockJsonTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/org/labkey/test/tests/PackageLockJsonTest.java b/src/org/labkey/test/tests/PackageLockJsonTest.java index df7494e95a..b9b040957b 100644 --- a/src/org/labkey/test/tests/PackageLockJsonTest.java +++ b/src/org/labkey/test/tests/PackageLockJsonTest.java @@ -27,7 +27,7 @@ @RunWith(Parameterized.class) public class PackageLockJsonTest { - private static final Set ALLOWED_SOURCES = Set.of("registry.npmjs.org", "labkey.jfrog.io"); + 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"); @@ -114,7 +114,7 @@ private void verifyPackage(String packageName, JSONObject packageJson, File pack { URI resolvedURL = new URI(resolved); String host = resolvedURL.getHost(); - if (!ALLOWED_SOURCES.contains(host)) + if (!ALLOWED_DEPENDENCY_HOSTS.contains(host)) { String message = "Package " + packageName + " resolved to unrecognized host [" + host + "] in " + packageLockFile.getAbsolutePath(); errors.add(message); From bd9ef0610ded4d5d2690adf28be7dd03de01f3f6 Mon Sep 17 00:00:00 2001 From: labkey-tchad Date: Thu, 29 Jan 2026 13:00:39 -0800 Subject: [PATCH 9/9] Add task to run package lock test --- build.gradle | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build.gradle b/build.gradle index 8b110bd5cb..aa002ab263 100644 --- a/build.gradle +++ b/build.gradle @@ -6,6 +6,7 @@ plugins { } import org.labkey.gradle.task.RunTestSuite +import org.labkey.gradle.task.RunUiTest import org.labkey.gradle.util.BuildUtils import org.labkey.gradle.util.GroupNames import org.labkey.gradle.util.PomFileHelper @@ -171,6 +172,10 @@ project.tasks.register("convertHarToStressXml", JavaExec) { } } +project.tasks.register("testPackageLockJson", RunUiTest) { + include "org/labkey/test/tests/PackageLockJsonTest.class" +} + project.tasks.named("uiTests").configure { dependsOn(initPropertiesTask) }