HarperFast / HarperFast/harper-pro

add_ssh_key generate=true: a minted key is unusable on peers with different secret custody, and same-name generates diverge — both reported as success

Open
#723 0 comments 0 reactions 0 assignees View on GitHub
area:replication bug
Dominant language
JavaScript
Stars
3
Forks
0
Avg merge
1d 21h
Merged PRs (30d)
80

Description

`add_ssh_key generate=true` (harper-pro#594) seals the minted private key against **the origin node's** secret custody and replicates that envelope. A peer whose custody differs rejects it, the origin still reports success, and because a generated key has no plaintext copy anywhere, the peer can never obtain it by any route.

## Peers routinely have different custody

The `sealSSHKey` docstring says custody is "a cluster keypair" generated on first boot, and `fileKeyCustody.ts:2` calls it "cluster-shared". That is the intent, but the only mechanism that actually shares it is `cloneNode`:

- `cloneNode.ts:663-668` clones the leader's key — and notes the leader "serves custody keys only to node-identity (cert-auth) requests, so a credential/token-auth clone cannot fetch this key — without it, this node generates its own keypair on first boot".
- `add_node` never syncs custody at all (no custody reference anywhere in `replication/`).
- The injected tier is host-provisioned per node.

So in any cluster assembled with `add_node`, or with credential/token-auth clones, peers hold different custody fingerprints. This is the common case, not an edge case.

## a) The peer rejects the envelope and the origin reports success anyway

`encryptEnvelope` stamps the origin's fingerprint as `kid` (`core/utility/secretEnvelope.ts:87`). On the peer, the replicated `add_ssh_key` carries an `enc:v1:` key, so `sealSSHKey` takes the already-sealed branch, compares `kid` to its own fingerprint, and throws `ClientError` (`security/sshKeyOperations.ts:59-64`). `replicateOperation` swallows that into a `{status:'failed'}` entry in `replicated[]` while the origin returns `200` and `message: "Added ssh key: "`.

For a client-supplied key this is recoverable — the operator still has their plaintext copy and can re-add it on the peer. For `generate` there is no plaintext anywhere: `get_ssh_key` returns the origin-sealed envelope, which re-fails the identical `kid` check on both `add_ssh_key` and `update_ssh_key`. The peer is permanently stuck, and the operator has already registered the `public_key` with their git host.

**Fix:** don't seal a generated key against one node's custody. Either fan out `get_secrets_public_key` and seal per destination node, or — minimally — make a peer rejection fatal for `generate`: unlink the key file, strip the config block, and return an error, so the operator is never handed a `public_key` only part of the cluster can use.

## b) Same-name `generate` on two nodes leaves divergent private keys, reported as success

Independent of custody. The duplicate-name guard (`security/sshKeyOperations.ts:169`) is per-node, so two `generate: true` calls for the same name at two different nodes each mint a **different** keypair and write locally. Each then replicates, and each peer rejects with `Key already exists` — again swallowed into `replicated[]` as `failed`, which reads like a benign already-done.

Both callers get `200` plus their own `public_key`. `list_ssh_keys` is identical on both nodes. Nothing in the cluster ever compares SSH key material, so only one node's public half is registered with the remote: git works on that node and fails on every other.

This needs no race — sequential adds on two nodes do it — and `generate` makes it reachable from *identical* request text, e.g. a config-management tool that runs the operation on every node, or an operator retrying against a second node after losing the first response (which #694 makes likely). With a client-supplied `key` the same collision converges on identical material, so the divergence is specific to `generate`.

**Fix:** pre-flight the name across peers before minting (fan out `list_ssh_keys`/`get_ssh_key` and refuse if taken anywhere), and make the response reflect peer failures — at minimum append failed peers to `message` instead of unconditionally reporting `Added ssh key`, so divergence is visible without parsing `replicated[]`.

## Common thread

Both halves share a root cause worth fixing on its own: **`replicateOperation` failures are invisible in the success message.** A caller has to parse `replicated[]` to notice that a write landed on one node and nowhere else.

## Provenance

Confirmed by code reading during a `deep-review` of harper-pro#594, including tracing `replicateOperation` → `sendOperationToNode` → `sendOperation` and the `OPERATION_REQUEST` receive path. Related: #693, #694, #722.

Contributor guide

Open the contributing guide

Research direction

Start by tracing replicateOperation through sendOperationToNode and sendOperation, then read security/sshKeyOperations.ts, core/utility/secretEnvelope.ts, and the custody references in cloneNode.ts and replication/. Reproduce the generate=true cases across peers with different custody or the same name. Done means generated keys cannot be reported as successful when peers reject them, and replication failures are visible to callers.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, typescript
Domain
distributed-systems, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.