Skip to content

Commit dad46f6

Browse files
OrKoNLightning00Blade
authored andcommitted
feat: ensure extensions for file outputs (ChromeDevTools#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 ChromeDevTools#1864 --------- Co-authored-by: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com>
1 parent de79df9 commit dad46f6

12 files changed

Lines changed: 116 additions & 21 deletions

File tree

src/McpContext.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ import {Locator} from './third_party/index.js';
3333
import {PredefinedNetworkConditions} from './third_party/index.js';
3434
import {listPages} from './tools/pages.js';
3535
import {CLOSE_PAGE_ERROR} from './tools/ToolDefinition.js';
36-
import type {Context, DevToolsData} from './tools/ToolDefinition.js';
36+
import type {
37+
Context,
38+
DevToolsData,
39+
SupportedExtensions,
40+
} from './tools/ToolDefinition.js';
3741
import type {TraceResult} from './trace-processing/parse.js';
3842
import type {
3943
EmulationSettings,
@@ -46,7 +50,7 @@ import {
4650
ExtensionRegistry,
4751
type InstalledExtension,
4852
} from './utils/ExtensionRegistry.js';
49-
import {saveTemporaryFile} from './utils/files.js';
53+
import {ensureExtension, saveTemporaryFile} from './utils/files.js';
5054
import {getNetworkMultiplierFromString} from './WaitForHelper.js';
5155

5256
interface McpContextOptions {
@@ -801,10 +805,14 @@ export class McpContext implements Context {
801805
}
802806
async saveFile(
803807
data: Uint8Array<ArrayBufferLike>,
804-
filename: string,
808+
clientProvidedFilePath: string,
809+
extension: SupportedExtensions,
805810
): Promise<{filename: string}> {
806811
try {
807-
const filePath = path.resolve(filename);
812+
const filePath = ensureExtension(
813+
path.resolve(clientProvidedFilePath),
814+
extension,
815+
);
808816
await fs.mkdir(path.dirname(filePath), {recursive: true});
809817
await fs.writeFile(filePath, data);
810818
return {filename: filePath};

src/McpResponse.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -414,11 +414,12 @@ export class McpResponse implements Response {
414414
if (textSnapshot) {
415415
const formatter = new SnapshotFormatter(textSnapshot);
416416
if (this.#snapshotParams.filePath) {
417-
await context.saveFile(
417+
const result = await context.saveFile(
418418
new TextEncoder().encode(formatter.toString()),
419419
this.#snapshotParams.filePath,
420+
'.txt',
420421
);
421-
snapshot = this.#snapshotParams.filePath;
422+
snapshot = result.filename;
422423
} else {
423424
snapshot = formatter;
424425
}
@@ -440,7 +441,8 @@ export class McpResponse implements Response {
440441
fetchData: true,
441442
requestFilePath: this.#attachedNetworkRequestOptions?.requestFilePath,
442443
responseFilePath: this.#attachedNetworkRequestOptions?.responseFilePath,
443-
saveFile: (data, filename) => context.saveFile(data, filename),
444+
saveFile: (data, filename, extension) =>
445+
context.saveFile(data, filename, extension),
444446
redactNetworkHeaders: this.#redactNetworkHeaders,
445447
});
446448
detailedNetworkRequest = formatter;
@@ -590,7 +592,8 @@ export class McpResponse implements Response {
590592
context.getNetworkRequestStableId(request) ===
591593
this.#networkRequestsOptions?.networkRequestIdInDevToolsUI,
592594
fetchData: false,
593-
saveFile: (data, filename) => context.saveFile(data, filename),
595+
saveFile: (data, filename, extension) =>
596+
context.saveFile(data, filename, extension),
594597
redactNetworkHeaders: this.#redactNetworkHeaders,
595598
}),
596599
),

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
@@ -137,6 +137,18 @@ export interface Response {
137137
setListWebMcpTools(): void;
138138
}
139139

140+
export type SupportedExtensions =
141+
| '.png'
142+
| '.jpeg'
143+
| '.webp'
144+
| '.json'
145+
| '.network-response'
146+
| '.network-request'
147+
| '.html'
148+
| '.txt'
149+
| '.csv'
150+
| '.json.gz';
151+
140152
/**
141153
* Only add methods required by tools/*.
142154
*/
@@ -171,7 +183,8 @@ export type Context = Readonly<{
171183
): Promise<{filepath: string}>;
172184
saveFile(
173185
data: Uint8Array<ArrayBufferLike>,
174-
filename: string,
186+
clientProvidedFilePath: string,
187+
extension: SupportedExtensions,
175188
): Promise<{filename: string}>;
176189
waitForTextOnPage(
177190
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)