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)
- 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
Assessment
This issue has not been assessed yet.