fix(curriculum): enforce course/module lock on manual-marker completion
POST /api/student/activities/:activityId/complete previously checked only that the activity
exists and the student holds an active PROGRAM enrollment — never whether the activity's own
course/module was actually unlocked for that student. A direct POST could mark any
manual_marker activity (lesson/demonstration/knowledge_check/guided_exercise/reflection)
complete regardless of the real prerequisite chain, the exact class of gap Phase 6B already
fixed once for the capstone's own start route, just never closed for this more general one.
Found while proving the real ROOT 01->02->03->04->05 chain via direct API (this phase's own
explicit requirement) — and confirmed to predate Phase 6E entirely by reproducing it against an
unrelated, pre-existing ROOT 05/06 pair before touching any Phase 6E content.
Fix: the route now resolves the activity's own course/module through the exact same
buildCourseTree() lock computation every other route already trusts, and returns 403 { error:
'locked' } if either is locked — reusing the existing mechanism, not a second, duplicated check.
Verified live: both a ROOT 02 bypass attempt and a ROOT 06 bypass attempt are now correctly
rejected; legitimate unlocked completions are unaffected.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
@@ -79,7 +79,11 @@ decided, and it never invents new state for anything that's independently verifi
|
|||||||
from (a lesson has no "pass/fail," a reflection has no validator). `POST /api/student/
|
from (a lesson has no "pass/fail," a reflection has no validator). `POST /api/student/
|
||||||
activities/:id/complete` refuses (409 `not_manually_completable`) for any activity whose
|
activities/:id/complete` refuses (409 `not_manually_completable`) for any activity whose
|
||||||
`completion_behavior` isn't `manual_marker` — a student can never fake a lab or assessment
|
`completion_behavior` isn't `manual_marker` — a student can never fake a lab or assessment
|
||||||
completion through this route.
|
completion through this route. **(Phase 6E)** the route also resolves the activity's own
|
||||||
|
course/module through the same `buildCourseTree()` lock computation every other route trusts,
|
||||||
|
and refuses (403 `locked`) if either is locked for that student — a real, pre-existing gap
|
||||||
|
found and fixed while proving the ROOT 01-04 prerequisite chain; see
|
||||||
|
`docs/PHASE6E_ROOT_FOUNDATIONS_COMPLETION.md`.
|
||||||
|
|
||||||
Course and module completion are pure aggregates over their activities' completion (`total`/
|
Course and module completion are pure aggregates over their activities' completion (`total`/
|
||||||
`completed`/`pct`/`complete` — see `totals()`), never separately stored.
|
`completed`/`pct`/`complete` — see `totals()`), never separately stored.
|
||||||
|
|||||||
@@ -44,12 +44,32 @@ module.exports = function registerCourseRoutes(app, deps) {
|
|||||||
// (course-progress.js) and can never be set this way — marking one "complete" here would be a
|
// (course-progress.js) and can never be set this way — marking one "complete" here would be a
|
||||||
// second, forgeable source of truth for something the platform already knows how to verify
|
// second, forgeable source of truth for something the platform already knows how to verify
|
||||||
// for real.
|
// for real.
|
||||||
|
//
|
||||||
|
// Phase 6E §11 — found by testing, not assumed: this route previously checked only that the
|
||||||
|
// activity exists and the student holds an active PROGRAM enrollment, never whether the
|
||||||
|
// activity's own course/module is actually unlocked for that student — a direct POST could
|
||||||
|
// mark any manual_marker activity complete regardless of the prerequisite chain, exactly the
|
||||||
|
// class of gap Phase 6B already fixed once for the capstone's own start route. Proven to
|
||||||
|
// predate this phase (reproduced against a ROOT 05/06 pair, unrelated to Phase 6E's own new
|
||||||
|
// content) before fixing — see docs/PHASE6E_ROOT_FOUNDATIONS_COMPLETION.md. Reuses the exact
|
||||||
|
// same buildCourseTree()/locked computation every other route already trusts, rather than a
|
||||||
|
// second, duplicated lock check.
|
||||||
app.post('/api/student/activities/:activityId/complete', studentSession, wrap(async (req, res) => {
|
app.post('/api/student/activities/:activityId/complete', studentSession, wrap(async (req, res) => {
|
||||||
const activity = await requireEnrolledActivity(req, res);
|
const activity = await requireEnrolledActivity(req, res);
|
||||||
if (!activity) return;
|
if (!activity) return;
|
||||||
if (activity.completion_behavior !== 'manual_marker') {
|
if (activity.completion_behavior !== 'manual_marker') {
|
||||||
return res.status(409).json({ error: 'not_manually_completable', message: 'This activity\'s completion is derived automatically.' });
|
return res.status(409).json({ error: 'not_manually_completable', message: 'This activity\'s completion is derived automatically.' });
|
||||||
}
|
}
|
||||||
|
const tree = await buildCourseTree(getPool(), { studentId: req.studentId, programId: activity.program_id, forInstructor: false });
|
||||||
|
let course = null, mod = null;
|
||||||
|
for (const c of tree) {
|
||||||
|
const m = c.modules.find((x) => x.module_id === activity.module_id);
|
||||||
|
if (m) { course = c; mod = m; break; }
|
||||||
|
}
|
||||||
|
if (!course || !mod) return res.status(404).json({ error: 'not_found' });
|
||||||
|
if (course.locked || mod.locked) {
|
||||||
|
return res.status(403).json({ error: 'locked', message: 'This module is not yet unlocked for this student.' });
|
||||||
|
}
|
||||||
await getPool().query(
|
await getPool().query(
|
||||||
`INSERT INTO activity_completions (student_id, activity_id) VALUES ($1, $2)
|
`INSERT INTO activity_completions (student_id, activity_id) VALUES ($1, $2)
|
||||||
ON CONFLICT (student_id, activity_id) DO NOTHING`,
|
ON CONFLICT (student_id, activity_id) DO NOTHING`,
|
||||||
|
|||||||
Reference in New Issue
Block a user