diff --git a/internal/app/app_nixos_linux_test.go b/internal/app/app_nixos_linux_test.go index e2aa5a4..b6f5e44 100644 --- a/internal/app/app_nixos_linux_test.go +++ b/internal/app/app_nixos_linux_test.go @@ -1,6 +1,7 @@ package app_test import ( + "context" "syscall" "hakurei.app/container" @@ -74,7 +75,7 @@ var testCasesNixos = []sealTestCase{ 0x4c, 0xf0, 0x73, 0xbd, 0xb4, 0x6e, 0xb5, 0xc1, }, - system.New(1000001). + system.New(context.TODO(), 1000001). Ensure("/tmp/hakurei.1971", 0711). Ensure("/tmp/hakurei.1971/runtime", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime", acl.Execute). Ensure("/tmp/hakurei.1971/runtime/1", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime/1", acl.Read, acl.Write, acl.Execute). diff --git a/internal/app/app_pd_linux_test.go b/internal/app/app_pd_linux_test.go index 384f5a0..7bc9473 100644 --- a/internal/app/app_pd_linux_test.go +++ b/internal/app/app_pd_linux_test.go @@ -1,6 +1,7 @@ package app_test import ( + "context" "os" "syscall" @@ -23,7 +24,7 @@ var testCasesPd = []sealTestCase{ 0xbd, 0x01, 0x78, 0x0e, 0xb9, 0xa6, 0x07, 0xac, }, - system.New(1000000). + system.New(context.TODO(), 1000000). Ensure("/tmp/hakurei.1971", 0711). Ensure("/tmp/hakurei.1971/runtime", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime", acl.Execute). Ensure("/tmp/hakurei.1971/runtime/0", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime/0", acl.Read, acl.Write, acl.Execute). @@ -115,7 +116,7 @@ var testCasesPd = []sealTestCase{ 0x82, 0xd4, 0x13, 0x36, 0x9b, 0x64, 0xce, 0x7c, }, - system.New(1000009). + system.New(context.TODO(), 1000009). Ensure("/tmp/hakurei.1971", 0711). Ensure("/tmp/hakurei.1971/runtime", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime", acl.Execute). Ensure("/tmp/hakurei.1971/runtime/9", 0700).UpdatePermType(system.User, "/tmp/hakurei.1971/runtime/9", acl.Read, acl.Write, acl.Execute). diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 8b96cfb..ab94b1c 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -30,7 +30,7 @@ func TestApp(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - a := app.NewWithID(tc.id, tc.os) + a := app.NewWithID(t.Context(), tc.id, tc.os) var ( gotSys *system.I gotContainer *container.Params diff --git a/internal/app/export_test.go b/internal/app/export_test.go index b7ee7f7..3e5db9f 100644 --- a/internal/app/export_test.go +++ b/internal/app/export_test.go @@ -1,17 +1,16 @@ package app import ( + "context" + "hakurei.app/container" "hakurei.app/internal/app/state" "hakurei.app/internal/sys" "hakurei.app/system" ) -func NewWithID(id state.ID, os sys.State) *App { - a := new(App) - a.id = newID(&id) - a.sys = os - return a +func NewWithID(ctx context.Context, id state.ID, os sys.State) *App { + return &App{id: newID(&id), sys: os, ctx: ctx} } func AppIParams(a *App, seal *Outcome) (*system.I, *container.Params) { diff --git a/internal/app/process.go b/internal/app/process.go index 4435569..b89f0fc 100644 --- a/internal/app/process.go +++ b/internal/app/process.go @@ -61,7 +61,7 @@ func (seal *Outcome) Run(rs *RunState) error { // read comp value early to allow for early failure hsuPath := internal.MustHsuPath() - if err := seal.sys.Commit(seal.ctx); err != nil { + if err := seal.sys.Commit(); err != nil { return err } store := state.NewMulti(seal.runDirPath.String()) diff --git a/internal/app/seal.go b/internal/app/seal.go index ec42dc2..047c3a7 100644 --- a/internal/app/seal.go +++ b/internal/app/seal.go @@ -146,8 +146,11 @@ type hsuUser struct { } func (seal *Outcome) finalise(ctx context.Context, sys sys.State, config *hst.Config) error { + if ctx == nil { + panic("invalid call to finalise") + } if seal.ctx != nil { - panic("finalise called twice") + panic("attempting to finalise twice") } seal.ctx = ctx @@ -306,7 +309,7 @@ func (seal *Outcome) finalise(ctx context.Context, sys sys.State, config *hst.Co share := &shareHost{seal: seal, sc: sys.Paths()} seal.runDirPath = share.sc.RunDirPath - seal.sys = system.New(seal.user.uid.unwrap()) + seal.sys = system.New(seal.ctx, seal.user.uid.unwrap()) seal.sys.Ensure(share.sc.SharePath.String(), 0711) { diff --git a/system/acl.go b/system/acl.go index 82777b9..0432056 100644 --- a/system/acl.go +++ b/system/acl.go @@ -9,37 +9,33 @@ import ( "hakurei.app/system/acl" ) -// UpdatePerm appends an ephemeral acl update Op. +// UpdatePerm appends [ACLUpdateOp] to [I] with the [Process] criteria. func (sys *I) UpdatePerm(path string, perms ...acl.Perm) *I { sys.UpdatePermType(Process, path, perms...) - return sys } -// UpdatePermType appends an acl update Op. +// UpdatePermType appends [ACLUpdateOp] to [I]. func (sys *I) UpdatePermType(et Enablement, path string, perms ...acl.Perm) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &ACL{et, path, perms}) - + sys.ops = append(sys.ops, &ACLUpdateOp{et, path, perms}) return sys } -type ACL struct { +// ACLUpdateOp maintains [acl.Perms] on a file until its [Enablement] is no longer satisfied. +type ACLUpdateOp struct { et Enablement path string perms acl.Perms } -func (a *ACL) Type() Enablement { return a.et } +func (a *ACLUpdateOp) Type() Enablement { return a.et } -func (a *ACL) apply(sys *I) error { +func (a *ACLUpdateOp) apply(sys *I) error { msg.Verbose("applying ACL", a) return newOpError("acl", acl.Update(a.path, sys.uid, a.perms...), false) } -func (a *ACL) revert(sys *I, ec *Criteria) error { +func (a *ACLUpdateOp) revert(sys *I, ec *Criteria) error { if ec.hasType(a) { msg.Verbose("stripping ACL", a) err := acl.Update(a.path, sys.uid) @@ -55,17 +51,17 @@ func (a *ACL) revert(sys *I, ec *Criteria) error { } } -func (a *ACL) Is(o Op) bool { - a0, ok := o.(*ACL) - return ok && a0 != nil && - a.et == a0.et && - a.path == a0.path && - slices.Equal(a.perms, a0.perms) +func (a *ACLUpdateOp) Is(o Op) bool { + target, ok := o.(*ACLUpdateOp) + return ok && a != nil && target != nil && + a.et == target.et && + a.path == target.path && + slices.Equal(a.perms, target.perms) } -func (a *ACL) Path() string { return a.path } +func (a *ACLUpdateOp) Path() string { return a.path } -func (a *ACL) String() string { +func (a *ACLUpdateOp) String() string { return fmt.Sprintf("%s type: %s path: %q", a.perms, TypeString(a.et), a.path) } diff --git a/system/acl_test.go b/system/acl_test.go index 886a62e..6b050bb 100644 --- a/system/acl_test.go +++ b/system/acl_test.go @@ -18,9 +18,9 @@ func TestUpdatePerm(t *testing.T) { for _, tc := range testCases { t.Run(tc.path+permSubTestSuffix(tc.perms), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.UpdatePerm(tc.path, tc.perms...) - (&tcOp{Process, tc.path}).test(t, sys.ops, []Op{&ACL{Process, tc.path, tc.perms}}, "UpdatePerm") + (&tcOp{Process, tc.path}).test(t, sys.ops, []Op{&ACLUpdateOp{Process, tc.path, tc.perms}}, "UpdatePerm") }) } } @@ -40,9 +40,9 @@ func TestUpdatePermType(t *testing.T) { for _, tc := range testCases { t.Run(tc.path+"_"+TypeString(tc.et)+permSubTestSuffix(tc.perms), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.UpdatePermType(tc.et, tc.path, tc.perms...) - tc.test(t, sys.ops, []Op{&ACL{tc.et, tc.path, tc.perms}}, "UpdatePermType") + tc.test(t, sys.ops, []Op{&ACLUpdateOp{tc.et, tc.path, tc.perms}}, "UpdatePermType") }) } } @@ -65,7 +65,7 @@ func TestACLString(t *testing.T) { for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - a := &ACL{et: tc.et, perms: tc.perms, path: container.Nonexistent} + a := &ACLUpdateOp{et: tc.et, perms: tc.perms, path: container.Nonexistent} if got := a.String(); got != tc.want { t.Errorf("String() = %v, want %v", got, tc.want) diff --git a/system/dbus.go b/system/dbus.go index b891089..d1ff06a 100644 --- a/system/dbus.go +++ b/system/dbus.go @@ -17,6 +17,7 @@ var ( ErrDBusConfig = errors.New("dbus config not supplied") ) +// MustProxyDBus calls ProxyDBus and panics if an error is returned. func (sys *I) MustProxyDBus(sessionPath string, session *dbus.Config, systemPath string, system *dbus.Config) *I { if _, err := sys.ProxyDBus(session, system, sessionPath, systemPath); err != nil { panic(err.Error()) @@ -25,8 +26,9 @@ func (sys *I) MustProxyDBus(sessionPath string, session *dbus.Config, systemPath } } +// ProxyDBus finalises configuration and appends [DBusProxyOp] to [I]. func (sys *I) ProxyDBus(session, system *dbus.Config, sessionPath, systemPath string) (func(), error) { - d := new(DBus) + d := new(DBusProxyOp) // session bus is required as otherwise this is effectively a very expensive noop if session == nil { @@ -39,7 +41,7 @@ func (sys *I) ProxyDBus(session, system *dbus.Config, sessionPath, systemPath st d.sessionBus[0], d.systemBus[0] = dbus.Address() d.sessionBus[1], d.systemBus[1] = sessionPath, systemPath - d.out = &scanToFmsg{msg: new(strings.Builder)} + d.out = &linePrefixWriter{println: log.Println, prefix: "(dbus) ", msg: new(strings.Builder)} if final, err := dbus.Finalise(d.sessionBus, d.systemBus, session, system); err != nil { if errors.Is(err, syscall.EINVAL) { return nil, newOpErrorMessage("dbus", err, @@ -65,20 +67,22 @@ func (sys *I) ProxyDBus(session, system *dbus.Config, sessionPath, systemPath st return d.out.Dump, nil } -type DBus struct { +// DBusProxyOp starts xdg-dbus-proxy via [dbus] and terminates it on revert. +// This [Op] is always [Process] scoped. +type DBusProxyOp struct { proxy *dbus.Proxy // populated during apply final *dbus.Final - out *scanToFmsg + out *linePrefixWriter // whether system bus proxy is enabled system bool sessionBus, systemBus dbus.ProxyPair } -func (d *DBus) Type() Enablement { return Process } +func (d *DBusProxyOp) Type() Enablement { return Process } -func (d *DBus) apply(sys *I) error { +func (d *DBusProxyOp) apply(sys *I) error { msg.Verbosef("session bus proxy on %q for upstream %q", d.sessionBus[1], d.sessionBus[0]) if d.system { msg.Verbosef("system bus proxy on %q for upstream %q", d.systemBus[1], d.systemBus[0]) @@ -94,7 +98,7 @@ func (d *DBus) apply(sys *I) error { return nil } -func (d *DBus) revert(*I, *Criteria) error { +func (d *DBusProxyOp) revert(*I, *Criteria) error { // criteria ignored here since dbus is always process-scoped msg.Verbose("terminating message bus proxy") d.proxy.Close() @@ -108,30 +112,34 @@ func (d *DBus) revert(*I, *Criteria) error { fmt.Sprintf("message bus proxy error: %v", err), true) } -func (d *DBus) Is(o Op) bool { - d0, ok := o.(*DBus) - return ok && d0 != nil && - ((d.proxy == nil && d0.proxy == nil) || - (d.proxy != nil && d0.proxy != nil && d.proxy.String() == d0.proxy.String())) +func (d *DBusProxyOp) Is(o Op) bool { + target, ok := o.(*DBusProxyOp) + return ok && d != nil && target != nil && + ((d.proxy == nil && target.proxy == nil) || + (d.proxy != nil && target.proxy != nil && + d.proxy.String() == target.proxy.String())) } -func (d *DBus) Path() string { return "(dbus proxy)" } -func (d *DBus) String() string { return d.proxy.String() } +func (d *DBusProxyOp) Path() string { return "(dbus proxy)" } +func (d *DBusProxyOp) String() string { return d.proxy.String() } -type scanToFmsg struct { - msg *strings.Builder - msgbuf []string +// linePrefixWriter calls println with a prefix for every line written. +type linePrefixWriter struct { + prefix string + println func(v ...any) + msg *strings.Builder + msgbuf []string mu sync.RWMutex } -func (s *scanToFmsg) Write(p []byte) (n int, err error) { +func (s *linePrefixWriter) Write(p []byte) (n int, err error) { s.mu.Lock() defer s.mu.Unlock() return s.write(p, 0) } -func (s *scanToFmsg) write(p []byte, a int) (int, error) { +func (s *linePrefixWriter) write(p []byte, a int) (int, error) { if i := bytes.IndexByte(p, '\n'); i == -1 { n, _ := s.msg.Write(p) return a + n, nil @@ -141,7 +149,7 @@ func (s *scanToFmsg) write(p []byte, a int) (int, error) { // allow container init messages through v := s.msg.String() if strings.HasPrefix(v, "init: ") { - log.Println("(dbus) " + v) + s.println(s.prefix + v) } else { s.msgbuf = append(s.msgbuf, v) } @@ -151,10 +159,10 @@ func (s *scanToFmsg) write(p []byte, a int) (int, error) { } } -func (s *scanToFmsg) Dump() { +func (s *linePrefixWriter) Dump() { s.mu.RLock() for _, m := range s.msgbuf { - log.Println("(dbus) " + m) + s.println(s.prefix + m) } s.mu.RUnlock() } diff --git a/system/enablement.go b/system/enablement.go index f3e74e0..9c1342d 100644 --- a/system/enablement.go +++ b/system/enablement.go @@ -5,7 +5,7 @@ import ( "strings" ) -// Enablement represents optional system resources. +// Enablement represents an optional host service to export to the target user. type Enablement byte const ( diff --git a/system/link.go b/system/link.go index 022ee4e..4ba74ff 100644 --- a/system/link.go +++ b/system/link.go @@ -5,32 +5,29 @@ import ( "os" ) -// Link registers an Op that links dst to src. +// Link appends [HardlinkOp] to [I] the [Process] criteria. func (sys *I) Link(oldname, newname string) *I { return sys.LinkFileType(Process, oldname, newname) } -// LinkFileType registers a file linking Op labelled with type et. +// LinkFileType appends [HardlinkOp] to [I]. func (sys *I) LinkFileType(et Enablement, oldname, newname string) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Hardlink{et, newname, oldname}) - + sys.ops = append(sys.ops, &HardlinkOp{et, newname, oldname}) return sys } -type Hardlink struct { +// HardlinkOp maintains a hardlink until its [Enablement] is no longer satisfied. +type HardlinkOp struct { et Enablement dst, src string } -func (l *Hardlink) Type() Enablement { return l.et } +func (l *HardlinkOp) Type() Enablement { return l.et } -func (l *Hardlink) apply(*I) error { +func (l *HardlinkOp) apply(*I) error { msg.Verbose("linking", l) return newOpError("hardlink", os.Link(l.src, l.dst), false) } -func (l *Hardlink) revert(_ *I, ec *Criteria) error { +func (l *HardlinkOp) revert(_ *I, ec *Criteria) error { if ec.hasType(l) { msg.Verbosef("removing hard link %q", l.dst) return newOpError("hardlink", os.Remove(l.dst), true) @@ -40,6 +37,10 @@ func (l *Hardlink) revert(_ *I, ec *Criteria) error { } } -func (l *Hardlink) Is(o Op) bool { l0, ok := o.(*Hardlink); return ok && l0 != nil && *l == *l0 } -func (l *Hardlink) Path() string { return l.src } -func (l *Hardlink) String() string { return fmt.Sprintf("%q from %q", l.dst, l.src) } +func (l *HardlinkOp) Is(o Op) bool { + target, ok := o.(*HardlinkOp) + return ok && l != nil && target != nil && *l == *target +} + +func (l *HardlinkOp) Path() string { return l.src } +func (l *HardlinkOp) String() string { return fmt.Sprintf("%q from %q", l.dst, l.src) } diff --git a/system/mkdir.go b/system/mkdir.go index 51fc274..4fba4af 100644 --- a/system/mkdir.go +++ b/system/mkdir.go @@ -6,38 +6,30 @@ import ( "os" ) -// Ensure the existence and mode of a directory. +// Ensure appends [MkdirOp] to [I] with its [Enablement] ignored. func (sys *I) Ensure(name string, perm os.FileMode) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Mkdir{User, name, perm, false}) - + sys.ops = append(sys.ops, &MkdirOp{User, name, perm, false}) return sys } -// Ephemeral ensures the temporary existence and mode of a directory through the life of et. +// Ephemeral appends an ephemeral [MkdirOp] to [I]. func (sys *I) Ephemeral(et Enablement, name string, perm os.FileMode) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Mkdir{et, name, perm, true}) - + sys.ops = append(sys.ops, &MkdirOp{et, name, perm, true}) return sys } -type Mkdir struct { +// MkdirOp ensures the existence of a directory. +// For ephemeral, the directory is destroyed once [Enablement] is no longer satisfied. +type MkdirOp struct { et Enablement path string perm os.FileMode ephemeral bool } -func (m *Mkdir) Type() Enablement { - return m.et -} +func (m *MkdirOp) Type() Enablement { return m.et } -func (m *Mkdir) apply(*I) error { +func (m *MkdirOp) apply(*I) error { msg.Verbose("ensuring directory", m) // create directory @@ -52,7 +44,7 @@ func (m *Mkdir) apply(*I) error { } } -func (m *Mkdir) revert(_ *I, ec *Criteria) error { +func (m *MkdirOp) revert(_ *I, ec *Criteria) error { if !m.ephemeral { // skip non-ephemeral dir and do not log anything return nil @@ -67,16 +59,14 @@ func (m *Mkdir) revert(_ *I, ec *Criteria) error { } } -func (m *Mkdir) Is(o Op) bool { - m0, ok := o.(*Mkdir) - return ok && m0 != nil && *m == *m0 +func (m *MkdirOp) Is(o Op) bool { + target, ok := o.(*MkdirOp) + return ok && m != nil && target != nil && *m == *target } -func (m *Mkdir) Path() string { - return m.path -} +func (m *MkdirOp) Path() string { return m.path } -func (m *Mkdir) String() string { +func (m *MkdirOp) String() string { t := "ensure" if m.ephemeral { t = TypeString(m.Type()) diff --git a/system/mkdir_test.go b/system/mkdir_test.go index 706b3d1..8fc12a1 100644 --- a/system/mkdir_test.go +++ b/system/mkdir_test.go @@ -19,9 +19,9 @@ func TestEnsure(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name+"_"+tc.perm.String(), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.Ensure(tc.name, tc.perm) - (&tcOp{User, tc.name}).test(t, sys.ops, []Op{&Mkdir{User, tc.name, tc.perm, false}}, "Ensure") + (&tcOp{User, tc.name}).test(t, sys.ops, []Op{&MkdirOp{User, tc.name, tc.perm, false}}, "Ensure") }) } } @@ -36,9 +36,9 @@ func TestEphemeral(t *testing.T) { } for _, tc := range testCases { t.Run(tc.path+"_"+tc.perm.String()+"_"+TypeString(tc.et), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.Ephemeral(tc.et, tc.path, tc.perm) - tc.test(t, sys.ops, []Op{&Mkdir{tc.et, tc.path, tc.perm, true}}, "Ephemeral") + tc.test(t, sys.ops, []Op{&MkdirOp{tc.et, tc.path, tc.perm, true}}, "Ephemeral") }) } } @@ -60,7 +60,7 @@ func TestMkdirString(t *testing.T) { } for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - m := &Mkdir{ + m := &MkdirOp{ et: tc.et, path: container.Nonexistent, perm: 0701, diff --git a/system/op.go b/system/system.go similarity index 56% rename from system/op.go rename to system/system.go index 25f3fa2..e8fa43f 100644 --- a/system/op.go +++ b/system/system.go @@ -1,4 +1,4 @@ -// Package system provides tools for safely interacting with the operating system. +// Package system provides helpers to apply and revert groups of operations to the system. package system import ( @@ -6,11 +6,10 @@ import ( "errors" "log" "strings" - "sync" ) const ( - // User type is reverted at final launcher exit. + // User type is reverted at final instance exit. User = EM << iota // Process type is unconditionally reverted on exit. Process @@ -32,12 +31,10 @@ func (ec *Criteria) hasType(o Op) bool { // Op is a reversible system operation. type Op interface { - // Type returns Op's enablement type. + // Type returns [Op]'s enablement type, for matching a revert criteria. Type() Enablement - // apply the Op apply(sys *I) error - // revert reverses the Op if criteria is met revert(sys *I, ec *Criteria) error Is(o Op) bool @@ -45,7 +42,7 @@ type Op interface { String() string } -// TypeString returns the string representation of a type stored as an [Enablement]. +// TypeString extends [Enablement.String] to support [User] and [Process]. func TypeString(e Enablement) string { switch e { case User: @@ -68,35 +65,39 @@ func TypeString(e Enablement) string { } } -// New initialises sys with no-op verbose functions. -func New(uid int) (sys *I) { - sys = new(I) - sys.uid = uid - return +// New returns the address of a new [I] targeting uid. +func New(ctx context.Context, uid int) (sys *I) { + if ctx == nil || uid < 0 { + panic("invalid call to New") + } + return &I{ctx: ctx, uid: uid} } -// An I provides indirect bulk operating system interaction. I must not be copied. +// An I provides deferred operating system interaction. [I] must not be copied. +// Methods of [I] must not be used concurrently. type I struct { + _ noCopy + uid int ops []Op ctx context.Context - // whether sys has been reverted - state bool - - lock sync.Mutex + // the behaviour of Commit is only defined for up to one call + committed bool + // the behaviour of Revert is only defined for up to one call + reverted bool } func (sys *I) UID() int { return sys.uid } -// Equal returns whether all [Op] instances held by v is identical to that of sys. -func (sys *I) Equal(v *I) bool { - if v == nil || sys.uid != v.uid || len(sys.ops) != len(v.ops) { +// Equal returns whether all [Op] instances held by sys matches that of target. +func (sys *I) Equal(target *I) bool { + if target == nil || sys.uid != target.uid || len(sys.ops) != len(target.ops) { return false } for i, o := range sys.ops { - if !o.Is(v.ops[i]) { + if !o.Is(target.ops[i]) { return false } } @@ -104,18 +105,15 @@ func (sys *I) Equal(v *I) bool { return true } -// Commit applies all [Op] held by [I] and reverts successful [Op] on first error encountered. +// Commit applies all [Op] held by [I] and reverts all successful [Op] on first error encountered. // Commit must not be called more than once. -func (sys *I) Commit(ctx context.Context) error { - sys.lock.Lock() - defer sys.lock.Unlock() - - if sys.ctx != nil { - panic("sys instance committed twice") +func (sys *I) Commit() error { + if sys.committed { + panic("attempting to commit twice") } - sys.ctx = ctx + sys.committed = true - sp := New(sys.uid) + sp := New(sys.ctx, sys.uid) sp.ops = make([]Op, 0, len(sys.ops)) // prevent copies during commits defer func() { // sp is set to nil when all ops are applied @@ -144,13 +142,10 @@ func (sys *I) Commit(ctx context.Context) error { // Revert reverts all [Op] meeting [Criteria] held by [I]. func (sys *I) Revert(ec *Criteria) error { - sys.lock.Lock() - defer sys.lock.Unlock() - - if sys.state { - panic("sys instance reverted twice") + if sys.reverted { + panic("attempting to revert twice") } - sys.state = true + sys.reverted = true // collect errors errs := make([]error, len(sys.ops)) @@ -162,3 +157,16 @@ func (sys *I) Revert(ec *Criteria) error { // errors.Join filters nils return errors.Join(errs...) } + +// noCopy may be added to structs which must not be copied +// after the first use. +// +// See https://golang.org/issues/8005#issuecomment-190753527 +// for details. +// +// Note that it must not be embedded, due to the Lock and Unlock methods. +type noCopy struct{} + +// Lock is a no-op used by -copylocks checker from `go vet`. +func (*noCopy) Lock() {} +func (*noCopy) Unlock() {} diff --git a/system/op_test.go b/system/system_test.go similarity index 81% rename from system/op_test.go rename to system/system_test.go index d11bcfa..323e35b 100644 --- a/system/op_test.go +++ b/system/system_test.go @@ -19,7 +19,7 @@ func TestNew(t *testing.T) { for _, tc := range testCases { t.Run("sys initialised with uid "+strconv.Itoa(tc.uid), func(t *testing.T) { - if got := system.New(tc.uid); got.UID() != tc.uid { + if got := system.New(t.Context(), tc.uid); got.UID() != tc.uid { t.Errorf("New(%d) uid = %d, want %d", tc.uid, got.UID(), tc.uid) @@ -63,57 +63,57 @@ func TestI_Equal(t *testing.T) { }{ { "simple UID", - system.New(150), - system.New(150), + system.New(t.Context(), 150), + system.New(t.Context(), 150), true, }, { "simple UID differ", - system.New(150), - system.New(151), + system.New(t.Context(), 150), + system.New(t.Context(), 151), false, }, { "simple UID nil", - system.New(150), + system.New(t.Context(), 150), nil, false, }, { "op length mismatch", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), false, }, { "op value mismatch", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0644), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), false, }, { "op type mismatch", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). CopyFile(new([]byte), "/home/ophestra/xdg/config/pulse/cookie", 0, 256), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), false, }, { "op equals", - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), - system.New(150). + system.New(t.Context(), 150). ChangeHosts("chronos"). Ensure("/run", 0755), true, diff --git a/system/tmpfiles.go b/system/tmpfiles.go index 44d170f..2850ba2 100644 --- a/system/tmpfiles.go +++ b/system/tmpfiles.go @@ -9,20 +9,16 @@ import ( "syscall" ) -// CopyFile registers an Op that copies from src. -// A buffer is initialised with size cap and the Op faults if bytes read exceed n. +// CopyFile appends [TmpfileOp] to [I]. func (sys *I) CopyFile(payload *[]byte, src string, cap int, n int64) *I { buf := new(bytes.Buffer) buf.Grow(cap) - - sys.lock.Lock() - sys.ops = append(sys.ops, &Tmpfile{payload, src, n, buf}) - sys.lock.Unlock() - + sys.ops = append(sys.ops, &TmpfileOp{payload, src, n, buf}) return sys } -type Tmpfile struct { +// TmpfileOp reads up to n bytes from src and writes the resulting byte slice to payload. +type TmpfileOp struct { payload *[]byte src string @@ -30,8 +26,8 @@ type Tmpfile struct { buf *bytes.Buffer } -func (t *Tmpfile) Type() Enablement { return Process } -func (t *Tmpfile) apply(*I) error { +func (t *TmpfileOp) Type() Enablement { return Process } +func (t *TmpfileOp) apply(*I) error { msg.Verbose("copying", t) if t.payload == nil { @@ -59,12 +55,12 @@ func (t *Tmpfile) apply(*I) error { *t.payload = t.buf.Bytes() return nil } -func (t *Tmpfile) revert(*I, *Criteria) error { t.buf.Reset(); return nil } +func (t *TmpfileOp) revert(*I, *Criteria) error { t.buf.Reset(); return nil } -func (t *Tmpfile) Is(o Op) bool { - t0, ok := o.(*Tmpfile) - return ok && t0 != nil && - t.src == t0.src && t.n == t0.n +func (t *TmpfileOp) Is(o Op) bool { + target, ok := o.(*TmpfileOp) + return ok && t != nil && target != nil && + t.src == target.src && t.n == target.n } -func (t *Tmpfile) Path() string { return t.src } -func (t *Tmpfile) String() string { return fmt.Sprintf("up to %d bytes from %q", t.n, t.src) } +func (t *TmpfileOp) Path() string { return t.src } +func (t *TmpfileOp) String() string { return fmt.Sprintf("up to %d bytes from %q", t.n, t.src) } diff --git a/system/tmpfiles_test.go b/system/tmpfiles_test.go index 27a21a0..1ed07c6 100644 --- a/system/tmpfiles_test.go +++ b/system/tmpfiles_test.go @@ -15,10 +15,10 @@ func TestCopyFile(t *testing.T) { } for _, tc := range testCases { t.Run("copy file "+tc.path+" with cap = "+strconv.Itoa(tc.cap)+" n = "+strconv.Itoa(int(tc.n)), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.CopyFile(new([]byte), tc.path, tc.cap, tc.n) tc.test(t, sys.ops, []Op{ - &Tmpfile{nil, tc.path, tc.n, nil}, + &TmpfileOp{nil, tc.path, tc.n, nil}, }, "CopyFile") }) } @@ -33,10 +33,10 @@ func TestLink(t *testing.T) { } for _, tc := range testCases { t.Run("link file "+tc.dst+" from "+tc.src, func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.Link(tc.src, tc.dst) (&tcOp{Process, tc.src}).test(t, sys.ops, []Op{ - &Hardlink{Process, tc.dst, tc.src}, + &HardlinkOp{Process, tc.dst, tc.src}, }, "Link") }) } @@ -52,10 +52,10 @@ func TestLinkFileType(t *testing.T) { } for _, tc := range testCases { t.Run("link file "+tc.dst+" from "+tc.path+" with type "+TypeString(tc.et), func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.LinkFileType(tc.et, tc.path, tc.dst) tc.test(t, sys.ops, []Op{ - &Hardlink{tc.et, tc.dst, tc.path}, + &HardlinkOp{tc.et, tc.dst, tc.path}, }, "LinkFileType") }) } @@ -73,7 +73,7 @@ func TestTmpfile_String(t *testing.T) { for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - if got := (&Tmpfile{src: tc.src, n: tc.n}).String(); got != tc.want { + if got := (&TmpfileOp{src: tc.src, n: tc.n}).String(); got != tc.want { t.Errorf("String() = %v, want %v", got, tc.want) } }) diff --git a/system/wayland.go b/system/wayland.go index f08f58b..999feee 100644 --- a/system/wayland.go +++ b/system/wayland.go @@ -9,17 +9,16 @@ import ( "hakurei.app/system/wayland" ) -// Wayland sets up a wayland socket with a security context attached. +// Wayland appends [WaylandOp] to [I]. func (sys *I) Wayland(syncFd **os.File, dst, src, appID, instanceID string) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, &Wayland{syncFd, dst, src, appID, instanceID, wayland.Conn{}}) - + sys.ops = append(sys.ops, &WaylandOp{syncFd, dst, src, appID, instanceID, wayland.Conn{}}) return sys } -type Wayland struct { +// WaylandOp maintains a wayland socket with security-context-v1 attached via [wayland]. +// The socket stops accepting connections once the pipe referred to by sync is closed. +// The socket is pathname only and is destroyed on revert. +type WaylandOp struct { sync **os.File dst, src string appID, instanceID string @@ -27,9 +26,9 @@ type Wayland struct { conn wayland.Conn } -func (w *Wayland) Type() Enablement { return Process } +func (w *WaylandOp) Type() Enablement { return Process } -func (w *Wayland) apply(sys *I) error { +func (w *WaylandOp) apply(sys *I) error { if w.sync == nil { // this is a misuse of the API; do not return a wrapped error return errors.New("invalid sync") @@ -59,7 +58,7 @@ func (w *Wayland) apply(sys *I) error { } } -func (w *Wayland) revert(_ *I, ec *Criteria) error { +func (w *WaylandOp) revert(_ *I, ec *Criteria) error { if ec.hasType(w) { msg.Verbosef("removing wayland socket on %q", w.dst) if err := os.Remove(w.dst); err != nil && !errors.Is(err, os.ErrNotExist) { @@ -74,11 +73,12 @@ func (w *Wayland) revert(_ *I, ec *Criteria) error { } } -func (w *Wayland) Is(o Op) bool { - w0, ok := o.(*Wayland) - return ok && w.dst == w0.dst && w.src == w0.src && - w.appID == w0.appID && w.instanceID == w0.instanceID +func (w *WaylandOp) Is(o Op) bool { + target, ok := o.(*WaylandOp) + return ok && w != nil && target != nil && + w.dst == target.dst && w.src == target.src && + w.appID == target.appID && w.instanceID == target.instanceID } -func (w *Wayland) Path() string { return w.dst } -func (w *Wayland) String() string { return fmt.Sprintf("wayland socket at %q", w.dst) } +func (w *WaylandOp) Path() string { return w.dst } +func (w *WaylandOp) String() string { return fmt.Sprintf("wayland socket at %q", w.dst) } diff --git a/system/xhost.go b/system/xhost.go index 27b2fc6..2ed1acf 100644 --- a/system/xhost.go +++ b/system/xhost.go @@ -4,27 +4,24 @@ import ( "hakurei.app/system/internal/xcb" ) -// ChangeHosts appends an X11 ChangeHosts command Op. +// ChangeHosts appends [XHostOp] to [I]. func (sys *I) ChangeHosts(username string) *I { - sys.lock.Lock() - defer sys.lock.Unlock() - - sys.ops = append(sys.ops, XHost(username)) - + sys.ops = append(sys.ops, XHostOp(username)) return sys } -type XHost string +// XHostOp inserts the target user into X11 hosts and deletes it once its [Enablement] is no longer satisfied. +type XHostOp string -func (x XHost) Type() Enablement { return EX11 } +func (x XHostOp) Type() Enablement { return EX11 } -func (x XHost) apply(*I) error { +func (x XHostOp) apply(*I) error { msg.Verbosef("inserting entry %s to X11", x) return newOpError("xhost", xcb.ChangeHosts(xcb.HostModeInsert, xcb.FamilyServerInterpreted, "localuser\x00"+string(x)), false) } -func (x XHost) revert(_ *I, ec *Criteria) error { +func (x XHostOp) revert(_ *I, ec *Criteria) error { if ec.hasType(x) { msg.Verbosef("deleting entry %s from X11", x) return newOpError("xhost", @@ -35,6 +32,6 @@ func (x XHost) revert(_ *I, ec *Criteria) error { } } -func (x XHost) Is(o Op) bool { x0, ok := o.(XHost); return ok && x == x0 } -func (x XHost) Path() string { return string(x) } -func (x XHost) String() string { return string("SI:localuser:" + x) } +func (x XHostOp) Is(o Op) bool { target, ok := o.(XHostOp); return ok && x == target } +func (x XHostOp) Path() string { return string(x) } +func (x XHostOp) String() string { return string("SI:localuser:" + x) } diff --git a/system/xhost_test.go b/system/xhost_test.go index 4b36443..df0e816 100644 --- a/system/xhost_test.go +++ b/system/xhost_test.go @@ -8,10 +8,10 @@ func TestChangeHosts(t *testing.T) { testCases := []string{"chronos", "keyring", "cat", "kbd", "yonah"} for _, tc := range testCases { t.Run("append ChangeHosts operation for "+tc, func(t *testing.T) { - sys := New(150) + sys := New(t.Context(), 150) sys.ChangeHosts(tc) (&tcOp{EX11, tc}).test(t, sys.ops, []Op{ - XHost(tc), + XHostOp(tc), }, "ChangeHosts") }) } @@ -26,7 +26,7 @@ func TestXHost_String(t *testing.T) { } for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { - if got := XHost(tc.username).String(); got != tc.want { + if got := XHostOp(tc.username).String(); got != tc.want { t.Errorf("String() = %v, want %v", got, tc.want) } })