HarperFast / HarperFast/harper

Conditional writes (If-Match/If-None-Match) are silently ignored → lost-update hazard

Open
#1,395 0 comments 0 reactions 0 assignees View on GitHub
area:rest-api bug
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Summary

Harper emits real, meaningful `ETag`s on reads, but **honors none of the HTTP write-side preconditions** (`If-Match`, `If-None-Match`) on `PUT`/`PATCH`/`DELETE`. Every conditional write succeeds and mutates state regardless of the precondition, so a client using `If-Match` for optimistic concurrency is **silently unprotected** — a lost-update hazard.

This is the write-side of the long-standing "no safe compare-and-set / `If-Match` ignored" gap.

## Severity

Medium — no single-write corruption, but two concurrent writers using `If-Match` for optimistic concurrency both succeed and one update is silently lost. The danger is that it's a *silent* no-op on a standard mechanism: clients believe they're protected and aren't.

## Repro (self-contained)

On any `@export` table `Doc` with a record `r1`:

```
# 1. read r1, capture its ETag
curl -i http://localhost:9926/Doc/r1 # -> 200, ETag: ""

# 2. conditional update with a WRONG etag -> should be 412 Precondition Failed
curl -i -X PUT -H 'Content-Type: application/json' \
-H 'If-Match: "deadbeef"' \
-d '{"id":"r1","v":2}' http://localhost:9926/Doc/r1
# ACTUAL: 204, record updated to v:2 (EXPECTED: 412, unchanged)

# 3. lost-update proof: two clients hold the same etag , both PUT If-Match: ""
# -> both return 204; the second silently overwrites the first. EXPECTED: 412 on the 2nd.
```

Full matrix observed (method × existence × precondition): **honored = 0**, ignored-hazard = 5, wrong-status = 7.
- `If-Match: ` on PUT/PATCH/DELETE → 204/200 + state changed (should be 412 on mismatch).
- `If-Match: *` on PUT-to-absent → 204 **create** (should be 412).
- `PATCH`-to-missing → 204 implicit create (should be 404); `DELETE`-of-missing → 200 (should be 404).
- `If-None-Match: *` on collection POST → 409 (intrinsic PK conflict; the header itself is ignored). Note: collection POST to a duplicate id *does* correctly 409 — that's an intrinsic check, not precondition handling.

## Root cause

`server/REST.ts` reads only `if-none-match`, and only on the **read** path. There is no `if-match` / `if-unmodified-since` / `412` handling anywhere on the write path.

## Recommendation

Either (a) implement `If-Match` / `If-None-Match` → `412` on writes (standard HTTP optimistic concurrency, pairs with the ETag already emitted), fixing the wrong-status `PATCH`/`DELETE`-to-missing edges alongside; or (b) document loudly that conditional writes are unsupported and silently ignored. Option (a) closes a real lost-update vector.

---
*Surfaced by the QA-explorer exploratory campaign against Harper `7aaa5a152`. The cited file (`server/REST.ts`) is byte-identical to `main` @`6797f091d`, so this reproduces on main. A ready-to-promote regression test (full precondition matrix) exists. Filed by Claude (Opus 4.8) for @kris.*

Contributor guide

Open the contributing guide

Research direction

Start in server/REST.ts and inspect how request headers are handled on write paths versus reads. Run the ready-to-promote precondition regression test and use its full method, existence, and header matrix to verify the expected statuses and unchanged state on failed writes. Confirm whether the project intends to implement the HTTP preconditions or document that they are unsupported.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
api, backend, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.