[Bug]: /customforms/metadata/ returns any teacher's form to any other teacher #5985

Description

While reading through PR #4583 — the one that locked down custom form response data — I noticed it covers four views, but there's a fifth that takes a form_id the same way and didn't get the same treatment.

After #4583, viewResponse, getExcelData, getData and bulkDownloadFiles all return 403 if a teacher asks for a form they didn't create. getRebuildData, which sits behind /customforms/metadata/, doesn't check anything. It just looks the form up by id and returns FormHandler.rebuildData() — the whole structure of the form: title, description, the perms string, link_type/link_id, and every page, section and question label on it.

What made me fairly sure this is a boundary the project actually intends, and not just a path nobody uses: formBuilder() already filters the "Create from existing form" dropdown to created_by=request.user for non-admins. So the UI deliberately never shows you another teacher's form. It's only the endpoint behind that dropdown that will hand one over.

While I was in there I also noticed the same view has two ways to 500:

• request.GET['form_id'] is wrapped in except ValueError only, so leaving the parameter off raises MultiValueDictKeyError.
• Form.objects.get(pk=form_id) isn't inside the try at all, so an id that doesn't exist raises Form.DoesNotExist.

The other four handle both of these. This one falls over.

Steps to Reproduce

1. Make two unrelated teacher accounts, A and B.
2. As A, build a form at /customforms/create/ and note its id.
3. Log in as B, open /customforms/create/, expand "Form information". A's form isn't in the "Create from existing form" dropdown — which is correct, and is the behaviour the rest of this report is measured against.
4. Still as B, from a tab on the same origin, run:

fetch('/customforms/metadata/?form_id=<A_FORM_ID>', {
  headers: {'X-Requested-With': 'XMLHttpRequest'}
}).then(r => r.json()).then(console.log)

5. For the 500, send the same request with form_id left off entirely.

Expected Behavior

403, the same as the other four form_id endpoints. Admins and morphed users should still get through, as they do elsewhere.

A missing or unknown form_id should come back as an error response rather than a 500.

Actual Behavior

200, with the other teacher's entire form.

Running this on a fresh checkout of main, signed in as teacher B and asking for teacher A's form, I got back:

"title": "Confidential - Teacher A Only"
"desc":  "Private screening notes for my seminar applicants. Not for other staff."
"perms": "Teacher"

plus every question label on the form. In my test data those were things like "Home address", "Guardian phone number" and "Financial aid notes (internal)" — so it isn't only the shape of the form that leaks, it's what the form is for.

The interesting part is the comparison. Same browser session, same form id:

/customforms/responses/1/         ->  403
/customforms/exceldata/1/         ->  403
/customforms/metadata/?form_id=1  ->  200

And with the parameter omitted:

django.utils.datastructures.MultiValueDictKeyError: 'form_id'  ->  500

Screenshots

Operating System

Windows 11, running the project under Docker Compose

Browser

Chrome

Additional Context

I think the reason this slipped through is that #4583's tests (FormOwnershipAccessTest in esp/esp/customforms/tests.py) cover all four of the views it changed, and there's no case in there for /customforms/metadata/. Nothing was failing, so there was nothing to notice.

I've got a fix working locally — same two-line check the siblings use, plus folding the Form lookup into the existing try — along with five tests added to that class. Happy to open a PR if this gets assigned to me.