Skip to content

Commit eee1067

Browse files
author
FTMahringer
committed
fix(plugins): harden marketplace abuse checks
1 parent f010cd3 commit eee1067

9 files changed

Lines changed: 305 additions & 13 deletions

File tree

‎packages/core/src/main/java/dev/synapse/core/common/domain/StoreEntry.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import org.hibernate.annotations.JdbcTypeCode;
55
import org.hibernate.type.SqlTypes;
66

7+
import java.io.Serializable;
78
import java.time.Instant;
89
import java.util.List;
910
import java.util.Map;
@@ -13,7 +14,9 @@
1314
*/
1415
@Entity
1516
@Table(name = "store_entries")
16-
public class StoreEntry {
17+
public class StoreEntry implements Serializable {
18+
19+
private static final long serialVersionUID = 1L;
1720

1821
@Id
1922
private String id;

‎packages/core/src/main/java/dev/synapse/plugins/PluginSafetyPolicy.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,19 @@ public static PluginSafetyPolicy unverified(String source) {
3838
)
3939
);
4040
}
41+
42+
public static PluginSafetyPolicy spoofedVerified(
43+
String source,
44+
String pluginId
45+
) {
46+
return new PluginSafetyPolicy(
47+
PluginTrustLevel.UNVERIFIED,
48+
true,
49+
List.of(
50+
"This plugin claims verified source '" + source + "' but does not match the store registry metadata for '" + pluginId + "'.",
51+
"Treat this as an unverified plugin and review the JAR and manifest before installing.",
52+
"Pass confirmed=true to proceed."
53+
)
54+
);
55+
}
4156
}

‎packages/core/src/main/java/dev/synapse/plugins/PluginSafetyService.java‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
package dev.synapse.plugins;
22

