Skip to content

fix(security): add path traversal validation to widget save path (GHSA-r553-q33m-v7pf) #41834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
subrata71 wants to merge 2 commits into
base: release
Choose a base branch
from
+134 −0
Open

fix(security): add path traversal validation to widget save path (GHSA-r553-q33m-v7pf) #41834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
602dc22
test: add failing test for widget name path traversal (GHSA-r553-q33m…
subrata71 May 21, 2026
eff0ba7
fix(security): add path traversal validation to widget save path (GHS…
subrata71 May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ protected Set<String> updateEntitiesInRepo(ApplicationGitReference applicationGi

Path path = Paths.get(
String.valueOf(pageSpecificDirectory.resolve(CommonConstants.WIDGETS)), childPath);
validatePathIsWithinGitRoot(path);
validatePathIsWithinGitRoot(path.resolve(widgetName + CommonConstants.JSON_EXTENSION));
validWidgetToParentMap.put(widgetName, path.toFile().toString());
fileOperations.saveWidgets(jsonObject, widgetName, path);
});
Expand Down
132 changes: 132 additions & 0 deletions app/server/appsmith-git/src/test/java/com/appsmith/git/helpers/FileUtilsImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.git.helpers;

import com.appsmith.external.dtos.ModifiedResources;
import com.appsmith.external.git.handler.FSGitHandler;
import com.appsmith.external.git.operations.FileOperations;
import com.appsmith.external.helpers.ObservationHelper;
Expand All @@ -11,6 +12,8 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.io.FileUtils;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.json.JSONArray;
import org.json.JSONObject;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -22,6 +25,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -201,6 +205,134 @@ public void reconstructMetadata_validPathWithinGitRoot_doesNotThrowSecurityExcep
Assertions.assertNotNull(result);
}

/**
* GHSA-r553-q33m-v7pf: Path traversal via malicious widgetName during Git serialization.
* A widget with a name containing path traversal sequences (e.g., "../../../../tmp/evil")
* must be rejected by validatePathIsWithinGitRoot before any file write occurs.
*/
@Test
public void saveApplicationRef_pathTraversalInWidgetName_throwsSecurityException_GHSA_r553()
throws GitAPIException, IOException {

Mockito.when(gitExecutor.resetToLastCommit(Mockito.any(Path.class), Mockito.any(), Mockito.anyBoolean()))
.thenReturn(Mono.just(true));

Files.createDirectories(localTestDirectoryPath);

ModifiedResources modifiedResources = new ModifiedResources();
modifiedResources.putResource("pageList", "TestPage");

String maliciousWidgetName = "../../../../tmp/ghsa-r553-traversal-test";

JSONObject maliciousWidget = new JSONObject();
maliciousWidget.put("widgetName", maliciousWidgetName);
maliciousWidget.put("type", "BUTTON_WIDGET");
maliciousWidget.put("widgetId", "exploit123");

JSONObject canvasWidget = new JSONObject();
canvasWidget.put("widgetName", "Canvas1");
canvasWidget.put("type", "CANVAS_WIDGET");
canvasWidget.put("children", new JSONArray().put(maliciousWidget));

JSONObject mainContainer = new JSONObject();
mainContainer.put("widgetName", "MainContainer");
mainContainer.put("type", "CANVAS_WIDGET");
mainContainer.put("widgetId", "0");
mainContainer.put("children", new JSONArray().put(canvasWidget));

ApplicationGitReference appRef = new ApplicationGitReference();
appRef.setApplication(new Object());
appRef.setTheme(new Object());
appRef.setMetadata(new Object());
appRef.setModifiedResources(modifiedResources);

Map<String, Object> pages = new HashMap<>();
pages.put("TestPage", new Object());
appRef.setPages(pages);

Map<String, String> pageDsl = new HashMap<>();
pageDsl.put("TestPage", mainContainer.toString());
appRef.setPageDsl(pageDsl);

appRef.setActions(new HashMap<>());
appRef.setActionCollections(new HashMap<>());
appRef.setDatasources(new HashMap<>());
appRef.setJsLibraries(new HashMap<>());

Assertions.assertThrows(RuntimeException.class, () -> {
fileUtils
.saveApplicationToGitRepo(Path.of(""), appRef, "branch", false)
.block();
});
Comment on lines +262 to +266

Assertions.assertFalse(
Files.exists(Path.of("/tmp/ghsa-r553-traversal-test.json")),
"Path traversal: file was created outside git root");
Comment on lines +268 to +270
}
Comment on lines +262 to +271
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Pre-clean the traversal artifact and tighten the expected exception type.

