support remote registry for production use#10
Conversation
turkosaurus
commented
Apr 5, 2026
turkosaurus
commented
- implement remote server
- better docs
There was a problem hiding this comment.
Pull request overview
This PR refactors how the server authorizes clients by introducing a Registry abstraction (file-backed and remote HTTP-backed), and updates the secrets server to load/refresh registry entries with optional background polling.
Changes:
- Introduce
Registryinterface withFileRegistry(YAML) and newRemoteRegistry(HTTP API) implementation. - Update
Serverto use aRegistry, optional polling refresh, and a pluggable request-allow policy. - Update tests to use
FileRegistryinstead of the removed standalone registry helpers.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| server.go | Switch server authorization to Registry, add polling refresh + request allow hook. |
| registry.go | Replace standalone registry helpers with Registry interface + FileRegistry. |
| registry_remote.go | Add HTTP-backed RemoteRegistry implementation. |
| registry_test.go | Update/expand tests for FileRegistry behavior. |
| locket.go | Add PathRegistry constant used by remote registry. |
| locket_test.go | Update E2E test to use FileRegistry when creating the server. |
| example/testreg.yml | Remove checked-in test registry file. |
| example/temp-1335530413.env | Add checked-in temp env file containing a private key (security risk). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
turkosaurus
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reconcile the remote-registry feature with the request-auth hardening that merged via #11. Conflicts resolved by keeping #10's architecture (Registry interface, registry polling, AllowRequestFunc policy, handlePost, body limit) and folding in #11's hardening: - server.go: nonce/timestamp binding via requestMessage, MaxClockSkew freshness check, seen-nonce replay cache + sweeper, Server.Close, drop decrypted-name log. CIDR enforcement is via #10's AllowCIDR policy (returns 403). - registry.go: #10's FileRegistry already carries the file-close and name normalization fixes; the old free functions are dropped. - tests: port the hardening regression tests and Register-dedup test to #10's FileRegistry/NewServer API.
Server.Close previously stopped only the nonce-cache sweeper, leaking the poll goroutine when the server was created with a non-cancelable context. Derive a child context for poll and cancel it in Close. Add TestServerCloseStopsPoll covering the lifecycle.
FileRegistry.Upsert stripped filepath.Base + ".env" from names, diverging from RemoteRegistry and the cloud registry API (grackleclub/cloud#277), which treat names as opaque, exact-match, case-sensitive identifiers. Remove the rewriting so both Registry implementations are substitutable; deriving a clean service name is the caller's responsibility. Update the dedup test: exact-name re-register dedups in place, while a distinct name (e.g. a .env suffix) is now its own entry.
- Tests now write registries under t.TempDir() instead of example/, so they don't pollute the tracked tree or collide under parallel/read-only runs (TestFileRegistryReadWrite, TestFileRegistryRegister, TestFileRegistryDelete, TestE2E). - NewServer validates Defaults.AllowCIDR up front when allow is nil, failing fast instead of rejecting every request with 403 later.
