Skip to content

Commit a92dee4

Browse files
authored
fix(security): Appsmith App Viewer datasource configuration leak via import helper (GHSA-93mf-9h52-gfxp) (#41764)
## Summary fix(security): Appsmith App Viewer datasource configuration leak via import helper (GHSA-93mf-9h52-gfxp) - **Primary fix:** `DatasourceImportableServiceCEImpl.getEntitiesPresentInWorkspace()` was calling `getAllByWorkspaceIdWithStorages(workspaceId, null)` — passing `null` as the ACL permission, which bypasses authorization entirely. Changed to enforce `datasourcePermission.getReadPermission()` (`READ_DATASOURCES`), consistent with the standard datasource listing endpoint. - **Constructor update:** Injected `DatasourcePermission` into both `DatasourceImportableServiceCEImpl` and `DatasourceImportableServiceImpl` constructors (Spring auto-wires the bean). - **Test coverage:** Added a regression test (`should_enforceReadPermission_when_getEntitiesPresentInWorkspace_isCalled`) annotated with GHSA ID, verifying that `READ_DATASOURCES` permission is always passed and `null` is never used. ## Vulnerability | Field | Value | |-------|-------| | **GHSA** | [GHSA-93mf-9h52-gfxp](https://github.com/appsmithorg/appsmith/security/advisories/GHSA-93mf-9h52-gfxp) | | **CVSS** | 7.7 (high) | | **CWE** | CWE-200, CWE-862 | | **Affected component** | `DatasourceImportableServiceCEImpl.getEntitiesPresentInWorkspace()` | ## Exposure Analysis - **Who can exploit this:** Any authenticated user with `App Viewer` access to a workspace. This is the lowest privilege role — no admin, developer, or owner access required. - **What an attacker can achieve:** Full read access to datasource configuration for all datasources in the workspace, including internal URLs, custom headers (e.g., `Authorization: Bearer <token>`), custom properties (e.g., API keys), and connection metadata. This is a confidentiality breach of backend infrastructure credentials. - **Evidence of exploitation:** No evidence of exploitation in the wild. Discovered via security advisory report. - **Blast radius:** All datasources in the target workspace are exposed. The attack is per-workspace — an App Viewer in workspace A cannot access workspace B's datasources. However, within a workspace, ALL datasource secrets are leaked regardless of which application they belong to. ## Fix - **Root cause:** `DatasourceImportableServiceCEImpl.getEntitiesPresentInWorkspace()` (line 602) called `datasourceService.getAllByWorkspaceIdWithStorages(workspaceId, null)`. When `AclPermission` is `null`, the MongoDB repository layer skips ACL filtering and returns all matching documents, bypassing the policy-based permission model. - **Fix strategy:** Inject `DatasourcePermission` into the service and replace `null` with `datasourcePermission.getReadPermission()` (`AclPermission.READ_DATASOURCES`). This is the same permission used by the standard `GET /api/v1/datasources?workspaceId=` endpoint — consistent, minimal, and at the correct layer. - **Intentionally NOT changed:** (1) `importEntities()` in the same class (line 117) also uses `null` permission — this is intentional because the import pipeline has its own authorization layer (edit permission on existing datasources, create permission on workspace). (2) `DatasourceForkableServiceCEImpl.getExistingEntitiesInTarget()` also uses `null` — fork operations require higher-level permissions upstream. - **Defense-in-depth:** The fix is at the data-access layer, which is the correct boundary. All callers of `getEntitiesPresentInWorkspace()` automatically inherit the permission enforcement. ## Test plan - [x] Failing test for the exploit scenario passes after fix - [x] All existing tests in affected modules pass (no regressions) — 9/9 tests green - [x] Codebase audit: grepped for `getAllByWorkspaceIdWithStorages.*null` — remaining instances are in authorized internal pipelines - [x] Compilation clean (`mvn compile -DskipTests`) - [ ] Manual reproduction of advisory PoC confirms the fix ## CE/EE sync CE-only safe: the modified files (`DatasourceImportableServiceCEImpl.java`, `DatasourceImportableServiceImpl.java`) are CE files that EE inherits. The hourly CE→EE sync will propagate the fix. The EE repo already has identical changes as uncommitted modifications, confirming alignment. ## Disclosure > **Do not merge until advisory is ready for disclosure coordination.** > > After merge: > 1. Confirm fix is in release branch > 2. Coordinate with security team on disclosure timeline > 3. Update advisory with patched version and publish > 4. Notify reporter ## Follow-ups - No additional vulnerable locations found during codebase audit — no follow-up PRs needed. - Linear ticket: [APP-15189](https://linear.app/appsmith/issue/APP-15189) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Datasource listings now respect computed read permissions so users only see workspace datasources they are allowed to access. * **Tests** * Added tests validating read-permission enforcement for datasource visibility to prevent unauthorized exposure. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Automation /ok-to-test tags="@tag.All" <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/24981307163> > Commit: 54b3c7f > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=24981307163&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Mon, 27 Apr 2026 08:09:14 UTC <!-- end of auto-generated comment: Cypress test results -->
1 parent 99d6918 commit a92dee4

3 files changed

Lines changed: 47 additions & 6 deletions

File tree

app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/importable/DatasourceImportableServiceCEImpl.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.appsmith.server.imports.importable.ImportableServiceCE;
2424
import com.appsmith.server.imports.importable.artifactbased.ArtifactBasedImportableService;
2525
import com.appsmith.server.services.WorkspaceService;
26+
import com.appsmith.server.solutions.DatasourcePermission;
2627
import lombok.extern.slf4j.Slf4j;
2728
import org.apache.commons.collections.CollectionUtils;
2829
import org.apache.commons.lang3.StringUtils;
@@ -48,14 +49,17 @@ public class DatasourceImportableServiceCEImpl implements ImportableServiceCE<Da
4849
private final DatasourceService datasourceService;
4950
private final WorkspaceService workspaceService;
5051
private final DatasourceStorageService datasourceStorageService;
52+
private final DatasourcePermission datasourcePermission;
5153

5254
public DatasourceImportableServiceCEImpl(
5355
DatasourceService datasourceService,
5456
WorkspaceService workspaceService,
55-
DatasourceStorageService datasourceStorageService) {
57+
DatasourceStorageService datasourceStorageService,
58+
DatasourcePermission datasourcePermission) {
5659
this.datasourceService = datasourceService;
5760
this.datasourceStorageService = datasourceStorageService;
5861
this.workspaceService = workspaceService;
62+
this.datasourcePermission = datasourcePermission;
5963
}
6064

6165
@Override
@@ -599,6 +603,6 @@ private void updateAuthenticationDTO(
599603

600604
@Override
601605
public Flux<Datasource> getEntitiesPresentInWorkspace(String workspaceId) {
602-
return datasourceService.getAllByWorkspaceIdWithStorages(workspaceId, null);
606+
return datasourceService.getAllByWorkspaceIdWithStorages(workspaceId, datasourcePermission.getReadPermission());
603607
}
604608
}

app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/importable/DatasourceImportableServiceImpl.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.appsmith.server.datasourcestorages.base.DatasourceStorageService;
66
import com.appsmith.server.imports.importable.ImportableService;
77
import com.appsmith.server.services.WorkspaceService;
8+
import com.appsmith.server.solutions.DatasourcePermission;
89
import org.springframework.stereotype.Service;
910

1011
@Service
@@ -14,7 +15,8 @@ public class DatasourceImportableServiceImpl extends DatasourceImportableService
1415
public DatasourceImportableServiceImpl(
1516
DatasourceService datasourceService,
1617
WorkspaceService workspaceService,
17-
DatasourceStorageService datasourceStorageService) {
18-
super(datasourceService, workspaceService, datasourceStorageService);
18+
DatasourceStorageService datasourceStorageService,
19+
DatasourcePermission datasourcePermission) {
20+
super(datasourceService, workspaceService, datasourceStorageService, datasourcePermission);
1921
}
2022
}

