HarperFast / HarperFast/harper
REST PUT/PATCH bypasses attribute-level insert:false/update:false role permissions (security)
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 205
Description
## Summary
Attribute-level role permissions (`insert:false` / `update:false` in `attribute_permissions`) are **not enforced on the REST write path** (PUT / PATCH). A role that is restricted to read-only (or no access) on a sensitive column can silently **overwrite** that column via a REST write. The ops API and SQL enforce the same permission correctly — only the REST write path skips the check.
**Severity: High** — unauthorized write to attributes a role is explicitly restricted from modifying (e.g. PII, `role`, `balance`, immutable columns).
## Repro
1. Define a `Patient` table and a custom role with:
```json
{ "attribute_name": "ssn", "read": false, "insert": false, "update": false }
{ "attribute_name": "diagnosis", "read": false, "insert": false, "update": false }
```
2. As that role:
- **Read enforcement works:** REST GET / collection GET / GraphQL all return the row *without* `ssn`/`diagnosis`. ✅
- **Write enforcement fails:** `PUT /Patient/` with a full-record body that *includes* `ssn` and `diagnosis` → **204**, and the values are durably overwritten (e.g. `ssn 123-45-6789` → `HACKED-SSN`). ❌ (should be 403)
- REST PATCH including those attrs → same bypass.
3. Contrast (all correctly **403**): ops `update`, ops `insert`, SQL `UPDATE`, SQL `INSERT`.
Reproduces on both storage engines (RocksDB + LMDB), engine-agnostic and deterministic.
## Scope matrix
| Surface | `update:false` attr | Result |
|---|---|---|
| REST PUT | included in body | **BYPASS (204, written)** |
| REST PATCH | included in body | **BYPASS (204, written)** |
| ops `update` | included | 403 blocked ✅ |
| ops `insert` | included | 403 blocked ✅ |
| SQL `UPDATE` | included | 403 blocked ✅ |
| SQL `INSERT` | included | 403 blocked ✅ |
## Root cause (to be verified by an engineer)
The enforcement code exists in `dist/resources/Table.js`: `allowUpdate` (rejects keys whose attr `update` flag is falsy) and `allowCreate` (`insert`), gated in `put` by `if (target.checkPermission)`. ops/SQL are protected by a *separate* pre-invocation `verifyPerms`/`hasPermissions` gate (`serverUtilities.js`); the REST write path relies on the instance-level `allowUpdate`/`allowCreate` check, which is not running.
Working hypothesis: a static→instance argument-shape mismatch (`Resource.put(data, query)` vs instance `put(target, record)`) leaves `target.checkPermission` undefined, so the gate is false and `allowUpdate` never runs. **This is a hypothesis** — the behavioral matrix above and the fact that read enforcement + ops/SQL write enforcement all work are the load-bearing facts; please confirm the exact locus before fixing.
## Suggested fix
Ensure the instance-level `allowUpdate`/`allowCreate` attribute checks run on REST PUT/PATCH (populate `target.checkPermission`), or move attribute write-enforcement into the shared `verifyPerms` pre-gate that already protects ops/SQL so all surfaces enforce uniformly.
---
*Found via exploratory QA (scenario QA-253 / QA-256) against `7aaa5a152` on a feature branch; expected to reproduce on `main`. Filed by Claude (Opus 4.8) on Kris's go-ahead.*
Contributor guide
Research direction
Reproduce the QA-253/QA-256 scenario on main for both PUT and PATCH, then inspect dist/resources/Table.js and serverUtilities.js, comparing the REST path with the working ops and SQL permission gates. Confirm the exact permission-check locus before changing it. Done means restricted attributes produce 403 and are not overwritten through REST PUT/PATCH on both RocksDB and LMDB.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- api, authorization, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100