33
import dev.synapse.core.common.domain.Plugin;
4-
import dev.synapse.core.infrastructure.exception.ResourceNotFoundException;
4+
import dev.synapse.core.common.domain.StoreEntry;
5+
import dev.synapse.core.common.repository.StoreEntryRepository;
56
import dev.synapse.core.infrastructure.logging.LogCategory;
67
import dev.synapse.core.infrastructure.logging.LogLevel;
78
import dev.synapse.core.infrastructure.logging.SystemLogService;
@@ -21,21 +22,32 @@ public class PluginSafetyService {
2122
private static final Set<String> COMMUNITY_SOURCES = Set.of("community", "skills_sh");
2223

2324
private final PluginLifecycleService lifecycleService;
25+
private final StoreEntryRepository storeEntryRepository;
2426
private final SystemLogService logService;
2527

26-
public PluginSafetyService(PluginLifecycleService lifecycleService, SystemLogService logService) {
28+
public PluginSafetyService(
29+
PluginLifecycleService lifecycleService,
30+
StoreEntryRepository storeEntryRepository,
31+
SystemLogService logService
32+
) {
2733
this.lifecycleService = lifecycleService;
34+
this.storeEntryRepository = storeEntryRepository;
2835
this.logService = logService;
2936
}
3037

3138
/**
3239
* Assess trust level for a manifest by its source field.
3340
*/
3441
public PluginSafetyPolicy assess(Map<String, Object> manifest) {
42+
String pluginId = manifest.getOrDefault("id", "unknown").toString();
3543
Object sourceObj = manifest.get("source");
36-
String source = sourceObj != null ? sourceObj.toString() : "";
44+
String source = sourceObj != null ? sourceObj.toString().trim().toLowerCase() : "";
45+
StoreEntry registryEntry = storeEntryRepository.findById(pluginId).orElse(null);
3746

3847
if (VERIFIED_SOURCES.contains(source)) {
48+
if (!matchesRegistryEntry(manifest, registryEntry, source)) {
49+
return PluginSafetyPolicy.spoofedVerified(source, pluginId);
50+
}
3951
return PluginSafetyPolicy.verified();
4052
}
4153
if (COMMUNITY_SOURCES.contains(source)) {
@@ -75,4 +87,37 @@ public Plugin safeInstall(Map<String, Object> manifest, boolean confirmed) {
7587

7688
return lifecycleService.install(manifest);
7789
}
90+
91+
private boolean matchesRegistryEntry(
92+
Map<String, Object> manifest,
93+
StoreEntry registryEntry,
94+
String source
95+
) {
96+
if (registryEntry == null) {
97+
return false;
98+
}
99+
if (!source.equalsIgnoreCase(registryEntry.getSource())) {
100+
return false;
101+
}
102+
103+
String manifestVersion = manifest.getOrDefault("version", "").toString();
104+
if (
105+
!manifestVersion.isBlank() &&
106+
registryEntry.getVersion() != null &&
107+
!manifestVersion.equals(registryEntry.getVersion())
108+
) {
109+
return false;
110+
}
111+
112+
Object author = manifest.get("author");
113+
if (
114+
author != null &&
115+
registryEntry.getAuthor() != null &&
116+
!author.toString().equalsIgnoreCase(registryEntry.getAuthor())
117+
) {
118+
return false;
119+
}
120+
121+
return true;
122+
}
78123
}

‎packages/core/src/main/java/dev/synapse/plugins/StoreRegistryService.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,11 @@ public List<StoreEntry> findByType(
114114
}
115115

116116
@Transactional(readOnly = true)
117-
@Cacheable(value = "plugin-metadata", key = "'id:' + #id")
117+
@Cacheable(
118+
value = "plugin-metadata",
119+
key = "'id:' + #id",
120+
unless = "#result == null"
121+
)
118122
public StoreEntry findById(String id) {
119123
return storeEntryRepository.findById(id).orElse(null);
120124
}

‎packages/core/src/main/java/dev/synapse/plugins/loader/BytecodeScanner.java‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,22 @@ public ScanResult scan(Path jarPath) throws IOException {
7676
}
7777

7878
try (InputStream is = jarFile.getInputStream(entry)) {
79-
ClassReader reader = new ClassReader(is);
80-
ClassVisitor visitor = new ForbiddenReferenceVisitor(
81-
violations,
82-
entry.getName()
83-
);
84-
reader.accept(visitor, ClassReader.SKIP_DEBUG);
79+
try {
80+
ClassReader reader = new ClassReader(is);
81+
ClassVisitor visitor = new ForbiddenReferenceVisitor(
82+
violations,
83+
entry.getName()
84+
);
85+
reader.accept(visitor, ClassReader.SKIP_DEBUG);
86+
} catch (IllegalArgumentException e) {
87+
violations.add(
88+
new Violation(
89+
entry.getName(),
90+
"Unreadable class file: " + e.getMessage(),
91+
ViolationType.CLASS_REFERENCE
92+
)
93+
);
94+
}
8595
}
8696
}
8797
}

‎packages/core/src/main/java/dev/synapse/plugins/loader/PluginSandboxController.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,16 @@ public class PluginSandboxController {
2323

2424
private final PluginSandboxService sandboxService;
2525
private final PluginLifecycleService lifecycleService;
26+
private final PluginStorageService storageService;
2627

2728
public PluginSandboxController(
2829
PluginSandboxService sandboxService,
29-
PluginLifecycleService lifecycleService
30+
PluginLifecycleService lifecycleService,
31+
PluginStorageService storageService
3032
) {
3133
this.sandboxService = sandboxService;
3234
this.lifecycleService = lifecycleService;
35+
this.storageService = storageService;
3336
}
3437

3538
/**
@@ -42,7 +45,14 @@ public Map<String, Object> scanJar(@RequestBody Map<String, String> body) {
4245
throw new IllegalArgumentException("jarPath is required");
4346
}
4447

45-
BytecodeScanner.ScanResult result = sandboxService.scanJar(Path.of(jarPath));
48+
Path managedJar;
49+
try {
50+
managedJar = storageService.resolveManagedJar(Path.of(jarPath));
51+
} catch (Exception e) {
52+
throw new IllegalArgumentException(e.getMessage());
53+
}
54+
55+
BytecodeScanner.ScanResult result = sandboxService.scanJar(managedJar);
4656
return Map.of(
4757
"clean", result.clean(),
4858
"violations", result.violations().stream().map(v -> Map.of(

‎packages/core/src/main/java/dev/synapse/plugins/loader/PluginStorageService.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,36 @@ public boolean hasOrphanedStagingJars() {
213213
return !listStagingJars().isEmpty();
214214
}
215215

216+
/**
217+
* Resolves a managed plugin JAR path inside system/ or staging/.
218+
* Rejects arbitrary filesystem paths outside plugin storage.
219+
*/
220+
public Path resolveManagedJar(Path requestedPath) throws IOException {
221+
if (requestedPath == null) {
222+
throw new IllegalArgumentException("jarPath is required");
223+
}
224+
225+
Path realRequested = requestedPath.toRealPath();
226+
Path realSystemDir = systemDir.toRealPath();
227+
Path realStagingDir = stagingDir.toRealPath();
228+
229+
boolean inSystem = realRequested.startsWith(realSystemDir);
230+
boolean inStaging = realRequested.startsWith(realStagingDir);
231+
if (!inSystem && !inStaging) {
232+
throw new IllegalArgumentException(
233+
"jarPath must point to a managed plugin JAR in system/ or staging/"
234+
);
235+
}
236+
if (!Files.isRegularFile(realRequested)) {
237+
throw new IllegalArgumentException("jarPath must point to a plugin JAR file");
238+
}
239+
if (!realRequested.getFileName().toString().endsWith(".jar")) {
240+
throw new IllegalArgumentException("jarPath must point to a .jar file");
241+
}
242+
243+
return realRequested;
244+
}
245+
216246
private List<Path> listJars(Path dir) {
217247
if (!Files.isDirectory(dir)) {
218248
return List.of();

‎packages/core/src/test/java/dev/synapse/core/plugins/PluginSafetyServiceTest.java‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package dev.synapse.core.plugins;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5+
import static org.mockito.Mockito.mock;
6+
import static org.mockito.Mockito.when;
7+
8+
import dev.synapse.core.common.domain.StoreEntry;
9+
import dev.synapse.core.common.repository.StoreEntryRepository;
10+
import dev.synapse.core.infrastructure.logging.SystemLogService;
11+
import dev.synapse.plugins.PluginLifecycleService;
12+
import dev.synapse.plugins.PluginSafetyPolicy;
13+
import dev.synapse.plugins.PluginSafetyService;
14+
import dev.synapse.plugins.PluginTrustLevel;
15+
import java.util.Map;
16+
import java.util.Optional;
17+
import org.junit.jupiter.api.Test;
18+
19+
class PluginSafetyServiceTest {
20+
21+
private final PluginLifecycleService lifecycleService = mock(
22+
PluginLifecycleService.class
23+
);
24+
private final StoreEntryRepository storeEntryRepository = mock(
25+
StoreEntryRepository.class
26+
);
27+
private final SystemLogService logService = mock(SystemLogService.class);
28+
29+
private final PluginSafetyService safetyService = new PluginSafetyService(
30+
lifecycleService,
31+
storeEntryRepository,
32+
logService
33+
);
34+
35+
@Test
36+
void verifiedSourceShouldRequireMatchingStoreEntry() {
37+
StoreEntry entry = new StoreEntry();
38+
entry.setId("anthropic");
39+
entry.setSource("official");
40+
entry.setVersion("1.0.0");
41+
entry.setAuthor("synapse-core");
42+
when(storeEntryRepository.findById("anthropic"))
43+
.thenReturn(Optional.of(entry));
44+
45+
PluginSafetyPolicy policy = safetyService.assess(
46+
Map.of(
47+
"id", "anthropic",
48+
"source", "official",
49+
"version", "1.0.0",
50+
"author", "synapse-core"
51+
)
52+
);
53+
54+
assertThat(policy.trustLevel()).isEqualTo(PluginTrustLevel.VERIFIED);
55+
assertThat(policy.requiresConfirmation()).isFalse();
56+
}
57+
58+
@Test
59+
void spoofedVerifiedSourceShouldBeDowngraded() {
60+
when(storeEntryRepository.findById("evil-plugin"))
61+
.thenReturn(Optional.empty());
62+
63+
PluginSafetyPolicy policy = safetyService.assess(
64+
Map.of(
65+
"id", "evil-plugin",
66+
"source", "official",
67+
"version", "1.0.0",
68+
"author", "attacker"
69+
)
70+
);
71+
72+
assertThat(policy.trustLevel()).isEqualTo(PluginTrustLevel.UNVERIFIED);
73+
assertThat(policy.requiresConfirmation()).isTrue();
74+
assertThat(policy.warnings().getFirst()).contains("claims verified source");
75+
}
76+
77+
@Test
78+
void spoofedVerifiedInstallShouldRequireConfirmation() {
79+
when(storeEntryRepository.findById("evil-plugin"))
80+
.thenReturn(Optional.empty());
81+
82+
assertThatThrownBy(() ->
83+
safetyService.safeInstall(
84+
Map.of(
85+
"id", "evil-plugin",
86+
"source", "official",
87+
"version", "1.0.0",
88+
"author", "attacker",
89+
"type", "channel",
90+
"description", "spoofed"
91+
),
92+
false
93+
)
94+
)
95+
.isInstanceOf(IllegalStateException.class)
96+
.hasMessageContaining("Confirmation required")
97+
.hasMessageContaining("verified source");
98+
}
99+
}

0 commit comments

Comments
 (0)