app/server/appsmith-server/src/test/java/com/appsmith/server/datasources/importable/DatasourceImportableServiceCEImplTest.java

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,24 @@
44
import com.appsmith.external.models.BasicAuth;
55
import com.appsmith.external.models.BearerTokenAuth;
66
import com.appsmith.external.models.DBAuth;
7+
import com.appsmith.external.models.Datasource;
78
import com.appsmith.external.models.DatasourceConfiguration;
89
import com.appsmith.external.models.DatasourceStorage;
910
import com.appsmith.external.models.DecryptedSensitiveFields;
1011
import com.appsmith.external.models.OAuth2;
12+
import com.appsmith.server.acl.AclPermission;
1113
import com.appsmith.server.datasources.base.DatasourceService;
1214
import com.appsmith.server.datasourcestorages.base.DatasourceStorageService;
1315
import com.appsmith.server.services.WorkspaceService;
16+
import com.appsmith.server.solutions.DatasourcePermission;
1417
import org.junit.jupiter.api.BeforeEach;
18+
import org.junit.jupiter.api.DisplayName;
1519
import org.junit.jupiter.api.Test;
1620
import org.junit.jupiter.api.extension.ExtendWith;
1721
import org.mockito.Mock;
1822
import org.mockito.junit.jupiter.MockitoExtension;
23+
import reactor.core.publisher.Flux;
24+
import reactor.test.StepVerifier;
1925

