HarperFast / HarperFast/harper
add_ssh_key / update_ssh_key private keys reach the operations log: `key` is missing from the processLocalTransaction strip list
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 205
Description
`processLocalTransaction` logs every operation body at `info` and strips a list of secret-bearing fields — but not `key`, which is where `add_ssh_key` and `update_ssh_key` carry an SSH **private key**. So the private key is written to the operations log verbatim.
`core/server/serverHelpers/serverUtilities.ts:98-111`:
```js
const {
hdb_user, hdbAuthHeader, password, payload, credentials,
registryAuth, value, values, envelope,
...cleanBody
} = req.body;
operationLog.info(cleanBody); // :111 — `key` is not stripped
```
## The lists already disagree with each other
harper-pro's replication path strips this exact field, deliberately. `harper-pro/replication/logRedaction.ts:13`:
```ts
const SENSITIVE_OPERATION_FIELDS = ['token', 'key', 'password', 'hdbAuthHeader'];
```
with a header comment naming the reason: *"Some replicated/forwarded operations carry secrets — SSH private keys (add_ssh_key / update_ssh_key) …"*. So `key` was already identified as private-key material that must not reach logs, the replication send/receive paths were hardened for it, and the operations log never got the same treatment.
## Two confirmed exposure paths
1. **Origin, client-supplied `key`.** The log line runs *before* the handler, so any sealing the handler does cannot help — the plaintext key as posted is already in the operations log.
2. **Any peer receiving a replicated `add_ssh_key`/`update_ssh_key`.** `harper-pro/replication/replicationConnection.ts:2952` carefully calls `redactOperationForLog` for its own debug line, then passes the **unredacted** object to `server.operation(...)` on the next line. That reaches `operation()` at `serverUtilities.ts:311` → `processLocalTransaction({ body: operation }, …)` → the log line at `:111`, with `key` already populated on the received body. Where the cluster has secret custody this logs the `enc:v1:` envelope (not plaintext, but still the stored credential); on a node with **no** custody, `sealSSHKey` passes the key through unchanged, so peers log the private key in the clear.
Worth noting what is *not* affected, since it is easy to assume otherwise: on the origin, `add_ssh_key generate=true` avoids path 1 entirely, because `req.key` is still `undefined` when the log line runs. The generate path is exposed only via path 2, on peers.
## Reachability
Gated on `harperLogger.logLevel` ∈ {`info`, `debug`, `trace`}; the default is `warn` (`core/static/defaultConfig.yaml`, `logging.level`). So it is off by default — but info/debug is a routine troubleshooting setting, and operations logs are commonly shipped off-host, where a private key then lands in a log aggregator with a different retention and access model than `/ssh/`.
## Fix
Add `key` to the destructured strip list at `serverUtilities.ts:98-109`. One word, and it covers `add_ssh_key` and `update_ssh_key` together. Worth a comment tying it to the `logRedaction.ts` list so the two do not drift again — and it may be worth reconciling the two lists outright, since `token` is stripped by one and not the other.
## Provenance
Found and confirmed by code reading during a `deep-review` of HarperFast/harper-pro#594 (server-side SSH keygen). Pre-existing and unrelated to that PR's diff; filed here rather than there because the defect and the fix are both in core. Cross-ref: HarperFast/harper-pro#594.
Contributor guide
Research direction
Start in core/server/serverHelpers/serverUtilities.ts:98-111 and compare its processLocalTransaction strip list with harper-pro/replication/logRedaction.ts:13. Verify the operations log no longer receives the key field for add_ssh_key and update_ssh_key, including replicated operations, and consider the payload's suggested comment or list reconciliation to prevent drift.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- nodejs, typescript
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100