Skip to content

Commit 3fa9324

Browse files
tsullivantsullivan
committed
Minor cleanup
1 parent 52e532c commit 3fa9324

File tree

4 files changed

+4
-82
lines changed

4 files changed

+4
-82
lines changed

‎src/platform/plugins/shared/files/server/routes/file_kind/helpers.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ type ResultOrHttpError =
2424
export async function getById(
2525
fileService: FileServiceStart,
2626
id: string,
27-
fileKind: string
27+
_fileKind: string
2828
): Promise<ResultOrHttpError> {
2929
let result: undefined | File;
3030
try {
@@ -57,7 +57,6 @@ export function validateMimeType(
5757
}
5858

5959
const allowedMimeTypes = fileKind.allowedMimeTypes;
60-
6160
if (!allowedMimeTypes || allowedMimeTypes.length === 0) {
6261
return;
6362
}

‎src/platform/plugins/shared/files/server/routes/file_kind/integration_tests/file_kind_http.test.ts‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -488,26 +488,6 @@ describe('File kind HTTP API', () => {
488488

489489
expect(result.body.file.mimeType).toBeUndefined();
490490
});
491-
492-
test('should not leak information about allowed MIME types in error messages', async () => {
493-
const result = await request
494-
.post(root, `/api/files/files/${fileKind}`)
495-
.set('x-elastic-internal-origin', 'files-test')
496-
.send({
497-
name: 'archive.zip',
498-
mimeType: 'application/zip',
499-
alt: 'zip archive',
500-
meta: {},
501-
})
502-
.expect(400);
503-
504-
// Should not reveal which MIME types are allowed
505-
expect(result.body.message).toBe('File type is not supported');
506-
expect(result.body.message).not.toContain('image/png');
507-
expect(result.body.message).not.toContain('image/jpeg');
508-
expect(result.body.message).not.toContain('application/pdf');
509-
expect(result.body.message).not.toContain('test-file-kind');
510-
});
511491
});
512492

513493
describe('file extension validation on download', () => {
@@ -517,7 +497,6 @@ describe('File kind HTTP API', () => {
517497
mimeType: 'image/png',
518498
});
519499

520-
// Upload content
521500
await request
522501
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
523502
.set('Content-Type', 'application/octet-stream')
@@ -547,7 +526,6 @@ describe('File kind HTTP API', () => {
547526
.send('image data')
548527
.expect(200);
549528

550-
// Download with correct extension should work
551529
const result = await request
552530
.get(root, `/api/files/files/${fileKind}/${id}/blob/image.png`)
553531
.set('x-elastic-internal-origin', 'files-test')
@@ -620,29 +598,5 @@ describe('File kind HTTP API', () => {
620598
.set('x-elastic-internal-origin', 'files-test')
621599
.expect(200);
622600
});
623-
624-
test('should not leak information about expected extensions in error messages', async () => {
625-
const { id } = await createFile({
626-
name: 'document.pdf',
627-
mimeType: 'application/pdf',
628-
});
629-
630-
await request
631-
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
632-
.set('Content-Type', 'application/octet-stream')
633-
.set('x-elastic-internal-origin', 'files-test')
634-
.send('pdf content')
635-
.expect(200);
636-
637-
const result = await request
638-
.get(root, `/api/files/files/${fileKind}/${id}/blob/document.txt`)
639-
.set('x-elastic-internal-origin', 'files-test')
640-
.expect(400);
641-
642-
// Should not reveal which extensions are expected
643-
expect(result.body.message).toBe('File extension does not match file type');
644-
expect(result.body.message).not.toContain('pdf');
645-
expect(result.body.message).not.toContain('application/pdf');
646-
});
647601
});
648602
});