Two small hygiene gaps in this regression test:

  1. Files.exists(Path.of("/tmp/ghsa-r553-traversal-test.json")) is not guarded against pre-existing state. If a prior run (with a regressed fix, or any unrelated process) left that file on disk, this assertion fails spuriously even when the current code is correct. Delete it before the action so the test is self-contained.
  2. assertThrows(RuntimeException.class, ...) is broader than necessary — the fix throws AppsmithPluginException. Asserting the specific type prevents an unrelated NullPointerException / RuntimeException from masquerading as a passing security test.
🛡️ Proposed tightening 
+        Path traversalArtifact = Path.of("/tmp/ghsa-r553-traversal-test.json");
+        Files.deleteIfExists(traversalArtifact);
+
-        Assertions.assertThrows(RuntimeException.class, () -> {
+        Assertions.assertThrows(AppsmithPluginException.class, () -> {
             fileUtils
                     .saveApplicationToGitRepo(Path.of(""), appRef, "branch", false)
                     .block();
         });

         Assertions.assertFalse(
-                Files.exists(Path.of("/tmp/ghsa-r553-traversal-test.json")),
+                Files.exists(traversalArtifact),
                 "Path traversal: file was created outside git root");
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/server/appsmith-git/src/test/java/com/appsmith/git/helpers/FileUtilsImplTest.java`
around lines 262 - 271, Ensure the test in FileUtilsImplTest cleans up the
traversal artifact and asserts the precise exception: before calling
fileUtils.saveApplicationToGitRepo(Path.of(""), appRef, "branch",
false).block(), delete the file at "/tmp/ghsa-r553-traversal-test.json" if it
exists so the assertion is not affected by prior state; replace
Assertions.assertThrows(RuntimeException.class, ...) with
Assertions.assertThrows(AppsmithPluginException.class, ...) so the test expects
the specific exception thrown by saveApplicationToGitRepo rather than any
RuntimeException.


/**
* GHSA-r553-q33m-v7pf: Verify that legitimate widget names still serialize correctly.
*/
@Test
public void saveApplicationRef_legitimateWidgetName_doesNotThrow_GHSA_r553() throws GitAPIException, IOException {

Mockito.when(gitExecutor.resetToLastCommit(Mockito.any(Path.class), Mockito.any(), Mockito.anyBoolean()))
.thenReturn(Mono.just(true));

Files.createDirectories(localTestDirectoryPath);

ModifiedResources modifiedResources = new ModifiedResources();
modifiedResources.putResource("pageList", "TestPage");

JSONObject safeWidget = new JSONObject();
safeWidget.put("widgetName", "Button1");
safeWidget.put("type", "BUTTON_WIDGET");
safeWidget.put("widgetId", "safe123");

JSONObject canvasWidget = new JSONObject();
canvasWidget.put("widgetName", "Canvas1");
canvasWidget.put("type", "CANVAS_WIDGET");
canvasWidget.put("children", new JSONArray().put(safeWidget));

JSONObject mainContainer = new JSONObject();
mainContainer.put("widgetName", "MainContainer");
mainContainer.put("type", "CANVAS_WIDGET");
mainContainer.put("widgetId", "0");
mainContainer.put("children", new JSONArray().put(canvasWidget));

ApplicationGitReference appRef = new ApplicationGitReference();
appRef.setApplication(new Object());
appRef.setTheme(new Object());
appRef.setMetadata(new Object());
appRef.setModifiedResources(modifiedResources);

Map<String, Object> pages = new HashMap<>();
pages.put("TestPage", new Object());
appRef.setPages(pages);

Map<String, String> pageDsl = new HashMap<>();
pageDsl.put("TestPage", mainContainer.toString());
appRef.setPageDsl(pageDsl);

appRef.setActions(new HashMap<>());
appRef.setActionCollections(new HashMap<>());
appRef.setDatasources(new HashMap<>());
appRef.setJsLibraries(new HashMap<>());

Assertions.assertDoesNotThrow(() -> {
fileUtils
.saveApplicationToGitRepo(Path.of(""), appRef, "branch", false)
.block();
});

Path widgetFile = localTestDirectoryPath
.resolve(PAGE_DIRECTORY)
.resolve("TestPage")
.resolve("widgets")
.resolve("Button1.json");
Assertions.assertTrue(Files.exists(widgetFile), "Legitimate widget file should exist");
}

/**
* This will delete localTestDirectory and its contents after the test is executed.
*/
Expand Down
Loading