geocrop-platform./apps/nextgen/.harness/changelogs/2026-07-17-p2-backup.md

153 lines
8.1 KiB
Markdown

# Changelog — 2026-07-17 — P2 backup daemon (Track G) + P2-5 bulk marks audit
Branch: `ops/sqlite-backup`
Worktree: `.worktrees/ops-sqlite-backup/`
Owner: developer (devops)
Date: 2026-07-17
This change adds the production-host backup / restore / verify tooling for
the Africa Alert SQLite database (P2-1), and documents an audit of the
existing bulk marks CSV import path (P2-5). No application code is
touched and no schema changes are introduced.
---
## Files added
| Path | Purpose |
|---|---|
| `scripts/backup/africa-alert-backup.sh` | Hot online backup via `sqlite3 .backup`. Configurable via `DB_PATH`, `BACKUP_DIR`, `LOG_DIR`, `RETAIN_DAILY` env vars. Copies `-wal`/`-shm` sidecars if present, writes a `sha256sum` next to the snapshot, and prunes the local retention window. |
| `scripts/backup/install-backup-cron.sh` | One-shot installer. Appends a `0 */12 * * *` cron entry (configurable via `CRON_SCHEDULE`), idempotent via a `crontab -l \| grep -q africa-alert` guard. Logs the next-run estimate to `logs/backup-install.log`. |
| `scripts/backup/restore.sh` | Destructive restore. Prompts for typed "yes" at a TTY (or `RESTORE_CONFIRM=yes` from automation), stops `africa-alert.service` via systemctl, takes a `pre-restore-YYYYmmdd-HHMMSS` safety snapshot, copies the backup (and its sidecars) over the live DB, removes any stale live sidecars that do not have a matching backup sidecar, then restarts the service. |
| `scripts/backup/verify-restore.sh` | Smoke-test. Copies the backup to a temp dir, spawns the server with `DB_PATH` and `PORT` overrides, polls `/api/auth/login` until the route is ready, asserts a 200 with the admin creds (`admin@school.com / admin123`), and cleans up. Exits 0 on success, 1 on failure. |
| `docs/BACKUP.md` | Operator runbook. Installation one-liner, manual trigger, restore drill, retention policy (30 daily + 12 weekly with a copy-pasteable find command), failure-alerting contract, and a full disaster-recovery scenario for a bare-metal rebuild. |
No `.sh` script in this change was executed on Windows. They are written
for the production Ubuntu 22.04 host. Verification on this dev machine is
limited to `bash -n` syntax checks (results in §Verification below).
---
## Bulk marks audit
Audit target: `server/src/controllers/marks.controller.js` — the
`POST /api/marks/bulk` endpoint that the teacher CSV importer calls.
### Expected CSV header
```
assignment_id,student_id,score,feedback
```
The controller uses `csv-parse/sync` with `columns: true`, so the first
non-empty row is the header. Columns are lowercased by `trim: true`; the
parser trims whitespace on each value and tolerates a UTF-8 BOM (`bom: true`).
### Per-row behaviour (summary)
1. Validate the four required fields: `assignment_id` and `student_id`
must be present and resolve to a real `assignments.id` / `users.id`;
`score` must be numeric, non-negative, and `<= assignments.max_score`
when `max_score` is non-null. The student must have `role='student'`.
2. Look up an existing `submissions` row by `(assignment_id, student_id)`.
If found, UPDATE the row's `grade`/`feedback`/`graded_by`/`graded_at`,
force `status='graded'`, set `sync_status='pending'`, and bump
`updated_at`. If not found, INSERT a new row with the same fields, a
freshly generated `uid`, and `sync_status='pending'`.
3. The whole loop is wrapped in a single `better-sqlite3` transaction, so
the per-row counters are consistent. Per-row errors are collected into
an `errors` array and the loop continues; a single bad row never
aborts the rest of the batch.
4. After the transaction, the controller writes a best-effort audit log
(`AuditService.log`, action `BULK_UPLOAD_MARKS`, entity type
`submission`) with `{ total_rows, inserted, updated, error_count,
filename }`. Audit failures do not fail the response.
5. Response: `{ inserted, updated, errors: [{ row, reason, data }], total_rows }`.
### Gap analysis
- **No header validation.** A CSV with the wrong header (e.g.
`student_name, score` because the teacher used the wrong export) will
produce 0 inserts and N errors all of the form `missing
assignment_id`, with no upfront "wrong header" message. The
recommended fix is a small header check at the top of the handler
that returns 400 with a clear message naming the expected columns.
- **No teacher ownership / class-scope guard.** The `adminOrTeacher`
middleware is permissive: any teacher can grade any student for any
assignment. A teacher could overwrite another teacher's grades by
re-uploading the same CSV. A `teacher.assigned_classes` /
`assignments.created_by` check would close this gap; not done here
because the existing RBAC refactor (Track C P1-10) is the right place
to add it consistently.
- **No `dry_run` mode.** The handler always commits. A query flag
(`?dry_run=true`) that runs the parse + validation + assignment/student
pre-cache but skips the transaction would let operators catch
malformed CSVs cheaply.
- **Score type is loosely validated.** `Number(scoreRaw)` accepts
floats (`75.5`) even when the assignment expects an integer. The
upper-bound check (`score > a.max_score`) catches overshoots but a
fractional score that happens to be ≤ max_score will be stored as a
float. Either pre-round or require integer explicitly.
- **MIME allowlist is permissive on purpose** (`application/octet-stream`
and empty MIME are accepted so that browsers and curl both work). This
is documented in the controller comment, but is worth a re-review
when the file-attachment work in Track H lands a unified multer
configuration.
### Recommendation
**Keep as-is** for this PR — these gaps are real but small, and the
fixes are best done in the same PR as the broader RBAC refactor
(Track C) and the dry-run work (Track B tests). Track G's scope is
backup tooling, not the marks controller. The audit findings above are
captured here so the next agent who picks up Track C / Track B has them.
---
## Verification
The repo's dev machine is Windows 11. The `.sh` scripts target the
production Ubuntu 22.04 host. Verification on this box is limited to
`bash -n` (syntax-only) using Git Bash. The actual end-to-end smoke
(`bash africa-alert-backup.sh && bash verify-restore.sh`) is run on
the production host after the branch is deployed.
| Check | Result |
|---|---|
| `bash -n scripts/backup/africa-alert-backup.sh` | `SYNTAX_OK` |
| `bash -n scripts/backup/install-backup-cron.sh` | `SYNTAX_OK` |
| `bash -n scripts/backup/restore.sh` | `SYNTAX_OK` |
| `bash -n scripts/backup/verify-restore.sh` | `SYNTAX_OK` |
| `wc -l docs/BACKUP.md` | `283 docs/BACKUP.md` (target: ≥80) |
| Marks bulk-audit findings | documented above; one-sentence recommendation: keep as-is, fix in Track B / Track C |
---
## Deviations from the plan
- **`.gitignore` covered already:** `logs/` is created at the project
root for `backup.log` / `verify-restore.log` / `backup-install.log`.
The existing `*.log` entry in `.gitignore` (line 9) catches the
per-run log files, so no `.gitignore` change is needed.
- **G.8 (Playwright `bulk-marks.spec.ts`) deferred to Track B.** The
plan notes "Skip this if it would risk breaking the existing 24/24
Playwright baseline." The Playwright test infrastructure is on a
different worktree (`feature/playwright-e2e`); adding specs from
this branch would either duplicate fixtures (fragile) or skip
them (defeats the purpose). Track B owns the test infra; the three
specs listed in the plan should be added there.
## Follow-ups
- On the production host, after deploy: run `bash -n` once more (paranoia),
then `sudo /usr/local/bin/install-backup-cron.sh` and `crontab -l` to
confirm the entry. Run one manual backup, then `verify-restore.sh`
against the new snapshot to confirm the round trip.
- Wire `logs/backup.log` to the operator's monitoring (Promtail, mailx,
Telegram webhook, etc.). The contract is in `docs/BACKUP.md` §6.
- Promote the 12-weekly retention in `docs/BACKUP.md` §5 to a real cron
entry. The runbook gives the script; the cron line is left for the
operator to paste.
- Add the four `server/src/controllers/marks.controller.js` audit items
to the Track C / Track B backlogs.