From 3ff9bbe6f769f2d7e17b4036211bbcb7f3a15275 Mon Sep 17 00:00:00 2001 From: turk Date: Sun, 5 Apr 2026 16:31:19 -0600 Subject: [PATCH 1/9] implement remote server --- example/temp-1335530413.env | 1 + example/testreg.yml | 5 - locket.go | 6 + locket_test.go | 14 +- registry.go | 180 +++++++++---------- registry_remote.go | 124 +++++++++++++ registry_test.go | 60 +++++-- server.go | 341 +++++++++++++++++++++++------------- 8 files changed, 484 insertions(+), 247 deletions(-) create mode 100644 example/temp-1335530413.env delete mode 100644 example/testreg.yml create mode 100644 registry_remote.go diff --git a/example/temp-1335530413.env b/example/temp-1335530413.env new file mode 100644 index 0000000..421bb52 --- /dev/null +++ b/example/temp-1335530413.env @@ -0,0 +1 @@ +MY_PRIVATE_KEY="-----BEGIN ED25519 PRIVATE KEY-----\ntm1s9ZofyF7lfrkHno5FnZOHlmMpPyjW2q1MhuWJROc=\n-----END ED25519 PRIVATE KEY-----\n" diff --git a/example/testreg.yml b/example/testreg.yml deleted file mode 100644 index 9492a35..0000000 --- a/example/testreg.yml +++ /dev/null @@ -1,5 +0,0 @@ -- name: SERVICE1 - keypub: | - -----BEGIN ED25519 PUBLIC KEY----- - aOnchd3XQTs0iNNJGbrf4uwknkhjCO4mmLSbnEMUSQo= - -----END ED25519 PUBLIC KEY----- diff --git a/locket.go b/locket.go index 1a00a1f..8d107ab 100644 --- a/locket.go +++ b/locket.go @@ -17,6 +17,12 @@ type defaults struct { BitsizeRSA int // bit size passed to RSA creation for client and server encryption } +// PathRegistry is the API endpoint for registry operations. +// - GET: list all entries +// - POST: upsert an entry (RegEntry JSON body) +// - DELETE: remove an entry (RegEntry JSON body with name) +var PathRegistry = "/locket/registry" + // map[serviceName]keyPrivateSigning type KeysPrivateSigning map[string]string diff --git a/locket_test.go b/locket_test.go index 980dd8b..305bdc5 100644 --- a/locket_test.go +++ b/locket_test.go @@ -17,24 +17,18 @@ func TestE2E(t *testing.T) { pub, priv, err := NewPairEd25519() require.NoError(t, err) - reg := []RegEntry{{ - Name: "SERVICE1", - KeyPub: pub, - }} testReg := path.Join("example", "testreg.yml") - err = WriteRegistry(testReg, reg) + fileReg := FileRegistry{Path: testReg} + err = fileReg.Upsert(RegEntry{Name: "SERVICE1", KeyPub: pub}) require.NoError(t, err) + defer os.Remove(testReg) source := Dotenv{ ServiceSecrets: testServiceMap, Path: path.Join("example", ".env"), } - registry, err := ReadRegistryFile(testReg) - require.NoError(t, err) - require.NotNil(t, registry) - require.Greater(t, len(registry), 0) - server, err := NewServer(source, registry) + server, err := NewServer(source, fileReg, 0, nil) require.NoError(t, err) handler := httptest.NewServer(http.HandlerFunc(server.Handler)) diff --git a/registry.go b/registry.go index 0868d2e..68dfee0 100644 --- a/registry.go +++ b/registry.go @@ -9,125 +9,125 @@ import ( "gopkg.in/yaml.v3" ) -/* -Registry is the process by which pre-computed signing keys (ed25519) -are created before deploying either server or client. - - Public signing keys for all allowed services are provided to the server via .yml - - Public and private keys are provided to the client for signing requests. - - Separately, both client and server create encryption keys on startup. -*/ - -// RegEntry is a single registry item, -// representing a single client which -// the server should recognize and authorize +// RegEntry is a single authorized client, identified by name +// and authenticated by its ed25519 public signing key. type RegEntry struct { - Name string `yaml:"name"` - KeyPub string `yaml:"keypub"` + Name string `yaml:"name" json:"name"` + KeyPub string `yaml:"keypub" json:"keypub"` } -// WriteRegistry creates a yaml file with a registry of allowed clients. -func WriteRegistry(path string, data []RegEntry) error { - f, err := os.Create(path) - if err != nil { - return fmt.Errorf("create file: %w", err) - } - - for i, item := range data { - data[i].Name = strings.TrimSuffix(filepath.Base(item.Name), ".env") - } +// Registry reads and writes authorized client entries. +// Implementations include FileRegistry (local YAML) and +// RemoteRegistry (HTTP API). +type Registry interface { + // Entries returns all authorized clients. + Entries() ([]RegEntry, error) + // Upsert inserts or updates a client entry by name. + Upsert(RegEntry) error + // Delete removes a client entry by name. + Delete(name string) error +} - b, err := yaml.Marshal(data) - if err != nil { - return fmt.Errorf("marshal: %w", err) - } - _, err = f.Write(b) - if err != nil { - return fmt.Errorf("write file: %w", err) - } - return nil +// FileRegistry is a Registry backed by a local YAML file. +type FileRegistry struct { + Path string } -// ReadRegistryFile turns a yaml file into a list of RegEntry -// for use in server authenticating client requests. -func ReadRegistryFile(filepath string) ([]RegEntry, error) { - f, err := os.ReadFile(filepath) +// Entries reads all authorized clients from the YAML file. +func (f FileRegistry) Entries() ([]RegEntry, error) { + b, err := os.ReadFile(f.Path) if err != nil { return nil, fmt.Errorf("read file: %w", err) } var out []RegEntry - err = yaml.Unmarshal(f, &out) + err = yaml.Unmarshal(b, &out) if err != nil { return nil, fmt.Errorf("unmarshal: %w", err) } return out, nil } -// UnmarshalRegistry turns a byte slice into a list of RegEntry -// for use in server authenticating client requests. -// Bytes format easier for embed.FS -func UnmarshalRegistry(bytes []byte) ([]RegEntry, error) { - var out []RegEntry - err := yaml.Unmarshal(bytes, &out) - if err != nil { - return nil, fmt.Errorf("unmarshal: %w", err) - } - return out, nil -} +// Upsert inserts or updates a client entry in the YAML file. +// If the file does not exist, it is created. +func (f FileRegistry) Upsert(entry RegEntry) error { + entry.Name = strings.TrimSuffix( + filepath.Base(entry.Name), ".env", + ) -// Register reads the existing registry file, upserts service key, and rewrites. -// If the registry file does not exist, it will be created. -// If no registry for the named service exists, a new entry will be created. -// An existing entry for the named service will be updated with new public key. -// Each new call of Register will generate new key pair, returning: -// public key, private key, or any error. -func Register(name string, registryPath string) (string, string, error) { - publicKey, privateKey, err := NewPairEd25519() - if err != nil { - return "", "", fmt.Errorf("generate key pair: %w", err) - } - - var registry []RegEntry - _, err = os.Stat(registryPath) - if err == nil { - registry, err = ReadRegistryFile(registryPath) + var entries []RegEntry + if _, err := os.Stat(f.Path); err == nil { + entries, err = f.Entries() if err != nil { - return "", "", fmt.Errorf("read registry file: %w", err) + return fmt.Errorf("read existing: %w", err) } - } else { - log.Debug( - "registry file does not exist (or other err); will create new one", - "registryPath", registryPath, - "statError", err, - ) } - // check if the service already exists in the registry replaced := false - for i, entry := range registry { - if entry.Name == name { - log.Debug("updating existing service in registry", - "service", name, - "publicKey", publicKey, - ) - registry[i].KeyPub = publicKey + for i, e := range entries { + if e.Name == entry.Name { + entries[i].KeyPub = entry.KeyPub replaced = true + break } } if !replaced { - log.Debug("adding new service to registry", - "service", name, - "publicKey", publicKey, + entries = append(entries, entry) + } + + return f.write(entries) +} + +// Delete removes a client entry by name from the YAML file. +func (f FileRegistry) Delete(name string) error { + entries, err := f.Entries() + if err != nil { + return fmt.Errorf("read existing: %w", err) + } + filtered := entries[:0] + for _, e := range entries { + if e.Name != name { + filtered = append(filtered, e) + } + } + return f.write(filtered) +} + +// Register generates a new ed25519 signing keypair, upserts the +// public key into the YAML file, and returns the keypair. +func (f FileRegistry) Register(name string) (string, string, error) { + pub, priv, err := NewPairEd25519() + if err != nil { + return "", "", fmt.Errorf("generate key pair: %w", err) + } + err = f.Upsert(RegEntry{Name: name, KeyPub: pub}) + if err != nil { + return "", "", fmt.Errorf("upsert: %w", err) + } + return pub, priv, nil +} + +// write serializes entries to the YAML file, creating or +// truncating it as needed. +func (f FileRegistry) write(entries []RegEntry) error { + file, err := os.Create(f.Path) + if err != nil { + return fmt.Errorf("create file: %w", err) + } + defer file.Close() + + for i, e := range entries { + entries[i].Name = strings.TrimSuffix( + filepath.Base(e.Name), ".env", ) - registry = append(registry, RegEntry{ - Name: name, - KeyPub: publicKey, - }) } - // write the updated registry - err = WriteRegistry(registryPath, registry) + b, err := yaml.Marshal(entries) + if err != nil { + return fmt.Errorf("marshal: %w", err) + } + _, err = file.Write(b) if err != nil { - return "", "", fmt.Errorf("write updated registry: %w", err) + return fmt.Errorf("write file: %w", err) } - return publicKey, privateKey, nil + return nil } diff --git a/registry_remote.go b/registry_remote.go new file mode 100644 index 0000000..9f9695f --- /dev/null +++ b/registry_remote.go @@ -0,0 +1,124 @@ +package locket + +import ( + "bytes" + "encoding/json" + "fmt" + "net/http" +) + +// RemoteRegistry is a Registry backed by an HTTP API. +// URL is the base URL (e.g. "http://api:8888") to which +// PathRegistry is appended for all operations. +// Token, if set, is sent as an X-Auth-Token header. +type RemoteRegistry struct { + URL string + Token string +} + +// endpoint returns the full URL to the registry API. +func (r RemoteRegistry) endpoint() string { + return r.URL + PathRegistry +} + +// Entries fetches all authorized clients from the remote API. +func (r RemoteRegistry) Entries() ([]RegEntry, error) { + req, err := http.NewRequest( + http.MethodGet, r.endpoint(), nil, + ) + if err != nil { + return nil, fmt.Errorf("new request: %w", err) + } + r.setHeaders(req) + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, fmt.Errorf("do request: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("status %d", resp.StatusCode) + } + + var entries []RegEntry + err = json.NewDecoder(resp.Body).Decode(&entries) + if err != nil { + return nil, fmt.Errorf("decode: %w", err) + } + return entries, nil +} + +// Upsert creates or updates an authorized client via the remote API. +func (r RemoteRegistry) Upsert(entry RegEntry) error { + b, err := json.Marshal(entry) + if err != nil { + return fmt.Errorf("marshal: %w", err) + } + + req, err := http.NewRequest( + http.MethodPost, r.endpoint(), bytes.NewReader(b), + ) + if err != nil { + return fmt.Errorf("new request: %w", err) + } + r.setHeaders(req) + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return fmt.Errorf("do request: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("status %d", resp.StatusCode) + } + return nil +} + +// Delete removes an authorized client by name via the remote API. +func (r RemoteRegistry) Delete(name string) error { + b, err := json.Marshal(RegEntry{Name: name}) + if err != nil { + return fmt.Errorf("marshal: %w", err) + } + + req, err := http.NewRequest( + http.MethodDelete, r.endpoint(), bytes.NewReader(b), + ) + if err != nil { + return fmt.Errorf("new request: %w", err) + } + r.setHeaders(req) + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return fmt.Errorf("do request: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("status %d", resp.StatusCode) + } + return nil +} + +// Register generates a new ed25519 signing keypair, upserts the +// public key via the remote API, and returns the keypair. +func (r RemoteRegistry) Register(name string) (string, string, error) { + pub, priv, err := NewPairEd25519() + if err != nil { + return "", "", fmt.Errorf("generate key pair: %w", err) + } + err = r.Upsert(RegEntry{Name: name, KeyPub: pub}) + if err != nil { + return "", "", fmt.Errorf("upsert: %w", err) + } + return pub, priv, nil +} + +// setHeaders applies auth headers to the request. +func (r RemoteRegistry) setHeaders(req *http.Request) { + if r.Token != "" { + req.Header.Set("X-Auth-Token", r.Token) + } +} diff --git a/registry_test.go b/registry_test.go index dd43024..2aefbbd 100644 --- a/registry_test.go +++ b/registry_test.go @@ -21,11 +21,15 @@ var testRegistryItems = []RegEntry{ var testExampleReg = path.Join("example", "registry.yml") -func TestReadWrite(t *testing.T) { - err := WriteRegistry(testExampleReg, testRegistryItems) - require.NoError(t, err) +func TestFileRegistryReadWrite(t *testing.T) { + reg := FileRegistry{Path: testExampleReg} + + for _, item := range testRegistryItems { + err := reg.Upsert(item) + require.NoError(t, err) + } - items, err := ReadRegistryFile(testExampleReg) + items, err := reg.Entries() require.NoError(t, err) require.NotNil(t, items) require.Equal(t, testRegistryItems, items) @@ -34,36 +38,58 @@ func TestReadWrite(t *testing.T) { } } -func TestRegister(t *testing.T) { +func TestFileRegistryRegister(t *testing.T) { testRegistry := path.Join("example", "test-registry.yml") + reg := FileRegistry{Path: testRegistry} services := []string{"service A", "service B", "service C"} - // no file exists + for _, service := range services { - pub, priv, err := Register(service, testRegistry) + pub, priv, err := reg.Register(service) require.NoError(t, err) require.NotEmpty(t, pub) require.NotEmpty(t, priv) t.Logf("Registered %q", service) - registry, err := ReadRegistryFile(testRegistry) + entries, err := reg.Entries() require.NoError(t, err) - for _, item := range registry { - t.Logf("%s\n%s", item.Name, item.KeyPub) + for _, e := range entries { + t.Logf("%s\n%s", e.Name, e.KeyPub) } } - // replace/update/upsert service A with new key - pub, priv, err := Register(services[0], testRegistry) + + // upsert service A with new key + pub, priv, err := reg.Register(services[0]) require.NoError(t, err) require.NotEmpty(t, pub) require.NotEmpty(t, priv) t.Logf("Updated %q", services[0]) - registry, err := ReadRegistryFile(testRegistry) + + entries, err := reg.Entries() require.NoError(t, err) - for _, item := range registry { - t.Logf("%s\n%s", item.Name, item.KeyPub) + for _, e := range entries { + t.Logf("%s\n%s", e.Name, e.KeyPub) } - // cleanup file + err = os.Remove(testRegistry) require.NoError(t, err) - t.Logf("Removed test registry file: %s", testRegistry) +} + +func TestFileRegistryDelete(t *testing.T) { + testRegistry := path.Join("example", "test-delete.yml") + reg := FileRegistry{Path: testRegistry} + defer os.Remove(testRegistry) + + require.NoError(t, reg.Upsert(RegEntry{Name: "a", KeyPub: "k1"})) + require.NoError(t, reg.Upsert(RegEntry{Name: "b", KeyPub: "k2"})) + + entries, err := reg.Entries() + require.NoError(t, err) + require.Len(t, entries, 2) + + require.NoError(t, reg.Delete("a")) + + entries, err = reg.Entries() + require.NoError(t, err) + require.Len(t, entries, 1) + require.Equal(t, "b", entries[0].Name) } diff --git a/server.go b/server.go index 7bdbbea..e0b1096 100644 --- a/server.go +++ b/server.go @@ -6,33 +6,84 @@ import ( "net" "net/http" "strings" + "sync" + "time" "github.com/google/uuid" ) +// AllowRequestFunc decides whether an HTTP request is permitted. +// Return nil to allow, or an error to deny with 403. +type AllowRequestFunc func(r *http.Request) error + +// AllowCIDR returns an AllowRequestFunc that permits requests from +// the given CIDR range only. This is the default policy when +// none is provided to NewServer. +func AllowCIDR(cidr string) AllowRequestFunc { + return func(r *http.Request) error { + ip, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return fmt.Errorf("parse remote addr: %w", err) + } + clientIP := net.ParseIP(ip) + _, network, err := net.ParseCIDR(cidr) + if err != nil { + return fmt.Errorf("parse CIDR: %w", err) + } + if !network.Contains(clientIP) { + return fmt.Errorf("IP %s not in %s", ip, cidr) + } + return nil + } +} + +// Server serves secrets to authenticated clients over HTTP. +// It validates client requests against a Registry of authorized +// signing keys, refreshing the registry on a configurable interval. type Server struct { - secrets map[string]Secrets // secret k/v pairs - registry []RegEntry // registered services - keyRsaPublic string // encryption public key - keyRsaPrivate string // encryption private key + secrets map[string]Secrets + reg Registry + entries []RegEntry + mu sync.RWMutex + allow AllowRequestFunc + keyRsaPublic string + keyRsaPrivate string } -// kvResponse is the server's response to the client's request, -// containing the encrypted secret value. +// kvResponse is the server's encrypted secret response. type kvResponse struct { Payload string `json:"payload"` } -// NewServer sets up a new secrets server when provided source options -// and registry of allowed services (and their public signing keys), -// expected to be read from file or embed before calling NewServer(). -func NewServer(opts source, registry []RegEntry) (*Server, error) { +// NewServer creates a Server, loading secrets from the given source +// and authorized clients from the given Registry. If pollInterval +// is positive, the server refreshes its registry in the background. +// If allow is nil, AllowCIDR(Defaults.AllowCIDR) is used. +func NewServer( + opts source, + reg Registry, + pollInterval time.Duration, + allow AllowRequestFunc, +) (*Server, error) { rsaPublic, rsaPrivate, err := newPairRSA(Defaults.BitsizeRSA) if err != nil { return nil, fmt.Errorf("generate key pair: %w", err) } - server := Server{ - registry: registry, + + entries, err := reg.Entries() + if err != nil { + return nil, fmt.Errorf("initial registry fetch: %w", err) + } + log.Info("registry loaded", "entries", len(entries)) + + if allow == nil { + allow = AllowCIDR(Defaults.AllowCIDR) + } + + server := &Server{ + reg: reg, + entries: entries, + allow: allow, keyRsaPublic: rsaPublic, keyRsaPrivate: rsaPrivate, } @@ -44,32 +95,69 @@ func NewServer(opts source, registry []RegEntry) (*Server, error) { return nil, fmt.Errorf("load env: %w", err) } server.secrets = secrets - return &server, nil case Dotenv: if len(opts.ServiceSecrets) == 0 { - return nil, fmt.Errorf("at least one service required to load *.env file") + return nil, fmt.Errorf( + "at least one service required to load *.env file", + ) } if opts.Path == "" { - return nil, fmt.Errorf("at least one path required to *.env file") + return nil, fmt.Errorf( + "at least one path required to *.env file", + ) } secrets, err := opts.Load() if err != nil { return nil, fmt.Errorf("load .env file: %w", err) } server.secrets = secrets - return &server, nil case Onepass: secrets, err := opts.Load() if err != nil { return nil, fmt.Errorf("load onepass: %w", err) } server.secrets = secrets - return &server, nil default: return nil, fmt.Errorf("invalid source") } + + if pollInterval > 0 { + go server.poll(pollInterval) + } + + return server, nil +} + +// poll refreshes the registry on a fixed interval. +func (s *Server) poll(interval time.Duration) { + ticker := time.NewTicker(interval) + defer ticker.Stop() + for range ticker.C { + entries, err := s.reg.Entries() + if err != nil { + log.Error("registry poll failed", "error", err) + continue + } + s.mu.Lock() + s.entries = entries + s.mu.Unlock() + log.Debug("registry refreshed", "entries", len(entries)) + } +} + +// registrySnapshot returns a point-in-time copy of the registry. +func (s *Server) registrySnapshot() []RegEntry { + s.mu.RLock() + defer s.mu.RUnlock() + out := make([]RegEntry, len(s.entries)) + copy(out, s.entries) + return out } +// Handler is the HTTP handler for the locket secret server. +// GET returns the server's RSA public encryption key. +// POST accepts an encrypted, signed secret request and returns +// the encrypted secret value. func (s *Server) Handler(w http.ResponseWriter, r *http.Request) { id := uuid.New().String() log.Info("received request", @@ -80,132 +168,135 @@ func (s *Server) Handler(w http.ResponseWriter, r *http.Request) { ) switch r.Method { case http.MethodOptions: - w.Header().Set("Allow", fmt.Sprintf("%s, %s", http.MethodGet, http.MethodPost)) + w.Header().Set("Allow", fmt.Sprintf( + "%s, %s", http.MethodGet, http.MethodPost, + )) return case http.MethodGet: w.Header().Set("Content-Type", "text/plain") w.Write([]byte(s.keyRsaPublic)) return case http.MethodPost: - var request kvRequest - // log.Debug("decoding request", "body", r.Body) - err := json.NewDecoder(r.Body).Decode(&request) - if err != nil { - http.Error(w, "bad request", http.StatusBadRequest) - return - } - log.Debug("request", - "payload", request.Payload, - "client_pubkey", request.ClientPubKey, - "signature", request.PayloadSignature, + s.handlePost(w, r, id) + default: + log.Warn("method not allowed", + "method", r.Method, "request_id", id, + "ip", r.RemoteAddr, ) - payload, err := decryptRSA(s.keyRsaPrivate, request.Payload) - if err != nil { - log.Error("decrypt payload", "request_id", id, "error", err) - http.Error(w, "bad request", http.StatusBadRequest) - } - log.Debug("request payload decrypted", "payload", payload, "request_id", id) - - // require from CIDR range DefaultAllowCIDR - ip, _, err := net.SplitHostPort(r.RemoteAddr) - if err != nil { - log.Error("split host port", "request_id", id, "error", err) - // maybe a 5xx, but probably only because of malformed host addr - http.Error(w, "bad request", http.StatusBadRequest) - return - } - clientIP := net.ParseIP(ip) - _, cidr, err := net.ParseCIDR(Defaults.AllowCIDR) - if err != nil { - log.Error("parse CIDR", "request_id", id, "error", err) - http.Error(w, "bad request", http.StatusBadRequest) - return - } - if !cidr.Contains(clientIP) { - log.Warn("IP rejected", - "request_id", id, - "ip", r.RemoteAddr, - "allowCIDR", Defaults.AllowCIDR, - ) - http.Error(w, "forbidden", http.StatusForbidden) - } else { - log.Debug("IP allowed", - "request_id", id, - "ip", r.RemoteAddr, - "allowCIDR", Defaults.AllowCIDR, - ) - } + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } +} - // verify signature against registry - var matches bool - var verifiedService string - log.Debug("verifying signature", "request_id", id) - for _, svc := range s.registry { - match, err := verifyEd25519(svc.KeyPub, payload, request.PayloadSignature) - if err != nil { - log.Error("verify signature", "request_id", id, "error", err) - http.Error(w, "bad request", http.StatusBadRequest) - } - if match { - matches = true - verifiedService = svc.Name - break - } - } - if !matches { - log.Error("signature mismatch", "request_id", id) - http.Error(w, "forbidden", http.StatusForbidden) - return - } else { - log.Debug("signature verified", - "service", verifiedService, - "request_id", id, - ) - } +// handlePost processes an encrypted secret request. +func (s *Server) handlePost( + w http.ResponseWriter, r *http.Request, id string, +) { + var request kvRequest + err := json.NewDecoder(r.Body).Decode(&request) + if err != nil { + http.Error(w, "bad request", http.StatusBadRequest) + return + } + log.Debug("request", + "payload", request.Payload, + "client_pubkey", request.ClientPubKey, + "signature", request.PayloadSignature, + "request_id", id, + ) - log.Debug("secrets for service", "service", verifiedService, "secrets_qty", len(s.secrets)) - secrets, ok := s.secrets[strings.ToLower(verifiedService)] - if !ok { - log.Warn("service not found in registry, check case sensitivity (expects lower)", - "service_requesting", verifiedService, - "request_id", id, - ) - http.Error(w, "forbidden", http.StatusForbidden) - return - } + payload, err := decryptRSA(s.keyRsaPrivate, request.Payload) + if err != nil { + log.Error("decrypt payload", + "request_id", id, "error", err, + ) + http.Error(w, "bad request", http.StatusBadRequest) + return + } + log.Debug("request payload decrypted", + "payload", payload, "request_id", id, + ) - value, ok := secrets[payload] - if !ok { - log.Warn("secret not found", "service", verifiedService, "key", payload, "request_id", id) - http.Error(w, "not found", http.StatusNotFound) - return - } + if err := s.allow(r); err != nil { + log.Warn("request denied", + "request_id", id, + "ip", r.RemoteAddr, + "error", err, + ) + http.Error(w, "forbidden", http.StatusForbidden) + return + } - ecryptedSecret, err := encryptRSA(request.ClientPubKey, value) - if err != nil { - log.Error("encrypt secret", "request_id", id, "error", err) - http.Error(w, "server error", http.StatusInternalServerError) - return - } - response := kvResponse{ - Payload: ecryptedSecret, - } - err = json.NewEncoder(w).Encode(response) + // verify signature against registry + registry := s.registrySnapshot() + var verifiedService string + for _, svc := range registry { + match, err := verifyEd25519( + svc.KeyPub, payload, request.PayloadSignature, + ) if err != nil { - log.Error("encode response", "request_id", id, "error", err) + log.Error("verify signature", + "request_id", id, "error", err, + ) http.Error(w, "bad request", http.StatusBadRequest) return } - log.Info("sending secret", + if match { + verifiedService = svc.Name + break + } + } + if verifiedService == "" { + log.Error("signature mismatch", "request_id", id) + http.Error(w, "forbidden", http.StatusForbidden) + return + } + log.Debug("signature verified", + "service", verifiedService, "request_id", id, + ) + + secrets, ok := s.secrets[strings.ToLower(verifiedService)] + if !ok { + log.Warn("service not found, check case (expects lower)", "service", verifiedService, - "name", payload, - "ip", r.RemoteAddr, "request_id", id, ) - w.Header().Set("Content-Type", "application/json") - default: - log.Warn("method not allowed", "method", r.Method, "request_id", id, "ip", r.RemoteAddr) - http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + http.Error(w, "forbidden", http.StatusForbidden) + return + } + + value, ok := secrets[payload] + if !ok { + log.Warn("secret not found", + "service", verifiedService, + "key", payload, + "request_id", id, + ) + http.Error(w, "not found", http.StatusNotFound) + return + } + + encrypted, err := encryptRSA(request.ClientPubKey, value) + if err != nil { + log.Error("encrypt secret", + "request_id", id, "error", err, + ) + http.Error(w, "server error", http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + err = json.NewEncoder(w).Encode(kvResponse{Payload: encrypted}) + if err != nil { + log.Error("encode response", + "request_id", id, "error", err, + ) + return } + log.Info("sending secret", + "service", verifiedService, + "name", payload, + "ip", r.RemoteAddr, + "request_id", id, + ) } From 7e2e25f74d267c0436f297d08a58c6060ef58200 Mon Sep 17 00:00:00 2001 From: turk Date: Sun, 5 Apr 2026 16:36:46 -0600 Subject: [PATCH 2/9] better docs --- registry.go | 13 +++++++++++-- registry_test.go | 6 +++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/registry.go b/registry.go index 68dfee0..7dd2e5e 100644 --- a/registry.go +++ b/registry.go @@ -9,8 +9,17 @@ import ( "gopkg.in/yaml.v3" ) -// RegEntry is a single authorized client, identified by name -// and authenticated by its ed25519 public signing key. +/* +Registry is the process by which pre-computed signing keys (ed25519) +are created before deploying either server or client. + - Public signing keys for all allowed services are provided to the server. + - Public and private keys are provided to the client for signing requests. + - Separately, both client and server create encryption keys on startup. +*/ + +// RegEntry is a single registry item, +// representing a single client which +// the server should recognize and authorize. type RegEntry struct { Name string `yaml:"name" json:"name"` KeyPub string `yaml:"keypub" json:"keypub"` diff --git a/registry_test.go b/registry_test.go index 2aefbbd..d2b516c 100644 --- a/registry_test.go +++ b/registry_test.go @@ -19,10 +19,10 @@ var testRegistryItems = []RegEntry{ }, } -var testExampleReg = path.Join("example", "registry.yml") - func TestFileRegistryReadWrite(t *testing.T) { - reg := FileRegistry{Path: testExampleReg} + p := path.Join("example", "test-readwrite.yml") + reg := FileRegistry{Path: p} + defer os.Remove(p) for _, item := range testRegistryItems { err := reg.Upsert(item) From 71f51659df2524f9e1c3ea67a2948df10524b7f2 Mon Sep 17 00:00:00 2001 From: turk Date: Sun, 5 Apr 2026 17:03:05 -0600 Subject: [PATCH 3/9] respond to pr --- .claude/settings.local.json | 8 ++++++ .gitignore | 2 ++ example/temp-1335530413.env | 1 - locket.go | 9 +++++++ locket_test.go | 3 ++- registry.go | 6 ++++- registry_remote.go | 20 ++++++++++---- server.go | 54 ++++++++++++++++++++++++++----------- 8 files changed, 79 insertions(+), 24 deletions(-) create mode 100644 .claude/settings.local.json create mode 100644 .gitignore delete mode 100644 example/temp-1335530413.env diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 0000000..4265275 --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,8 @@ +{ + "permissions": { + "allow": [ + "Read(//home/turk/dotfiles/**)", + "Read(//home/turk/.local/bin/**)" + ] + } +} diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..58c8ccb --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +temp-*.env +plan.md diff --git a/example/temp-1335530413.env b/example/temp-1335530413.env deleted file mode 100644 index 421bb52..0000000 --- a/example/temp-1335530413.env +++ /dev/null @@ -1 +0,0 @@ -MY_PRIVATE_KEY="-----BEGIN ED25519 PRIVATE KEY-----\ntm1s9ZofyF7lfrkHno5FnZOHlmMpPyjW2q1MhuWJROc=\n-----END ED25519 PRIVATE KEY-----\n" diff --git a/locket.go b/locket.go index 8d107ab..b42d2a6 100644 --- a/locket.go +++ b/locket.go @@ -23,6 +23,15 @@ type defaults struct { // - DELETE: remove an entry (RegEntry JSON body with name) var PathRegistry = "/locket/registry" +// Environment variable names used by locket clients and servers. +var ( + EnvURL = "LOCKET_URL" // locket server URL + EnvPublic = "LOCKET_PUBLIC" // ed25519 public signing key + EnvPrivate = "LOCKET_PRIVATE" // ed25519 private signing key + EnvRegistryURL = "LOCKET_REGISTRY_URL" // registry API base URL + EnvRegistryToken = "LOCKET_REGISTRY_TOKEN" // registry API auth token +) + // map[serviceName]keyPrivateSigning type KeysPrivateSigning map[string]string diff --git a/locket_test.go b/locket_test.go index 305bdc5..fc14c2b 100644 --- a/locket_test.go +++ b/locket_test.go @@ -1,6 +1,7 @@ package locket import ( + "context" "fmt" "net/http" "net/http/httptest" @@ -28,7 +29,7 @@ func TestE2E(t *testing.T) { Path: path.Join("example", ".env"), } - server, err := NewServer(source, fileReg, 0, nil) + server, err := NewServer(context.Background(), source, fileReg, 0, nil) require.NoError(t, err) handler := httptest.NewServer(http.HandlerFunc(server.Handler)) diff --git a/registry.go b/registry.go index 7dd2e5e..390cd1c 100644 --- a/registry.go +++ b/registry.go @@ -64,11 +64,15 @@ func (f FileRegistry) Upsert(entry RegEntry) error { ) var entries []RegEntry - if _, err := os.Stat(f.Path); err == nil { + _, err := os.Stat(f.Path) + switch { + case err == nil: entries, err = f.Entries() if err != nil { return fmt.Errorf("read existing: %w", err) } + case !os.IsNotExist(err): + return fmt.Errorf("stat file: %w", err) } replaced := false diff --git a/registry_remote.go b/registry_remote.go index 9f9695f..d6ce2bb 100644 --- a/registry_remote.go +++ b/registry_remote.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "time" ) // RemoteRegistry is a Registry backed by an HTTP API. @@ -12,8 +13,17 @@ import ( // PathRegistry is appended for all operations. // Token, if set, is sent as an X-Auth-Token header. type RemoteRegistry struct { - URL string - Token string + URL string + Token string + Client *http.Client +} + +// client returns the configured HTTP client, or a default with timeout. +func (r RemoteRegistry) client() *http.Client { + if r.Client != nil { + return r.Client + } + return &http.Client{Timeout: 10 * time.Second} } // endpoint returns the full URL to the registry API. @@ -31,7 +41,7 @@ func (r RemoteRegistry) Entries() ([]RegEntry, error) { } r.setHeaders(req) - resp, err := http.DefaultClient.Do(req) + resp, err := r.client().Do(req) if err != nil { return nil, fmt.Errorf("do request: %w", err) } @@ -64,7 +74,7 @@ func (r RemoteRegistry) Upsert(entry RegEntry) error { r.setHeaders(req) req.Header.Set("Content-Type", "application/json") - resp, err := http.DefaultClient.Do(req) + resp, err := r.client().Do(req) if err != nil { return fmt.Errorf("do request: %w", err) } @@ -91,7 +101,7 @@ func (r RemoteRegistry) Delete(name string) error { r.setHeaders(req) req.Header.Set("Content-Type", "application/json") - resp, err := http.DefaultClient.Do(req) + resp, err := r.client().Do(req) if err != nil { return fmt.Errorf("do request: %w", err) } diff --git a/server.go b/server.go index e0b1096..2347b78 100644 --- a/server.go +++ b/server.go @@ -1,6 +1,7 @@ package locket import ( + "context" "encoding/json" "fmt" "net" @@ -20,16 +21,17 @@ type AllowRequestFunc func(r *http.Request) error // the given CIDR range only. This is the default policy when // none is provided to NewServer. func AllowCIDR(cidr string) AllowRequestFunc { + _, network, cidrErr := net.ParseCIDR(cidr) + return func(r *http.Request) error { + if cidrErr != nil { + return fmt.Errorf("parse CIDR: %w", cidrErr) + } ip, _, err := net.SplitHostPort(r.RemoteAddr) if err != nil { return fmt.Errorf("parse remote addr: %w", err) } clientIP := net.ParseIP(ip) - _, network, err := net.ParseCIDR(cidr) - if err != nil { - return fmt.Errorf("parse CIDR: %w", err) - } if !network.Contains(clientIP) { return fmt.Errorf("IP %s not in %s", ip, cidr) } @@ -60,11 +62,16 @@ type kvResponse struct { // is positive, the server refreshes its registry in the background. // If allow is nil, AllowCIDR(Defaults.AllowCIDR) is used. func NewServer( + ctx context.Context, opts source, reg Registry, pollInterval time.Duration, allow AllowRequestFunc, ) (*Server, error) { + if reg == nil { + return nil, fmt.Errorf("registry must not be nil") + } + rsaPublic, rsaPrivate, err := newPairRSA(Defaults.BitsizeRSA) if err != nil { return nil, fmt.Errorf("generate key pair: %w", err) @@ -122,26 +129,31 @@ func NewServer( } if pollInterval > 0 { - go server.poll(pollInterval) + go server.poll(ctx, pollInterval) } return server, nil } -// poll refreshes the registry on a fixed interval. -func (s *Server) poll(interval time.Duration) { +// poll refreshes the registry on a fixed interval until ctx is cancelled. +func (s *Server) poll(ctx context.Context, interval time.Duration) { ticker := time.NewTicker(interval) defer ticker.Stop() - for range ticker.C { - entries, err := s.reg.Entries() - if err != nil { - log.Error("registry poll failed", "error", err) - continue + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + entries, err := s.reg.Entries() + if err != nil { + log.Error("registry poll failed", "error", err) + continue + } + s.mu.Lock() + s.entries = entries + s.mu.Unlock() + log.Debug("registry refreshed", "entries", len(entries)) } - s.mu.Lock() - s.entries = entries - s.mu.Unlock() - log.Debug("registry refreshed", "entries", len(entries)) } } @@ -192,9 +204,19 @@ func (s *Server) Handler(w http.ResponseWriter, r *http.Request) { func (s *Server) handlePost( w http.ResponseWriter, r *http.Request, id string, ) { + const maxBody = 1 << 20 + r.Body = http.MaxBytesReader(w, r.Body, maxBody) + var request kvRequest err := json.NewDecoder(r.Body).Decode(&request) if err != nil { + if _, ok := err.(*http.MaxBytesError); ok { + http.Error(w, + "request entity too large", + http.StatusRequestEntityTooLarge, + ) + return + } http.Error(w, "bad request", http.StatusBadRequest) return } From d5a9ea68daa16a6fffa569012c018b7e07b42158 Mon Sep 17 00:00:00 2001 From: turk Date: Sun, 5 Apr 2026 17:18:03 -0600 Subject: [PATCH 4/9] more fixes --- .claude/settings.local.json | 8 -------- .gitignore | 1 + locket.go | 9 --------- registry.go | 6 ------ registry_remote.go | 6 +++--- registry_test.go | 5 ++--- server.go | 23 +++++++++++++---------- 7 files changed, 19 insertions(+), 39 deletions(-) delete mode 100644 .claude/settings.local.json diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index 4265275..0000000 --- a/.claude/settings.local.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "permissions": { - "allow": [ - "Read(//home/turk/dotfiles/**)", - "Read(//home/turk/.local/bin/**)" - ] - } -} diff --git a/.gitignore b/.gitignore index 58c8ccb..b175375 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ temp-*.env plan.md +.claude/settings.local.json diff --git a/locket.go b/locket.go index b42d2a6..8d107ab 100644 --- a/locket.go +++ b/locket.go @@ -23,15 +23,6 @@ type defaults struct { // - DELETE: remove an entry (RegEntry JSON body with name) var PathRegistry = "/locket/registry" -// Environment variable names used by locket clients and servers. -var ( - EnvURL = "LOCKET_URL" // locket server URL - EnvPublic = "LOCKET_PUBLIC" // ed25519 public signing key - EnvPrivate = "LOCKET_PRIVATE" // ed25519 private signing key - EnvRegistryURL = "LOCKET_REGISTRY_URL" // registry API base URL - EnvRegistryToken = "LOCKET_REGISTRY_TOKEN" // registry API auth token -) - // map[serviceName]keyPrivateSigning type KeysPrivateSigning map[string]string diff --git a/registry.go b/registry.go index 390cd1c..1423bdc 100644 --- a/registry.go +++ b/registry.go @@ -128,12 +128,6 @@ func (f FileRegistry) write(entries []RegEntry) error { } defer file.Close() - for i, e := range entries { - entries[i].Name = strings.TrimSuffix( - filepath.Base(e.Name), ".env", - ) - } - b, err := yaml.Marshal(entries) if err != nil { return fmt.Errorf("marshal: %w", err) diff --git a/registry_remote.go b/registry_remote.go index d6ce2bb..bc81a7e 100644 --- a/registry_remote.go +++ b/registry_remote.go @@ -47,7 +47,7 @@ func (r RemoteRegistry) Entries() ([]RegEntry, error) { } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("status %d", resp.StatusCode) + return nil, fmt.Errorf("status %s", resp.Status) } var entries []RegEntry @@ -80,7 +80,7 @@ func (r RemoteRegistry) Upsert(entry RegEntry) error { } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return fmt.Errorf("status %d", resp.StatusCode) + return fmt.Errorf("status %s", resp.Status) } return nil } @@ -107,7 +107,7 @@ func (r RemoteRegistry) Delete(name string) error { } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return fmt.Errorf("status %d", resp.StatusCode) + return fmt.Errorf("status %s", resp.Status) } return nil } diff --git a/registry_test.go b/registry_test.go index d2b516c..b0490cf 100644 --- a/registry_test.go +++ b/registry_test.go @@ -41,6 +41,8 @@ func TestFileRegistryReadWrite(t *testing.T) { func TestFileRegistryRegister(t *testing.T) { testRegistry := path.Join("example", "test-registry.yml") reg := FileRegistry{Path: testRegistry} + defer os.Remove(testRegistry) + services := []string{"service A", "service B", "service C"} for _, service := range services { @@ -69,9 +71,6 @@ func TestFileRegistryRegister(t *testing.T) { for _, e := range entries { t.Logf("%s\n%s", e.Name, e.KeyPub) } - - err = os.Remove(testRegistry) - require.NoError(t, err) } func TestFileRegistryDelete(t *testing.T) { diff --git a/server.go b/server.go index 2347b78..f75dbf3 100644 --- a/server.go +++ b/server.go @@ -68,6 +68,9 @@ func NewServer( pollInterval time.Duration, allow AllowRequestFunc, ) (*Server, error) { + if ctx == nil { + ctx = context.Background() + } if reg == nil { return nil, fmt.Errorf("registry must not be nil") } @@ -204,6 +207,16 @@ func (s *Server) Handler(w http.ResponseWriter, r *http.Request) { func (s *Server) handlePost( w http.ResponseWriter, r *http.Request, id string, ) { + if err := s.allow(r); err != nil { + log.Warn("request denied", + "request_id", id, + "ip", r.RemoteAddr, + "error", err, + ) + http.Error(w, "forbidden", http.StatusForbidden) + return + } + const maxBody = 1 << 20 r.Body = http.MaxBytesReader(w, r.Body, maxBody) @@ -239,16 +252,6 @@ func (s *Server) handlePost( "payload", payload, "request_id", id, ) - if err := s.allow(r); err != nil { - log.Warn("request denied", - "request_id", id, - "ip", r.RemoteAddr, - "error", err, - ) - http.Error(w, "forbidden", http.StatusForbidden) - return - } - // verify signature against registry registry := s.registrySnapshot() var verifiedService string From b0b6069a272168b686b270ad163a2c5b76e4b1c9 Mon Sep 17 00:00:00 2001 From: turk Date: Sun, 5 Apr 2026 21:15:22 -0600 Subject: [PATCH 5/9] fixes --- registry_remote.go | 38 +++++++++---- registry_remote_test.go | 119 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 registry_remote_test.go diff --git a/registry_remote.go b/registry_remote.go index bc81a7e..5d53be9 100644 --- a/registry_remote.go +++ b/registry_remote.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "time" ) @@ -26,16 +27,23 @@ func (r RemoteRegistry) client() *http.Client { return &http.Client{Timeout: 10 * time.Second} } -// endpoint returns the full URL to the registry API. -func (r RemoteRegistry) endpoint() string { - return r.URL + PathRegistry +// endpoint returns the full URL to the registry API, +// safely joining the base URL and PathRegistry. +func (r RemoteRegistry) endpoint() (string, error) { + joined, err := url.JoinPath(r.URL, PathRegistry) + if err != nil { + return "", fmt.Errorf("join url: %w", err) + } + return joined, nil } // Entries fetches all authorized clients from the remote API. func (r RemoteRegistry) Entries() ([]RegEntry, error) { - req, err := http.NewRequest( - http.MethodGet, r.endpoint(), nil, - ) + endpoint, err := r.endpoint() + if err != nil { + return nil, fmt.Errorf("endpoint: %w", err) + } + req, err := http.NewRequest(http.MethodGet, endpoint, nil) if err != nil { return nil, fmt.Errorf("new request: %w", err) } @@ -46,7 +54,7 @@ func (r RemoteRegistry) Entries() ([]RegEntry, error) { return nil, fmt.Errorf("do request: %w", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { return nil, fmt.Errorf("status %s", resp.Status) } @@ -65,8 +73,12 @@ func (r RemoteRegistry) Upsert(entry RegEntry) error { return fmt.Errorf("marshal: %w", err) } + endpoint, err := r.endpoint() + if err != nil { + return fmt.Errorf("endpoint: %w", err) + } req, err := http.NewRequest( - http.MethodPost, r.endpoint(), bytes.NewReader(b), + http.MethodPost, endpoint, bytes.NewReader(b), ) if err != nil { return fmt.Errorf("new request: %w", err) @@ -79,7 +91,7 @@ func (r RemoteRegistry) Upsert(entry RegEntry) error { return fmt.Errorf("do request: %w", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { return fmt.Errorf("status %s", resp.Status) } return nil @@ -92,8 +104,12 @@ func (r RemoteRegistry) Delete(name string) error { return fmt.Errorf("marshal: %w", err) } + endpoint, err := r.endpoint() + if err != nil { + return fmt.Errorf("endpoint: %w", err) + } req, err := http.NewRequest( - http.MethodDelete, r.endpoint(), bytes.NewReader(b), + http.MethodDelete, endpoint, bytes.NewReader(b), ) if err != nil { return fmt.Errorf("new request: %w", err) @@ -106,7 +122,7 @@ func (r RemoteRegistry) Delete(name string) error { return fmt.Errorf("do request: %w", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { return fmt.Errorf("status %s", resp.Status) } return nil diff --git a/registry_remote_test.go b/registry_remote_test.go new file mode 100644 index 0000000..7530895 --- /dev/null +++ b/registry_remote_test.go @@ -0,0 +1,119 @@ +package locket + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRemoteRegistryEntries(t *testing.T) { + want := []RegEntry{ + {Name: "svc1", KeyPub: "pub1"}, + {Name: "svc2", KeyPub: "pub2"}, + } + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodGet, r.Method) + require.Equal(t, PathRegistry, r.URL.Path) + require.Equal(t, "tok", r.Header.Get("X-Auth-Token")) + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(want)) + }, + )) + defer srv.Close() + + // Trailing slash on URL; JoinPath should normalize. + reg := RemoteRegistry{URL: srv.URL + "/", Token: "tok"} + got, err := reg.Entries() + require.NoError(t, err) + require.Equal(t, want, got) +} + +func TestRemoteRegistryUpsert(t *testing.T) { + want := RegEntry{Name: "svc1", KeyPub: "pub1"} + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + require.Equal(t, PathRegistry, r.URL.Path) + require.Equal(t, + "application/json", r.Header.Get("Content-Type"), + ) + + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + var got RegEntry + require.NoError(t, json.Unmarshal(body, &got)) + require.Equal(t, want, got) + + // Return 201 to confirm 2xx acceptance. + w.WriteHeader(http.StatusCreated) + }, + )) + defer srv.Close() + + reg := RemoteRegistry{URL: srv.URL} + require.NoError(t, reg.Upsert(want)) +} + +func TestRemoteRegistryDelete(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodDelete, r.Method) + require.Equal(t, PathRegistry, r.URL.Path) + + body, err := io.ReadAll(r.Body) + require.NoError(t, err) + var got RegEntry + require.NoError(t, json.Unmarshal(body, &got)) + require.Equal(t, "svc1", got.Name) + + // Return 204 to confirm 2xx acceptance. + w.WriteHeader(http.StatusNoContent) + }, + )) + defer srv.Close() + + reg := RemoteRegistry{URL: srv.URL} + require.NoError(t, reg.Delete("svc1")) +} + +func TestRemoteRegistryRegister(t *testing.T) { + var seen RegEntry + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, http.MethodPost, r.Method) + require.NoError(t, json.NewDecoder(r.Body).Decode(&seen)) + w.WriteHeader(http.StatusOK) + }, + )) + defer srv.Close() + + reg := RemoteRegistry{URL: srv.URL} + pub, priv, err := reg.Register("svc1") + require.NoError(t, err) + require.NotEmpty(t, pub) + require.NotEmpty(t, priv) + require.Equal(t, "svc1", seen.Name) + require.Equal(t, pub, seen.KeyPub) +} + +func TestRemoteRegistryNon2xx(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "boom", http.StatusInternalServerError) + }, + )) + defer srv.Close() + + reg := RemoteRegistry{URL: srv.URL} + _, err := reg.Entries() + require.Error(t, err) + require.ErrorContains(t, err, "500") +} From da1a01072d1517f42dd43fe29f27ef8ad615762d Mon Sep 17 00:00:00 2001 From: turk Date: Sun, 5 Apr 2026 21:56:43 -0600 Subject: [PATCH 6/9] more fixes --- client.go | 2 +- registry_remote.go | 12 +++++++++++- registry_remote_test.go | 7 +++++++ server.go | 5 ++++- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/client.go b/client.go index 9181795..383ddc6 100644 --- a/client.go +++ b/client.go @@ -34,7 +34,7 @@ type kvRequest struct { // Pre-computed ed25519 signing keys (via NewPairEd25519() or any other means) // must be passed to a new client, with the expectation that the public key // be made available to the server to facilitate authentication. -// see: WriteRegistry() for details +// see: FileRegistry.Register() for details func NewClient(serverURL, keyPub, keyPriv string) (*Client, error) { rsaPublic, rsaPrivate, err := newPairRSA(Defaults.BitsizeRSA) if err != nil { diff --git a/registry_remote.go b/registry_remote.go index 5d53be9..7248988 100644 --- a/registry_remote.go +++ b/registry_remote.go @@ -28,12 +28,22 @@ func (r RemoteRegistry) client() *http.Client { } // endpoint returns the full URL to the registry API, -// safely joining the base URL and PathRegistry. +// safely joining the base URL and PathRegistry. It validates +// that r.URL is an absolute URL with scheme and host. func (r RemoteRegistry) endpoint() (string, error) { joined, err := url.JoinPath(r.URL, PathRegistry) if err != nil { return "", fmt.Errorf("join url: %w", err) } + u, err := url.Parse(joined) + if err != nil { + return "", fmt.Errorf("parse url: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return "", fmt.Errorf( + "invalid base URL %q: missing scheme or host", r.URL, + ) + } return joined, nil } diff --git a/registry_remote_test.go b/registry_remote_test.go index 7530895..3cfaadb 100644 --- a/registry_remote_test.go +++ b/registry_remote_test.go @@ -104,6 +104,13 @@ func TestRemoteRegistryRegister(t *testing.T) { require.Equal(t, pub, seen.KeyPub) } +func TestRemoteRegistryInvalidBaseURL(t *testing.T) { + reg := RemoteRegistry{URL: "api:8888"} + _, err := reg.Entries() + require.Error(t, err) + require.ErrorContains(t, err, "missing scheme or host") +} + func TestRemoteRegistryNon2xx(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { diff --git a/server.go b/server.go index f75dbf3..13862fd 100644 --- a/server.go +++ b/server.go @@ -32,6 +32,9 @@ func AllowCIDR(cidr string) AllowRequestFunc { return fmt.Errorf("parse remote addr: %w", err) } clientIP := net.ParseIP(ip) + if clientIP == nil { + return fmt.Errorf("parse ip: %q", ip) + } if !network.Contains(clientIP) { return fmt.Errorf("IP %s not in %s", ip, cidr) } @@ -113,7 +116,7 @@ func NewServer( } if opts.Path == "" { return nil, fmt.Errorf( - "at least one path required to *.env file", + "opts.Path must be set to the .env file path", ) } secrets, err := opts.Load() From c906c219c82416e3ab3b60b4b914c9075d4537d2 Mon Sep 17 00:00:00 2001 From: Travis Turk <55367187+turkosaurus@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:19:30 -0600 Subject: [PATCH 7/9] fix: Close stops the registry poll goroutine 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. --- server.go | 15 ++++++++++++--- server_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/server.go b/server.go index 1591a10..4ffc5ff 100644 --- a/server.go +++ b/server.go @@ -118,6 +118,7 @@ type Server struct { keyRsaPublic string keyRsaPrivate string seen *nonceCache + cancel context.CancelFunc // stops the registry poll goroutine } // kvResponse is the server's encrypted secret response. @@ -201,15 +202,23 @@ func NewServer( } if pollInterval > 0 { - go server.poll(ctx, pollInterval) + // derive a child context so Close can stop polling independently of + // the caller's context (which may be context.Background()). + pollCtx, cancel := context.WithCancel(ctx) + server.cancel = cancel + go server.poll(pollCtx, pollInterval) } return server, nil } -// Close releases the server's background resources (the nonce-cache sweeper). -// The Server must not be used after Close. +// Close releases the server's background resources: the registry poll +// goroutine (if any) and the nonce-cache sweeper. The Server must not be used +// after Close. func (s *Server) Close() { + if s.cancel != nil { + s.cancel() + } s.seen.close() } diff --git a/server_test.go b/server_test.go index 6d39847..c7c0414 100644 --- a/server_test.go +++ b/server_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "path/filepath" + "sync" "testing" "time" @@ -175,6 +176,51 @@ func TestHandlerRejectsReplay(t *testing.T) { assertSecretNotLeaked(t, body2, clientPriv) } +// countingRegistry records how many times Entries is called, to verify the +// poll goroutine's lifecycle. +type countingRegistry struct { + mu sync.Mutex + count int +} + +func (c *countingRegistry) Entries() ([]RegEntry, error) { + c.mu.Lock() + c.count++ + c.mu.Unlock() + return nil, nil +} +func (c *countingRegistry) Upsert(RegEntry) error { return nil } +func (c *countingRegistry) Delete(string) error { return nil } +func (c *countingRegistry) Register(string) (string, string, error) { + return "", "", nil +} +func (c *countingRegistry) calls() int { + c.mu.Lock() + defer c.mu.Unlock() + return c.count +} + +// TestServerCloseStopsPoll is the regression test for the lifecycle gap: Close +// must stop the registry poll goroutine even when the server was created with a +// non-cancelable context. +func TestServerCloseStopsPoll(t *testing.T) { + reg := &countingRegistry{} + source := Dotenv{Path: testEnvFile, ServiceSecrets: testServiceMap} + interval := 5 * time.Millisecond + + server, err := NewServer(context.Background(), source, reg, interval, nil) + require.NoError(t, err) + + time.Sleep(40 * time.Millisecond) + require.Greater(t, reg.calls(), 1, "poll should have run several times") + + server.Close() + time.Sleep(20 * time.Millisecond) // let any in-flight tick finish + stopped := reg.calls() + time.Sleep(40 * time.Millisecond) // several more intervals + require.Equal(t, stopped, reg.calls(), "poll must not run after Close") +} + // TestHandlerRejectsStaleTimestamp is the regression test for the replay // window: a request whose signed timestamp is outside MaxClockSkew is rejected // even though the signature itself is valid. From 57a9aef99d879a1904c720044c9737403f20a6a7 Mon Sep 17 00:00:00 2001 From: Travis Turk <55367187+turkosaurus@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:29:46 -0600 Subject: [PATCH 8/9] fix: store registry names verbatim (match cloud contract) 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. --- registry.go | 10 +++------- registry_test.go | 33 +++++++++++++++++++++------------ 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/registry.go b/registry.go index 1423bdc..5bd0fa7 100644 --- a/registry.go +++ b/registry.go @@ -3,8 +3,6 @@ package locket import ( "fmt" "os" - "path/filepath" - "strings" "gopkg.in/yaml.v3" ) @@ -57,12 +55,10 @@ func (f FileRegistry) Entries() ([]RegEntry, error) { } // Upsert inserts or updates a client entry in the YAML file. -// If the file does not exist, it is created. +// If the file does not exist, it is created. The name is stored verbatim +// (exact-match, case-sensitive) to match the RemoteRegistry / cloud contract; +// derive a clean service name before calling if needed. func (f FileRegistry) Upsert(entry RegEntry) error { - entry.Name = strings.TrimSuffix( - filepath.Base(entry.Name), ".env", - ) - var entries []RegEntry _, err := os.Stat(f.Path) switch { diff --git a/registry_test.go b/registry_test.go index c7a45c7..3b8a9e6 100644 --- a/registry_test.go +++ b/registry_test.go @@ -39,28 +39,37 @@ func TestFileRegistryReadWrite(t *testing.T) { } } -// TestFileRegistryNoDuplicate is the regression test for name normalization in -// Upsert: re-registering a service (including with a .env suffix that Upsert -// strips) must update the existing entry rather than append a duplicate. -func TestFileRegistryNoDuplicate(t *testing.T) { +// TestFileRegistryUpsertDedup verifies Upsert dedups on the exact name +// (verbatim, case-sensitive) per the cloud registry contract: re-registering +// the same name updates the entry in place, while a different name (e.g. one +// with a .env suffix) is a distinct entry — Upsert does not normalize. +func TestFileRegistryUpsertDedup(t *testing.T) { reg := FileRegistry{Path: filepath.Join(t.TempDir(), "registry.yml")} - _, _, err := reg.Register("svc.env") + pub1, _, err := reg.Register("svc") require.NoError(t, err) - pub2, _, err := reg.Register("svc.env") + // re-registering the exact name updates in place (no duplicate) + pub2, _, err := reg.Register("svc") require.NoError(t, err) - // the plain name normalizes to the same entry too - _, _, err = reg.Register("svc") + // a different, un-normalized name is a distinct entry + _, _, err = reg.Register("svc.env") require.NoError(t, err) entries, err := reg.Entries() require.NoError(t, err) - require.Len(t, entries, 1, "re-registering the same service must not duplicate") - require.Equal(t, "svc", entries[0].Name) - // last write wins on the key - require.NotEqual(t, pub2, entries[0].KeyPub) + require.Len(t, entries, 2, + "exact-name re-register dedups; a distinct name does not") + + byName := make(map[string]string, len(entries)) + for _, e := range entries { + byName[e.Name] = e.KeyPub + } + require.Contains(t, byName, "svc") + require.Contains(t, byName, "svc.env") + require.Equal(t, pub2, byName["svc"], "last write wins for the exact name") + require.NotEqual(t, pub1, pub2) } func TestFileRegistryRegister(t *testing.T) { From 8745b14bac6b2206a840fac12fc42bd865881733 Mon Sep 17 00:00:00 2001 From: Travis Turk <55367187+turkosaurus@users.noreply.github.com> Date: Sun, 28 Jun 2026 02:03:05 -0600 Subject: [PATCH 9/9] address review: temp-dir tests + validate default CIDR - 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. --- locket_test.go | 11 ++++------- registry_test.go | 14 +++----------- server.go | 8 ++++++++ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/locket_test.go b/locket_test.go index 56194d1..f339a2f 100644 --- a/locket_test.go +++ b/locket_test.go @@ -6,7 +6,7 @@ import ( "net/http" "net/http/httptest" "os" - "path" + "path/filepath" "strings" "testing" @@ -18,15 +18,13 @@ func TestE2E(t *testing.T) { pub, priv, err := NewPairEd25519() require.NoError(t, err) - testReg := path.Join("example", "testreg.yml") - fileReg := FileRegistry{Path: testReg} + fileReg := FileRegistry{Path: filepath.Join(t.TempDir(), "registry.yml")} err = fileReg.Upsert(RegEntry{Name: "SERVICE1", KeyPub: pub}) require.NoError(t, err) - defer os.Remove(testReg) source := Dotenv{ ServiceSecrets: testServiceMap, - Path: path.Join("example", ".env"), + Path: filepath.Join("example", ".env"), } server, err := NewServer(context.Background(), source, fileReg, 0, nil) @@ -46,9 +44,8 @@ func TestE2E(t *testing.T) { text := MarshalDotenv(envVars) t.Logf("formatted env vars: %s", text) // write the env vars to a file - f, err := os.CreateTemp("example", "temp-*.env") + f, err := os.CreateTemp(t.TempDir(), "temp-*.env") require.NoError(t, err) - defer os.Remove(f.Name()) _, err = f.WriteString(text) require.NoError(t, err) err = f.Close() diff --git a/registry_test.go b/registry_test.go index 3b8a9e6..7fcb023 100644 --- a/registry_test.go +++ b/registry_test.go @@ -1,8 +1,6 @@ package locket import ( - "os" - "path" "path/filepath" "testing" @@ -21,9 +19,7 @@ var testRegistryItems = []RegEntry{ } func TestFileRegistryReadWrite(t *testing.T) { - p := path.Join("example", "test-readwrite.yml") - reg := FileRegistry{Path: p} - defer os.Remove(p) + reg := FileRegistry{Path: filepath.Join(t.TempDir(), "registry.yml")} for _, item := range testRegistryItems { err := reg.Upsert(item) @@ -73,9 +69,7 @@ func TestFileRegistryUpsertDedup(t *testing.T) { } func TestFileRegistryRegister(t *testing.T) { - testRegistry := path.Join("example", "test-registry.yml") - reg := FileRegistry{Path: testRegistry} - defer os.Remove(testRegistry) + reg := FileRegistry{Path: filepath.Join(t.TempDir(), "registry.yml")} services := []string{"service A", "service B", "service C"} @@ -108,9 +102,7 @@ func TestFileRegistryRegister(t *testing.T) { } func TestFileRegistryDelete(t *testing.T) { - testRegistry := path.Join("example", "test-delete.yml") - reg := FileRegistry{Path: testRegistry} - defer os.Remove(testRegistry) + reg := FileRegistry{Path: filepath.Join(t.TempDir(), "registry.yml")} require.NoError(t, reg.Upsert(RegEntry{Name: "a", KeyPub: "k1"})) require.NoError(t, reg.Upsert(RegEntry{Name: "b", KeyPub: "k2"})) diff --git a/server.go b/server.go index 4ffc5ff..62513f3 100644 --- a/server.go +++ b/server.go @@ -156,6 +156,14 @@ func NewServer( log.Info("registry loaded", "entries", len(entries)) if allow == nil { + // validate up front so a bad Defaults.AllowCIDR fails fast here + // instead of silently rejecting every request with 403 later. + if _, _, err := net.ParseCIDR(Defaults.AllowCIDR); err != nil { + return nil, fmt.Errorf( + "invalid default allow CIDR %q: %w", + Defaults.AllowCIDR, err, + ) + } allow = AllowCIDR(Defaults.AllowCIDR) }