‎src/platform/plugins/shared/files/server/routes/integration_tests/routes.test.ts‎

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ describe('File HTTP API', () => {
360360
.expect(200);
361361

362362
const { body: buffer, header } = await request
363-
// By providing a file name like "myfilename.pdf" we imply that we want a pdf
363+
// "myfilename.pdf" has a mime type that matches the metadata
364364
.get(root, `/api/files/public/blob/myfilename.pdf?token=${token}`)
365365
.set('x-elastic-internal-origin', 'files-test')
366366
.buffer()
@@ -377,7 +377,6 @@ describe('File HTTP API', () => {
377377
mimeType: 'application/pdf',
378378
});
379379

380-
// Share the file
381380
const {
382381
body: { token },
383382
} = await request
@@ -386,7 +385,6 @@ describe('File HTTP API', () => {
386385
.send({})
387386
.expect(200);
388387

389-
// Upload content
390388
await request
391389
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
392390
.set('Content-Type', 'application/octet-stream')
@@ -409,7 +407,6 @@ describe('File HTTP API', () => {
409407
mimeType: 'image/png',
410408
});
411409

412-
// Share the file
413410
const {
414411
body: { token },
415412
} = await request
@@ -418,7 +415,6 @@ describe('File HTTP API', () => {
418415
.send({})
419416
.expect(200);
420417

421-
// Upload content
422418
await request
423419
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
424420
.set('Content-Type', 'application/octet-stream')
@@ -444,7 +440,6 @@ describe('File HTTP API', () => {
444440
mimeType: 'text/plain',
445441
});
446442

447-
// Share the file
448443
const {
449444
body: { token },
450445
} = await request
@@ -453,7 +448,6 @@ describe('File HTTP API', () => {
453448
.send({})
454449
.expect(200);
455450

456-
// Upload content
457451
await request
458452
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
459453
.set('Content-Type', 'application/octet-stream')
@@ -468,8 +462,6 @@ describe('File HTTP API', () => {
468462
.buffer()
469463
.expect(200);
470464

471-
// Content-type comes from stored file MIME type, not URL filename
472-
// Should be either the stored MIME type or default fallback - but never from URL filename
473465
expect(['text/plain', 'text/plain; charset=utf-8', 'application/octet-stream']).toContain(
474466
header['content-type']
475467
);
@@ -482,7 +474,6 @@ describe('File HTTP API', () => {
482474
mimeType: 'image/jpeg',
483475
});
484476

485-
// Share the file
486477
const {
487478
body: { token },
488479
} = await request
@@ -491,7 +482,6 @@ describe('File HTTP API', () => {
491482
.send({})
492483
.expect(200);
493484

494-
// Upload content
495485
await request
496486
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
497487
.set('Content-Type', 'application/octet-stream')
@@ -517,7 +507,6 @@ describe('File HTTP API', () => {
517507
mimeType: 'text/plain',
518508
});
519509

520-
// Share the file
521510
const {
522511
body: { token },
523512
} = await request
@@ -526,7 +515,6 @@ describe('File HTTP API', () => {
526515
.send({})
527516
.expect(200);
528517

529-
// Upload content
530518
await request
531519
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
532520
.set('Content-Type', 'application/octet-stream')
@@ -542,8 +530,6 @@ describe('File HTTP API', () => {
542530
.buffer()
543531
.expect(200);
544532

545-
// Content-type comes from stored file MIME type, not URL filename
546-
// Note: Text files may include charset information
547533
expect(response.header['content-type']).toMatch(/^text\/plain(; charset=utf-8)?$/);
548534

549535
// For text content with .buffer(), use response.text instead of response.body
@@ -563,7 +549,6 @@ describe('File HTTP API', () => {
563549
mimeType: 'application/json',
564550
});
565551

566-
// Share the file
567552
const {
568553
body: { token },
569554
} = await request
@@ -572,7 +557,6 @@ describe('File HTTP API', () => {
572557
.send({})
573558
.expect(200);
574559

575-
// Upload content
576560
await request
577561
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
578562
.set('Content-Type', 'application/octet-stream')
@@ -592,13 +576,12 @@ describe('File HTTP API', () => {
592576
expect(result.body.message).not.toContain('application/json');
593577
});
594578

595-
test('prevents MIME type manipulation through URL filename (security fix)', async () => {
579+
test('prevents MIME type manipulation through URL filename', async () => {
596580
const { id } = await createFile({
597581
name: 'safe-document.pdf',
598582
mimeType: 'application/pdf',
599583
});
600584

601-
// Share the file
602585
const {
603586
body: { token },
604587
} = await request
@@ -607,31 +590,18 @@ describe('File HTTP API', () => {
607590
.send({})
608591
.expect(200);
609592

610-
// Upload content
611593
await request
612594
.put(root, `/api/files/files/${fileKind}/${id}/blob`)
613595
.set('Content-Type', 'application/octet-stream')
614596
.set('x-elastic-internal-origin', 'files-test')
615597
.send('PDF content')
616598
.expect(200);
617599

618-
// Security layer 1: Extension validation blocks dangerous mismatched downloads
600+
// Extension validation blocks dangerous mismatched downloads
619601
await request
620602
.get(root, `/api/files/public/blob/malicious-script.html?token=${token}`)
621603
.set('x-elastic-internal-origin', 'files-test')
622604
.expect(400); // Correctly blocked!
623-
624-
// Security layer 2: When download is allowed, MIME type comes from server, not URL
625-
const { body: buffer, header } = await request
626-
.get(root, `/api/files/public/blob/document.pdf?token=${token}`)
627-
.set('x-elastic-internal-origin', 'files-test')
628-
.buffer()
629-
.expect(200);
630-
631-
// Content-type comes from stored file MIME type, never from URL filename
632-
expect(header['content-type']).toEqual('application/pdf');
633-
expect(header['content-disposition']).toEqual('attachment; filename=document.pdf');
634-
expect(buffer.toString('utf8')).toEqual('PDF content');
635605
});
636606
});
637607
});

‎src/platform/plugins/shared/files/server/test_utils/setup_integration_environment.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ export async function setupIntegrationEnvironment() {
100100
const testHttpConfig = { requiredPrivileges: ['myapp'] };
101101
const myFileKind = {
102102
id: fileKind,
103-
// Allow MIME types used in existing tests, so we can test validation with forbidden types
104103
allowedMimeTypes: [
105104
'image/png',
106105
'image/jpeg',

0 commit comments

Comments
 (0)