Skip to content

Commit 60303e1

Browse files
OrKoNLightning00Blade
authored andcommitted
feat: ensure extensions for file outputs (#1867)
This PR ensures the extensions for the file outputs of different types minimizing the chance of misuse. The input filePath, thus, might be modified but it should not be an issue for clients as the final output path is returned to the clients in the response. Closes #1864 --------- Co-authored-by: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com>
1 parent 345deac commit 60303e1

12 files changed

Lines changed: 112 additions & 20 deletions

File tree

src/McpContext.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import type {
3737
Context,
3838
DevToolsData,
3939
ContextPage,
40+
SupportedExtensions,
4041
} from './tools/ToolDefinition.js';
4142
import type {TraceResult} from './trace-processing/parse.js';
4243
import type {
@@ -50,7 +51,7 @@ import {
5051
ExtensionRegistry,
5152
type InstalledExtension,
5253
} from './utils/ExtensionRegistry.js';
53-
import {saveTemporaryFile} from './utils/files.js';
54+
import {ensureExtension, saveTemporaryFile} from './utils/files.js';
5455
import {getNetworkMultiplierFromString} from './WaitForHelper.js';
5556

5657
interface McpContextOptions {
@@ -954,10 +955,14 @@ export class McpContext implements Context {
954955
}
955956
async saveFile(
956957
data: Uint8Array<ArrayBufferLike>,
957-
filename: string,
958+
clientProvidedFilePath: string,
959+
extension: SupportedExtensions,
958960
): Promise<{filename: string}> {
959961
try {
960-
const filePath = path.resolve(filename);
962+
const filePath = ensureExtension(
963+
path.resolve(clientProvidedFilePath),
964+
extension,
965+
);
961966
await fs.mkdir(path.dirname(filePath), {recursive: true});
962967
await fs.writeFile(filePath, data);
963968
return {filename: filePath};

src/McpResponse.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -403,11 +403,12 @@ export class McpResponse implements Response {
403403
if (textSnapshot) {
404404
const formatter = new SnapshotFormatter(textSnapshot);
405405
if (this.#snapshotParams.filePath) {
406-
await context.saveFile(
406+
const result = await context.saveFile(
407407
new TextEncoder().encode(formatter.toString()),
408408
this.#snapshotParams.filePath,
409+
'.txt',
409410
);
410-
snapshot = this.#snapshotParams.filePath;
411+
snapshot = result.filename;
411412
} else {
412413
snapshot = formatter;
413414
}
@@ -429,7 +430,8 @@ export class McpResponse implements Response {
429430
fetchData: true,
430431
requestFilePath: this.#attachedNetworkRequestOptions?.requestFilePath,
431432
responseFilePath: this.#attachedNetworkRequestOptions?.responseFilePath,
432-
saveFile: (data, filename) => context.saveFile(data, filename),
433+
saveFile: (data, filename, extension) =>
434+
context.saveFile(data, filename, extension),
433435
redactNetworkHeaders: this.#redactNetworkHeaders,
434436
});
435437
detailedNetworkRequest = formatter;
@@ -573,7 +575,8 @@ export class McpResponse implements Response {
573575
context.getNetworkRequestStableId(request) ===
574576
this.#networkRequestsOptions?.networkRequestIdInDevToolsUI,
575577
fetchData: false,
576-
saveFile: (data, filename) => context.saveFile(data, filename),
578+
saveFile: (data, filename, extension) =>
579+
context.saveFile(data, filename, extension),
577580
redactNetworkHeaders: this.#redactNetworkHeaders,
578581
}),
579582
),

src/formatters/NetworkFormatter.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export interface NetworkFormatterOptions {
2424
saveFile?: (
2525
data: Uint8Array<ArrayBufferLike>,
2626
filename: string,
27+
extension: '.network-request' | '.network-response',
2728
) => Promise<{filename: string}>;
2829
redactNetworkHeaders: boolean;
2930
}
@@ -88,11 +89,12 @@ export class NetworkFormatter {
8889
throw new Error('saveFile is not provided');
8990
}
9091
if (data) {
91-
await this.#options.saveFile(
92+
const result = await this.#options.saveFile(
9293
Buffer.from(data),
9394
this.#options.requestFilePath,
95+
'.network-request',
9496
);
95-
this.#requestBodyFilePath = this.#options.requestFilePath;
97+
this.#requestBodyFilePath = result.filename;
9698
} else {
9799
this.#requestBody = requestBodyNotAvailableMessage;
98100
}
@@ -119,8 +121,12 @@ export class NetworkFormatter {
119121
if (!this.#options.saveFile) {
120122
throw new Error('saveFile is not provided');
121123
}
122-
await this.#options.saveFile(buffer, this.#options.responseFilePath);
123-
this.#responseBodyFilePath = this.#options.responseFilePath;
124+
const result = await this.#options.saveFile(
125+
buffer,
126+
this.#options.responseFilePath,
127+
'.network-response',
128+
);
129+
this.#responseBodyFilePath = result.filename;
124130
} catch {
125131
// Flatten error handling for buffer() failure and save failure
126132
}

src/tools/ToolDefinition.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,18 @@ export interface Response {
138138
setListInPageTools(): void;
139139
}
140140

141+
export type SupportedExtensions =
142+
| '.png'
143+
| '.jpeg'
144+
| '.webp'
145+
| '.json'
146+
| '.network-response'
147+
| '.network-request'
148+
| '.html'
149+
| '.txt'
150+
| '.csv'
151+
| '.json.gz';
152+
141153
/**
142154
* Only add methods required by tools/*.
143155
*/
@@ -172,7 +184,8 @@ export type Context = Readonly<{
172184
): Promise<{filepath: string}>;
173185
saveFile(
174186
data: Uint8Array<ArrayBufferLike>,
175-
filename: string,
187+
clientProvidedFilePath: string,
188+
extension: SupportedExtensions,
176189
): Promise<{filename: string}>;
177190
waitForTextOnPage(
178191
text: string[],

src/tools/lighthouse.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,12 @@ export const lighthouseAudit = definePageTool({
107107
const report = generateReport(lhr, format);
108108
const data = encoder.encode(report);
109109
if (outputDirPath) {
110-
const reportPath = path.join(outputDirPath, `report.${format}`);
111-
const {filename} = await context.saveFile(data, reportPath);
110+
const reportPath = path.join(outputDirPath, `report`);
111+
const {filename} = await context.saveFile(
112+
data,
113+
reportPath,
114+
`.${format}`,
115+
);
112116
reportPaths.push(filename);
113117
} else {
114118
const {filepath} = await context.saveTemporaryFile(

src/tools/memory.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
import {zod} from '../third_party/index.js';
8+
import {ensureExtension} from '../utils/files.js';
89

910
import {ToolCategory} from './categories.js';
1011
import {definePageTool} from './ToolDefinition.js';
@@ -25,7 +26,7 @@ export const takeMemorySnapshot = definePageTool({
2526
const page = request.page;
2627

2728
await page.pptrPage.captureHeapSnapshot({
28-
path: request.params.filePath,
29+
path: ensureExtension(request.params.filePath, '.heapsnapshot'),
2930
});
3031

3132
response.appendResponseLine(

src/tools/performance.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,11 @@ async function stopTracingAndAppendOutput(
197197
});
198198
});
199199
}
200-
const file = await context.saveFile(dataToWrite, filePath);
200+
const file = await context.saveFile(
201+
dataToWrite,
202+
filePath,
203+
filePath.endsWith('.gz') ? '.json.gz' : '.json',
204+
);
201205
response.appendResponseLine(
202206
`The raw trace data was saved to ${file.filename}.`,
203207
);

src/tools/screencast.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import path from 'node:path';
1010

1111
import {zod} from '../third_party/index.js';
1212
import type {ScreenRecorder} from '../third_party/index.js';
13+
import {ensureExtension} from '../utils/files.js';
1314

1415
import {ToolCategory} from './categories.js';
1516
import {definePageTool} from './ToolDefinition.js';
@@ -46,7 +47,7 @@ export const startScreencast = definePageTool({
4647
}
4748

4849
const filePath = request.params.path ?? (await generateTempFilePath());
49-
const resolvedPath = path.resolve(filePath);
50+
const resolvedPath = ensureExtension(path.resolve(filePath), '.mp4');
5051

5152
const page = request.page;
5253

src/tools/screenshot.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,12 @@ export const screenshot = definePageTool({
8787
}
8888

8989
if (request.params.filePath) {
90-
const file = await context.saveFile(screenshot, request.params.filePath);
91-
response.appendResponseLine(`Saved screenshot to ${file.filename}.`);
90+
const result = await context.saveFile(
91+
screenshot,
92+
request.params.filePath,
93+
`.${format}`,
94+
);
95+
response.appendResponseLine(`Saved screenshot to ${result.filename}.`);
9296
} else if (screenshot.length >= 2_000_000) {
9397
const {filepath} = await context.saveTemporaryFile(
9498
screenshot,

src/utils/files.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,11 @@ export async function saveTemporaryFile(
2424
throw new Error('Could not save a file', {cause: err});
2525
}
2626
}
27+
28+
export function ensureExtension(
29+
filepath: string,
30+
extension: `.${string}`,
31+
): string {
32+
const ext = path.extname(filepath);
33+
return filepath.slice(0, filepath.length - ext.length) + extension;
34+
}

0 commit comments

Comments
 (0)