feat: add publisher agreement compliance check #954

xai · 2024-06-26T17:33:02Z

This commit enforces that only extension versions compliant with the publisher agreement can be published.

Already existing extensions versions are checked as part of a migration job.

Contributed on behalf of STMicroelectronics

...in/java/org/eclipse/openvsx/migration/CheckPotentiallyMaliciousExtensionVersionsService.java

server/src/main/java/org/eclipse/openvsx/migration/PotentiallyMaliciousJobRequestHandler.java

amvanbaren · 2024-06-27T07:38:35Z

server/src/main/java/org/eclipse/openvsx/migration/PotentiallyMaliciousJobRequestHandler.java

+    }
+
+    @Override
+    @Job(name = "Check published extensions for maliciously compressed vsix files", retries = 3)


... for potentially malicious ...

amvanbaren · 2024-06-27T07:40:47Z

server/src/main/java/org/eclipse/openvsx/migration/PotentiallyMaliciousJobRequestHandler.java

+    public void run(MigrationJobRequest jobRequest) throws Exception {
+        var download = migrations.getResource(jobRequest);
+        var extVersion = download.getExtension();
+        logger.info("Checking extension version for malicious content: {}", NamingUtil.toLogFormat(extVersion));


... for potentially malicious vsix file: ...

amvanbaren · 2024-06-27T07:46:42Z

server/src/main/java/org/eclipse/openvsx/migration/PotentiallyMaliciousJobRequestHandler.java

+                return;
+            }
+
+            logger.info("Checking vsix file for malicious metadata: {}", download.getName());


... for potentially malicious ...

amvanbaren · 2024-06-27T07:55:41Z

server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java

@@ -204,6 +204,11 @@ public void publishAsync(FileResource download, TempFile extensionFile, Extensio
        service.storeDownload(download, extensionFile);
        service.persistResource(download);
        try(var processor = new ExtensionProcessor(extensionFile, ObservationRegistry.NOOP)) {
+            extVersion.setPotentiallyMalicious(processor.isPotentiallyMalicious());
+            if (extVersion.isPotentiallyMalicious()) {
+                throw new ErrorResultException("The extension version is potentially malicious and cannot be published: " + NamingUtil.toLogFormat(extVersion));


This runs asynchronous in the background. The publisher won't receive the exception.
Log and return would be good enough:

logger.warn("Extension version is potentially malicious: {}", NamingUtil.toLogFormat(extVersion)); return;

amvanbaren · 2024-06-27T07:57:12Z

server/src/main/java/org/eclipse/openvsx/repositories/RepositoryService.java

@@ -495,6 +495,11 @@ public Streamable<MigrationItem> findNotMigratedSha256Checksums() {
        return findNotMigratedItems("V1_35__FileResource_Generate_Sha256_Checksum.sql");
    }

+    public Streamable<MigrationItem> findNotMigratedPotentiallyMalicious() {
+        return findNotMigratedItems("V1_46__ExtensionVersion_PotentiallyMalicious.sql");
+        // return migrationItemRepo.findByMigrationScriptAndMigrationScheduledFalseOrderById("V1_46__ExtensionVersion_PotentiallyMalicious.sql");


Remove line

amvanbaren · 2024-06-27T08:00:37Z

server/src/main/java/org/eclipse/openvsx/UserAPI.java

@@ -203,6 +203,7 @@ public List<ExtensionJson> getOwnExtensions() {
                    var json = latest.toExtensionJson();
                    json.preview = latest.isPreview();
                    json.active = latest.getExtension().isActive();
+                    json.potentiallyMalicious = latest.isPotentiallyMalicious();


This is going to be used in the webui?

Not planned yet, therefore I removed it again. We can add it back if there ever is a plan to show it in the webui.

amvanbaren · 2024-06-27T08:00:45Z

server/src/main/java/org/eclipse/openvsx/admin/AdminService.java

@@ -275,6 +275,7 @@ public UserPublishInfoJson getUserPublishInfo(String provider, String loginName)
                    var json = latest.toExtensionJson();
                    json.preview = latest.isPreview();
                    json.active = latest.getExtension().isActive();
+                    json.potentiallyMalicious = latest.isPotentiallyMalicious();


This is going to be used in the webui?

Not planned yet, therefore I removed it again. We can add it back if there ever is a plan to show it in the webui.

xai · 2024-07-02T09:05:18Z

I believe I addressed all comments. Should I change anything else, @amvanbaren ?

...in/java/org/eclipse/openvsx/migration/CheckPotentiallyMaliciousExtensionVersionsService.java

server/src/main/java/org/eclipse/openvsx/migration/PotentiallyMaliciousJobRequestHandler.java

amvanbaren · 2024-07-03T08:51:02Z

server/src/main/java/org/eclipse/openvsx/publish/PublishExtensionVersionHandler.java

@@ -194,7 +194,7 @@ private String checkBundledExtension(String bundledExtension) {

    @Async
    @Retryable
-    public void publishAsync(FileResource download, TempFile extensionFile, ExtensionService extensionService) {
+    public void publishAsync(FileResource download, TempFile extensionFile, ExtensionService extensionService) throws ErrorResultException {


It no longer throws an ErrorResultException

amvanbaren · 2024-07-03T09:17:27Z

server/src/main/java/org/eclipse/openvsx/json/ExtensionJson.java

@@ -72,6 +72,9 @@ public static ExtensionJson error(String message) {
    @Schema(hidden = true)
    public Boolean active;

+    @Schema(description = "The value 'true' means that the extension contains potentially malicious content.")


Is this used?

No, not yet => also removed for now. We can reintroduce this if needed.

This commit enforces that only extension versions compliant with the publisher agreement can be published. Already existing extensions versions are checked as part of a migration job. Contributed on behalf of STMicroelectronics Signed-off-by: Olaf Lessenich <olessenich@eclipsesource.com>

xai · 2024-07-03T10:10:22Z

Thanks for your help and the feedback @amvanbaren!
I amended my commit accordingly.

amvanbaren requested changes Jun 27, 2024

View reviewed changes

xai force-pushed the compliance-check branch from a7d0a0c to aabb9eb Compare June 27, 2024 09:23

xai requested a review from amvanbaren June 27, 2024 09:25

amvanbaren reviewed Jul 3, 2024

View reviewed changes

xai force-pushed the compliance-check branch from aabb9eb to 286a976 Compare July 3, 2024 10:08

xai requested a review from amvanbaren July 3, 2024 10:10

amvanbaren approved these changes Jul 4, 2024

View reviewed changes

amvanbaren merged commit 66f62a2 into eclipse:master Jul 4, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add publisher agreement compliance check #954

feat: add publisher agreement compliance check #954

xai commented Jun 26, 2024

amvanbaren Jun 27, 2024

amvanbaren Jun 27, 2024

amvanbaren Jun 27, 2024

amvanbaren Jun 27, 2024

amvanbaren Jun 27, 2024

amvanbaren Jun 27, 2024

xai Jun 27, 2024

amvanbaren Jun 27, 2024

xai Jun 27, 2024

xai commented Jul 2, 2024

amvanbaren Jul 3, 2024

amvanbaren Jul 3, 2024

xai Jul 3, 2024

xai commented Jul 3, 2024

feat: add publisher agreement compliance check #954

feat: add publisher agreement compliance check #954

Conversation

xai commented Jun 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xai commented Jul 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xai commented Jul 3, 2024