2026
import java.lang.reflect.Method;
2127

@@ -24,6 +30,11 @@
2430
import static org.junit.jupiter.api.Assertions.assertNull;
2531
import static org.junit.jupiter.api.Assertions.assertTrue;
2632
import static org.junit.jupiter.api.Assertions.fail;
33+
import static org.mockito.ArgumentMatchers.eq;
34+
import static org.mockito.ArgumentMatchers.isNull;
35+
import static org.mockito.Mockito.never;
36+
import static org.mockito.Mockito.verify;
37+
import static org.mockito.Mockito.when;
2738

2839
@ExtendWith(MockitoExtension.class)
2940
class DatasourceImportableServiceCEImplTest { // Renamed class to match convention
@@ -37,12 +48,15 @@ class DatasourceImportableServiceCEImplTest { // Renamed class to match conventi
3748
@Mock
3849
DatasourceStorageService datasourceStorageService;
3950

51+
@Mock
52+
DatasourcePermission datasourcePermission;
53+
4054
DatasourceImportableServiceCEImpl importService;
4155

4256
@BeforeEach
4357
void setUp() {
44-
importService =
45-
new DatasourceImportableServiceCEImpl(datasourceService, workspaceService, datasourceStorageService);
58+
importService = new DatasourceImportableServiceCEImpl(
59+
datasourceService, workspaceService, datasourceStorageService, datasourcePermission);
4660
}
4761

4862
// Helper to call the private method using reflection
@@ -228,4 +242,25 @@ void updateAuthenticationDTO_WhenAuthTypeIsNull_ShouldDoNothingAndNotSetAuth() {
228242

229243
assertNull(dsConfig.getAuthentication(), "Authentication should not be set if authType is null");
230244
}
245+
246+
@Test
247+
@DisplayName("GHSA-93mf-9h52-gfxp: getEntitiesPresentInWorkspace must enforce READ_DATASOURCES permission")
248+
void should_enforceReadPermission_when_getEntitiesPresentInWorkspace_isCalled() {
249+
// Given
250+
String workspaceId = "workspace1";
251+
Datasource ds = new Datasource();
252+
ds.setId("ds1");
253+
ds.setName("TestDS");
254+
when(datasourcePermission.getReadPermission()).thenReturn(AclPermission.READ_DATASOURCES);
255+
when(datasourceService.getAllByWorkspaceIdWithStorages(workspaceId, AclPermission.READ_DATASOURCES))
256+
.thenReturn(Flux.just(ds));
257+
258+
// When
259+
Flux<Datasource> result = importService.getEntitiesPresentInWorkspace(workspaceId);
260+
261+
// Then
262+
StepVerifier.create(result).expectNextCount(1).verifyComplete();
263+
verify(datasourceService).getAllByWorkspaceIdWithStorages(workspaceId, AclPermission.READ_DATASOURCES);
264+
verify(datasourceService, never()).getAllByWorkspaceIdWithStorages(eq(workspaceId), isNull());
265+
}
231266
}

0 commit comments

Comments
 (0)