From 0c0714857984406c8a07b82e43ca2201d90a453c Mon Sep 17 00:00:00 2001 From: Stefan Kairinos <118008817+SKairinos@users.noreply.github.com> Date: Wed, 29 Nov 2023 16:43:07 +0000 Subject: [PATCH] feat: validate imports (#1838) * feat: validate imports * disable codecov comments --- .codecov.yml | 9 +- game_frontend/src/pyodide/syntax.test.ts | 112 ++++++++++++++++++++ game_frontend/src/pyodide/syntax.ts | 126 +++++++++++++++++++++++ game_frontend/src/pyodide/webWorker.ts | 43 +++++++- 4 files changed, 279 insertions(+), 11 deletions(-) create mode 100644 game_frontend/src/pyodide/syntax.test.ts create mode 100644 game_frontend/src/pyodide/syntax.ts diff --git a/.codecov.yml b/.codecov.yml index c71949362..18dff23f7 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -20,11 +20,4 @@ ignore: - "test_utils/*" - "setup.py" -comment: - layout: "reach, diff, flags, files" - behavior: new - require_changes: false - require_base: no - require_head: yes - branches: - - "!master" +comment: false diff --git a/game_frontend/src/pyodide/syntax.test.ts b/game_frontend/src/pyodide/syntax.test.ts new file mode 100644 index 000000000..a4cc905e3 --- /dev/null +++ b/game_frontend/src/pyodide/syntax.test.ts @@ -0,0 +1,112 @@ +/* eslint-env jest */ +import { matchFromImports, matchImports } from './syntax'; +jest.mock('threads/worker') + + +describe('validate imports', () => { + it('match imports', () => { + const imports = matchImports(` +import a +import b, c + +def import_d(): + import d + +def import_e(): import e + +import f.f2 +`); + + return expect(imports).toEqual(new Set([ + 'a', + 'b', + 'c', + 'd', + 'e', + 'f.f2' + ])); + }); + + it('match imports with comments', () => { + const imports = matchImports(` +import a # some comment +import b # +import c#touching +import d # some # comment +import e, f # +`); + + return expect(imports).toEqual(new Set([ + 'a', + 'b', + 'c', + 'd', + 'e', + 'f' + ])); + }); + + it('match imports with irregular spacing', () => { + const imports = matchImports(` +import a +import b, c +import d , e +import f,g +`); + + return expect(imports).toEqual(new Set([ + 'a', + 'b', + 'c', + 'd', + 'e', + 'f', + 'g' + ])); + }); + + it('match imports with invalid syntax', () => { + const imports = matchImports(` +import a, +import b,,c +import d. +`); + + return expect(imports).toEqual(new Set()); + }); + + it('match from-imports', () => { + const fromImports = matchFromImports(` +from a import ( # after import + b, # after b + c, # after b +) # after end + +from d.d1 import ( + e, + f +) + +from g import ( + h,, + i +) + +def foo(): from j import ( + k, + l +) + +from m import (n, o) +from p import q, r # some comment +`); + + return expect(fromImports).toEqual({ + 'a': new Set(['b', 'c']), + 'd.d1': new Set(['e', 'f']), + 'j': new Set(['k', 'l']), + 'm': new Set(['n', 'o']), + 'p': new Set(['q', 'r']), + }); + }); +}); \ No newline at end of file diff --git a/game_frontend/src/pyodide/syntax.ts b/game_frontend/src/pyodide/syntax.ts new file mode 100644 index 000000000..be975c51d --- /dev/null +++ b/game_frontend/src/pyodide/syntax.ts @@ -0,0 +1,126 @@ +const namePattern = '[{base}][{base}0-9]*'.replace(/{base}/g, '_a-zA-Z'); +const modulePattern = '{name}(?:\\.{name})*'.replace(/{name}/g, namePattern); + +export function funcPattern({ + lineStart, + captureName, + captureArgs +}: { + lineStart: boolean + captureName: boolean + captureArgs: boolean +}) { + let pattern = ' *def +{name} *\\({args}\\) *:'; + + if (lineStart) pattern = '^' + pattern; + + // TODO: refine + const argsPattern = '.*'; + + pattern = pattern.replace( + /{name}/g, + captureName ? `(${namePattern})` : namePattern + ); + pattern = pattern.replace( + /{args}/g, + captureArgs ? `(${argsPattern})` : argsPattern + ); + + return pattern; +} + +function splitImports(imports: string) { + return new Set(imports.split(',').map((_import) => _import.trim())); +} + +export function matchImports(code: string) { + const pattern = new RegExp( + [ + '^', + '(?:{func})?'.replace( + /{func}/g, + funcPattern({ + lineStart: false, + captureName: false, + captureArgs: false + }) + ), + ' *import +({module}(?: *, *{module})*)'.replace( + /{module}/g, + modulePattern + ), + ' *(?:#.*)?', + '$' + ].join(''), + 'gm' + ); + + const imports: Set = new Set(); + for (const match of code.matchAll(pattern)) { + splitImports(match[1]).forEach((_import) => { imports.add(_import); }); + } + + return imports; +} + +export function matchFromImports(code: string) { + const pattern = new RegExp( + [ + '^', + '(?:{func})?'.replace( + /{func}/g, + funcPattern({ + lineStart: false, + captureName: false, + captureArgs: false + }) + ), + ' *from +({module}) +import'.replace( + /{module}/g, + modulePattern + ), + '(?: *\\(([^)]+)\\)| +({name}(?: *, *{name})*))'.replace( + /{name}/g, + namePattern + ), + ' *(?:#.*)?', + '$' + ].join(''), + 'gm' + ); + + const fromImports: Record> = {}; + for (const match of code.matchAll(pattern)) { + let imports: Set; + if (match[3] === undefined) { + // Get imports as string and remove comments. + let importsString = match[2].replace( + /#.*(\r|\n|\r\n|$)/g, + '' + ); + + // If imports have a trailing comma, remove it. + importsString = importsString.trim(); + if (importsString.endsWith(',')) { + importsString = importsString.slice(0, -1); + } + + // Split imports by comma. + imports = splitImports(importsString); + + // If any imports are invalid, don't save them. + const importPattern = new RegExp(`^${namePattern}$`, 'gm') + if (imports.has('') || + [...imports].every((_import) => importPattern.test(_import)) + ) { + continue; + } + } else { + imports = splitImports(match[3]); + } + + fromImports[match[1]] = imports; + } + + return fromImports; +} diff --git a/game_frontend/src/pyodide/webWorker.ts b/game_frontend/src/pyodide/webWorker.ts index 8c7672ada..193190360 100644 --- a/game_frontend/src/pyodide/webWorker.ts +++ b/game_frontend/src/pyodide/webWorker.ts @@ -1,7 +1,8 @@ /* eslint-env worker */ -import { expose } from 'threads/worker' -import { checkIfBadgeEarned, filterByWorksheet } from './badges' -import ComputedTurnResult from './computedTurnResult' +import { expose } from 'threads/worker'; +import { checkIfBadgeEarned, filterByWorksheet } from './badges'; +import ComputedTurnResult from './computedTurnResult'; +import { matchFromImports, matchImports } from './syntax'; let pyodide: Pyodide @@ -98,6 +99,31 @@ export function simplifyErrorMessageInLog(log: string): string { return log.split('\n').slice(-2).join('\n') } +const IMPORT_WHITE_LIST: Array<{ + name: string + allowAnySubmodule: boolean + from?: Set +}> = [ + { + name: 'random', + allowAnySubmodule: true, + } + ]; + +function validateImportInWhiteList(_import: string, turnCount: number) { + if (IMPORT_WHITE_LIST.every(({ name, allowAnySubmodule }) => + _import !== name || (_import.startsWith(name) && !allowAnySubmodule) + )) { + return Promise.resolve({ + action: { action_type: 'wait' }, + log: `Import "${_import}" is not allowed.`, + turnCount: turnCount, + }) + } + + return undefined; +} + export async function updateAvatarCode( userCode: string, gameState: any, @@ -109,6 +135,17 @@ export async function updateAvatarCode( } try { + for (const _import of matchImports(userCode)) { + const promise = validateImportInWhiteList(_import, turnCount); + if (promise) return promise; + } + + for (const _import in matchFromImports(userCode)) { + const promise = validateImportInWhiteList(_import, turnCount); + if (promise) return promise; + // TODO: validate from imports + } + await pyodide.runPythonAsync(userCode) if (gameState) { return computeNextAction(gameState, playerAvatarID, false)