HarperFast / HarperFast/harper

`X-Replicate-To: 0` bypasses the super-user check and pins a record to one node

Open
#2,546 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

`checkContextPermissions` (`resources/Table.ts:7258`) blocks a non-super-user from setting
replication parameters, but it gates on **truthiness**:

```js
if (context.replicateTo)
throw new ClientError('Can not specify replication parameters without super user permissions', 403);
```

`0` is falsy, so `X-Replicate-To: 0` skips the 403 entirely.

## What that request then does

`server/REST.ts:259-271` parses a single numeric value straight through
(`parsed.length === 1 && +parsed[0] >= 0 ? +parsed[0]`), so `context.replicateTo === 0` reaches
`getResidency` (`resources/Table.ts:1574`). There the guard is `!= undefined`, not truthiness, and
`0 >= 0` takes the count branch:

```js
if (context.replicateTo != undefined) {

if (context.replicateTo >= 0) count = context.replicateTo; // count = 0
}
if (count >= 0 && server.nodes) {
const replicateTo = [server.hostname]; // self only, nothing appended

return replicateTo;
}
```

Residency comes back as `[server.hostname]`. **The record is pinned to the receiving node, replicates
nowhere, and is lost when that node is lost** — which is exactly the caller-specified replication
parameter the 403 exists to prevent.

## Why this is worse than it looks

- **Any authenticated user with write permission on the table can do it.** No super-user role, no
configuration change.
- **It is silent.** The write returns 200. Nothing logs, nothing counts it, and the record looks
normal on the node that holds it. The loss is only visible when that node is.
- **It persists.** Residency is stored with the record, and an update carries the previous residency
forward (`context.previousResidency`), so a single request can take a record out of replication
indefinitely rather than for one write.

## Fix

Gate on presence rather than truthiness, matching what `getResidency` already does:

```js
if (context.replicateTo != undefined) throw new ClientError(…, 403);
if (context.replicatedConfirmation != undefined) throw new ClientError(…, 403);
```

`replicatedConfirmation` has the same truthiness gate at `:7263`. `confirm=0` is benign today —
`server/REST.ts:265` only assigns when `node.next.value >= 0`, and a zero confirmation is a no-op —
but the guard should be corrected with its sibling rather than left as the one that happens not to
matter.

Worth checking as part of the fix whether any other permission or capability guard in the request
path gates a numeric caller-supplied value on truthiness; this one was found by tracing a single
header.

## How this was found

Tracing `X-Replicate-To` semantics while writing the `lock()` guarantee contract for #2498 — the
design note needed to know whether `X-Replicate-To;confirm=` was reachable as a caller-side
mitigation. It is not, for a different reason than the code appears to say.

Refs #483

🤖 Filed by Claude Opus 5 on behalf of Kris.

Contributor guide

Open the contributing guide

Research direction

Start in resources/Table.ts at checkContextPermissions and getResidency, then trace numeric header parsing in server/REST.ts. Verify that presence-based checks cover both replicateTo and replicatedConfirmation, that zero-valued inputs no longer bypass permission checks, and that ordinary replication requests retain their existing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
backend, databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.