From 607ded3219a85b2866e066c5e67892b3cadc756a Mon Sep 17 00:00:00 2001 From: Michael Pursifull Date: Sat, 18 Apr 2026 20:42:05 -0500 Subject: [PATCH] =?UTF-8?q?chore(rules):=20add=20Rule=2012=20=E2=80=94=20n?= =?UTF-8?q?ever=20return=20internal=20pointers=20from=20locked=20accessors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the rule added to marvel (orc finding-032). Store.Get/List return value copies; mutations go through Update* methods that take the lock. Rule applies to any Go type that owns mutable state behind a mutex. Refs: orc finding-032 --- .claude/rules/go.md | 49 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/.claude/rules/go.md b/.claude/rules/go.md index 0348482..42d1a31 100644 --- a/.claude/rules/go.md +++ b/.claude/rules/go.md @@ -92,6 +92,55 @@ time.Now().UTC() // yes time.Now() // no ``` +**12. Never return internal pointers from a locked accessor** + +If a type owns state behind a mutex, its `Get*`/`List*` methods return +value copies (deep enough — clone nested slices/maps/pointers). Mutation +goes through methods on that owning type; those methods take the lock +and mutate in place. A returned `*T` into concurrently-mutated state is +a bug — the lock protects the map, not the values. + +```go +// WRONG — caller holds a pointer into the store's live object, +// mutates it after the lock is dropped, other callers see torn writes +func (s *Store) ListSessions() []*Session { + s.mu.RLock() + defer s.mu.RUnlock() + out := make([]*Session, 0, len(s.sessions)) + for _, sess := range s.sessions { + out = append(out, sess) // leaks internal pointer + } + return out +} + +// RIGHT — snapshots fully decoupled from internal state +func (s *Store) ListSessions() []Session { + s.mu.RLock() + defer s.mu.RUnlock() + out := make([]Session, 0, len(s.sessions)) + for _, sess := range s.sessions { + out = append(out, cloneSession(sess)) + } + return out +} + +// RIGHT — mutation goes through the owner under the lock +func (s *Store) UpdateSession(key string, fn func(*Session) error) error { + s.mu.Lock() + defer s.mu.Unlock() + sess, ok := s.sessions[key] + if !ok { + return fmt.Errorf("session %s: %w", key, ErrNotFound) + } + return fn(sess) +} +``` + +The Kubernetes lister/informer pattern is the canonical example: cached +reads give immutable snapshots, writes go through the API. `go test -race` +is the backstop; the type signature is the fence. See orc +finding-032-store-sync-contract-leak. + ## Error handling - Every exported function that can fail returns `error` last