Security Fixes

[KesterTan]

Description

This PR builds on the work by @NicholasMy . Original work is found here which verifies that the requested submissions belong to the current assessment/course and here which adds a auth check for tweak_total.

Thanks for the original implementation!

Additionally, this PR verifies that submissions belong to the same assessment in download batch, adds some CSRF protection for endpoints, and also enforces error handling without information disclosure.

How Has This Been Tested?

Test that everything works as expected in manage submissions.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] I have run rubocop and erblint for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
• [ ] My change requires a change to the documentation, which is located at Autolab Docs
• [ ] I have updated the documentation accordingly, included in this PR

Other issues / help required

If unsure, feel free to submit first and we'll help you along.

[coderabbitai[bot]]

📝 Walkthrough

Walkthrough

The SubmissionsController is updated to add CSRF protection with JSON-specific exceptions, improve error handling in score_details, rework batch operation permission checks, update flash messaging for batch operations, rename submission_info authorization to tweak_total, and introduce a json_request? helper method.

Changes

Cohort / File(s)	Summary
CSRF Protection & Security app/controllers/submissions_controller.rb	Adds protect_from_forgery with exceptions for destroy_batch and excuse_batch; introduces json_request? private helper to enforce authenticity tokens for JSON-bound actions.
Error Handling app/controllers/submissions_controller.rb	Updates score_details to log exceptions and return generic error message instead of exposing raw exception text.
Batch Operations & Authorization app/controllers/submissions_controller.rb	Reworks permission checks in destroy_batch and download_batch to verify submissions belong to current assessment and user has permission; consolidates allowed flag to gate processing and replaces per-submission redirects with single outcome.
UI Messaging app/controllers/submissions_controller.rb	Adjusts flash messages and success/failure phrasing for batch operations to reflect aggregated counts.
Action Routing app/controllers/submissions_controller.rb	Renames authorization from submission_info to tweak_total and updates corresponding action declaration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Single-file scope simplifies review, but the changes span multiple distinct concerns (CSRF security, authorization logic, error handling, and action renaming) requiring careful verification of each subsystem's correctness and interaction.

Possibly related PRs

• autolab/Autolab#2271: Also modifies SubmissionsController batch actions (destroy_batch, excuse_batch) to handle JSON/AJAX requests with JSON response handling.
• autolab/Autolab#2248: Modifies the same SubmissionsController methods, including tweak_total, score_details, and batch operations.

Suggested reviewers

• dwang3851
• coder6583

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check	❓ Inconclusive	The title "Security Fixes" is generic and vague, failing to provide meaningful specificity about the changeset. While it is related to the changes (which do address security), it uses a non-descriptive term that could apply to countless different security-related PRs without differentiating this specific work. The changes involve multiple distinct security aspects—CSRF protection, authorization verification, error handling without information disclosure—yet the title does not indicate which are primary or what specifically is being fixed. A teammate scanning the git history would not gain clear understanding from this title alone.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	The description covers all major required sections from the template: it provides a description of the changes (with context from prior work), addresses motivation through the security improvements being made, includes a brief testing summary, specifies the change type (bug fix), and completes the relevant checklist items. While the "Motivation and Context" section could be more detailed in explaining specific vulnerabilities or problems being solved, and the testing description is somewhat minimal, the PR contains sufficient information across all key sections. The description is mostly complete and addresses the core requirements despite some sections being brief.

✨ Finishing touches

• [ ] 📝 Generate docstrings

🧪 Generate unit tests (beta)

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment
• [ ] Commit unit tests in branch security-fixes

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}