fix(connectors): follow-up hardening for callApi (#256) (#261)

- Percent-encode caller-supplied integration types and connector IDs in
  request URLs, so a runtime-built identifier can only select a
  connector, never re-target another route under the privileged token
- Type ConnectorApiResponse.data as T | null to match the documented
  and implemented contract (null for binary and proxy-error responses)
- Treat host: null like undefined when omitting the host field, since
  untyped callers write `host: x ?? null`
- Add nock.disableNetConnect() to the proxy tests so interceptor
  mismatches fail fast instead of escaping as real network requests
- Run test:types in CI so the type-contract tests are enforced

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: ChenMachBase

.github/workflows/unit-tests.yml

@@ -26,5 +26,8 @@ jobs:
       - name: Install dependencies
         run: npm ci
 
+      - name: Run type tests
+        run: npm run test:types
+
       - name: Run unit tests
         run: npm run test:unit

src/modules/connectors.ts

@@ -46,7 +46,7 @@ export function createConnectorsModule(
       }
 
       const response = await axios.get<ConnectorAccessTokenResponse>(
-        `/apps/${appId}/external-auth/tokens/${integrationType}`
+        `/apps/${appId}/external-auth/tokens/${encodeURIComponent(integrationType)}`
       );
 
       // @ts-expect-error
@@ -61,7 +61,7 @@ export function createConnectorsModule(
       }
 
       const response = await axios.get<ConnectorAccessTokenResponse>(
-        `/apps/${appId}/external-auth/tokens/${integrationType}`
+        `/apps/${appId}/external-auth/tokens/${encodeURIComponent(integrationType)}`
       );
 
       const data = response as unknown as ConnectorAccessTokenResponse;
@@ -79,7 +79,7 @@ export function createConnectorsModule(
       }
 
       const response = await axios.get<ConnectorAccessTokenResponse>(
-        `/apps/${appId}/external-auth/tokens/connectors/${connectorId}`
+        `/apps/${appId}/external-auth/tokens/connectors/${encodeURIComponent(connectorId)}`
       );
 
       const data = response as unknown as ConnectorAccessTokenResponse;
@@ -100,7 +100,7 @@ export function createConnectorsModule(
       }
 
       const response = await axios.get(
-        `/apps/${appId}/app-user-auth/connectors/${connectorId}/token`
+        `/apps/${appId}/app-user-auth/connectors/${encodeURIComponent(connectorId)}/token`
       );
 
       const data = response as unknown as { access_token: string };
@@ -115,7 +115,7 @@ export function createConnectorsModule(
       }
 
       const response = await axios.get(
-        `/apps/${appId}/app-user-auth/connectors/${connectorId}/token`
+        `/apps/${appId}/app-user-auth/connectors/${encodeURIComponent(connectorId)}/token`
       );
 
       const data = response as unknown as ConnectorAccessTokenResponse;
@@ -130,9 +130,11 @@ export function createConnectorsModule(
       request: ConnectorApiRequest
     ): Promise<ConnectorApiResponse<T>> {
       assertNonEmptyString(integrationType, "Integration type");
+      // Encoded so a runtime-built identifier can only ever select a
+      // connector, never re-target another route under this token.
       return proxyCall<T>(
         axios,
-        `/apps/${appId}/connectors/${integrationType}/call`,
+        `/apps/${appId}/connectors/${encodeURIComponent(integrationType)}/call`,
         request
       );
     },
@@ -172,9 +174,9 @@ async function proxyCall<T>(
 
   const response = await axios.post(url, {
     method,
-    // Omitted rather than sent as null so the proxy applies the connector's
-    // declared default host.
-    ...(request.host === undefined ? {} : { host: request.host }),
+    // Omitted when unset (undefined or null, since untyped callers write
+    // either) so the proxy applies the connector's declared default host.
+    ...(request.host == null ? {} : { host: request.host }),
     path: request.path,
     query: request.query ?? {},
     headers: request.headers ?? {},
@@ -213,7 +215,7 @@ export function createUserConnectorsModule(
       }
 
       const response = await axios.post(
-        `/apps/${appId}/app-user-auth/connectors/${connectorId}/initiate`
+        `/apps/${appId}/app-user-auth/connectors/${encodeURIComponent(connectorId)}/initiate`
       );
 
       const data = response as unknown as { redirect_url: string };
@@ -226,7 +228,7 @@ export function createUserConnectorsModule(
       }
 
       await axios.delete(
-        `/apps/${appId}/app-user-auth/connectors/${connectorId}`
+        `/apps/${appId}/app-user-auth/connectors/${encodeURIComponent(connectorId)}`
       );
     },
   };

src/modules/connectors.types.ts

@@ -102,7 +102,7 @@ export interface ConnectorApiResponse<T = unknown> {
    * The parsed upstream response body, or proxy error details when no response
    * was received. `null` when the response was binary — see {@link dataBase64}.
    */
-  data: T;
+  data: T | null;
   /**
    * The response body base64-encoded, for the media types the connector declares
    * as binary (images, PDFs). Set instead of {@link data}, never alongside it.

tests/types/connectors.types.ts

@@ -47,8 +47,17 @@ const rejectsLowercaseMethod = {
   path: "/2/tweets",
 } satisfies ConnectorApiRequest;
 
+// Even with an explicit type argument, data stays nullable: binary and
+// proxy-error responses carry null, so it must be narrowed before use.
+declare const typedResponse: ConnectorApiResponse<{ id: string }>;
+const narrowableData: { id: string } | null = typedResponse.data;
+// @ts-expect-error data may be null until narrowed.
+const unnarrowedData: { id: string } = typedResponse.data;
+
 void request;
 void response;
 void binaryResponse;
 void hostedRequest;
 void rejectsLowercaseMethod;
+void narrowableData;
+void unnarrowedData;

tests/unit/connectors-proxy.test.ts

@@ -12,10 +12,12 @@ describe("Connectors module – metered connector proxy", () => {
   beforeEach(() => {
     base44 = createClient({ serverUrl, appId, serviceToken });
     scope = nock(serverUrl);
+    nock.disableNetConnect();
   });
 
   afterEach(() => {
     nock.cleanAll();
+    nock.enableNetConnect();
   });
 
   const proxyResponse = {
@@ -51,6 +53,23 @@ describe("Connectors module – metered connector proxy", () => {
     expect(received.headers).toEqual({});
   });
 
+  test("percent-encodes the integration type so it stays on the connectors route", async () => {
+    // The route carries the service-role token, so a runtime-built identifier
+    // containing slashes must select a (nonexistent) connector, not another route.
+    scope
+      .post(
+        `/api/apps/${appId}/connectors/${encodeURIComponent("../evil/route")}/call`
+      )
+      .reply(200, proxyResponse);
+
+    const res = await base44.asServiceRole.connectors.callApi(
+      "../evil/route" as any,
+      { path: "/x" }
+    );
+
+    expect(res.success).toBe(true);
+  });
+
   test("forwards a named host, and omits it entirely when unset", async () => {
     // The payload is built field by field, so anything not explicitly forwarded
     // is silently dropped — which is what happened to `host` before this.
@@ -60,7 +79,7 @@ describe("Connectors module – metered connector proxy", () => {
         bodies.push(body);
         return true;
       })
-      .twice()
+      .times(3)
       .reply(200, proxyResponse);
 
     await base44.asServiceRole.connectors.callApi("googlemaps", {
@@ -70,10 +89,16 @@ describe("Connectors module – metered connector proxy", () => {
     await base44.asServiceRole.connectors.callApi("googlemaps", {
       path: "/maps/api/geocode/json",
     });
+    await base44.asServiceRole.connectors.callApi("googlemaps", {
+      host: null as any,
+      path: "/maps/api/geocode/json",
+    });
 
     expect(bodies[0].host).toBe("places");
     // Absent rather than null, so the proxy picks the connector's default host.
     expect("host" in bodies[1]).toBe(false);
+    // Untyped callers write `host: x ?? null`; null must mean unset, not a host.
+    expect("host" in bodies[2]).toBe(false);
   });
 
   test("maps a binary response to dataBase64 + contentType", async () => {