diff --git a/src/tools/percy-sdk.ts b/src/tools/percy-sdk.ts index f116077e..94f62f2f 100644 --- a/src/tools/percy-sdk.ts +++ b/src/tools/percy-sdk.ts @@ -2,8 +2,7 @@ import { trackMCP } from "../index.js"; import { BrowserStackConfig } from "../lib/types.js"; import { fetchPercyChanges } from "./review-agent.js"; import { addListTestFiles } from "./list-test-files.js"; -// PMAA-100: runPercyScan tool temporarily disabled due to plaintext token leak in tool output. -// import { runPercyScan } from "./run-percy-scan.js"; +import { runPercyScan } from "./run-percy-scan.js"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { SetUpPercyParamsShape } from "./sdk-utils/common/schema.js"; import { updateTestsWithPercyCommands } from "./add-percy-snapshots.js"; @@ -22,8 +21,7 @@ import { import { UpdateTestFileWithInstructionsParams } from "./percy-snapshot-utils/constants.js"; import { - // PMAA-100: kept commented so the registration block below is easy to restore once the proper fix lands. - // RunPercyScanParamsShape, + RunPercyScanParamsShape, FetchPercyChangesParamsShape, ManagePercyBuildApprovalParamsShape, } from "./sdk-utils/common/schema.js"; @@ -135,22 +133,19 @@ export function registerPercyTools( }, ); - // PMAA-100: runPercyScan temporarily disabled — fetched Percy token was being - // returned in plaintext within tool output (see HackerOne #3576387). Re-enable - // once the token is replaced with a placeholder in run-percy-scan.ts. - // tools.runPercyScan = server.tool( - // "runPercyScan", - // "Run a Percy visual test scan. Example prompts : Run this Percy build/scan. Never run percy scan/build without this tool", - // RunPercyScanParamsShape, - // async (args) => { - // try { - // trackMCP("runPercyScan", server.server.getClientVersion()!, config); - // return runPercyScan(args, config); - // } catch (error) { - // return handleMCPError("runPercyScan", server, config, error); - // } - // }, - // ); + tools.runPercyScan = server.tool( + "runPercyScan", + "Run a Percy visual test scan. Example prompts : Run this Percy build/scan. Never run percy scan/build without this tool", + RunPercyScanParamsShape, + async (args) => { + try { + trackMCP("runPercyScan", server.server.getClientVersion()!, config); + return runPercyScan(args); + } catch (error) { + return handleMCPError("runPercyScan", server, config, error); + } + }, + ); tools.fetchPercyChanges = server.tool( "fetchPercyChanges", diff --git a/src/tools/run-percy-scan.ts b/src/tools/run-percy-scan.ts index 6302dc1a..28ba5906 100644 --- a/src/tools/run-percy-scan.ts +++ b/src/tools/run-percy-scan.ts @@ -1,8 +1,5 @@ import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { PercyIntegrationTypeEnum } from "./sdk-utils/common/types.js"; -import { BrowserStackConfig } from "../lib/types.js"; -import { getBrowserStackAuth } from "../lib/get-auth.js"; -import { fetchPercyToken } from "./sdk-utils/percy-web/fetchPercyToken.js"; import { storedPercyResults } from "../lib/inmemory-store.js"; import { getFrameworkTestCommand, @@ -10,19 +7,12 @@ import { } from "./sdk-utils/percy-web/constants.js"; import path from "path"; -export async function runPercyScan( - args: { - projectName: string; - integrationType: PercyIntegrationTypeEnum; - instruction?: string; - }, - config: BrowserStackConfig, -): Promise { - const { projectName, integrationType, instruction } = args; - const authorization = getBrowserStackAuth(config); - const percyToken = await fetchPercyToken(projectName, authorization, { - type: integrationType, - }); +export async function runPercyScan(args: { + projectName: string; + integrationType: PercyIntegrationTypeEnum; + instruction?: string; +}): Promise { + const { projectName, instruction } = args; // Check if we have stored data and project matches const stored = storedPercyResults.get(); @@ -33,7 +23,7 @@ export async function runPercyScan( // Build steps array with conditional spread const steps = [ - generatePercyTokenInstructions(percyToken), + generatePercyTokenInstructions(), ...(hasUpdatedFiles ? generateUpdatedFilesSteps(stored, updatedFiles) : []), ...(instruction && !hasUpdatedFiles ? generateInstructionSteps(instruction) @@ -55,12 +45,14 @@ export async function runPercyScan( }; } -function generatePercyTokenInstructions(percyToken: string): string { - return `Set the environment variable for your project: +function generatePercyTokenInstructions(): string { + return `Set the PERCY_TOKEN environment variable for your project. Retrieve your project's token from the Percy dashboard (https://percy.io → Project Settings → Project Token) and add it to your project's .env file (PERCY_TOKEN=) or export it in your shell: -export PERCY_TOKEN="${percyToken}" + - macOS/Linux: export PERCY_TOKEN="" + - Windows (PS): $env:PERCY_TOKEN="" + - Windows (CMD): set PERCY_TOKEN= -(For Windows: use 'setx PERCY_TOKEN "${percyToken}"' or 'set PERCY_TOKEN=${percyToken}' as appropriate.)`; +Do not paste the token into chat or commit it.`; } const toAbs = (p: string): string | undefined => diff --git a/src/tools/sdk-utils/handler.ts b/src/tools/sdk-utils/handler.ts index 5c339eaf..e04a5ed2 100644 --- a/src/tools/sdk-utils/handler.ts +++ b/src/tools/sdk-utils/handler.ts @@ -2,8 +2,6 @@ import { formatToolResult } from "./common/utils.js"; import { BrowserStackConfig } from "../../lib/types.js"; import { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { PercyIntegrationTypeEnum } from "./common/types.js"; -import { getBrowserStackAuth } from "../../lib/get-auth.js"; -import { fetchPercyToken } from "./percy-web/fetchPercyToken.js"; import { runPercyWeb } from "./percy-web/handler.js"; import { runPercyAutomateOnly } from "./percy-automate/handler.js"; import { runBstackSDKOnly } from "./bstack/sdkHandler.js"; @@ -60,8 +58,6 @@ export async function setUpPercyHandler( testFiles: {}, }); - const authorization = getBrowserStackAuth(config); - const folderPaths = input.folderPaths || []; const filePaths = input.filePaths || []; @@ -86,14 +82,7 @@ export async function setUpPercyHandler( ); } - // Fetch the Percy token - const percyToken = await fetchPercyToken( - input.projectName, - authorization, - { type: PercyIntegrationTypeEnum.WEB }, - ); - - const result = runPercyWeb(percyInput, percyToken); + const result = runPercyWeb(percyInput); return await formatToolResult(result, "percy-web"); } else if (input.integrationType === PercyIntegrationTypeEnum.AUTOMATE) { // First try Percy with BrowserStack SDK @@ -142,15 +131,7 @@ export async function setUpPercyHandler( }; const sdkResult = await runBstackSDKOnly(sdkInput, config, true); // Percy Automate instructions - const percyToken = await fetchPercyToken( - input.projectName, - authorization, - { type: PercyIntegrationTypeEnum.AUTOMATE }, - ); - const percyAutomateResult = runPercyAutomateOnly( - percyInput, - percyToken, - ); + const percyAutomateResult = runPercyAutomateOnly(percyInput); // Combine steps: warning, SDK steps, Percy Automate steps const steps = [ diff --git a/src/tools/sdk-utils/percy-automate/handler.ts b/src/tools/sdk-utils/percy-automate/handler.ts index f5e43135..e3d79543 100644 --- a/src/tools/sdk-utils/percy-automate/handler.ts +++ b/src/tools/sdk-utils/percy-automate/handler.ts @@ -5,7 +5,6 @@ import { SDKSupportedLanguage } from "../common/types.js"; export function runPercyAutomateOnly( input: SetUpPercyInput, - percyToken: string, ): RunTestsInstructionResult { const steps: RunTestsStep[] = []; @@ -26,7 +25,7 @@ export function runPercyAutomateOnly( steps.push({ type: "instruction", title: "Set Percy Token in Environment", - content: `Here is percy token if required {${percyToken}}`, + content: `Retrieve your project's token from the Percy dashboard (https://percy.io → Project Settings → Project Token) and add it to your project's .env file (PERCY_TOKEN=) or export it in your shell (e.g. export PERCY_TOKEN=""). Do not paste the token into chat or commit it.`, }); steps.push({ diff --git a/src/tools/sdk-utils/percy-web/handler.ts b/src/tools/sdk-utils/percy-web/handler.ts index 681fa614..ebf96320 100644 --- a/src/tools/sdk-utils/percy-web/handler.ts +++ b/src/tools/sdk-utils/percy-web/handler.ts @@ -10,10 +10,7 @@ import { export let percyWebSetupInstructions = ""; -export function runPercyWeb( - input: SetUpPercyInput, - percyToken: string, -): RunTestsInstructionResult { +export function runPercyWeb(input: SetUpPercyInput): RunTestsInstructionResult { const steps: RunTestsStep[] = []; // Assume configuration is supported due to guardrails at orchestration layer @@ -32,9 +29,12 @@ export function runPercyWeb( steps.push({ type: "instruction", title: "Set Percy Token in Environment", - content: `Set the environment variable for your project: - export PERCY_TOKEN="${percyToken}" - (For Windows: use 'setx PERCY_TOKEN "${percyToken}"' or 'set PERCY_TOKEN=${percyToken}' as appropriate.)`, + content: `Retrieve your project's token from the Percy dashboard (https://percy.io → Project Settings → Project Token) and add it to your project's .env file (PERCY_TOKEN=) or export it in your shell: + macOS/Linux: export PERCY_TOKEN="" + Windows (PS): $env:PERCY_TOKEN="" + Windows (CMD): set PERCY_TOKEN= + +Do not paste the token into chat or commit it.`, }); steps.push({ diff --git a/tests/tools/percySdk.test.ts b/tests/tools/percySdk.test.ts index 0916d482..afeca3c0 100644 --- a/tests/tools/percySdk.test.ts +++ b/tests/tools/percySdk.test.ts @@ -2,8 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { registerPercyTools } from "../../src/tools/percy-sdk"; import { setUpPercyHandler, simulatePercyChangeHandler } from "../../src/tools/sdk-utils/handler"; import { updateTestsWithPercyCommands } from "../../src/tools/add-percy-snapshots"; -// PMAA-100: runPercyScan registration disabled — restore import alongside the test. -// import { runPercyScan } from "../../src/tools/run-percy-scan"; +import { runPercyScan } from "../../src/tools/run-percy-scan"; import { fetchPercyChanges } from "../../src/tools/review-agent"; import { approveOrDeclinePercyBuild } from "../../src/tools/review-agent-utils/percy-approve-reject"; @@ -14,10 +13,9 @@ vi.mock("../../src/tools/sdk-utils/handler", () => ({ vi.mock("../../src/tools/add-percy-snapshots", () => ({ updateTestsWithPercyCommands: vi.fn(), })); -// PMAA-100: runPercyScan registration disabled — restore mock alongside the test. -// vi.mock("../../src/tools/run-percy-scan", () => ({ -// runPercyScan: vi.fn(), -// })); +vi.mock("../../src/tools/run-percy-scan", () => ({ + runPercyScan: vi.fn(), +})); vi.mock("../../src/tools/review-agent", () => ({ fetchPercyChanges: vi.fn(), })); @@ -66,8 +64,7 @@ describe("Percy SDK Tools", () => { expect(toolNames).toContain("expandPercyVisualTesting"); expect(toolNames).toContain("addPercySnapshotCommands"); expect(toolNames).toContain("listTestFiles"); - // PMAA-100: runPercyScan registration disabled — restore once the token leak is fixed. - // expect(toolNames).toContain("runPercyScan"); + expect(toolNames).toContain("runPercyScan"); expect(toolNames).toContain("fetchPercyChanges"); expect(toolNames).toContain("managePercyBuildApproval"); }); @@ -121,15 +118,14 @@ describe("Percy SDK Tools", () => { expect(result.content[0].text).toContain("Commands added"); }); - // PMAA-100: runPercyScan registration disabled — restore once the token leak is fixed. - // it("runPercyScan - SUCCESS", async () => { - // (runPercyScan as any).mockResolvedValue({ - // content: [{ type: "text", text: "Percy scan started" }], - // }); - // - // const result = await handlers["runPercyScan"]({ projectName: "test" }); - // expect(result.content[0].text).toContain("Percy scan"); - // }); + it("runPercyScan - SUCCESS", async () => { + (runPercyScan as any).mockResolvedValue({ + content: [{ type: "text", text: "Percy scan started" }], + }); + + const result = await handlers["runPercyScan"]({ projectName: "test" }); + expect(result.content[0].text).toContain("Percy scan"); + }); it("fetchPercyChanges - SUCCESS", async () => { (fetchPercyChanges as any).mockResolvedValue({ diff --git a/tests/tools/runPercyScan.test.ts b/tests/tools/runPercyScan.test.ts index b7620515..c7988ea8 100644 --- a/tests/tools/runPercyScan.test.ts +++ b/tests/tools/runPercyScan.test.ts @@ -1,15 +1,8 @@ import { describe, it, expect, vi, beforeEach, Mock } from "vitest"; import { runPercyScan } from "../../src/tools/run-percy-scan"; -import { fetchPercyToken } from "../../src/tools/sdk-utils/percy-web/fetchPercyToken"; import { storedPercyResults } from "../../src/lib/inmemory-store"; import { PercyIntegrationTypeEnum } from "../../src/tools/sdk-utils/common/types"; -vi.mock("../../src/lib/get-auth", () => ({ - getBrowserStackAuth: vi.fn().mockReturnValue("fake-user:fake-key"), -})); -vi.mock("../../src/tools/sdk-utils/percy-web/fetchPercyToken", () => ({ - fetchPercyToken: vi.fn(), -})); vi.mock("../../src/lib/inmemory-store", () => ({ storedPercyResults: { get: vi.fn(), set: vi.fn() }, })); @@ -21,32 +14,24 @@ vi.mock("../../src/logger", () => ({ default: { error: vi.fn(), info: vi.fn(), debug: vi.fn() }, })); -const mockConfig = { - "browserstack-username": "fake-user", - "browserstack-access-key": "fake-key", -}; - describe("runPercyScan", () => { beforeEach(() => vi.clearAllMocks()); - it("SUCCESS: returns Percy token and run instructions", async () => { - (fetchPercyToken as Mock).mockResolvedValue("percy-token-abc"); + it("renders PERCY_TOKEN setup instructions with placeholder", async () => { (storedPercyResults.get as Mock).mockReturnValue(null); - const result = await runPercyScan( - { - projectName: "my-project", - integrationType: PercyIntegrationTypeEnum.WEB, - }, - mockConfig, - ); + const result = await runPercyScan({ + projectName: "my-project", + integrationType: PercyIntegrationTypeEnum.WEB, + }); - expect(result.content[0].text).toContain("percy-token-abc"); - expect(result.content[0].text).toContain("PERCY_TOKEN"); + const text = result.content[0].text as string; + expect(text).toContain("PERCY_TOKEN"); + expect(text).toContain(""); + expect(text).toContain(".env"); }); - it("SUCCESS: includes updated file instructions when available", async () => { - (fetchPercyToken as Mock).mockResolvedValue("percy-token-abc"); + it("includes updated file instructions when available", async () => { (storedPercyResults.get as Mock).mockReturnValue({ projectName: "my-project", testFiles: { "/tests/login.test.js": true }, @@ -54,48 +39,25 @@ describe("runPercyScan", () => { detectedTestingFramework: "jest", }); - const result = await runPercyScan( - { - projectName: "my-project", - integrationType: PercyIntegrationTypeEnum.WEB, - }, - mockConfig, - ); + const result = await runPercyScan({ + projectName: "my-project", + integrationType: PercyIntegrationTypeEnum.WEB, + }); - expect(result.content[0].text).toContain("percy-token-abc"); + const text = result.content[0].text as string; + expect(text).toContain("Updated files to run"); }); - it("SUCCESS: includes custom instruction steps", async () => { - (fetchPercyToken as Mock).mockResolvedValue("percy-token-abc"); + it("includes custom instruction steps", async () => { (storedPercyResults.get as Mock).mockReturnValue(null); - const result = await runPercyScan( - { - projectName: "my-project", - integrationType: PercyIntegrationTypeEnum.WEB, - instruction: "npx percy exec -- npx playwright test", - }, - mockConfig, - ); - - expect(result.content[0].text).toContain("percy-token-abc"); - expect(result.content[0].text).toContain("npx percy exec"); - }); - - it("FAIL: throws when Percy token fetch fails", async () => { - (fetchPercyToken as Mock).mockRejectedValue( - new Error("Percy token not found"), - ); - (storedPercyResults.get as Mock).mockReturnValue(null); + const result = await runPercyScan({ + projectName: "my-project", + integrationType: PercyIntegrationTypeEnum.WEB, + instruction: "npx percy exec -- npx playwright test", + }); - await expect( - runPercyScan( - { - projectName: "bad-project", - integrationType: PercyIntegrationTypeEnum.WEB, - }, - mockConfig, - ), - ).rejects.toThrow("Percy token not found"); + const text = result.content[0].text as string; + expect(text).toContain("npx percy exec"); }); });