Potential fix for code scanning alert no. 10: Server-side request forgery #1

2026-05-12T22:22:42+02:00

FTMahringer commented

2026-05-12 22:22:42 +02:00

(Migrated from github.com)

Potential fix for https://github.com/FTMahringer/Synapse/security/code-scanning/10

Use strict canonical path validation before converting to URL/classloading:

In PluginLoaderService.loadPlugin(...), resolve both the plugins root and input JAR path with toRealPath(...) (after existence checks), then enforce containment using canonical paths.
Reject non-regular files.
Build the URL from the validated canonical path using toUri().toURL() (safe after canonical allowlist check), removing manual string URL construction.
This preserves functionality (still loads local plugin JARs) while preventing traversal/symlink escape and satisfying SSRF dataflow concerns.

Target file/region:

packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderService.java
- Replace the existing normalize()/startsWith + manual file:// URL assembly block in loadPlugin(...) (around lines 86–134 in provided snippet).

No new dependencies are required.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Potential fix for [https://github.com/FTMahringer/Synapse/security/code-scanning/10](https://github.com/FTMahringer/Synapse/security/code-scanning/10) Use **strict canonical path validation** before converting to URL/classloading: - In `PluginLoaderService.loadPlugin(...)`, resolve both the plugins root and input JAR path with `toRealPath(...)` (after existence checks), then enforce containment using canonical paths. - Reject non-regular files. - Build the URL from the validated canonical path using `toUri().toURL()` (safe after canonical allowlist check), removing manual string URL construction. - This preserves functionality (still loads local plugin JARs) while preventing traversal/symlink escape and satisfying SSRF dataflow concerns. Target file/region: - `packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderService.java` - Replace the existing `normalize()/startsWith` + manual `file://` URL assembly block in `loadPlugin(...)` (around lines 86–134 in provided snippet). No new dependencies are required. _Suggested fixes powered by Copilot Autofix. Review carefully before merging._

github-advanced-security[bot] (Migrated from github.com) reviewed

2026-05-12 22:24:14 +02:00

packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderService.java

					
				@ -123,0 +121,4 @@

				                    e

				                );

				            }

CodeQL / Uncontrolled data used in path expression

This path depends on a user-provided value.

Show more details

## CodeQL / Uncontrolled data used in path expression This path depends on a [user-provided value](1). [Show more details](https://github.com/FTMahringer/Synapse/security/code-scanning/12)

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2026-05-12 22:34:00 +02:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull request overview

This PR aims to address CodeQL alert #10 (server-side request forgery / path confusion) by tightening plugin JAR path validation before converting paths to URLs and classloading, primarily in PluginLoaderService.loadPlugin(...).

Changes:

Canonicalize and validate plugin JAR paths using toRealPath() + containment checks, and build the JAR URL via toUri().toURL() only after validation.
Add stricter input handling for the plugin update endpoint by attempting to restrict jarPath to a “safe filename”.
Adjust update flow to use the extracted filename for staging/promoting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/core/src/main/java/dev/synapse/plugins/loader/PluginUpdateService.java	Adds filename-only validation and switches staging/promote inputs to the filename.
packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderService.java	Replaces normalize/startsWith + manual `file://` building with `toRealPath()` containment and safe URL creation.
packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderController.java	Adds request validation and passes only the filename into the update service.

Comments suppressed due to low confidence (1)

packages/core/src/main/java/dev/synapse/plugins/loader/PluginUpdateService.java:117

stageJar(...) copies from the provided sourceJar path. Passing jarFileName here means the source becomes a relative path like myplugin.jar, which will typically not exist (and will break updates) unless the process working directory happens to contain that file. Pass the actual source path being updated from, or (if the API is meant to accept only a filename) resolve the source against a known directory (e.g., staging/incoming) and validate it exists as a regular file before copying.

        // 3. Stage new JAR
        try {
            storageService.stageJar(jarFileName);
        } catch (Exception e) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview This PR aims to address CodeQL alert #10 (server-side request forgery / path confusion) by tightening plugin JAR path validation before converting paths to URLs and classloading, primarily in `PluginLoaderService.loadPlugin(...)`. **Changes:** - Canonicalize and validate plugin JAR paths using `toRealPath()` + containment checks, and build the JAR URL via `toUri().toURL()` only after validation. - Add stricter input handling for the plugin update endpoint by attempting to restrict `jarPath` to a “safe filename”. - Adjust update flow to use the extracted filename for staging/promoting. ### Reviewed changes Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments. | File | Description | | ---- | ----------- | | packages/core/src/main/java/dev/synapse/plugins/loader/PluginUpdateService.java | Adds filename-only validation and switches staging/promote inputs to the filename. | | packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderService.java | Replaces normalize/startsWith + manual `file://` building with `toRealPath()` containment and safe URL creation. | | packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderController.java | Adds request validation and passes only the filename into the update service. | <details> <summary>Comments suppressed due to low confidence (1)</summary> **packages/core/src/main/java/dev/synapse/plugins/loader/PluginUpdateService.java:117** * `stageJar(...)` copies from the provided `sourceJar` path. Passing `jarFileName` here means the source becomes a relative path like `myplugin.jar`, which will typically not exist (and will break updates) unless the process working directory happens to contain that file. Pass the actual source path being updated from, or (if the API is meant to accept only a filename) resolve the source against a known directory (e.g., staging/incoming) and validate it exists as a regular file before copying. ``` // 3. Stage new JAR try { storageService.stageJar(jarFileName); } catch (Exception e) { ``` </details> --- 💡 <a href="/FTMahringer/Synapse/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

packages/core/src/main/java/dev/synapse/plugins/loader/PluginLoaderController.java

					
				@ -183,1 +194,4 @@

				            id,

				            Path.of(trimmed).getFileName()

				        );

				        Plugin dbPlugin = lifecycleService.findById(id);

The endpoint now reduces the request to Path.of(trimmed).getFileName() (i.e., only a filename). But the update flow still stages/copies the JAR from the provided path, so sending only a filename will usually fail to locate the source file. Either accept a full path (with canonical allowlist validation) or make the endpoint semantics explicitly “jar name already present in staging/” and adjust the service accordingly.

The endpoint now reduces the request to `Path.of(trimmed).getFileName()` (i.e., only a filename). But the update flow still stages/copies the JAR from the provided path, so sending only a filename will usually fail to locate the source file. Either accept a full path (with canonical allowlist validation) or make the endpoint semantics explicitly “jar name already present in staging/” and adjust the service accordingly.

packages/core/src/main/java/dev/synapse/plugins/loader/PluginUpdateService.java

					
				@ -64,0 +67,4 @@

				                pluginId,

				                "Invalid JAR path: only a file name is allowed"

				            );

				        }

This new validation requires newJarPath to be only a single filename segment. That changes the updatePlugin contract (it previously accepted a path to the new JAR) and doesn’t match the rest of the method, which stages/copies from a source path. Consider keeping newJarPath as a path but applying canonical containment/regular-file validation (similar to PluginLoaderService), or update the staging logic to treat the input as a jar name already present in a specific directory.

This issue also appears on line 114 of the same file.

This new validation requires `newJarPath` to be only a single filename segment. That changes the `updatePlugin` contract (it previously accepted a path to the new JAR) and doesn’t match the rest of the method, which stages/copies from a source path. Consider keeping `newJarPath` as a path but applying canonical containment/regular-file validation (similar to `PluginLoaderService`), or update the staging logic to treat the input as a jar name already present in a specific directory. This issue also appears on line 114 of the same file.

Sign in to join this conversation.

No reviewers