Skip to content

Fix CodeTailor security vulnerabilities#1231

Merged
bnmnetp merged 4 commits into
RunestoneInteractive:mainfrom
aspadiyath:main
Jun 10, 2026
Merged

Fix CodeTailor security vulnerabilities#1231
bnmnetp merged 4 commits into
RunestoneInteractive:mainfrom
aspadiyath:main

Conversation

@aspadiyath

Copy link
Copy Markdown
Contributor
  • Add authentication (Depends(auth_manager)) to parsons_scaffolding endpoint
  • Remove exec()-based Python test runner; replace with JOBE sandbox via evaluate_fixed_code.py
  • Fetch test code from DB (question_json.suffix_code) instead of accepting it from request body
  • Add clean_python_testcase() to transform Runestone unittest.gui format for JOBE compatibility

- Add authentication (Depends(auth_manager)) to parsons_scaffolding endpoint
- Remove exec()-based Python test runner; replace with JOBE sandbox via evaluate_fixed_code.py
- Fetch test code from DB (question_json.suffix_code) instead of accepting it from request body
- Add clean_python_testcase() to transform Runestone unittest.gui format for JOBE compatibility

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses CodeTailor security hardening by restricting access to the Parsons scaffolding endpoint and removing server-side exec()-based Python test execution in favor of running tests inside a JOBE sandbox. It also stops accepting test code from the request body and instead loads instructor-authored test code (suffix_code) from the database, with a small normalization step for Runestone’s unittest.gui format.

Changes:

  • Add authentication dependency (Depends(auth_manager)) to the /coach/parsons_scaffolding endpoint.
  • Replace Python exec() test execution with JOBE-based execution in evaluate_fixed_code.py.
  • Fetch suffix_code from Question.question_json (DB) and normalize Python tests via clean_python_testcase().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
bases/rsptx/book_server_api/routers/personalized_parsons/evaluate_fixed_code.py Switch Python test evaluation to JOBE and return a minimal wasSuccessful()-compatible result.
bases/rsptx/book_server_api/routers/coach.py Require auth for parsons_scaffolding, fetch test code from DB, and normalize Runestone Python test format for JOBE.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bases/rsptx/book_server_api/routers/personalized_parsons/evaluate_fixed_code.py Outdated
Comment thread bases/rsptx/book_server_api/routers/coach.py
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@bnmnetp

bnmnetp commented Jun 9, 2026

Copy link
Copy Markdown
Member

Have you tested this locally so you know it works?

- Suppress __name__ == "__main__" guards in student code sent to JOBE
- Check last stdout line for PASS/FAIL to tolerate student debug prints
- Remove redundant HEAD/PUT file-store calls (sourcecode already sent inline)
- Pass basecourse to fetch_question to prevent wrong result from duplicate names

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aspadiyath

Copy link
Copy Markdown
Contributor Author

Yup, I've tested locally and it works!

@bnmnetp

bnmnetp commented Jun 9, 2026

Copy link
Copy Markdown
Member

Great, I'll look at it a little more and get it merged. Thanks for addressing this so quickly.

@bnmnetp bnmnetp merged commit 880c30b into RunestoneInteractive:main Jun 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants