Skip to content

Commit 2f3377c

Browse files
ruturaj-browserstackruturaj-browserstack
authored
Merge pull request #288 from manoj-k04/PMAA-103-fix-percy-token-leak
fix(percy): replace runPercyScan token interpolation with placeholder (PMAA-103)
2 parents 8a5b31a + 2b085ce commit 2f3377c

7 files changed

Lines changed: 75 additions & 150 deletions

File tree

‎src/tools/percy-sdk.ts‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import { trackMCP } from "../index.js";
22
import { BrowserStackConfig } from "../lib/types.js";
33
import { fetchPercyChanges } from "./review-agent.js";
44
import { addListTestFiles } from "./list-test-files.js";
5-
// PMAA-100: runPercyScan tool temporarily disabled due to plaintext token leak in tool output.
6-
// import { runPercyScan } from "./run-percy-scan.js";
5+
import { runPercyScan } from "./run-percy-scan.js";
76
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
87
import { SetUpPercyParamsShape } from "./sdk-utils/common/schema.js";
98
import { updateTestsWithPercyCommands } from "./add-percy-snapshots.js";
@@ -22,8 +21,7 @@ import {
2221
import { UpdateTestFileWithInstructionsParams } from "./percy-snapshot-utils/constants.js";
2322

2423
import {
25-
// PMAA-100: kept commented so the registration block below is easy to restore once the proper fix lands.
26-
// RunPercyScanParamsShape,
24+
RunPercyScanParamsShape,
2725
FetchPercyChangesParamsShape,
2826
ManagePercyBuildApprovalParamsShape,
2927
} from "./sdk-utils/common/schema.js";
@@ -135,22 +133,19 @@ export function registerPercyTools(
135133
},
136134
);
137135

138-
// PMAA-100: runPercyScan temporarily disabled — fetched Percy token was being
139-
// returned in plaintext within tool output (see HackerOne #3576387). Re-enable
140-
// once the token is replaced with a placeholder in run-percy-scan.ts.
141-
// tools.runPercyScan = server.tool(
142-
// "runPercyScan",
143-
// "Run a Percy visual test scan. Example prompts : Run this Percy build/scan. Never run percy scan/build without this tool",
144-
// RunPercyScanParamsShape,
145-
// async (args) => {
146-
// try {
147-
// trackMCP("runPercyScan", server.server.getClientVersion()!, config);
148-
// return runPercyScan(args, config);
149-
// } catch (error) {
150-
// return handleMCPError("runPercyScan", server, config, error);
151-
// }
152-
// },
153-
// );
136+
tools.runPercyScan = server.tool(
137+
"runPercyScan",
138+
"Run a Percy visual test scan. Example prompts : Run this Percy build/scan. Never run percy scan/build without this tool",
139+
RunPercyScanParamsShape,
140+
async (args) => {
141+
try {
142+
trackMCP("runPercyScan", server.server.getClientVersion()!, config);
143+
return runPercyScan(args);
144+
} catch (error) {
145+
return handleMCPError("runPercyScan", server, config, error);
146+
}
147+
},
148+
);
154149

