search

github-education-resources / autograding

GitHub Education Auto-grading and Feedback for GitHub Classroom
MIT License
59 stars 68 forks source link

give child processes access to all of process.env #19

Open titaniumbones opened 4 years ago

titaniumbones commented 4 years ago

right now, it doesn't seem to be possible to pass additonal environment variables to one (or all) of the autograde tests. This is a bit of a limitation when we might want to use the same test suites for students and autograding, with environment parameters modulating the precise test outcome. Looks like this should be doable in the spawn function of runner.ts, but whe nI try to make the changes myself I run into errors. Sorry I can't provide working code.

jeffrafter commented 4 years ago

Sorry for the slow reply! You're right that passing the env is possible in spawn, unfortunately restricting the environment is done for security reasons - for example if the student has access to the GitHub Actions environment they could craft code that would automatically pass the build, change the outcome of the tests, and in some cases modify the tamper-seals, etc.

Instead of restricting specific environment variables we went with a much more strict policy to be more future-proof. We could potentially allow-list specific variables or construct a configuration for that. Can you give a more specific example of which variables you would be looking for?

titaniumbones commented 4 years ago

Hmm, mostly I want the GitHub user id in github_actor - I want to check that there are at least n commits by the student, and maybe they the tree includes only instructor and GitHub user id commits. Is that a reasonable ask?

On May 12, 2020 11:59 a.m., Jeff Rafter notifications@github.com wrote:

Sorry for the slow reply! You're right that passing the env is possible in spawn, unfortunately restricting the environment is done for security reasons - for example if the student has access to the GitHub Actions environment they could craft code that would automatically pass the build, change the outcome of the tests, and in some cases modify the tamper-seals, etc.

Instead of restricting specific environment variables we went with a much more strict policy to be more future-proof. We could potentially allow-list specific variables or construct a configuration for that. Can you give a more specific example of which variables you would be looking for?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/education/autograding/issues/19#issuecomment-627435284, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AACY6NB7IX5QQ6ZBQSJQL7TRRFW5ZANCNFSM4MZIOM3A.

jeffrafter commented 4 years ago

I can look into the actor's GitHub user id. I think that would be reasonable and easy to add.

I want to check that there are at least n commits by the student, and maybe they the tree includes only instructor and GitHub user id commits

Can you clarify this?

you06 commented 3 years ago

As @jeffrafter says, the security issue can not be ignored, however, some variables need to be passed to the child process since they need them. e.g. I'm currently making a course that uses golang, this language requires inheriting GOROOT and HOME variables for its development environment. I did some hard code in my fork as a workaround.

What about an allow list config so that we can specify which variables can be passed to child processes?

diamondburned commented 1 year ago

You're right that passing the env is possible in spawn, unfortunately restricting the environment is done for security reasons - for example if the student has access to the GitHub Actions environment they could craft code that would automatically pass the build, change the outcome of the tests, and in some cases modify the tamper-seals, etc.

Isn't this already possible? I'm not sure if GitHub Classroom has anything that verifies the author of commits made to .github. (I actually cannot verify that our university does this correctly, however I ended up having to write a custom solution to verify .github integrity. See this old GitHub Education Community post or this fairly-recent discussion).

I'm also making this comment as a +1: I currently rely on a few NIX_ environment variables, and the build tools don't work right without any of them.

Is it possible to make it configurable somewhere?