HarperFast / HarperFast/harper-pro

Replicated set_configuration can brick every node in the cluster simultaneously (accepted-then-brick config values propagate with no deep validation)

Open
#558 0 comments 0 reactions 1 assignee Claimed by @kriszyp View on GitHub
area:replication bug
Dominant language
JavaScript
Stars
3
Forks
0
Avg merge
1d 21h
Merged PRs (30d)
80

Description

## Summary

`set_configuration` with `replicated:true` propagates an "accepted-then-brick" config value to **every node in the cluster** with no deep validation anywhere in the path. A subsequent restart of the cluster — a routine rolling-restart or upgrade — bricks every node **simultaneously**, with no healthy peer left to recover from.

This is the same "accepted-then-brick" class as harper#1286 (negative TTL), but `replicated:true` removes the blast-radius protection that made #1286 a single-node inconvenience: since origin and every peer run the identical shallow validator, the whole cluster inherits the exact same landmine at once.

## Repro

harper-pro `e40e8ba4` / core `14ed49560`. 2-node cluster.

1. `set_configuration` on node A: `{ threads_maxHeapMemory: 1, replicated: true }`.
- `threads_maxHeapMemory: 1` (1MB V8 old-generation heap) passes Joi's `number.min(0)` validation (`core/validation/configValidator.ts`) — there's no upper bound configured.
- The call returns `200`, and `response.replicated` shows successful propagation to node B. No rejection anywhere.
2. Restart both nodes (concurrently, simulating a rolling restart/upgrade).
3. **Both nodes fail to start.** Each crashes with an identical native V8 fatal error: `Fatal JavaScript out of memory: GC during deserialization` (uncatchable — a process-level abort, not a JS exception). Confirmed independently (outside the cluster test) that spawning a bare `worker_threads.Worker` with `resourceLimits.maxOldGenerationSizeMb: 1` crashes the same way — `threads_maxHeapMemory` feeds this value directly (`server/threads/manageThreads.js:333`).

## Root cause

`setConfiguration()` (`core/config/configUtils.ts` ~848-859) applies the value locally via `updateConfigValue()` unconditionally, then — only if `replicated:true` — fans it out peer-by-peer via `replication/replicator.ts`'s live per-connection `sendOperationToNode`. The only validation anywhere in the path is a shallow Joi schema (`core/validation/configValidator.ts`), and origin and every peer run the **identical** validator. There's no deeper "will this actually boot" check, and no dry-run/canary step before fan-out — so whatever passes locally propagates identically everywhere.

## Secondary finding (same investigation, lower severity)

A peer that's offline when a replicated config change lands does **not** catch up on that config once it reconnects — `replicated:true` fan-out is live-connection-only, with no durable/audit-logged path for config changes. This is a silent config-divergence gap, worth a follow-up even though it's not the main finding here.

## Suggested fix direction

Either (a) add deep/dry-run validation for config values known to be consumed by native or constructor-level APIs at boot — actually attempt to construct/apply the value in an isolated context before accepting it, not just type/range-check it — generalizing whatever fix direction #1286 eventually takes, applied cluster-wide; or (b) at minimum, canary a replicated change on one peer and confirm it survives a restart before fanning out further.

Test file (not yet promoted to the permanent suite): `integrationTests/cluster/qa-scratch/qa471-replicated-config-brick.test.mjs`.

— KrAIs 🤖 (exploratory QA, on Kris's behalf)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.