155150
tools.fetchPercyChanges = server.tool(
156151
"fetchPercyChanges",

‎src/tools/run-percy-scan.ts‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,18 @@
11
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
22
import { PercyIntegrationTypeEnum } from "./sdk-utils/common/types.js";
3-
import { BrowserStackConfig } from "../lib/types.js";
4-
import { getBrowserStackAuth } from "../lib/get-auth.js";
5-
import { fetchPercyToken } from "./sdk-utils/percy-web/fetchPercyToken.js";
63
import { storedPercyResults } from "../lib/inmemory-store.js";
74
import {
85
getFrameworkTestCommand,
96
PERCY_FALLBACK_STEPS,
107
} from "./sdk-utils/percy-web/constants.js";
118
import path from "path";
129

13-
export async function runPercyScan(
14-
args: {
15-
projectName: string;
16-
integrationType: PercyIntegrationTypeEnum;
17-
instruction?: string;
18-
},
19-
config: BrowserStackConfig,
20-
): Promise<CallToolResult> {
21-
const { projectName, integrationType, instruction } = args;
22-
const authorization = getBrowserStackAuth(config);
23-
const percyToken = await fetchPercyToken(projectName, authorization, {
24-
type: integrationType,
25-
});
10+
export async function runPercyScan(args: {
11+
projectName: string;
12+
integrationType: PercyIntegrationTypeEnum;
13+
instruction?: string;
14+
}): Promise<CallToolResult> {
15+
const { projectName, instruction } = args;
2616

2717
// Check if we have stored data and project matches
2818
const stored = storedPercyResults.get();
@@ -33,7 +23,7 @@ export async function runPercyScan(
3323

3424
// Build steps array with conditional spread
3525
const steps = [
36-
generatePercyTokenInstructions(percyToken),
26+
generatePercyTokenInstructions(),
3727
...(hasUpdatedFiles ? generateUpdatedFilesSteps(stored, updatedFiles) : []),
3828
...(instruction && !hasUpdatedFiles
3929
? generateInstructionSteps(instruction)
@@ -55,12 +45,14 @@ export async function runPercyScan(
5545
};
5646
}
5747

58-
function generatePercyTokenInstructions(percyToken: string): string {
59-
return `Set the environment variable for your project:
48+
function generatePercyTokenInstructions(): string {
49+
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=<your Percy project token>) or export it in your shell:
6050
61-
export PERCY_TOKEN="${percyToken}"
51+
- macOS/Linux: export PERCY_TOKEN="<your Percy project token>"
52+
- Windows (PS): $env:PERCY_TOKEN="<your Percy project token>"
53+
- Windows (CMD): set PERCY_TOKEN=<your Percy project token>
6254
63-
(For Windows: use 'setx PERCY_TOKEN "${percyToken}"' or 'set PERCY_TOKEN=${percyToken}' as appropriate.)`;
55+
Do not paste the token into chat or commit it.`;
6456
}
6557

6658
const toAbs = (p: string): string | undefined =>

‎src/tools/sdk-utils/handler.ts‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ import { formatToolResult } from "./common/utils.js";
22
import { BrowserStackConfig } from "../../lib/types.js";
33
import { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
44
import { PercyIntegrationTypeEnum } from "./common/types.js";
5-
import { getBrowserStackAuth } from "../../lib/get-auth.js";
6-
import { fetchPercyToken } from "./percy-web/fetchPercyToken.js";
75
import { runPercyWeb } from "./percy-web/handler.js";
86
import { runPercyAutomateOnly } from "./percy-automate/handler.js";
97
import { runBstackSDKOnly } from "./bstack/sdkHandler.js";
@@ -60,8 +58,6 @@ export async function setUpPercyHandler(
6058
testFiles: {},
6159
});
6260

63-
const authorization = getBrowserStackAuth(config);
64-
6561
const folderPaths = input.folderPaths || [];
6662
const filePaths = input.filePaths || [];
6763

@@ -86,14 +82,7 @@ export async function setUpPercyHandler(
8682
);
8783
}
8884

89-
// Fetch the Percy token
90-
const percyToken = await fetchPercyToken(
91-
input.projectName,
92-
authorization,
93-
{ type: PercyIntegrationTypeEnum.WEB },
94-
);
95-
96-
const result = runPercyWeb(percyInput, percyToken);
85+
const result = runPercyWeb(percyInput);
9786
return await formatToolResult(result, "percy-web");
9887
} else if (input.integrationType === PercyIntegrationTypeEnum.AUTOMATE) {
9988
// First try Percy with BrowserStack SDK
@@ -142,15 +131,7 @@ export async function setUpPercyHandler(
142131
};
143132
const sdkResult = await runBstackSDKOnly(sdkInput, config, true);
144133
// Percy Automate instructions
145-
const percyToken = await fetchPercyToken(
146-
input.projectName,
147-
authorization,
148-
{ type: PercyIntegrationTypeEnum.AUTOMATE },
149-
);
150-
const percyAutomateResult = runPercyAutomateOnly(
151-
percyInput,
152-
percyToken,
153-
);
134+
const percyAutomateResult = runPercyAutomateOnly(percyInput);
154135

155136
// Combine steps: warning, SDK steps, Percy Automate steps
156137
const steps = [

‎src/tools/sdk-utils/percy-automate/handler.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { SDKSupportedLanguage } from "../common/types.js";
55

66
export function runPercyAutomateOnly(
77
input: SetUpPercyInput,
8-
percyToken: string,
98
): RunTestsInstructionResult {
109
const steps: RunTestsStep[] = [];
1110

@@ -26,7 +25,7 @@ export function runPercyAutomateOnly(
2625
steps.push({
2726
type: "instruction",
2827
title: "Set Percy Token in Environment",
29-
content: `Here is percy token if required {${percyToken}}`,
28+
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=<your Percy project token>) or export it in your shell (e.g. export PERCY_TOKEN="<your Percy project token>"). Do not paste the token into chat or commit it.`,
3029
});
3130

3231
steps.push({

‎src/tools/sdk-utils/percy-web/handler.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ import {
1010

1111
export let percyWebSetupInstructions = "";
1212

13-
export function runPercyWeb(
14-
input: SetUpPercyInput,
15-
percyToken: string,
16-
): RunTestsInstructionResult {
13+
export function runPercyWeb(input: SetUpPercyInput): RunTestsInstructionResult {
1714
const steps: RunTestsStep[] = [];
1815

1916
// Assume configuration is supported due to guardrails at orchestration layer
@@ -32,9 +29,12 @@ export function runPercyWeb(
3229
steps.push({
3330
type: "instruction",
3431
title: "Set Percy Token in Environment",
35-
content: `Set the environment variable for your project:
36-
export PERCY_TOKEN="${percyToken}"
37-
(For Windows: use 'setx PERCY_TOKEN "${percyToken}"' or 'set PERCY_TOKEN=${percyToken}' as appropriate.)`,
32+
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=<your Percy project token>) or export it in your shell:
33+
macOS/Linux: export PERCY_TOKEN="<your Percy project token>"
34+
Windows (PS): $env:PERCY_TOKEN="<your Percy project token>"
35+
Windows (CMD): set PERCY_TOKEN=<your Percy project token>
36+
37+
Do not paste the token into chat or commit it.`,
3838
});
3939

4040
steps.push({

‎tests/tools/percySdk.test.ts‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
22
import { registerPercyTools } from "../../src/tools/percy-sdk";
33
import { setUpPercyHandler, simulatePercyChangeHandler } from "../../src/tools/sdk-utils/handler";
44
import { updateTestsWithPercyCommands } from "../../src/tools/add-percy-snapshots";
5-
// PMAA-100: runPercyScan registration disabled — restore import alongside the test.
6-
// import { runPercyScan } from "../../src/tools/run-percy-scan";
5+
import { runPercyScan } from "../../src/tools/run-percy-scan";
76
import { fetchPercyChanges } from "../../src/tools/review-agent";
87
import { approveOrDeclinePercyBuild } from "../../src/tools/review-agent-utils/percy-approve-reject";
98

@@ -14,10 +13,9 @@ vi.mock("../../src/tools/sdk-utils/handler", () => ({
1413
vi.mock("../../src/tools/add-percy-snapshots", () => ({
1514
updateTestsWithPercyCommands: vi.fn(),
1615
}));
17-
// PMAA-100: runPercyScan registration disabled — restore mock alongside the test.
18-
// vi.mock("../../src/tools/run-percy-scan", () => ({
19-
// runPercyScan: vi.fn(),
20-
// }));
16+
vi.mock("../../src/tools/run-percy-scan", () => ({
17+
runPercyScan: vi.fn(),
18+
}));
2119
vi.mock("../../src/tools/review-agent", () => ({
2220
fetchPercyChanges: vi.fn(),
2321
}));
@@ -66,8 +64,7 @@ describe("Percy SDK Tools", () => {
6664
expect(toolNames).toContain("expandPercyVisualTesting");
6765
expect(toolNames).toContain("addPercySnapshotCommands");
6866
expect(toolNames).toContain("listTestFiles");
69-
// PMAA-100: runPercyScan registration disabled — restore once the token leak is fixed.
70-
// expect(toolNames).toContain("runPercyScan");
67+
expect(toolNames).toContain("runPercyScan");
7168
expect(toolNames).toContain("fetchPercyChanges");
7269
expect(toolNames).toContain("managePercyBuildApproval");
7370
});
@@ -121,15 +118,14 @@ describe("Percy SDK Tools", () => {
121118
expect(result.content[0].text).toContain("Commands added");
122119
});
123120

124-
// PMAA-100: runPercyScan registration disabled — restore once the token leak is fixed.
125-
// it("runPercyScan - SUCCESS", async () => {
126-
// (runPercyScan as any).mockResolvedValue({
127-
// content: [{ type: "text", text: "Percy scan started" }],
128-
// });
129-
//
130-
// const result = await handlers["runPercyScan"]({ projectName: "test" });
131-
// expect(result.content[0].text).toContain("Percy scan");
132-
// });
121+
it("runPercyScan - SUCCESS", async () => {
122+
(runPercyScan as any).mockResolvedValue({
123+
content: [{ type: "text", text: "Percy scan started" }],
124+
});
125+
126+
const result = await handlers["runPercyScan"]({ projectName: "test" });
127+
expect(result.content[0].text).toContain("Percy scan");
128+
});
133129

134130
it("fetchPercyChanges - SUCCESS", async () => {
135131
(fetchPercyChanges as any).mockResolvedValue({

‎tests/tools/runPercyScan.test.ts‎

Lines changed: 24 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,8 @@
11
import { describe, it, expect, vi, beforeEach, Mock } from "vitest";
22
import { runPercyScan } from "../../src/tools/run-percy-scan";
3-
import { fetchPercyToken } from "../../src/tools/sdk-utils/percy-web/fetchPercyToken";
43
import { storedPercyResults } from "../../src/lib/inmemory-store";
54
import { PercyIntegrationTypeEnum } from "../../src/tools/sdk-utils/common/types";
65

7-
vi.mock("../../src/lib/get-auth", () => ({
8-
getBrowserStackAuth: vi.fn().mockReturnValue("fake-user:fake-key"),
9-
}));
10-
vi.mock("../../src/tools/sdk-utils/percy-web/fetchPercyToken", () => ({
11-
fetchPercyToken: vi.fn(),
12-
}));
136
vi.mock("../../src/lib/inmemory-store", () => ({
147
storedPercyResults: { get: vi.fn(), set: vi.fn() },
158
}));
@@ -21,81 +14,50 @@ vi.mock("../../src/logger", () => ({
2114
default: { error: vi.fn(), info: vi.fn(), debug: vi.fn() },
2215
}));
2316

24-
const mockConfig = {
25-
"browserstack-username": "fake-user",
26-
"browserstack-access-key": "fake-key",
27-
};
28-
2917
describe("runPercyScan", () => {
3018
beforeEach(() => vi.clearAllMocks());
3119

32-
it("SUCCESS: returns Percy token and run instructions", async () => {
33-
(fetchPercyToken as Mock).mockResolvedValue("percy-token-abc");
20+
it("renders PERCY_TOKEN setup instructions with placeholder", async () => {
3421
(storedPercyResults.get as Mock).mockReturnValue(null);
3522

36-
const result = await runPercyScan(
37-
{
38-
projectName: "my-project",
39-
integrationType: PercyIntegrationTypeEnum.WEB,
40-
},
41-
mockConfig,
42-
);
23+
const result = await runPercyScan({
24+
projectName: "my-project",
25+
integrationType: PercyIntegrationTypeEnum.WEB,
26+
});
4327

44-
expect(result.content[0].text).toContain("percy-token-abc");
45-
expect(result.content[0].text).toContain("PERCY_TOKEN");
28+
const text = result.content[0].text as string;
29+
expect(text).toContain("PERCY_TOKEN");
30+
expect(text).toContain("<your Percy project token>");
31+
expect(text).toContain(".env");
4632
});
4733

48-
it("SUCCESS: includes updated file instructions when available", async () => {
49-
(fetchPercyToken as Mock).mockResolvedValue("percy-token-abc");
34+
it("includes updated file instructions when available", async () => {
5035
(storedPercyResults.get as Mock).mockReturnValue({
5136
projectName: "my-project",
5237
testFiles: { "/tests/login.test.js": true },
5338
detectedLanguage: "javascript",
5439
detectedTestingFramework: "jest",
5540
});
5641

57-
const result = await runPercyScan(
58-
{
59-
projectName: "my-project",
60-
integrationType: PercyIntegrationTypeEnum.WEB,
61-
},
62-
mockConfig,
63-
);
42+
const result = await runPercyScan({
43+
projectName: "my-project",
44+
integrationType: PercyIntegrationTypeEnum.WEB,
45+
});
6446

65-
expect(result.content[0].text).toContain("percy-token-abc");
47+
const text = result.content[0].text as string;
48+
expect(text).toContain("Updated files to run");
6649
});
6750

68-
it("SUCCESS: includes custom instruction steps", async () => {
69-
(fetchPercyToken as Mock).mockResolvedValue("percy-token-abc");
51+
it("includes custom instruction steps", async () => {
7052
(storedPercyResults.get as Mock).mockReturnValue(null);
7153

72-
const result = await runPercyScan(
73-
{
74-
projectName: "my-project",
75-
integrationType: PercyIntegrationTypeEnum.WEB,
76-
instruction: "npx percy exec -- npx playwright test",
77-
},
78-
mockConfig,
79-
);
80-
81-
expect(result.content[0].text).toContain("percy-token-abc");
82-
expect(result.content[0].text).toContain("npx percy exec");
83-
});
84-
85-
it("FAIL: throws when Percy token fetch fails", async () => {
86-
(fetchPercyToken as Mock).mockRejectedValue(
87-
new Error("Percy token not found"),
88-
);
89-
(storedPercyResults.get as Mock).mockReturnValue(null);
54+
const result = await runPercyScan({
55+
projectName: "my-project",
56+
integrationType: PercyIntegrationTypeEnum.WEB,
57+
instruction: "npx percy exec -- npx playwright test",
58+
});
9059

91-
await expect(
92-
runPercyScan(
93-
{
94-
projectName: "bad-project",
95-
integrationType: PercyIntegrationTypeEnum.WEB,
96-
},
97-
mockConfig,
98-
),
99-
).rejects.toThrow("Percy token not found");
60+
const text = result.content[0].text as string;
61+
expect(text).toContain("npx percy exec");
10062
});
10163
});

0 commit comments

Comments
 (0)