Add server-side master key rotation support - #46
Conversation
After upserting keys, addData now encodes the resulting filename + updated_at pairs in the response body. The client decodes this and calls os.Chtimes to align each local file's mtime with the server timestamp, eliminating false "server file is newer" prompts on subsequent uploads. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add encapsulation_key field to Machine model and repository for ML-KEM-768 public key storage - Add MasterKeyRotation model and repository (upsert/get/delete per machine) - Add /api/v1/key-rotation routes (POST/GET/DELETE) for distributing and consuming rotation entries - Add /api/v1/machines/public-keys endpoint to fetch all machine public keys for rotation - Store encapsulation key on machine creation (setup and live flows) - In postKeyRotation, skip inserting a rotation entry for the rotating machine itself (server enforces this) - Update docker-compose to reference new DB image tag Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
If the uploading machine has a row in master_key_rotations, return 409 Conflict with a message instructing the user to run 'ssh-sync download' first. This is a server-enforced guard so stale-key uploads cannot silently corrupt data even from old or buggy clients. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… tests The upload guard added to addData requires MasterKeyRotationRepository in the DI injector. Add a hand-written mock and wire it into the three affected tests (TestAddData, TestAddDataBadRequest, TestAddDataError) returning sql.ErrNoRows to indicate no pending rotation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
therealpaulgg
left a comment
There was a problem hiding this comment.
Overall the server-side design is solid — the 409 guard in addData, the machine ID ownership check in postKeyRotation, and the optional encapsulation key on setup/update are all correct. A few things to address:
Bug — zerolog call won't fire (data.go)
log.Err(errors.New("could not get machine from context"))log.Err(err) returns an *zerolog.Event that you must chain .Msg(...) or .Send() on; without it, nothing is logged. Every other error log in this file correctly chains .Msg(...). This one should be:
log.Error().Msg("could not get machine from context")Dev artifact in docker-compose.yaml
The image was changed from therealpaulgg/ssh-sync-db:latest to ssh-sync-db (a locally-built image name). Looks like a leftover from local testing — should be reverted before merge.
Missing tests for keyrotation.go
data_test.go was correctly updated, but there are no tests for the three new key-rotation route handlers (postKeyRotation, getKeyRotation, deleteKeyRotation). Worth adding at minimum: happy-path GET/POST/DELETE, the 404 case from GET, and the forbidden-machine-ID case in POST.
Minor — no 409 test in addData
The new pending-rotation 409 path in addData isn't exercised by any test. Would be good to add a test where GetRotationForMachine returns (rotation, nil) and assert the handler returns 409.
Summary
Machinemodel gainsEncapsulationKey []bytefield; stored on machine creation (setup + live flows) and updateable viaUpdateMachineEncapsulationKey.MasterKeyRotationmodel +MasterKeyRotationRepository(upsert/get/delete per machine)./api/v1/key-rotationroutes (POST/GET/DELETE) for distributing and consuming rotation entries./api/v1/machines/public-keysendpoint returns all machine public keys (EC + PQ) for the rotating client to encrypt to.postKeyRotationskips inserting a rotation entry for the machine performing the rotation — it already has the new key saved locally. This is enforced server-side.Test plan
POST /api/v1/key-rotationwith a payload including the rotating machine's own ID — confirm no row is inserted for it.GET /api/v1/key-rotationfrom another machine that has a pending entry — confirm 200 with encrypted key.DELETE /api/v1/key-rotation— confirm row is removed.GET /api/v1/machines/public-keys— confirm all machines returned with correct keys.🤖 Generated with Claude Code