[Bug]: LineItemsModule raises 500 on stale or invalid line item ids in edit and delete flows

[moksha-hub]

Description

esp/esp/program/modules/handlers/lineitemsmodule.py uses direct .objects.get(...) calls on request-supplied IDs in several edit/delete/import paths without graceful handling for invalid or stale IDs.

Why this matters

LineItemsModule is an admin-facing accounting/configuration workflow. If an item has been deleted, an ID is stale, or the request is tampered with, the module should show a recoverable error rather than crashing.

Current flows include patterns such as:

• LineItemType.objects.get(id=request.GET['id'])
• LineItemType.objects.get(id=request.POST['id'])
• Program.objects.get(id=request.POST['program'])

with no surrounding DoesNotExist or parsing guards.

Evidence

Relevant code in esp/esp/program/modules/handlers/lineitemsmodule.py:

• edit flow:
  LineItemType.objects.get(id=request.GET['id'])
• delete confirmation:
  LineItemType.objects.get(id=request.GET['id'])
• confirmed deletion:
  LineItemType.objects.get(id=request.POST['id'])
• import confirmation:
  Program.objects.get(id=request.POST['program'])

These request-derived lookups are performed directly with no defensive handling.

Steps to Reproduce

1. Open the Line Items management UI.
2. Trigger an edit, delete, or import path using an invalid, deleted, or tampered ID in the request.
3. Observe that the handler performs direct .objects.get(...) lookups from request parameters.
4. If the referenced object does not exist, the flow can raise an unhandled exception.

Expected Behavior

The module should detect invalid IDs and return a clear admin-facing error, without crashing the entire page.

Actual Behavior

Stale or invalid IDs can lead to DoesNotExist exceptions and 500 responses.

Suggested Fix

Wrap request-derived object lookups in validation and exception handling:

• catch LineItemType.DoesNotExist
• catch Program.DoesNotExist
• catch ValueError where IDs are parsed
• return a clean ESPError instead of a raw exception

Additional Context

I checked the current issue tracker with searches around lineitemsmodule, LineItemType.objects.get, and line-item edit/delete crashes. I only found the test-coverage issue #5210, not an existing bug issue for this invalid-ID crash path.

[github-actions]

👋 Thanks for opening this issue!

To keep the tracker focused, new issues are locked for conversation until a maintainer has reviewed them. This helps us avoid duplicates and ensure issues are actionable.

What happens next:

• A maintainer will review this issue shortly.
• If approved, the conversation will be unlocked and the needs-triage label removed.
• If this is a duplicate or out of scope, a maintainer will close it with an explanation.

Please don't worry — your issue hasn't been rejected, it's just waiting for review. 🙏

[workonlly]

/assign

[github-actions]

👋 Hi @workonlly! To keep work distributed across contributors, we limit each person to 5 assigned open issue(s) at a time.

You're currently assigned to 5 open issue(s):

• [Bug]: ProgramPrintables.paid_list() crashes when filter contains a non-integer value #5731
• [Bug]: TeacherPreviewModule raises 500 when admin ?user= points to a missing teacher #5657
• [Bug]: stripeToken and ponumber read from POST without validation #5446
• [Bug]: Invalid registration date ranges allowed in program creation (missing validation) #5326
• [Bug]: Missing Unit Tests for TeacherOnsite (teacheronsite.py) #5196

Once you close or unassign yourself from an existing issue, feel free to come back and request this one!

[workonlly]

@moksha-hub i want a help from you can you close the previous issues in which i have provided solution so that i can work on these issues to that would be great help because i want to work on these new issues to

[moksha-hub]

@moksha-hub i want a help from you can you close the previous issues in which i have provided solution so that i can work on these issues to that would be great help because i want to work on these new issues to

Actually, I am a contributor, and maintainers will definitely close them. It takes time; you have to wait.

[Sameer-00001]

/assign

[github-actions]

✅ @Sameer-00001 has been assigned to this issue. You have 5/5 issue(s) assigned.

📖 Getting Started · Contribution Rules · Code of Conduct

[Sameer-00001]

Hi @moksha-hub, thanks for the excellent and detailed write-up on this! I've assigned myself and am jumping into the codebase now.

My plan of attack:
I'll be updating esp/esp/program/modules/handlers/lineitemsmodule.py to add defensive error handling around the request-derived object lookups (like LineItemType.objects.get() and Program.objects.get()).

Specifically, I'll wrap these direct lookups in try/except blocks to catch DoesNotExist, ValueError, and KeyError. Instead of allowing the server to 500 crash, I'll ensure the flow returns a clean, graceful ESPError to the admin UI if a stale, invalid, or tampered ID is detected.

I'll get a PR up for this shortly!