csivitu/code-executor

A CLI/library to execute code against test cases in various languages and obtain relevant results. :rocket:

code-executor's Issues

Promise is Always Pending ...

• First of all the promise is not resolved when i try to execute runCode
• documentation is not done correctly you have no information about how to use it and if information is provided how to use it they it is half only not the full info. how to use it..

Add more languages

Add more languages

code-executor currently supports most of the languages in src/langs. More languages need to be added so that code-executor can fulfill all requirements for popular use-cases such as Competitive Coding websites.

Structure

To add languages, you can add a Dockerfile and a start.sh script like in Python.

Note: Make sure to test the Dockerfile and the start.sh script with some sample code before making a PR to the dev branch.

Probably do not need internal resolve/reject map

I was browsing through the bulljs reference docs, and found that every job has job.finished() which returns a promise that resolves when the job has finished
https://github.com/OptimalBits/bull/blob/develop/REFERENCE.md#jobfinished

So technically, we do not need the internal map logic in code-executor which maintains maps from job IDs to promises, as bulljs already does this internally.

code-executor/src/CodeExecutor.ts

Lines 15 to 39 in b69766f

	this.jobs = new Map();
	this.queue.on('global:completed', (_job: Bull.Job, result: string) => {
	const { id } = <Result>JSON.parse(result);
	logger.debug(`Running on complete for id: ${id}`);
	const currentJob = this.jobs.get(id);
	if (currentJob) {
	currentJob.resolve(result);
	this.jobs.delete(id);
	}
	});
	}
	async runCode(codeOptions: CodeParams): Promise<void> {
	const id = uuid();
	const codeObject = { ...codeOptions, id };
	logger.info(`Running code with id: ${id}`);
	return new Promise((resolve, reject) => {
	this.jobs.set(id, { resolve, reject });
	this.queue.add(codeObject);
	});
	}

throw error when if invalid Redis url is passed

The current published version of code-executor doesn't throw error when trying to connect to an invalid Redis URL. Instead it connects to localhost:6379. Need to fix this.

A lenient way to match outputs

Right now, comparison is as simple as :

remarks = expectedOutput === obtainedOutput ? 'Pass' : 'Fail';

It should be more tolerant to outputs where there can be an extra space, a carriage return etc.

Alternatively, best way to do this is by serializing the return value and then compare these serialized objects instead of strings. This has a lot of benefits:

• Data structures can be returned as a result of procedure calls, instead of providing a specific output format for each problem.
• Sometimes people interpret non printable characters in a wrong way (different from intended). This solves that problem so that users never get a penalty just because they counted one space less or more.

CLI to spawn Workers

CLI to spawn Workers

The scripts used to spawn workers consist of a lot of repetitive code, it would be really useful to have a CLI instead.
The CLI could have options like:

code-executor spawn-worker --name myExecutor -n 10 --redis "redis://127.0.0.1:6379"

This spawns 10 workers on the server with the redis instance for managing the queue located at redis://127.0.0.1:6379.
This is just an example of the bare minimum requirement. Feel free to suggest features that could be included in the CLI 💯

Proposed structure

I'd propose you have a file bin/code-executor.js which just imports the CLI from dist/src/cli.js and runs it.
dist/src/cli.js is the build output from src/cli.ts, which has the CLI in typescript.

Write tests for all languages

Write tests for all languages

Currently, code-executor supports all languages mentioned in src/langs. There are no script yet to test if these languages work properly. You can add tests in test for each language.

Proposed structure

• Each language has a folder inside test, the name of the folder is the name of the language
• There are two scripts in each folder, one for master and one for the worker.

You might consider using a testing library such as mocha or chai. However, for this, timeouts need to be configured properly since these tests will be running on Github Actions in the future. Nonetheless, adding a unit testing library for each language is not a priority right now, verifying that the Dockerfiles for all the languages work can be done with or without mocha or chai.

Add docs for worker parameters memory and CPUs

Add docs for worker parameters memory and CPUs

Optional parameters for memory ( in MBs ) and no. of CPUs to allocate are added. Needs to be updated in README.

Memory default value: 0 (ie no limit)
CPUs default value: 0.5

Return errors in CodeExecutor class

Return errors in CodeExecutor class

Errors (if any) thrown from the by this.work() are not caught/handled in Worker.ts.

code-executor/src/Worker.ts

Lines 50 to 58 in b69766f

	start() {
	this.queue.process(async (job, done) => {
	logger.info(`Received: ${job.data.id}`);
	const result = await this.work(job.data);
	logger.debug(JSON.stringify(result));
	done(null, result);
	});
	}

The goal is to reject the promise in this.queue.on('global:completed') in the CodeExecutor class if the Worker ran into an error.

code-executor/src/CodeExecutor.ts

Lines 17 to 27 in b69766f

	this.queue.on('global:completed', (_job: Bull.Job, result: string) => {
	const { id } = <Result>JSON.parse(result);
	logger.debug(`Running on complete for id: ${id}`);
	const currentJob = this.jobs.get(id);
	if (currentJob) {
	currentJob.resolve(result);
	this.jobs.delete(id);
	}
	});

Test Cases may be leaked through RCE

Test Cases may be leaked through RCE

Since the input is stored as a file inside the docker image, there is a possibility that someone could leak the test cases. For example, they could do the following in Python:

import subprocess
input_file = subprocess.check_output('./input', shell=True)
print(input_file)

This can be fixed by modifying the permissions inside the Docker containers. An alternative could be to take input through stdin. This is a critical bug and needs to be fixed.

However, a user might still be able to spawn a reverse shell inside the container or run a fork bomb. A feasible way to prevent this would be to block all outgoing network traffic and using something like nsjail inside the containers. This too is a security risk of critical severity.

Add contributors

Support more runner strategies

Support more runner strategies

Currently, the runner is very straightforward, it executes every test case sequentially, there are different strategies it can support:

1. Maybe the runner can execute multiple test cases in parallel, and you can possibly configure how many in parallel at a time. This is linked to the issue about leaking test cases from other tests you are not executing against, I mentioned a possible approach to handle this there (#15 (comment))
2. The runner has the option of executing a certain group of test cases first, and only executing the others if the first group passes. This is a very common use case, as judging platforms want to often not run against private test cases if the sample test cases didn't pass in the first place. There are many solutions for this, maybe each test case can have a "depend_on" attribute which tells it depends on other test case to succeed, and only runs if that test case passed successfully.

Return error if no instances of worker class is running

code-executor needs atleast one instance of worker class running to run the jobs. If a user tries to run a job without starting any worker instance, the promise returned by the code-executor will never resolve or reject. Need to find a way to check if any worker instances are running, if not, throw an error.