Skip to content

Commit

Permalink
Update file-type to 18.2.0 (#8304)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Cousens <[email protected]>
Co-authored-by: Emma Hamilton <[email protected]>
Co-authored-by: Josh Calder <[email protected]>
  • Loading branch information
4 people authored Feb 27, 2023
1 parent 279a50c commit aad6314
Show file tree
Hide file tree
Showing 11 changed files with 1,168 additions and 194 deletions.
5 changes: 5 additions & 0 deletions .changeset/esm-not-found.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-6/core': patch
---

Updates `image-size` to `5.0.0` to mitigate [CVE-2022-36313](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-36313)
16 changes: 14 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"@types/node-fetch": "^2.5.12",
"@typescript-eslint/eslint-plugin": "^5.7.0",
"@typescript-eslint/parser": "^5.7.0",
"esbuild-jest": "^0.5.0",
"eslint": "^8.0.0",
"eslint-plugin-import": "2.25.3",
"eslint-plugin-jest": "^26.0.0",
Expand Down Expand Up @@ -96,7 +97,10 @@
"prisma-utils",
"scripts/*"
],
"exports": true
"exports": true,
"___experimentalFlags_WILL_CHANGE_IN_PATCH": {
"keepDynamicImportAsDynamicImportInCommonJS": true
}
},
"manypkg": {
"defaultBranch": "main"
Expand All @@ -108,7 +112,15 @@
"!**/*.d.ts",
"!packages/**/dist/**",
"!packages/core/src/fields/**/test-fixtures.{js,ts}"
]
],
"transform": {
"^.+\\.[tj]sx?$": [
"esbuild-jest",
{
"target": "esnext"
}
]
}
},
"resolutions": {
"**/@types/node": "^18.11.14"
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,13 @@
"execa": "^5.1.1",
"express": "^4.17.1",
"fast-deep-equal": "^3.1.3",
"file-type": "^18.2.0",
"filenamify": "^4.3.0",
"form-data": "4.0.0",
"fs-extra": "^11.0.0",
"graphql": "^16.6.0",
"graphql-upload": "^15.0.2",
"image-size": "^1.0.0",
"image-type": "^4.1.0",
"inflection": "^1.13.1",
"intersection-observer": "^0.12.0",
"meow": "^9.0.0",
Expand Down
13 changes: 6 additions & 7 deletions packages/core/src/lib/assets/createImagesContext.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,27 @@
import { v4 as uuid } from 'uuid';
import fromBuffer from 'image-type';
import imageSize from 'image-size';
import { KeystoneConfig, ImageMetadata, ImagesContext } from '../../types';
import { KeystoneConfig, ImagesContext } from '../../types';
import { ImageAdapter } from './types';
import { localImageAssetsAPI } from './local';
import { s3ImageAssetsAPI } from './s3';
import { streamToBuffer } from './utils';

export function getImageMetadataFromBuffer(buffer: Buffer): ImageMetadata {
const fileType = fromBuffer(buffer);
async function getImageMetadataFromBuffer(buffer: Buffer) {
const fileType = await (await import('file-type')).fileTypeFromBuffer(buffer);
if (!fileType) {
throw new Error('File type not found');
}

const extension = fileType.ext;
const { ext: extension } = fileType;
if (extension !== 'jpg' && extension !== 'png' && extension !== 'webp' && extension !== 'gif') {
throw new Error(`${extension} is not a supported image type`);
}

const { height, width } = imageSize(buffer);

if (width === undefined || height === undefined) {
throw new Error('Height and width could not be found for image');
}

return { width, height, filesize: buffer.length, extension };
}

Expand Down Expand Up @@ -54,7 +53,7 @@ export function createImagesContext(config: KeystoneConfig): ImagesContext {
const { transformName = () => uuid() } = storageConfig;

const buffer = await streamToBuffer(stream);
const { extension, ...rest } = getImageMetadataFromBuffer(buffer);
const { extension, ...rest } = await getImageMetadataFromBuffer(buffer);

const id = await transformName(originalFilename, extension);

Expand Down
3 changes: 0 additions & 3 deletions packages/core/src/lib/assets/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { pipeline } from 'stream';
import fs from 'fs-extra';

import { StorageConfig } from '../../types';
// import { getImageMetadataFromBuffer } from './createImagesContext';
import { FileAdapter, ImageAdapter } from './types';

export function localImageAssetsAPI(
Expand All @@ -14,8 +13,6 @@ export function localImageAssetsAPI(
return storageConfig.generateUrl(`/${id}.${extension}`);
},
async upload(buffer, id, extension) {
// const buffer = await streamToBuffer(stream);

await fs.writeFile(path.join(storageConfig.storagePath, `${id}.${extension}`), buffer);
},
async delete(id, extension) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/lib/assets/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Readable } from 'stream';
import { ImageExtension, FileMetadata } from '../../types';

export type ImageAdapter = {
upload(stream: Buffer, id: string, extension: string): Promise<void>;
upload(buffer: Buffer, id: string, extension: string): Promise<void>;
delete(id: string, extension: ImageExtension): Promise<void>;
url(id: string, extension: ImageExtension): Promise<string>;
};
Expand Down
237 changes: 237 additions & 0 deletions tests/api-tests/fields/files.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
import path from 'path';
import { createHash } from 'crypto';
import os from 'os';
import fs from 'fs-extra';
import fetch from 'node-fetch';
// @ts-ignore
import Upload from 'graphql-upload/Upload.js';
import mime from 'mime';
import { file, text } from '@keystone-6/core/fields';
import { list } from '@keystone-6/core';
import { KeystoneConfig, StorageConfig } from '@keystone-6/core/types';
import { setupTestRunner } from '@keystone-6/api-tests/test-runner';
import { allowAll } from '@keystone-6/core/access';
import { apiTestConfig } from '../utils';

const fieldPath = path.resolve(__dirname, '../../..', 'packages/core/src/fields/types');

export const prepareFile = (_filePath: string, kind: 'image' | 'file') => {
const filePath = path.resolve(fieldPath, kind, 'test-files', _filePath);
const upload = new Upload();
upload.resolve({
createReadStream: () => fs.createReadStream(filePath),
filename: path.basename(filePath),
// @ts-ignore
mimetype: mime.getType(filePath),
encoding: 'utf-8',
});
return { upload };
};

type MatrixValue = 's3' | 'local';
export const testMatrix: Array<MatrixValue> = ['local'];

if (process.env.S3_BUCKET_NAME) {
testMatrix.push('s3');
}

const s3DefaultStorage = {
kind: 's3',
bucketName: process.env.S3_BUCKET_NAME!,
accessKeyId: process.env.S3_ACCESS_KEY_ID!,
secretAccessKey: process.env.S3_SECRET_ACCESS_KEY!,
region: process.env.S3_REGION!,
endpoint: process.env.S3_ENDPOINT,
forcePathStyle: process.env.S3_FORCE_PATH_STYLE === 'true',
} as const;

const getRunner = ({
storage,
fields,
}: {
storage: Record<string, StorageConfig>;
fields: KeystoneConfig['lists'][string]['fields'];
}) =>
setupTestRunner({
config: apiTestConfig({
db: {},
storage,
lists: {
Test: list({
access: allowAll,
fields: {
name: text(),
...fields,
},
}),
},
}),
});

const getFileHash = async (
filename: string,
config: { matrixValue: 's3' } | { matrixValue: 'local'; folder: string }
) => {
let contentFromURL;

if (config.matrixValue === 's3') {
contentFromURL = await fetch(filename).then(x => x.buffer());
} else {
contentFromURL = await fs.readFile(path.join(config.folder, filename));
}

return createHash('sha1').update(contentFromURL).digest('hex');
};

const checkFile = async (
filename: string,
config: { matrixValue: 's3' } | { matrixValue: 'local'; folder: string }
) => {
if (config.matrixValue === 's3') {
return await fetch(filename).then(x => x.status === 200);
} else {
return fs.existsSync(path.join(config.folder, filename));
}
};

describe('File - Crud special tests', () => {
const filename = 'keystone.jpeg';
const fileHash = createHash('sha1')
.update(fs.readFileSync(path.resolve(fieldPath, 'image/test-files', filename)))
.digest('hex');

const createItem = (context: any) =>
context.query.Test.createOne({
data: { secretFile: prepareFile(filename, 'image') },
query: `
id
secretFile {
filename
__typename
filesize
url
}
`,
});

for (let matrixValue of testMatrix) {
const getConfig = (): StorageConfig => ({
...(matrixValue === 's3'
? { ...s3DefaultStorage, preserve: false, type: 'file' }
: {
kind: 'local',
type: 'file',
storagePath: fs.mkdtempSync(path.join(os.tmpdir(), 'file-local-test')),
serverRoute: { path: '/files' },
generateUrl: (path: string) => `http://localhost:3000/files${path}`,
}),
});

const fields = { secretFile: file({ storage: 'test_file' }) };

describe(matrixValue, () => {
describe('Create - upload', () => {
const config = getConfig();
const hashConfig: { matrixValue: 'local'; folder: string } | { matrixValue: 's3' } =
config.kind === 'local'
? { matrixValue: 'local', folder: `${config.storagePath}/`! }
: { matrixValue: config.kind };
test(
'Upload values should match expected corrected',
getRunner({ storage: { test_file: { ...config } }, fields })(async ({ context }) => {
const data = await createItem(context);
expect(data).not.toBe(null);

expect(data.secretFile).toEqual({
/*
The url and filename here include a hash, and currently what we are doing
is just checking the url is modified correctly - that said, this sucks
as a test
*/
url:
matrixValue === 's3'
? expect.stringContaining(`/${data.secretFile.filename}`)
: `http://localhost:3000/files/${data.secretFile.filename}`,
filename: data.secretFile.filename,
__typename: 'FileFieldOutput',
filesize: 3250,
});
// check file exists at location
expect(fileHash).toEqual(await getFileHash(data.secretFile.filename, hashConfig));
})
);
});

describe('After Operation Hook', () => {
const config = getConfig();
const hashConfig =
config.kind === 'local'
? { matrixValue: config.kind, folder: `${config.storagePath}/`! }
: { matrixValue: config.kind };
test(
'with preserve: true',
getRunner({ storage: { test_file: { ...config, preserve: true } }, fields })(
async ({ context }) => {
const {
id,
secretFile: { filename },
} = await createItem(context);

expect(await checkFile(filename, hashConfig)).toBeTruthy();

const {
secretFile: { filename: filename2 },
} = await context.query.Test.updateOne({
where: { id },
data: { secretFile: prepareFile('thinkmill.jpg', 'file') },
query: `secretFile { filename }`,
});

expect(await checkFile(filename, hashConfig)).toBeTruthy();
expect(await checkFile(filename2, hashConfig)).toBeTruthy();

await context.query.Test.deleteOne({ where: { id } });

expect(await checkFile(filename, hashConfig)).toBeTruthy();
// TODO test that just nulling the field doesn't delete it
}
)
);
test(
'with preserve: false',
getRunner({
storage: { test_file: { ...config, preserve: false } },
fields,
})(async ({ context }) => {
const {
id,
secretFile: { filename },
} = await createItem(context);

expect(await checkFile(filename, hashConfig)).toBeTruthy();

const {
secretFile: { filename: filename2 },
} = await context.query.Test.updateOne({
where: { id },
data: { secretFile: prepareFile('thinkmill.jpg', 'file') },
query: `
secretFile {
filename
}`,
});

expect(await checkFile(filename2, hashConfig)).toBeTruthy();
expect(await checkFile(filename, hashConfig)).toBeFalsy();

await context.query.Test.deleteOne({ where: { id } });

expect(await checkFile(filename2, hashConfig)).toBeFalsy();

// TODO test that just nulling the field removes the file
})
);
});
});
}
});
Loading

0 comments on commit aad6314

Please sign in to comment.