From 790d77075e40ee3162d7925e983f01747b5c2c89 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Tue, 14 Oct 2025 01:33:44 +0900 Subject: [PATCH] system/dbus: remove builder state leak This enables external testing of system.I state. Signed-off-by: Ophestra --- internal/app/app_test.go | 25 ++++++++++++++--- internal/app/dispatcher.go | 6 +++++ internal/app/dispatcher_test.go | 9 ++++++- internal/app/spdbus.go | 11 ++++---- system/dbus.go | 21 ++++++++------- system/dbus/dbus.go | 48 ++++++++++++++++++--------------- system/dbus_test.go | 35 ++++++++++++++++-------- system/dispatcher.go | 6 ----- system/dispatcher_test.go | 6 ----- 9 files changed, 102 insertions(+), 65 deletions(-) diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 2f42f70..c89910a 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -25,6 +25,7 @@ import ( "hakurei.app/message" "hakurei.app/system" "hakurei.app/system/acl" + "hakurei.app/system/dbus" ) func TestApp(t *testing.T) { @@ -213,7 +214,7 @@ func TestApp(t *testing.T) { Ensure(m("/run/user/1971"), 0700).UpdatePermType(system.User, m("/run/user/1971"), acl.Execute). // this is ordered as is because the previous Ensure only calls mkdir if XDG_RUNTIME_DIR is unset Ephemeral(system.Process, m("/run/user/1971/hakurei/ebf083d1b175911782d413369b64ce7c"), 0700).UpdatePermType(system.Process, m("/run/user/1971/hakurei/ebf083d1b175911782d413369b64ce7c"), acl.Execute). Link(m("/run/user/1971/pulse/native"), m("/run/user/1971/hakurei/ebf083d1b175911782d413369b64ce7c/pulse")). - MustProxyDBus(m("/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/bus"), &hst.BusConfig{ + MustProxyDBus(&hst.BusConfig{ Talk: []string{ "org.freedesktop.Notifications", "org.freedesktop.FileManager1", @@ -235,13 +236,19 @@ func TestApp(t *testing.T) { "org.freedesktop.portal.*": "@/org/freedesktop/portal/*", }, Filter: true, - }, m("/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/system_bus_socket"), &hst.BusConfig{ + }, &hst.BusConfig{ Talk: []string{ "org.bluez", "org.freedesktop.Avahi", "org.freedesktop.UPower", }, Filter: true, + }, dbus.ProxyPair{ + "unix:path=/run/user/1971/bus", + "/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/bus", + }, dbus.ProxyPair{ + "unix:path=/var/run/dbus/system_bus_socket", + "/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/system_bus_socket", }). UpdatePerm(m("/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/bus"), acl.Read, acl.Write). UpdatePerm(m("/tmp/hakurei.0/ebf083d1b175911782d413369b64ce7c/system_bus_socket"), acl.Read, acl.Write), @@ -364,7 +371,7 @@ func TestApp(t *testing.T) { Ephemeral(system.Process, m("/run/user/1971/hakurei/8e2c76b066dabe574cf073bdb46eb5c1"), 0700).UpdatePermType(system.Process, m("/run/user/1971/hakurei/8e2c76b066dabe574cf073bdb46eb5c1"), acl.Execute). Link(m("/run/user/1971/pulse/native"), m("/run/user/1971/hakurei/8e2c76b066dabe574cf073bdb46eb5c1/pulse")). Ephemeral(system.Process, m("/tmp/hakurei.0/8e2c76b066dabe574cf073bdb46eb5c1"), 0711). - MustProxyDBus(m("/tmp/hakurei.0/8e2c76b066dabe574cf073bdb46eb5c1/bus"), &hst.BusConfig{ + MustProxyDBus(&hst.BusConfig{ Talk: []string{ "org.freedesktop.FileManager1", "org.freedesktop.Notifications", "org.freedesktop.ScreenSaver", "org.freedesktop.secrets", @@ -377,13 +384,19 @@ func TestApp(t *testing.T) { }, Call: map[string]string{}, Broadcast: map[string]string{}, Filter: true, - }, m("/tmp/hakurei.0/8e2c76b066dabe574cf073bdb46eb5c1/system_bus_socket"), &hst.BusConfig{ + }, &hst.BusConfig{ Talk: []string{ "org.bluez", "org.freedesktop.Avahi", "org.freedesktop.UPower", }, Filter: true, + }, dbus.ProxyPair{ + "unix:path=/run/user/1971/bus", + "/tmp/hakurei.0/8e2c76b066dabe574cf073bdb46eb5c1/bus", + }, dbus.ProxyPair{ + "unix:path=/var/run/dbus/system_bus_socket", + "/tmp/hakurei.0/8e2c76b066dabe574cf073bdb46eb5c1/system_bus_socket", }). UpdatePerm(m("/tmp/hakurei.0/8e2c76b066dabe574cf073bdb46eb5c1/bus"), acl.Read, acl.Write). UpdatePerm(m("/tmp/hakurei.0/8e2c76b066dabe574cf073bdb46eb5c1/system_bus_socket"), acl.Read, acl.Write), @@ -739,6 +752,10 @@ func (k *stubNixOS) overflowGid(message.Msg) int { return 65534 } func (k *stubNixOS) mustHsuPath() *check.Absolute { return m("/proc/nonexistent/hsu") } +func (k *stubNixOS) dbusAddress() (string, string) { + return "unix:path=/run/user/1971/bus", "unix:path=/var/run/dbus/system_bus_socket" +} + func (k *stubNixOS) fatalf(format string, v ...any) { panic(fmt.Sprintf(format, v...)) } func (k *stubNixOS) isVerbose() bool { return true } diff --git a/internal/app/dispatcher.go b/internal/app/dispatcher.go index d56278a..d298d6b 100644 --- a/internal/app/dispatcher.go +++ b/internal/app/dispatcher.go @@ -13,6 +13,7 @@ import ( "hakurei.app/container/check" "hakurei.app/internal" "hakurei.app/message" + "hakurei.app/system/dbus" ) // osFile represents [os.File]. @@ -63,6 +64,9 @@ type syscallDispatcher interface { // mustHsuPath provides [internal.MustHsuPath]. mustHsuPath() *check.Absolute + // dbusAddress provides [dbus.Address]. + dbusAddress() (session, system string) + // fatalf provides [log.Fatalf]. fatalf(format string, v ...any) } @@ -99,4 +103,6 @@ func (direct) overflowGid(msg message.Msg) int { return container.OverflowGid(ms func (direct) mustHsuPath() *check.Absolute { return internal.MustHsuPath() } +func (k direct) dbusAddress() (session, system string) { return dbus.Address() } + func (direct) fatalf(format string, v ...any) { log.Fatalf(format, v...) } diff --git a/internal/app/dispatcher_test.go b/internal/app/dispatcher_test.go index 3e608c3..d00170a 100644 --- a/internal/app/dispatcher_test.go +++ b/internal/app/dispatcher_test.go @@ -97,7 +97,7 @@ func checkOpBehaviour(t *testing.T, testCases []opBehaviourTestCase) { if err := s.populateLocal(k, k); err != nil { t.Fatalf("populateLocal: error = %v", err) } - stateSys := s.newSys(config, newI()) + stateSys := s.newSys(config, system.New(panicMsgContext{}, k, checkExpectUid)) if tc.pStateSys != nil { tc.pStateSys(stateSys) } @@ -218,6 +218,12 @@ func (k *kstub) mustHsuPath() *check.Absolute { return k.Expects("mustHsuPath").Ret.(*check.Absolute) } +func (k *kstub) dbusAddress() (session, system string) { + k.Helper() + ret := k.Expects("dbusAddress").Ret.([2]string) + return ret[0], ret[1] +} + func (k *kstub) GetLogger() *log.Logger { panic("unreachable") } func (k *kstub) IsVerbose() bool { k.Helper(); return k.Expects("isVerbose").Ret.(bool) } @@ -320,4 +326,5 @@ func (panicDispatcher) cmdOutput(*exec.Cmd) ([]byte, error) { panic("unreachab func (panicDispatcher) overflowUid(message.Msg) int { panic("unreachable") } func (panicDispatcher) overflowGid(message.Msg) int { panic("unreachable") } func (panicDispatcher) mustHsuPath() *check.Absolute { panic("unreachable") } +func (panicDispatcher) dbusAddress() (string, string) { panic("unreachable") } func (panicDispatcher) fatalf(string, ...any) { panic("unreachable") } diff --git a/internal/app/spdbus.go b/internal/app/spdbus.go index b818cbe..c2c1fef 100644 --- a/internal/app/spdbus.go +++ b/internal/app/spdbus.go @@ -13,8 +13,7 @@ func init() { gob.Register(new(spDBusOp)) } // spDBusOp maintains an xdg-dbus-proxy instance for the container. type spDBusOp struct { - // Whether to bind the system bus socket. - // Populated during toSystem. + // Whether to bind the system bus socket. Populated during toSystem. ProxySystem bool } @@ -30,10 +29,10 @@ func (s *spDBusOp) toSystem(state *outcomeStateSys) error { // downstream socket paths sessionPath, systemPath := state.instance().Append("bus"), state.instance().Append("system_bus_socket") - if err := state.sys.ProxyDBus( - state.sessionBus, state.systemBus, - sessionPath, systemPath, - ); err != nil { + var sessionBus, systemBus dbus.ProxyPair + sessionBus[0], systemBus[0] = state.k.dbusAddress() + sessionBus[1], systemBus[1] = sessionPath.String(), systemPath.String() + if err := state.sys.ProxyDBus(state.sessionBus, state.systemBus, sessionBus, systemBus); err != nil { return err } diff --git a/system/dbus.go b/system/dbus.go index 1cc24a2..424f9df 100644 --- a/system/dbus.go +++ b/system/dbus.go @@ -12,18 +12,19 @@ import ( "syscall" "hakurei.app/container" - "hakurei.app/container/check" "hakurei.app/hst" "hakurei.app/system/dbus" ) -var ( - ErrDBusConfig = errors.New("dbus config not supplied") -) +// ErrDBusConfig is returned when a required [hst.BusConfig] argument is nil. +var ErrDBusConfig = errors.New("dbus config not supplied") // MustProxyDBus calls ProxyDBus and panics if an error is returned. -func (sys *I) MustProxyDBus(sessionPath *check.Absolute, session *hst.BusConfig, systemPath *check.Absolute, system *hst.BusConfig) *I { - if err := sys.ProxyDBus(session, system, sessionPath, systemPath); err != nil { +func (sys *I) MustProxyDBus( + session, system *hst.BusConfig, + sessionBus, systemBus dbus.ProxyPair, +) *I { + if err := sys.ProxyDBus(session, system, sessionBus, systemBus); err != nil { panic(err.Error()) } else { return sys @@ -32,7 +33,10 @@ func (sys *I) MustProxyDBus(sessionPath *check.Absolute, session *hst.BusConfig, // ProxyDBus finalises configuration ahead of time and starts xdg-dbus-proxy via [dbus] and terminates it on revert. // This [Op] is always [Process] scoped. -func (sys *I) ProxyDBus(session, system *hst.BusConfig, sessionPath, systemPath *check.Absolute) error { +func (sys *I) ProxyDBus( + session, system *hst.BusConfig, + sessionBus, systemBus dbus.ProxyPair, +) error { d := new(dbusProxyOp) // session bus is required as otherwise this is effectively a very expensive noop @@ -44,9 +48,6 @@ func (sys *I) ProxyDBus(session, system *hst.BusConfig, sessionPath, systemPath // system bus is optional d.system = system != nil - var sessionBus, systemBus dbus.ProxyPair - sessionBus[0], systemBus[0] = sys.dbusAddress() - sessionBus[1], systemBus[1] = sessionPath.String(), systemPath.String() d.out = &linePrefixWriter{println: log.Println, prefix: "(dbus) ", buf: new(strings.Builder)} if final, err := sys.dbusFinalise(sessionBus, systemBus, session, system); err != nil { if errors.Is(err, syscall.EINVAL) { diff --git a/system/dbus/dbus.go b/system/dbus/dbus.go index 08db8f1..9ea89f5 100644 --- a/system/dbus/dbus.go +++ b/system/dbus/dbus.go @@ -8,27 +8,31 @@ import ( ) const ( - /*SessionBusAddress is the name of the environment variable where the address of the login session message bus is given in. + /* + SessionBusAddress is the name of the environment variable where the address of the login session message bus is given in. - If that variable is not set, applications may also try to read the address from the X Window System root window property _DBUS_SESSION_BUS_ADDRESS. - The root window property must have type STRING. The environment variable should have precedence over the root window property. + If that variable is not set, applications may also try to read the address from the X Window System root window property _DBUS_SESSION_BUS_ADDRESS. + The root window property must have type STRING. The environment variable should have precedence over the root window property. - The address of the login session message bus is given in the DBUS_SESSION_BUS_ADDRESS environment variable. - If DBUS_SESSION_BUS_ADDRESS is not set, or if it's set to the string "autolaunch:", - the system should use platform-specific methods of locating a running D-Bus session server, - or starting one if a running instance cannot be found. - Note that this mechanism is not recommended for attempting to determine if a daemon is running. - It is inherently racy to attempt to make this determination, since the bus daemon may be started just before or just after the determination is made. - Therefore, it is recommended that applications do not try to make this determination for their functionality purposes, and instead they should attempt to start the server. + The address of the login session message bus is given in the DBUS_SESSION_BUS_ADDRESS environment variable. + If DBUS_SESSION_BUS_ADDRESS is not set, or if it's set to the string "autolaunch:", + the system should use platform-specific methods of locating a running D-Bus session server, + or starting one if a running instance cannot be found. + Note that this mechanism is not recommended for attempting to determine if a daemon is running. + It is inherently racy to attempt to make this determination, since the bus daemon may be started just before or just after the determination is made. + Therefore, it is recommended that applications do not try to make this determination for their functionality purposes, and instead they should attempt to start the server. - This package diverges from the specification, as the caller is unlikely to be an X client, or be in a position to autolaunch a dbus server. - So a fallback address with a socket located in the well-known default XDG_RUNTIME_DIR formatting is used.*/ + This package diverges from the specification, as the caller is unlikely to be an X client, or be in a position to autolaunch a dbus server. + So a fallback address with a socket located in the well-known default XDG_RUNTIME_DIR formatting is used. + */ SessionBusAddress = "DBUS_SESSION_BUS_ADDRESS" - /*SystemBusAddress is the name of the environment variable where the address of the system message bus is given in. + /* + SystemBusAddress is the name of the environment variable where the address of the system message bus is given in. - If that variable is not set, applications should try to connect to the well-known address unix:path=/var/run/dbus/system_bus_socket. - Implementations of the well-known system bus should listen on an address that will result in that connection being successful.*/ + If that variable is not set, applications should try to connect to the well-known address unix:path=/var/run/dbus/system_bus_socket. + Implementations of the well-known system bus should listen on an address that will result in that connection being successful. + */ SystemBusAddress = "DBUS_SYSTEM_BUS_ADDRESS" // FallbackSystemBusAddress is used when [SystemBusAddress] is not set. @@ -36,28 +40,30 @@ const ( ) var ( - addresses [2]string + address [2]string addressOnce sync.Once ) +// Address returns the session and system bus addresses copied from environment, +// or appropriate fallback values if they are not set. func Address() (session, system string) { addressOnce.Do(func() { // resolve upstream session bus address if addr, ok := os.LookupEnv(SessionBusAddress); !ok { // fall back to default format - addresses[0] = fmt.Sprintf("unix:path=/run/user/%d/bus", os.Getuid()) + address[0] = fmt.Sprintf("unix:path=/run/user/%d/bus", os.Getuid()) } else { - addresses[0] = addr + address[0] = addr } // resolve upstream system bus address if addr, ok := os.LookupEnv(SystemBusAddress); !ok { // fall back to default hardcoded value - addresses[1] = FallbackSystemBusAddress + address[1] = FallbackSystemBusAddress } else { - addresses[1] = addr + address[1] = addr } }) - return addresses[0], addresses[1] + return address[0], address[1] } diff --git a/system/dbus_test.go b/system/dbus_test.go index 2a1f12d..a346711 100644 --- a/system/dbus_test.go +++ b/system/dbus_test.go @@ -85,7 +85,7 @@ func TestDBusProxyOp(t *testing.T) { Op: "dbus", Err: ErrDBusConfig, Msg: "attempted to create message bus proxy args without session bus config", } - if err := sys.ProxyDBus(nil, new(hst.BusConfig), nil, nil); !reflect.DeepEqual(err, wantErr) { + if err := sys.ProxyDBus(nil, new(hst.BusConfig), dbus.ProxyPair{}, dbus.ProxyPair{}); !reflect.DeepEqual(err, wantErr) { t.Errorf("ProxyDBus: error = %v, want %v", err, wantErr) } }, nil, stub.Expect{}}, @@ -99,15 +99,20 @@ func TestDBusProxyOp(t *testing.T) { }() sys.MustProxyDBus( - m("/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus"), &hst.BusConfig{ + &hst.BusConfig{ // use impossible value here as an implicit assert that it goes through the stub Talk: []string{"session\x00"}, Filter: true, - }, m("/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket"), &hst.BusConfig{ + }, &hst.BusConfig{ // use impossible value here as an implicit assert that it goes through the stub Talk: []string{"system\x00"}, Filter: true, + }, dbus.ProxyPair{ + "unix:path=/run/user/1000/bus", + "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus", + }, dbus.ProxyPair{ + "unix:path=/run/dbus/system_bus_socket", + "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket", }) }, nil, stub.Expect{Calls: []stub.Call{ - call("dbusAddress", stub.ExpectArgs{}, [2]string{"unix:path=/run/user/1000/bus", "unix:path=/run/dbus/system_bus_socket"}, nil), call("dbusFinalise", stub.ExpectArgs{ dbus.ProxyPair{"unix:path=/run/user/1000/bus", "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus"}, dbus.ProxyPair{"unix:path=/run/dbus/system_bus_socket", "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket"}, @@ -128,13 +133,16 @@ func TestDBusProxyOp(t *testing.T) { }, &hst.BusConfig{ // use impossible value here as an implicit assert that it goes through the stub Talk: []string{"system\x00"}, Filter: true, - }, - m("/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus"), - m("/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket")); !reflect.DeepEqual(err, wantErr) { + }, dbus.ProxyPair{ + "unix:path=/run/user/1000/bus", + "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus", + }, dbus.ProxyPair{ + "unix:path=/run/dbus/system_bus_socket", + "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket", + }); !reflect.DeepEqual(err, wantErr) { t.Errorf("ProxyDBus: error = %v", err) } }, nil, stub.Expect{Calls: []stub.Call{ - call("dbusAddress", stub.ExpectArgs{}, [2]string{"unix:path=/run/user/1000/bus", "unix:path=/run/dbus/system_bus_socket"}, nil), call("dbusFinalise", stub.ExpectArgs{ dbus.ProxyPair{"unix:path=/run/user/1000/bus", "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus"}, dbus.ProxyPair{"unix:path=/run/dbus/system_bus_socket", "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket"}, @@ -145,12 +153,18 @@ func TestDBusProxyOp(t *testing.T) { {"full", 0xcafebabe, func(_ *testing.T, sys *I) { sys.MustProxyDBus( - m("/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus"), &hst.BusConfig{ + &hst.BusConfig{ // use impossible value here as an implicit assert that it goes through the stub Talk: []string{"session\x00"}, Filter: true, - }, m("/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket"), &hst.BusConfig{ + }, &hst.BusConfig{ // use impossible value here as an implicit assert that it goes through the stub Talk: []string{"system\x00"}, Filter: true, + }, dbus.ProxyPair{ + "unix:path=/run/user/1000/bus", + "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus", + }, dbus.ProxyPair{ + "unix:path=/run/dbus/system_bus_socket", + "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket", }) }, []Op{ &dbusProxyOp{ @@ -158,7 +172,6 @@ func TestDBusProxyOp(t *testing.T) { system: true, }, }, stub.Expect{Calls: []stub.Call{ - call("dbusAddress", stub.ExpectArgs{}, [2]string{"unix:path=/run/user/1000/bus", "unix:path=/run/dbus/system_bus_socket"}, nil), call("dbusFinalise", stub.ExpectArgs{ dbus.ProxyPair{"unix:path=/run/user/1000/bus", "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/bus"}, dbus.ProxyPair{"unix:path=/run/dbus/system_bus_socket", "/tmp/hakurei.0/99dd71ee2146369514e0d10783368f8f/system_bus_socket"}, diff --git a/system/dispatcher.go b/system/dispatcher.go index 3f17dc1..cdc9507 100644 --- a/system/dispatcher.go +++ b/system/dispatcher.go @@ -48,8 +48,6 @@ type syscallDispatcher interface { // xcbChangeHosts provides [xcb.ChangeHosts]. xcbChangeHosts(mode xcb.HostMode, family xcb.Family, address string) error - // dbusAddress provides [dbus.Address]. - dbusAddress() (session, system string) // dbusFinalise provides [dbus.Finalise]. dbusFinalise(sessionBus, systemBus dbus.ProxyPair, session, system *hst.BusConfig) (final *dbus.Final, err error) // dbusProxyStart provides the Start method of [dbus.Proxy]. @@ -82,10 +80,6 @@ func (k direct) xcbChangeHosts(mode xcb.HostMode, family xcb.Family, address str return xcb.ChangeHosts(mode, family, address) } -func (k direct) dbusAddress() (session, system string) { - return dbus.Address() -} - func (k direct) dbusFinalise(sessionBus, systemBus dbus.ProxyPair, session, system *hst.BusConfig) (final *dbus.Final, err error) { return dbus.Finalise(sessionBus, systemBus, session, system) } diff --git a/system/dispatcher_test.go b/system/dispatcher_test.go index 5edc275..fd6e5d0 100644 --- a/system/dispatcher_test.go +++ b/system/dispatcher_test.go @@ -276,12 +276,6 @@ func (k *kstub) xcbChangeHosts(mode xcb.HostMode, family xcb.Family, address str stub.CheckArg(k.Stub, "address", address, 2)) } -func (k *kstub) dbusAddress() (session, system string) { - k.Helper() - ret := k.Expects("dbusAddress").Ret.([2]string) - return ret[0], ret[1] -} - func (k *kstub) dbusFinalise(sessionBus, systemBus dbus.ProxyPair, session, system *hst.BusConfig) (final *dbus.Final, err error) { k.Helper() expect := k.Expects("dbusFinalise")