Skip to content

Commit 0d6c82a

Browse files
nathanielsherrynathanielsherry
committed
Additional bugfixes in bolt
1 parent 7b00819 commit 0d6c82a

7 files changed

Lines changed: 300 additions & 46 deletions

File tree

‎Framework/Bolt/Bolt/src/main/java/org/peakaboo/framework/bolt/plugin/core/container/BoltURLContainer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public boolean delete() {
4646
try {
4747
File f = new File(url.toURI());
4848
return f.delete();
49-
} catch (URISyntaxException e) {
49+
} catch (URISyntaxException | IllegalArgumentException e) {
5050
return false;
5151
}
5252
}

‎Framework/Bolt/Bolt/src/main/java/org/peakaboo/framework/bolt/repository/PluginMetadata.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ public <T extends BoltPlugin> Optional<PluginDescriptor<T>> getUpgradeTarget(Plu
9494
*/
9595
public <T extends BoltPlugin> Optional<PluginDescriptor<T>> getUpgradeTarget(ExtensionPointRegistry reg) {
9696
for (var subreg : reg.getRegistries()) {
97-
var result = getUpgradeTarget(subreg);
97+
Optional<PluginDescriptor<T>> result = this.<T>getUpgradeTarget((PluginRegistry<T>) subreg);
9898
if (result.isPresent()) {
99-
return (Optional<PluginDescriptor<T>>) result;
99+
return result;
100100
}
101101
}
102102
return Optional.empty();
@@ -168,6 +168,7 @@ public boolean validateChecksum(String filename) {
168168
OneLog.log(Level.WARNING, "Checksum failed to match for " + filename);
169169
return false;
170170
}
171+
if (this.checksum == null) { return false; }
171172
boolean matched = this.checksum.equalsIgnoreCase(md5sum);
172173
if (!matched) {
173174
OneLog.log(Level.WARNING, "Checksum failed to match for " + filename);
@@ -249,10 +250,12 @@ public boolean equals(Object obj) {
249250
if (this == obj) return true;
250251
if (obj == null) return false;
251252
if (obj instanceof PluginMetadata other) {
252-
return this.uuid.equals(other.uuid) &&
253-
this.name.equals(other.name) &&
254-
this.repositoryUrl.equals(other.repositoryUrl) &&
255-
this.downloadUrl.equals(other.downloadUrl);
253+
return this.uuid != null &&
254+
other.uuid != null &&
255+
this.uuid.equals(other.uuid) &&
256+
Objects.equals(this.name, other.name) &&
257+
Objects.equals(this.repositoryUrl, other.repositoryUrl) &&
258+
Objects.equals(this.downloadUrl, other.downloadUrl);
256259
}
257260
return false;
258261
}

‎Framework/Bolt/Bolt/src/main/java/org/peakaboo/framework/bolt/repository/RepositoryMetadata.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public boolean validate(String repoUrl, int appVersion) {
7575

7676
// DOWNLOAD URL
7777
// Ensure the plugin's download url starts with the repository URL
78-
if (!validateString(plugin.downloadUrl, 200) || this.repositoryUrl.contains("..") || !plugin.downloadUrl.startsWith(this.repositoryUrl)) {
78+
if (!validateString(plugin.downloadUrl, 200) || plugin.downloadUrl.contains("..") || !plugin.downloadUrl.startsWith(this.repositoryUrl)) {
7979
OneLog.log(Level.WARNING, "Plugin '" + plugin.name + "' has an invalid download URL: " + plugin.downloadUrl + ". It must begin with same path as the repository inventory file.");
8080
return false;
8181
}

‎Framework/Bolt/Bolt/src/test/java/net/sciencestudio/bolt/AppTest.java‎

Lines changed: 0 additions & 38 deletions
This file was deleted.

‎Framework/Bolt/Bolt/src/test/java/org/peakaboo/framework/bolt/plugin/core/container/BoltURLContainerTest.java‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package org.peakaboo.framework.bolt.plugin.core.container;
2+
3+
import static org.junit.Assert.*;
4+
5+
import java.io.File;
6+
import java.net.MalformedURLException;
7+
import java.net.URL;
8+
import java.util.List;
9+
10+
import org.junit.Test;
11+
import org.junit.Rule;
12+
import org.junit.rules.TemporaryFolder;
13+
14+
import org.peakaboo.framework.bolt.plugin.core.BoltPlugin;
15+
import org.peakaboo.framework.bolt.plugin.core.PluginDescriptor;
16+
import org.peakaboo.framework.bolt.plugin.core.PluginRegistry;
17+
import org.peakaboo.framework.bolt.plugin.core.issue.BoltIssue;
18+
19+
public class BoltURLContainerTest {
20+
21+
@Rule
22+
public TemporaryFolder tempFolder = new TemporaryFolder();
23+
24+
private static class TestURLContainer extends BoltURLContainer<BoltPlugin> {
25+
TestURLContainer(URL url, boolean deletable) { super(url, deletable); }
26+
public List<PluginDescriptor<BoltPlugin>> getPlugins() { return List.of(); }
27+
public List<BoltIssue<BoltPlugin>> getIssues() { return List.of(); }
28+
public PluginRegistry<BoltPlugin> getManager() { return null; }
29+
}
30+
31+
@Test
32+
public void getSourcePath_fileUrl_returnsAbsolutePath() throws Exception {
33+
File f = tempFolder.newFile("test-plugin.jar");
34+
URL url = f.toURI().toURL();
35+
TestURLContainer container = new TestURLContainer(url, false);
36+
assertEquals(f.getAbsolutePath(), container.getSourcePath());
37+
}
38+
39+
/** Regression: jar:file: URLs previously threw IllegalArgumentException (only URISyntaxException was caught). */
40+
@Test
41+
public void getSourcePath_jarUrl_returnsFallbackPath() throws Exception {
42+
URL url = new URL("jar:file:/opt/plugins/myplugin.jar!/plugin.yaml");
43+
TestURLContainer container = new TestURLContainer(url, false);
44+
String path = container.getSourcePath();
45+
assertNotNull(path);
46+
assertTrue("Expected path to contain 'plugin.yaml', got: " + path, path.contains("plugin.yaml"));
47+
}
48+
49+
/** Regression: jar:file: URLs previously threw IllegalArgumentException in getSourceName(). */
50+
@Test
51+
public void getSourceName_jarUrl_returnsFallbackName() throws Exception {
52+
URL url = new URL("jar:file:/opt/plugins/myplugin.jar!/plugin.yaml");
53+
TestURLContainer container = new TestURLContainer(url, false);
54+
assertEquals("plugin.yaml", container.getSourceName());
55+
}
56+
57+
@Test
58+
public void delete_fileUrl_deletesFile() throws Exception {
59+
File f = tempFolder.newFile("deleteme.jar");
60+
assertTrue(f.exists());
61+
URL url = f.toURI().toURL();
62+
TestURLContainer container = new TestURLContainer(url, true);
63+
assertTrue(container.delete());
64+
assertFalse(f.exists());
65+
}
66+
67+
/**
68+
* Bug: delete() only catches URISyntaxException, not IllegalArgumentException.
69+
* A jar:file: URL causes new File(url.toURI()) to throw IllegalArgumentException
70+
* which should be caught and return false, like getSourcePath/getSourceName do.
71+
*/
72+
@Test
73+
public void delete_jarUrl_returnsFalse() throws Exception {
74+
URL url = new URL("jar:file:/opt/plugins/myplugin.jar!/plugin.yaml");
75+
TestURLContainer container = new TestURLContainer(url, true);
76+
assertFalse("delete() should return false for non-file URLs", container.delete());
77+
}
78+
79+
}

‎Framework/Bolt/Bolt/src/test/java/org/peakaboo/framework/bolt/repository/PluginMetadataTest.java‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
package org.peakaboo.framework.bolt.repository;
2+
3+
import static org.junit.Assert.*;
4+
5+
import java.io.File;
6+
import java.nio.file.Files;
7+
8+
import org.junit.Rule;
9+
import org.junit.Test;
10+
import org.junit.rules.TemporaryFolder;
11+
12+
public class PluginMetadataTest {
13+
14+
@Rule
15+
public TemporaryFolder tempFolder = new TemporaryFolder();
16+
17+
@Test
18+
public void checksum_validFile_returnsHex() throws Exception {
19+
File f = tempFolder.newFile("testfile.jar");
20+
Files.writeString(f.toPath(), "hello world");
21+
String result = PluginMetadata.checksum(f.getAbsolutePath());
22+
assertNotNull(result);
23+
assertTrue("Expected 31-32 hex chars, got: " + result, result.matches("[0-9a-fA-F]{31,32}"));
24+
}
25+
26+
@Test
27+
public void checksum_nonexistentFile_returnsNull() {
28+
String result = PluginMetadata.checksum("/nonexistent/path/file.jar");
29+
assertNull(result);
30+
}
31+
32+
@Test
33+
public void validateChecksum_matching_returnsTrue() throws Exception {
34+
File f = tempFolder.newFile("checksumtest.jar");
35+
Files.writeString(f.toPath(), "test content for checksum");
36+
PluginMetadata meta = new PluginMetadata();
37+
meta.checksum = PluginMetadata.checksum(f.getAbsolutePath());
38+
assertNotNull(meta.checksum);
39+
assertTrue(meta.validateChecksum(f.getAbsolutePath()));
40+
}
41+
42+
@Test
43+
public void validateChecksum_mismatched_returnsFalse() throws Exception {
44+
File f = tempFolder.newFile("mismatch.jar");
45+
Files.writeString(f.toPath(), "some content");
46+
PluginMetadata meta = new PluginMetadata();
47+
meta.checksum = "00000000000000000000000000000000";
48+
assertFalse(meta.validateChecksum(f.getAbsolutePath()));
49+
}
50+
51+
/**
52+
* Bug: line 171 calls {@code this.checksum.equalsIgnoreCase(md5sum)} but
53+
* this.checksum is null when not set, causing a NullPointerException.
54+
* Should return false when checksum has not been set.
55+
*/
56+
@Test
57+
public void validateChecksum_nullStoredChecksum_returnsFalse() throws Exception {
58+
File f = tempFolder.newFile("nullcheck.jar");
59+
Files.writeString(f.toPath(), "content");
60+
PluginMetadata meta = new PluginMetadata();
61+
// meta.checksum is null by default
62+
assertFalse("validateChecksum should return false when stored checksum is null", meta.validateChecksum(f.getAbsolutePath()));
63+
}
64+
65+
@Test
66+
public void equals_sameFields_returnsTrue() {
67+
PluginMetadata a = buildMeta();
68+
PluginMetadata b = buildMeta();
69+
assertEquals(a, b);
70+
assertEquals(a.hashCode(), b.hashCode());
71+
}
72+
73+
/**
74+
* Bug: line 252 calls {@code this.uuid.equals(other.uuid)} but uuid is null
75+
* when not set, causing a NullPointerException. A PluginMetadata with a null
76+
* UUID has no identity and should not be equal to anything other than itself.
77+
*/
78+
@Test
79+
public void equals_nullUuid_returnsFalse() {
80+
PluginMetadata a = new PluginMetadata();
81+
PluginMetadata b = new PluginMetadata();
82+
assertNotEquals("PluginMetadata with null UUID should not equal another", a, b);
83+
}
84+
85+
private static PluginMetadata buildMeta() {
86+
PluginMetadata p = new PluginMetadata();
87+
p.uuid = "12345678-1234-1234-1234-123456789abc";
88+
p.name = "Test Plugin";
89+
p.repositoryUrl = "https://example.com/repo";
90+
p.downloadUrl = "https://example.com/repo/plugin.jar";
91+
return p;
92+
}
93+
94+
}

‎Framework/Bolt/Bolt/src/test/java/org/peakaboo/framework/bolt/repository/RepositoryMetadataTest.java‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package org.peakaboo.framework.bolt.repository;
2+
3+
import static org.junit.Assert.*;
4+
5+
import org.junit.Test;
6+
7+
public class RepositoryMetadataTest {
8+
9+
private static final String REPO_URL = "https://example.com/plugins/repo";
10+
private static final int APP_VERSION = 100;
11+
12+
/** Builds a fully valid RepositoryMetadata with one plugin. */
13+
private static RepositoryMetadata buildValidRepo(String repoUrl) {
14+
RepositoryMetadata repo = new RepositoryMetadata();
15+
repo.specVersion = 1;
16+
repo.repositoryName = "Test Repo";
17+
repo.repositoryUrl = repoUrl;
18+
repo.repositoryDescription = "A test repository";
19+
repo.applicationName = "Peakaboo";
20+
repo.plugins.add(buildValidPlugin(repoUrl));
21+
return repo;
22+
}
23+
24+
/** Builds a fully valid PluginMetadata that passes all checks. */
25+
private static PluginMetadata buildValidPlugin(String repoUrl) {
26+
PluginMetadata p = new PluginMetadata();
27+
p.name = "Test Plugin";
28+
p.category = "Filter";
29+
p.version = "1-0-0";
30+
p.minAppVersion = 1;
31+
p.uuid = "12345678-1234-1234-1234-123456789abc";
32+
p.downloadUrl = repoUrl + "/test-plugin.jar";
33+
p.repositoryUrl = repoUrl;
34+
p.description = "A test plugin";
35+
p.author = "Test Author";
36+
p.checksum = "d41d8cd98f00b204e9800998ecf8427e";
37+
p.releaseNotes = "Initial release";
38+
return p;
39+
}
40+
41+
@Test
42+
public void validate_allValid_returnsTrue() {
43+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
44+
assertTrue(repo.validate(REPO_URL, APP_VERSION));
45+
}
46+
47+
@Test
48+
public void validate_specVersionZero_returnsFalse() {
49+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
50+
repo.specVersion = 0;
51+
assertFalse(repo.validate(REPO_URL, APP_VERSION));
52+
}
53+
54+
@Test
55+
public void validate_repoNameContainsBuilt_returnsFalse() {
56+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
57+
repo.repositoryName = "Built-In Extras";
58+
assertFalse(repo.validate(REPO_URL, APP_VERSION));
59+
}
60+
61+
@Test
62+
public void validate_repoUrlMismatch_returnsFalse() {
63+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
64+
assertFalse(repo.validate("https://evil.com/repo", APP_VERSION));
65+
}
66+
67+
@Test
68+
public void validate_pluginDownloadUrlNotPrefixedByRepoUrl_returnsFalse() {
69+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
70+
repo.plugins.get(0).downloadUrl = "https://evil.com/malware.jar";
71+
assertFalse(repo.validate(REPO_URL, APP_VERSION));
72+
}
73+
74+
/**
75+
* Bug: line 78 checks {@code this.repositoryUrl.contains("..")} instead of
76+
* {@code plugin.downloadUrl.contains("..")}. A download URL with path traversal
77+
* should be rejected but passes because the wrong field is checked.
78+
*/
79+
@Test
80+
public void validate_pluginDownloadUrlContainsTraversal_returnsFalse() {
81+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
82+
repo.plugins.get(0).downloadUrl = REPO_URL + "/../../../etc/passwd";
83+
assertFalse("Download URL with path traversal should be rejected", repo.validate(REPO_URL, APP_VERSION));
84+
}
85+
86+
/**
87+
* Documents bug: line 85 checks {@code this.repositoryUrl.contains("..")} instead
88+
* of {@code plugin.repositoryUrl.contains("..")}. The traversal check is dead code
89+
* because:
90+
* <ol>
91+
* <li>Line 55 already rejects {@code this.repositoryUrl} containing ".."</li>
92+
* <li>Line 85's equality check catches any mismatch between plugin and repo URLs</li>
93+
* </ol>
94+
* So a plugin.repositoryUrl with ".." is caught by the equality check, not
95+
* the traversal check. This test verifies the equality check does catch it,
96+
* and documents that the traversal check on line 85 is checking the wrong field.
97+
*/
98+
@Test
99+
public void validate_pluginRepoUrlTraversalCheck_isDeadCode() {
100+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
101+
// Plugin has ".." but repo does not — should be caught by traversal check,
102+
// but that check looks at this.repositoryUrl (clean), so it passes.
103+
// The equality check (!plugin.repositoryUrl.equals(this.repositoryUrl)) catches
104+
// it instead — but only because the URLs don't match, not because of "..".
105+
repo.plugins.get(0).repositoryUrl = REPO_URL + "/../../../etc/passwd";
106+
assertFalse("Caught by equality check, not traversal check", repo.validate(REPO_URL, APP_VERSION));
107+
}
108+
109+
@Test
110+
public void validate_pluginMinAppVersionExceedsApp_returnsFalse() {
111+
RepositoryMetadata repo = buildValidRepo(REPO_URL);
112+
repo.plugins.get(0).minAppVersion = APP_VERSION + 1;
113+
assertFalse(repo.validate(REPO_URL, APP_VERSION));
114+
}
115+
116+
}

0 commit comments

Comments
 (0)