diff --git a/src/lib/tm-base-url.ts b/src/lib/tm-base-url.ts index 88c4e201..ad0c9f2a 100644 --- a/src/lib/tm-base-url.ts +++ b/src/lib/tm-base-url.ts @@ -2,6 +2,7 @@ import { apiClient } from "./apiClient.js"; import logger from "../logger.js"; import { BrowserStackConfig } from "./types.js"; import { getBrowserStackAuth } from "./get-auth.js"; +import appConfig from "../config.js"; const TM_BASE_URLS = [ "https://test-management.browserstack.com", @@ -14,7 +15,10 @@ let cachedBaseUrl: string | null = null; export async function getTMBaseURL( config: BrowserStackConfig, ): Promise { - if (cachedBaseUrl) { + // Skip the module-level cache in remote (multi-tenant) mode: it is process-shared, + // so the first user's region would be served to every subsequent user — breaking + // requests for users on a different region's BrowserStack account. + if (!appConfig.REMOTE_MCP && cachedBaseUrl) { logger.debug(`Using cached TM base URL: ${cachedBaseUrl}`); return cachedBaseUrl; } @@ -37,7 +41,11 @@ export async function getTMBaseURL( }); if (res.ok) { - cachedBaseUrl = baseUrl; + // Only populate the cache in single-tenant (stdio) mode; in remote mode + // the cache must stay empty so each user discovers their own region. + if (!appConfig.REMOTE_MCP) { + cachedBaseUrl = baseUrl; + } logger.info(`Selected TM base URL: ${baseUrl}`); return baseUrl; } diff --git a/src/tools/appautomate-utils/native-execution/appautomate.ts b/src/tools/appautomate-utils/native-execution/appautomate.ts index 00a2f8b6..19e6fe09 100644 --- a/src/tools/appautomate-utils/native-execution/appautomate.ts +++ b/src/tools/appautomate-utils/native-execution/appautomate.ts @@ -2,6 +2,7 @@ import fs from "fs"; import FormData from "form-data"; import { apiClient } from "../../../lib/apiClient.js"; import { customFuzzySearch } from "../../../lib/fuzzy.js"; +import { getBrowserStackAuth } from "../../../lib/get-auth.js"; import { BrowserStackConfig } from "../../../lib/types.js"; interface Device { @@ -258,18 +259,15 @@ export async function triggerEspressoBuild( test_suite_url: string, devices: string[], project: string, + config: BrowserStackConfig, ): Promise { - const auth = { - username: process.env.BROWSERSTACK_USERNAME || "", - password: process.env.BROWSERSTACK_ACCESS_KEY || "", - }; + const authHeader = + "Basic " + Buffer.from(getBrowserStackAuth(config)).toString("base64"); const response = await apiClient.post({ url: "https://api-cloud.browserstack.com/app-automate/espresso/v2/build", headers: { - Authorization: - "Basic " + - Buffer.from(`${auth.username}:${auth.password}`).toString("base64"), + Authorization: authHeader, "Content-Type": "application/json", }, body: { diff --git a/src/tools/appautomate.ts b/src/tools/appautomate.ts index 192218f4..edcb246c 100644 --- a/src/tools/appautomate.ts +++ b/src/tools/appautomate.ts @@ -233,6 +233,7 @@ async function runAppTestsOnBrowserStack( test_suite_url, deviceStrings, args.project, + config, ); return { diff --git a/src/tools/sdk-utils/bstack/commands.ts b/src/tools/sdk-utils/bstack/commands.ts index 1f600d72..d2b0815f 100644 --- a/src/tools/sdk-utils/bstack/commands.ts +++ b/src/tools/sdk-utils/bstack/commands.ts @@ -43,7 +43,8 @@ const GRADLE_SETUP_INSTRUCTIONS = ` // Generates Maven archetype command for Windows platform function getMavenCommandForWindows( - framework: string, + username: string, + accessKey: string, mavenFramework: string, ): string { return ( @@ -54,8 +55,8 @@ function getMavenCommandForWindows( `-DgroupId="${MAVEN_ARCHETYPE_GROUP_ID}" ` + `-DartifactId="${MAVEN_ARCHETYPE_ARTIFACT_ID}" ` + `-Dversion="${MAVEN_ARCHETYPE_VERSION}" ` + - `-DBROWSERSTACK_USERNAME="${process.env.BROWSERSTACK_USERNAME}" ` + - `-DBROWSERSTACK_ACCESS_KEY="${process.env.BROWSERSTACK_ACCESS_KEY}" ` + + `-DBROWSERSTACK_USERNAME="${username}" ` + + `-DBROWSERSTACK_ACCESS_KEY="${accessKey}" ` + `-DBROWSERSTACK_FRAMEWORK="${mavenFramework}"` ); } @@ -85,7 +86,7 @@ function getJavaSDKInstructions( const platformLabel = isWindows ? "Windows" : "macOS/Linux"; const mavenCommand = isWindows - ? getMavenCommandForWindows(framework, mavenFramework) + ? getMavenCommandForWindows(username, accessKey, mavenFramework) : getMavenCommandForUnix(username, accessKey, mavenFramework); return `---STEP--- diff --git a/src/tools/testmanagement-utils/update-testcase.ts b/src/tools/testmanagement-utils/update-testcase.ts index ba4ed60f..7546a9d4 100644 --- a/src/tools/testmanagement-utils/update-testcase.ts +++ b/src/tools/testmanagement-utils/update-testcase.ts @@ -97,10 +97,7 @@ export const UpdateTestCaseSchema = z.object({ "Replacement list of linked Jira/Asana/Azure issue IDs for the test case.", ), custom_fields: z - .record( - z.string(), - z.union([z.string(), z.number(), z.boolean()]), - ) + .record(z.string(), z.union([z.string(), z.number(), z.boolean()])) .optional() .describe( "Map of custom field name/id to value. Valid field names and value types are per-project; discover them via the project's form fields.", @@ -118,7 +115,11 @@ export const UpdateTestCaseSchema = z.object({ * pass the raw value through so the backend can surface its own error. */ function normalizeDefaultFieldValue( - fieldValues: Array<{ internal_name?: string | null; name?: string; value: any }>, + fieldValues: Array<{ + internal_name?: string | null; + name?: string; + value: any; + }>, input: string, emit: "name" | "internal_name", ): string | undefined { diff --git a/tests/lib/tm-base-url.test.ts b/tests/lib/tm-base-url.test.ts new file mode 100644 index 00000000..f4ba3940 --- /dev/null +++ b/tests/lib/tm-base-url.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("../../src/lib/apiClient", () => ({ + apiClient: { + get: vi.fn(), + }, +})); + +vi.mock("../../src/logger", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn() }, +})); + +// Re-imported per-test by resetting modules so the module-level +// `cachedBaseUrl` and the mocked `appConfig.REMOTE_MCP` stay isolated +// across cases. +async function loadModule(remoteMcp: boolean) { + vi.resetModules(); + vi.doMock("../../src/config", () => ({ + __esModule: true, + default: { REMOTE_MCP: remoteMcp }, + })); + const apiClientMod = await import("../../src/lib/apiClient"); + const tmMod = await import("../../src/lib/tm-base-url"); + return { apiClient: apiClientMod.apiClient, getTMBaseURL: tmMod.getTMBaseURL }; +} + +const mockConfig = { + "browserstack-username": "u", + "browserstack-access-key": "k", +}; + +describe("getTMBaseURL — multi-tenant cache discipline", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("stdio mode (REMOTE_MCP=false): caches the discovered base URL across calls", async () => { + const { apiClient, getTMBaseURL } = await loadModule(false); + (apiClient.get as any).mockResolvedValueOnce({ ok: true }); + + const first = await getTMBaseURL(mockConfig); + expect(first).toBe("https://test-management.browserstack.com"); + expect(apiClient.get).toHaveBeenCalledTimes(1); + + // Second call must hit the cache; no additional HTTP call. + const second = await getTMBaseURL(mockConfig); + expect(second).toBe(first); + expect(apiClient.get).toHaveBeenCalledTimes(1); + }); + + it("remote mode (REMOTE_MCP=true): never caches, re-discovers each call", async () => { + const { apiClient, getTMBaseURL } = await loadModule(true); + // First user — region 1 succeeds on the first URL. + (apiClient.get as any).mockResolvedValueOnce({ ok: true }); + const userA = await getTMBaseURL(mockConfig); + expect(userA).toBe("https://test-management.browserstack.com"); + + // Second user (different region) — first URL fails, EU succeeds. + (apiClient.get as any) + .mockResolvedValueOnce({ ok: false }) + .mockResolvedValueOnce({ ok: true }); + const userB = await getTMBaseURL(mockConfig); + expect(userB).toBe("https://test-management-eu.browserstack.com"); + + // Three HTTP calls total: one for user A, two for user B. + // If the cache leaked across users, user B would have been served userA's URL with zero new calls. + expect(apiClient.get).toHaveBeenCalledTimes(3); + }); +}); diff --git a/tests/tools/sdk-utils-commands.test.ts b/tests/tools/sdk-utils-commands.test.ts new file mode 100644 index 00000000..1cb95729 --- /dev/null +++ b/tests/tools/sdk-utils-commands.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect, afterEach, beforeEach } from "vitest"; +import { getSDKPrefixCommand } from "../../src/tools/sdk-utils/bstack/commands"; + +describe("getSDKPrefixCommand", () => { + const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform"); + + afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, "platform", originalPlatform); + } + }); + + beforeEach(() => { + // Guard: ensure these env vars never leak into the rendered command. + // The fix forwards `username` / `accessKey` parameters; reading process.env + // would silently return undefined or a stale value in remote (multi-tenant) mode. + delete process.env.BROWSERSTACK_USERNAME; + delete process.env.BROWSERSTACK_ACCESS_KEY; + }); + + it("nodejs: embeds passed username/accessKey, never reads process.env", () => { + const out = getSDKPrefixCommand("nodejs", "testng", "u-from-config", "k-from-config"); + expect(out).toContain("--username u-from-config"); + expect(out).toContain("--key k-from-config"); + expect(out).not.toContain("undefined"); + expect(out).not.toContain("process.env"); + }); + + it("java/unix: Maven command uses passed username/accessKey", () => { + Object.defineProperty(process, "platform", { value: "darwin" }); + const out = getSDKPrefixCommand("java", "testng", "u-from-config", "k-from-config"); + expect(out).toContain('-DBROWSERSTACK_USERNAME="u-from-config"'); + expect(out).toContain('-DBROWSERSTACK_ACCESS_KEY="k-from-config"'); + expect(out).not.toContain("undefined"); + expect(out).not.toContain("process.env"); + }); + + it("java/windows: Maven command uses passed username/accessKey (regression)", () => { + // Regression for a bug where the Windows branch read process.env.BROWSERSTACK_* + // while the Unix branch correctly took params. In remote mode this leaked the + // string "undefined" into the Maven command shown to the user. + Object.defineProperty(process, "platform", { value: "win32" }); + const out = getSDKPrefixCommand("java", "testng", "u-from-config", "k-from-config"); + expect(out).toContain('-DBROWSERSTACK_USERNAME="u-from-config"'); + expect(out).toContain('-DBROWSERSTACK_ACCESS_KEY="k-from-config"'); + expect(out).not.toContain("undefined"); + expect(out).not.toContain("process.env"); + }); +}); diff --git a/tests/tools/triggerEspressoBuild.test.ts b/tests/tools/triggerEspressoBuild.test.ts new file mode 100644 index 00000000..5fe1a251 --- /dev/null +++ b/tests/tools/triggerEspressoBuild.test.ts @@ -0,0 +1,61 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("../../src/lib/apiClient", () => ({ + apiClient: { + post: vi.fn(), + }, +})); + +vi.mock("../../src/logger", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn() }, +})); + +import { apiClient } from "../../src/lib/apiClient"; +import { triggerEspressoBuild } from "../../src/tools/appautomate-utils/native-execution/appautomate"; + +const mockConfig = { + "browserstack-username": "config-user", + "browserstack-access-key": "config-key", +}; + +describe("triggerEspressoBuild — credential sourcing", () => { + beforeEach(() => { + vi.clearAllMocks(); + // Guard: even if process.env happens to be set, the function must not use it. + process.env.BROWSERSTACK_USERNAME = "env-user-should-be-ignored"; + process.env.BROWSERSTACK_ACCESS_KEY = "env-key-should-be-ignored"; + }); + + it("uses creds from config, not process.env", async () => { + (apiClient.post as any).mockResolvedValue({ data: { build_id: "BUILD-1" } }); + + const buildId = await triggerEspressoBuild( + "app.apk", + "tests.apk", + ["Samsung Galaxy S20-12.0"], + "p1", + mockConfig, + ); + + expect(buildId).toBe("BUILD-1"); + + const call = (apiClient.post as any).mock.calls[0][0]; + const authHeader = call.headers.Authorization as string; + expect(authHeader.startsWith("Basic ")).toBe(true); + + const decoded = Buffer.from(authHeader.slice("Basic ".length), "base64").toString(); + expect(decoded).toBe("config-user:config-key"); + expect(decoded).not.toContain("env-user-should-be-ignored"); + }); + + it("throws a clear error when config is missing creds (does not silently auth)", async () => { + await expect( + triggerEspressoBuild("app.apk", "tests.apk", ["d"], "p", { + "browserstack-username": "", + "browserstack-access-key": "", + } as any), + ).rejects.toThrow(/credentials not set/i); + + expect(apiClient.post).not.toHaveBeenCalled(); + }); +});