From edd5ade4717aa316ea3e7a60891d5da7c471a06a Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Wed, 12 Aug 2026 11:01:29 +0200 Subject: [PATCH] refactor: simplify bypass - toolsrv is the broker, olliesrv is just a client - Remove olliesrv's bypass broker machinery (pendingCh, EvaluateRequest, etc) - Session bypass loop now just reads from toolsrv's bypass/pending and notifies - Notification handler writes directly to toolsrv's bypass/resolve (fire and forget) - Remove bypass/ directory from olliesrv's 9P namespace - Remove session/*/bypass file (policy can be added back to toolsrv later if needed) The flow is now: 1. toolsrv blocks tool execution, exposes request via bypass/pending 2. olliesrv reads from toolsrv, shows D-Bus notification 3. User clicks approve/deny, olliesrv writes to toolsrv's bypass/resolve 4. toolsrv unblocks and executes (or denies) --- cmd/olliesrv/bypass_notify.go | 86 +++++++++++++---------- cmd/olliesrv/internal/fs/newroot.go | 5 +- cmd/olliesrv/internal/fs/spec.go | 79 +-------------------- cmd/olliesrv/internal/session/persist.go | 6 +- cmd/olliesrv/internal/session/registry.go | 24 +++---- cmd/olliesrv/internal/session/session.go | 37 +++------- cmd/olliesrv/main.go | 32 ++------- 7 files changed, 82 insertions(+), 187 deletions(-) diff --git a/cmd/olliesrv/bypass_notify.go b/cmd/olliesrv/bypass_notify.go index 22780b5..f6de01b 100644 --- a/cmd/olliesrv/bypass_notify.go +++ b/cmd/olliesrv/bypass_notify.go @@ -5,30 +5,37 @@ import ( "sync" "github.com/godbus/dbus/v5" - "ollie/cmd/olliesrv/internal/bypass" + "ollie/toolsrv" ) -// bypassNotifier handles desktop notifications for bypass requests -// and maps notification action callbacks back to the broker. +// bypassNotifier handles desktop notifications for bypass requests. +// When a notification action is clicked, it writes the resolution directly to toolsrv. type bypassNotifier struct { - conn *dbus.Conn - broker *bypass.Broker + conn *dbus.Conn mu sync.Mutex - notifID map[uint32]string // notification ID -> request ID + notifID map[uint32]*pendingRequest // notification ID -> request info +} + +type pendingRequest struct { + id string + session *sessionRef +} + +type sessionRef struct { + dialFn func() *toolsrv.Conn } var notifier *bypassNotifier // initBypassNotifier sets up the notification action listener. -func initBypassNotifier(conn *dbus.Conn, broker *bypass.Broker) { - if conn == nil || broker == nil { +func initBypassNotifier(conn *dbus.Conn) { + if conn == nil { return } notifier = &bypassNotifier{ conn: conn, - broker: broker, - notifID: make(map[uint32]string), + notifID: make(map[uint32]*pendingRequest), } // Listen for ActionInvoked signals from the notification daemon @@ -43,26 +50,26 @@ func initBypassNotifier(conn *dbus.Conn, broker *bypass.Broker) { } // notifyBypass sends a desktop notification for a bypass request. -func notifyBypass(req *bypass.Request) { +// When the user responds, it writes the resolution directly to toolsrv. +func notifyBypass(req *toolsrv.BypassRequest, sessionID string, dialFn func() *toolsrv.Conn) { if notifier == nil { return } - notifier.sendNotification(req) + notifier.sendNotification(req, sessionID, dialFn) } -func (n *bypassNotifier) sendNotification(req *bypass.Request) { +func (n *bypassNotifier) sendNotification(req *toolsrv.BypassRequest, sessionID string, dialFn func() *toolsrv.Conn) { obj := n.conn.Object("org.freedesktop.Notifications", "/org/freedesktop/Notifications") summary := "Elevation Request" body := fmt.Sprintf("%s\ncwd: %s", escapeMarkup(req.Cmd), escapeMarkup(req.Cwd)) - if req.SessionID != "" { - body = fmt.Sprintf("session: %s\n%s", escapeMarkup(req.SessionID), body) + if sessionID != "" { + body = fmt.Sprintf("session: %s\n%s", escapeMarkup(sessionID), body) } actions := []string{ "approve", "Approve", "deny", "Deny", - "persist", "Persist", } hints := map[string]dbus.Variant{ @@ -77,7 +84,7 @@ func (n *bypassNotifier) sendNotification(req *bypass.Request) { body, actions, hints, - int32(300000), // timeout ms (matches TTL) + int32(300000), // timeout ms (5 minutes) ) if call.Err != nil { return @@ -87,7 +94,10 @@ func (n *bypassNotifier) sendNotification(req *bypass.Request) { call.Store(¬ifID) n.mu.Lock() - n.notifID[notifID] = req.ID + n.notifID[notifID] = &pendingRequest{ + id: req.ID, + session: &sessionRef{dialFn: dialFn}, + } n.mu.Unlock() } @@ -102,26 +112,16 @@ func (n *bypassNotifier) listenSignals(ch chan *dbus.Signal) { action, _ := sig.Body[1].(string) n.mu.Lock() - reqID, ok := n.notifID[nid] + pr, ok := n.notifID[nid] delete(n.notifID, nid) n.mu.Unlock() - if !ok { + if !ok || pr == nil { continue } - var res bypass.Resolution - switch action { - case "approve": - res = bypass.ResolveApprove - case "deny": - res = bypass.ResolveDeny - case "persist": - res = bypass.ResolvePersist - default: - res = bypass.ResolveDeny - } - n.broker.Resolve(reqID, res) + approved := action == "approve" + go n.resolve(pr, approved) case "org.freedesktop.Notifications.NotificationClosed": if len(sig.Body) < 2 { @@ -131,25 +131,33 @@ func (n *bypassNotifier) listenSignals(ch chan *dbus.Signal) { reason, _ := sig.Body[1].(uint32) n.mu.Lock() - reqID, ok := n.notifID[nid] + pr, ok := n.notifID[nid] delete(n.notifID, nid) n.mu.Unlock() - if !ok { + if !ok || pr == nil { continue } - // reason 2 = dismissed by user, 1 = expired, 3 = closed by call, 4 = undefined - // Treat dismiss as deny - if reason == 2 { - n.broker.Resolve(reqID, bypass.ResolveDeny) + // reason 2 = dismissed by user, 1 = expired + // Treat dismiss/expire as deny + if reason == 1 || reason == 2 { + go n.resolve(pr, false) } } } } +func (n *bypassNotifier) resolve(pr *pendingRequest, approved bool) { + conn := pr.session.dialFn() + if conn == nil { + return + } + conn.ResolveBypass(pr.id, approved, "") //nolint:errcheck + conn.Close() +} + func escapeMarkup(s string) string { - // Basic XML/HTML entity escaping for notification body var out []byte for _, c := range []byte(s) { switch c { diff --git a/cmd/olliesrv/internal/fs/newroot.go b/cmd/olliesrv/internal/fs/newroot.go index 4239b87..91834b2 100644 --- a/cmd/olliesrv/internal/fs/newroot.go +++ b/cmd/olliesrv/internal/fs/newroot.go @@ -3,7 +3,6 @@ package fs import ( "context" - "ollie/cmd/olliesrv/internal/bypass" "ollie/virtfs" olog "ollie/log" "ollie/cmd/olliesrv/internal/session" @@ -19,7 +18,7 @@ type Config struct { SessionsDir string Yolo bool ModelCache *ModelCache - BypassBroker *bypass.Broker + BypassNotify session.BypassNotifyFunc Shutdown func() Invalidate func() } @@ -34,7 +33,7 @@ func NewRoot(cfg Config) *Tree { AgentsDir: cfg.AgentsDir, SessionsDir: cfg.SessionsDir, Yolo: cfg.Yolo, - BypassBroker: cfg.BypassBroker, + BypassNotify: cfg.BypassNotify, }) // Build the tree from the spec. diff --git a/cmd/olliesrv/internal/fs/spec.go b/cmd/olliesrv/internal/fs/spec.go index 380f82e..c0c73a6 100644 --- a/cmd/olliesrv/internal/fs/spec.go +++ b/cmd/olliesrv/internal/fs/spec.go @@ -13,7 +13,6 @@ import ( "ollie/cmd/olliesrv/internal/agent" "ollie/cmd/olliesrv/internal/backend" - "ollie/cmd/olliesrv/internal/bypass" "ollie/cmd/olliesrv/internal/session" "ollie/paths" "ollie/toolsrv" @@ -40,7 +39,6 @@ var treeSpecUID, treeSpecGID string // buildTreeSpec constructs the full 9P namespace. func buildTreeSpec(cfg *Config) virtfs.FsNodeDecl { mc := cfg.ModelCache - broker := cfg.BypassBroker u, _ := user.Current() treeSpecUID = "ollie" @@ -166,54 +164,6 @@ func buildTreeSpec(cfg *Config) virtfs.FsNodeDecl { }), ), - // bypass/ - virtfs.DirNode("bypass", - virtfs.Doc("Bypass broker for sandboxed command approval"), - virtfs.FileNode("pending", 0444, - virtfs.Doc("Blocks until bypass request; returns JSON {id, cmd, cwd, env}"), - virtfs.BlockOnce(func(ctx context.Context, _ string) ([]byte, string, error) { - if broker == nil { - return nil, "", errNoBypass - } - req := broker.NextPending(ctx) - if req == nil { - return nil, "", fmt.Errorf("context cancelled") - } - data, err := json.Marshal(req) - if err != nil { - return nil, "", err - } - return append(data, '\n'), "", nil - }), - ), - virtfs.FileNode("resolve", 0222, - virtfs.Doc("Resolve bypass request: write JSON {id, approved} or {id, error}"), - virtfs.Write(func(data []byte) error { - if broker == nil { - return errNoBypass - } - var msg struct { - ID string `json:"id"` - Approved bool `json:"approved"` - Error string `json:"error"` - } - if err := json.Unmarshal(data, &msg); err != nil { - return fmt.Errorf("invalid JSON: %w", err) - } - var res bypass.Resolution - if msg.Approved { - res = bypass.ResolveApprove - } else { - res = bypass.ResolveDeny - } - if !broker.Resolve(msg.ID, res) { - return fmt.Errorf("unknown request ID: %s", msg.ID) - } - return nil - }), - ), - ), - // session/ virtfs.DirNode("session", virtfs.Doc("Session management"), @@ -244,16 +194,14 @@ func buildTreeSpec(cfg *Config) virtfs.FsNodeDecl { virtfs.Read(func() ([]byte, error) { return session.BuildIndex(), nil }), ), virtfs.Each("{sname}", func() ([]virtfs.FsNodeDecl, error) { - return buildSessionEntries(broker) + return buildSessionEntries() }), ), ) } -var errNoBypass = fmt.Errorf("bypass broker not available") - // buildSessionEntries produces one FsNodeDecl per session. -func buildSessionEntries(broker *bypass.Broker) ([]virtfs.FsNodeDecl, error) { +func buildSessionEntries() ([]virtfs.FsNodeDecl, error) { sessions := session.Sessions() var out []virtfs.FsNodeDecl @@ -274,7 +222,7 @@ func buildSessionEntries(broker *bypass.Broker) ([]virtfs.FsNodeDecl, error) { Aliases: []string{s.ID}, Remove: removeFn, Rename: renameFn, - Children: buildSessionChildren(s, broker, removeFn, renameFn), + Children: buildSessionChildren(s, removeFn, renameFn), }) } return out, nil @@ -283,7 +231,6 @@ func buildSessionEntries(broker *bypass.Broker) ([]virtfs.FsNodeDecl, error) { // buildSessionChildren returns the file nodes for a single session. func buildSessionChildren( s *session.Session, - broker *bypass.Broker, removeFn func() error, renameFn func(string) error, ) []virtfs.FsNodeDecl { @@ -350,26 +297,6 @@ func buildSessionChildren( virtfs.FileNode("id", 0444, virtfs.Read(func() ([]byte, error) { return []byte(s.ID + "\n"), nil }), ), - virtfs.FileNode("bypass", 0666, - virtfs.Read(func() ([]byte, error) { - if broker == nil { - return nil, errNoBypass - } - p := broker.SessionPolicy(s.ID) - return p.Marshal() - }), - virtfs.Write(func(data []byte) error { - if broker == nil { - return errNoBypass - } - var p bypass.Policy - if err := bypass.ParsePolicy(data, &p); err != nil { - return err - } - broker.SetSessionPolicy(s.ID, p) - return nil - }), - ), virtfs.DirNode("agent", virtfs.FileNode("new", 0666, virtfs.Request(func(_ context.Context, data []byte) ([]byte, error) { diff --git a/cmd/olliesrv/internal/session/persist.go b/cmd/olliesrv/internal/session/persist.go index fd8f2b3..59983ee 100644 --- a/cmd/olliesrv/internal/session/persist.go +++ b/cmd/olliesrv/internal/session/persist.go @@ -256,9 +256,9 @@ func restoreMultiAgentSession(ps *PersistedSession) (*RestoredSession, error) { sess.Uname = sess.Agents()[0].ID() Register(sess.Name(), sess) - // Start bypass approval loop for restored sessions (if broker is configured) - if pkgBypassBroker != nil && !ps.Paused { - sess.StartBypassLoop(pkgBypassBroker) + // Start bypass approval loop for restored sessions (if notify function is configured) + if pkgBypassNotify != nil && !ps.Paused { + sess.StartBypassLoop(pkgBypassNotify) } toolCount := 0 diff --git a/cmd/olliesrv/internal/session/registry.go b/cmd/olliesrv/internal/session/registry.go index 962eb43..b708785 100644 --- a/cmd/olliesrv/internal/session/registry.go +++ b/cmd/olliesrv/internal/session/registry.go @@ -29,13 +29,13 @@ var ( sessions = make(map[string]*Session) // Package config, set via Init. - serverCtx context.Context - pkgLog *olog.Logger - pkgSink *olog.Sink - pkgAgentsDir string - pkgSessionsDir string - pkgYolo bool - pkgBypassBroker BypassEvaluator + serverCtx context.Context + pkgLog *olog.Logger + pkgSink *olog.Sink + pkgAgentsDir string + pkgSessionsDir string + pkgYolo bool + pkgBypassNotify BypassNotifyFunc ) // InitConfig configures the session package. @@ -46,7 +46,7 @@ type InitConfig struct { AgentsDir string SessionsDir string Yolo bool - BypassBroker BypassEvaluator // may be nil if bypass is disabled + BypassNotify BypassNotifyFunc // may be nil if bypass is disabled } // Init initializes the session package with the given configuration. @@ -57,7 +57,7 @@ func Init(cfg InitConfig) { pkgAgentsDir = cfg.AgentsDir pkgSessionsDir = cfg.SessionsDir pkgYolo = cfg.Yolo - pkgBypassBroker = cfg.BypassBroker + pkgBypassNotify = cfg.BypassNotify } // Sessions returns a snapshot of all sessions. @@ -320,9 +320,9 @@ func CreateEmpty(name, remote string) (*Session, error) { sess.Keeper = infra.Keeper sess.SetToolsConn(infra.ToolsConn) - // Start bypass approval loop (if broker is configured) - if pkgBypassBroker != nil { - sess.StartBypassLoop(pkgBypassBroker) + // Start bypass approval loop (if notify function is configured) + if pkgBypassNotify != nil { + sess.StartBypassLoop(pkgBypassNotify) } mu.Lock() diff --git a/cmd/olliesrv/internal/session/session.go b/cmd/olliesrv/internal/session/session.go index df0e802..671283c 100644 --- a/cmd/olliesrv/internal/session/session.go +++ b/cmd/olliesrv/internal/session/session.go @@ -112,19 +112,17 @@ func (s *Session) SetToolsConn(conn *toolsrv.Conn) { s.toolsConn = conn } +// BypassNotifyFunc is called when a bypass request arrives. +// It receives the request, session ID, and a dial function to get a connection for resolution. +type BypassNotifyFunc func(req *toolsrv.BypassRequest, sessionID string, dialFn func() *toolsrv.Conn) + // StartBypassLoop starts a goroutine that reads bypass requests from toolsrv -// and evaluates them via the bypass broker. This should be called after -// SetToolsConn when the toolsrv connection is ready. -func (s *Session) StartBypassLoop(broker BypassEvaluator) { - go s.runBypassLoop(broker) +// and calls notifyFn for each one. This should be called after SetToolsConn. +func (s *Session) StartBypassLoop(notifyFn BypassNotifyFunc) { + go s.runBypassLoop(notifyFn) } -// BypassEvaluator evaluates bypass requests. Implemented by bypass.Broker. -type BypassEvaluator interface { - EvaluateRequest(sessionID, cmd, cwd string, env map[string]string) (bool, error) -} - -func (s *Session) runBypassLoop(broker BypassEvaluator) { +func (s *Session) runBypassLoop(notifyFn BypassNotifyFunc) { for { // Get a fresh connection for each request (blocking reads don't multiplex well) conn := s.DialToolServer() @@ -150,23 +148,10 @@ func (s *Session) runBypassLoop(broker BypassEvaluator) { } } - // Evaluate the request - approved, evalErr := broker.EvaluateRequest(s.ID, req.Cmd, req.Cwd, req.Env) - - // Send resolution back to toolsrv - resolveConn := s.DialToolServer() - if resolveConn == nil { - s.log.Warn("bypass: can't dial to resolve request %s", req.ID) - continue + // Notify (non-blocking) - the notifier will write resolution when user responds + if notifyFn != nil { + notifyFn(req, s.ID, s.DialToolServer) } - errMsg := "" - if evalErr != nil { - errMsg = evalErr.Error() - } - if err := resolveConn.ResolveBypass(req.ID, approved, errMsg); err != nil { - s.log.Warn("bypass: resolve failed for %s: %v", req.ID, err) - } - resolveConn.Close() } } diff --git a/cmd/olliesrv/main.go b/cmd/olliesrv/main.go index 33fd03f..0337957 100644 --- a/cmd/olliesrv/main.go +++ b/cmd/olliesrv/main.go @@ -13,12 +13,10 @@ import ( "9fans.net/go/plan9/client" "ollie/cmd/olliesrv/internal/agent" - "ollie/cmd/olliesrv/internal/bypass" "ollie/env" "ollie/cmd/olliesrv/internal/fs" olog "ollie/log" "ollie/paths" - "ollie/cmd/olliesrv/internal/session" "github.com/godbus/dbus/v5" ) @@ -65,31 +63,9 @@ func runServer(sockPath string) { modelCache := fs.NewModelCache() - // Start bypass broker (must be before NewRoot so it can be passed in) - policyPath := filepath.Join(paths.DataDir(), "bypass-policy.yaml") - var bypassBroker *bypass.Broker - { - notifyFn := func(req *bypass.Request) { - notifyBypass(req) - } - var err error - bypassBroker, err = bypass.NewBroker(bypass.BrokerConfig{ - PolicyPath: policyPath, - Notify: notifyFn, - Logf: func(f string, a ...any) { fmt.Printf(f+"\n", a...) }, - SessionValid: func(id string) bool { return session.Lookup(id) != nil }, - }) - if err != nil { - fmt.Fprintf(os.Stderr, "warning: bypass broker: %v\n", err) - } else { - // Desktop notifications for bypass prompts (uses D-Bus notifications API directly) - if conn, err := dbus.SessionBus(); err == nil { - initBypassNotifier(conn, bypassBroker) - } - } - } - if bypassBroker != nil { - defer bypassBroker.Close() + // Desktop notifications for bypass prompts + if conn, err := dbus.SessionBus(); err == nil { + initBypassNotifier(conn) } rootTree := fs.NewRoot(fs.Config{ @@ -100,7 +76,7 @@ func runServer(sockPath string) { Sink: sink, Yolo: *yolo, ModelCache: modelCache, - BypassBroker: bypassBroker, + BypassNotify: notifyBypass, }) // Create 9P server