From ccc80eabc7b752a51d3c8c64e2df18b70dc3b41c Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Thu, 26 Feb 2026 22:53:09 +0100 Subject: [PATCH] fix: path inconsistency in skills.go and race conditions in server.go --- internal/p9/server.go | 27 ++++++++++++++++----------- internal/p9/skills.go | 17 +++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/internal/p9/server.go b/internal/p9/server.go index bdb6a89..38a86d7 100644 --- a/internal/p9/server.go +++ b/internal/p9/server.go @@ -607,13 +607,14 @@ func (s *Server) create(cs *connState, fc *plan9.Fcall) *plan9.Fcall { func (s *Server) read(cs *connState, fc *plan9.Fcall) *plan9.Fcall { cs.mu.RLock() f, ok := cs.fids[fc.Fid] - cs.mu.RUnlock() if !ok { + cs.mu.RUnlock() return errFcall(fc, "bad fid") } // Streaming /events: block until next event arrives (or channel closed). if f.eventCh != nil { + cs.mu.RUnlock() e, ok := <-f.eventCh if !ok { // Channel closed (subscription cancelled); signal EOF. @@ -623,12 +624,16 @@ func (s *Server) read(cs *connState, fc *plan9.Fcall) *plan9.Fcall { return &plan9.Fcall{Type: plan9.Rread, Tag: fc.Tag, Count: uint32(len(data)), Data: data} } + path := f.path + isDir := f.qid.Type&QTDir != 0 + cs.mu.RUnlock() + var data []byte - if f.qid.Type&QTDir != 0 { - data = s.readDir(f.path, fc.Offset, fc.Count) + if isDir { + data = s.readDir(path, fc.Offset, fc.Count) } else { - content := s.readFile(f.path) + content := s.readFile(path) if fc.Offset < uint64(len(content)) { end := min(int(fc.Offset)+int(fc.Count), len(content)) data = []byte(content[fc.Offset:end]) @@ -639,14 +644,14 @@ func (s *Server) read(cs *connState, fc *plan9.Fcall) *plan9.Fcall { } func (s *Server) write(cs *connState, fc *plan9.Fcall) *plan9.Fcall { - cs.mu.Lock() + cs.mu.RLock() f, ok := cs.fids[fc.Fid] if !ok { - cs.mu.Unlock() + cs.mu.RUnlock() return errFcall(fc, "bad fid") } path := f.path - cs.mu.Unlock() + cs.mu.RUnlock() input := strings.TrimSpace(string(fc.Data)) parts := strings.Split(strings.TrimPrefix(path, "/"), "/") @@ -656,9 +661,9 @@ func (s *Server) write(cs *connState, fc *plan9.Fcall) *plan9.Fcall { // Handle beads writes if strings.HasPrefix(path, "/beads/") { if s.beads != nil { - cs.mu.RLock() + cs.mu.Lock() sessionID := cs.sessionID - cs.mu.RUnlock() + cs.mu.Unlock() if err := s.beads.Write(strings.TrimPrefix(path, "/beads/"), fc.Data, sessionID); err != nil { return errFcall(fc, err.Error()) } @@ -1027,7 +1032,7 @@ func (s *Server) remove(cs *connState, fc *plan9.Fcall) *plan9.Fcall { msgID := strings.TrimSuffix(parts[2], ".json") // Validate ownership: only allow removing from own inbox or user inbox - if sessionID == "" || (sessID != "user" && sessID != sessionID) { + if sessID != "user" && sessID != sessionID { return errFcall(fc, "permission denied: can only remove from own inbox") } @@ -1278,7 +1283,7 @@ func (s *Server) readDir(path string, offset uint64, count uint32) []byte { mailMgr := s.mgr.GetMailManager() if mailMgr == nil { - return nil + return []byte("[]") } var messages []*mailbox.Message diff --git a/internal/p9/skills.go b/internal/p9/skills.go index 86a2874..29be8f4 100644 --- a/internal/p9/skills.go +++ b/internal/p9/skills.go @@ -276,8 +276,8 @@ func (s *SkillsFS) List(path string) ([]plan9.Dir, error) { func (s *SkillsFS) Read(path string) ([]byte, error) { parts := strings.Split(strings.TrimPrefix(path, "/"), "/") - // /skills/help - if len(parts) == 2 && parts[0] == "skills" && parts[1] == "help" { + // /agent/skills/help + if len(parts) == 3 && parts[0] == "agent" && parts[1] == "skills" && parts[2] == "help" { help, err := s.generateHelp() if err != nil { return nil, err @@ -285,14 +285,14 @@ func (s *SkillsFS) Read(path string) ([]byte, error) { return []byte(help), nil } - // /skills/// - if len(parts) < 4 || parts[0] != "skills" { + // /agent/skills/// + if len(parts) < 5 || parts[0] != "agent" || parts[1] != "skills" { return nil, fmt.Errorf("not found") } - intent := parts[1] - skillName := parts[2] - fileName := strings.Join(parts[3:], "/") + intent := parts[2] + skillName := parts[3] + fileName := strings.Join(parts[4:], "/") // Prevent path traversal cleanName := filepath.Clean(fileName) @@ -309,7 +309,8 @@ func (s *SkillsFS) Read(path string) ([]byte, error) { if skill.Name == skillName { filePath := filepath.Join(skill.Path, cleanName) // Verify result stays within skill directory - if !strings.HasPrefix(filePath, skill.Path+string(filepath.Separator)) && filePath != skill.Path { + relPath, err := filepath.Rel(skill.Path, filePath) + if err != nil || strings.HasPrefix(relPath, "..") { return nil, fmt.Errorf("invalid path") } return os.ReadFile(filePath)