HarperFast / HarperFast/harper

REST PUT/PATCH bypasses attribute-level insert:false/update:false role permissions (security)

Open
#1,434 0 comments 0 reactions 0 assignees View on GitHub
area:security bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.