Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: validate imports #1838

Merged
merged 2 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions .codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
112 changes: 112 additions & 0 deletions game_frontend/src/pyodide/syntax.test.ts
Original file line number Diff line number Diff line change
@@ -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']),
});
});
});
126 changes: 126 additions & 0 deletions game_frontend/src/pyodide/syntax.ts
Original file line number Diff line number Diff line change
@@ -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<string> = 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<string, Set<string>> = {};
for (const match of code.matchAll(pattern)) {
let imports: Set<string>;
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;
}
43 changes: 40 additions & 3 deletions game_frontend/src/pyodide/webWorker.ts
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -98,6 +99,31 @@
return log.split('\n').slice(-2).join('\n')
}

const IMPORT_WHITE_LIST: Array<{
name: string
allowAnySubmodule: boolean
from?: Set<string>
}> = [
{
name: 'random',
allowAnySubmodule: true,
}
];

function validateImportInWhiteList(_import: string, turnCount: number) {

Check warning on line 113 in game_frontend/src/pyodide/webWorker.ts

View check run for this annotation

Codecov / codecov/patch

game_frontend/src/pyodide/webWorker.ts#L113

Added line #L113 was not covered by tests
if (IMPORT_WHITE_LIST.every(({ name, allowAnySubmodule }) =>
_import !== name || (_import.startsWith(name) && !allowAnySubmodule)
)) {
return Promise.resolve({

Check warning on line 117 in game_frontend/src/pyodide/webWorker.ts

View check run for this annotation

Codecov / codecov/patch

game_frontend/src/pyodide/webWorker.ts#L117

Added line #L117 was not covered by tests
action: { action_type: 'wait' },
log: `Import "${_import}" is not allowed.`,
turnCount: turnCount,
})
}

return undefined;

Check warning on line 124 in game_frontend/src/pyodide/webWorker.ts

View check run for this annotation

Codecov / codecov/patch

game_frontend/src/pyodide/webWorker.ts#L124

Added line #L124 was not covered by tests
}

export async function updateAvatarCode(
userCode: string,
gameState: any,
Expand All @@ -109,6 +135,17 @@
}

try {
for (const _import of matchImports(userCode)) {
const promise = validateImportInWhiteList(_import, turnCount);

Check warning on line 139 in game_frontend/src/pyodide/webWorker.ts

View check run for this annotation

Codecov / codecov/patch

game_frontend/src/pyodide/webWorker.ts#L138-L139

Added lines #L138 - L139 were not covered by tests
if (promise) return promise;
}

for (const _import in matchFromImports(userCode)) {
const promise = validateImportInWhiteList(_import, turnCount);

Check warning on line 144 in game_frontend/src/pyodide/webWorker.ts

View check run for this annotation

Codecov / codecov/patch

game_frontend/src/pyodide/webWorker.ts#L143-L144

Added lines #L143 - L144 were not covered by tests
if (promise) return promise;
// TODO: validate from imports
}

await pyodide.runPythonAsync(userCode)
if (gameState) {
return computeNextAction(gameState, playerAvatarID, false)
Expand Down