From 5d37bc76cf647861fef31984db04e641fb83e865 Mon Sep 17 00:00:00 2001 From: et-nik Date: Sat, 16 May 2026 14:12:38 +0200 Subject: [PATCH] steamcmd fixes --- README.md | 26 +++++++ internal/app/components/executor_unix.go | 63 ++++++++++++++-- internal/app/config/config.go | 5 ++ .../game_server_commands/install_server.go | 24 ++++++- .../install_server_test.go | 44 ++++++------ internal/app/osowner/owner.go | 71 +++++++++++++++++++ internal/app/osowner/owner_test.go | 53 ++++++++++++++ internal/app/osowner/owner_unix.go | 36 ++++++++++ internal/app/osowner/owner_windows.go | 4 ++ 9 files changed, 299 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 8bbdff8..ad893fd 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,32 @@ private_key: | | stats_update_period | no | integer | Stats update period | stats_db_update_period | no | integer | Update database period +### Steam + +| Parameter | Required | Type | Info +|---------------------------|-----------------------|-----------|------------ +| steamcmd_path | no | string | Path to the directory that contains steamcmd +| steam_config.login | no | string | Steam account login (anonymous when empty) +| steam_config.password | no | string | Steam account password +| steam_config.group | no | string | Shared OS group for the steamcmd directory (see below) + +When the daemon runs as `root` and a game server has its own `su_user`, steamcmd is +executed under that unprivileged user (least privilege; files end up owned correctly +for both install and updates). Because `steamcmd.sh` self-updates and writes into its +own directory, that directory must be writable by every `su_user`. + +Before running steamcmd the daemon applies, recursively, the setgid bit plus group +`rwx`/`rw` to `steamcmd_path`, changing only the group (the owner is preserved). The +group is taken from `steam_config.group`, falling back to the `su_user` primary group +when empty. + +On a node with several different `su_user`s, set `steam_config.group` to a shared +group and add every `su_user` to it (e.g. `usermod -aG `). The +daemon then keeps the steamcmd directory consistently group-shared so self-updates +succeed regardless of which server triggers them. `steam_config` is read from the +yaml config only (it is not pushed from the API). This whole step is a no-op when +the daemon does not run as `root`. + ### Other #### Only on Windows diff --git a/internal/app/components/executor_unix.go b/internal/app/components/executor_unix.go index 4df4f68..ec95c7c 100644 --- a/internal/app/components/executor_unix.go +++ b/internal/app/components/executor_unix.go @@ -8,10 +8,12 @@ import ( "os/exec" "os/user" "strconv" + "strings" "syscall" "github.com/gameap/daemon/internal/app/contracts" "github.com/pkg/errors" + log "github.com/sirupsen/logrus" ) func setCMDSysProcCredential(cmd *exec.Cmd, options contracts.ExecutorOptions) (*exec.Cmd, error) { @@ -29,14 +31,67 @@ func setCMDSysProcCredential(cmd *exec.Cmd, options contracts.ExecutorOptions) ( if err != nil { return nil, errors.WithMessage(err, "[game_server_commands.installator] invalid user gid") } - cmd.SysProcAttr = &syscall.SysProcAttr{} - cmd.SysProcAttr.Credential = &syscall.Credential{ + + credential := &syscall.Credential{ Uid: uint32(uid), Gid: uint32(gid), } - cmd.Env = os.Environ() - cmd.Env = append(cmd.Env, "HOME="+u.HomeDir) + if groups := supplementaryGroups(u); len(groups) > 0 { + credential.Groups = groups + } + + cmd.SysProcAttr = &syscall.SysProcAttr{} + cmd.SysProcAttr.Credential = credential + + env := os.Environ() + for key, value := range options.Env { + env = upsertEnv(env, key, value) + } + if _, ok := options.Env["HOME"]; !ok { + env = upsertEnv(env, "HOME", u.HomeDir) + } + cmd.Env = env return cmd, nil } + +// supplementaryGroups resolves the user's full group membership so the +// privilege-dropped process keeps access to shared groups (e.g. the steamcmd +// group). Without it the child would call setgroups([]) and lose them. +func supplementaryGroups(u *user.User) []uint32 { + groupIDs, err := u.GroupIds() + if err != nil { + log.Error(errors.Wrapf(err, "failed to resolve supplementary groups for user %s", u.Username)) + + return nil + } + + groups := make([]uint32, 0, len(groupIDs)) + for _, g := range groupIDs { + n, convErr := strconv.Atoi(g) + if convErr != nil { + log.Error(errors.Wrapf(convErr, "invalid supplementary group id %q for user %s", g, u.Username)) + + continue + } + groups = append(groups, uint32(n)) + } + + return groups +} + +// upsertEnv replaces the KEY=... entry in env or appends it, keeping a single +// definition per key so a caller-supplied value (e.g. HOME) is not shadowed. +func upsertEnv(env []string, key, value string) []string { + prefix := key + "=" + for i, e := range env { + if strings.HasPrefix(e, prefix) { + env[i] = prefix + value + + return env + } + } + + return append(env, prefix+value) +} diff --git a/internal/app/config/config.go b/internal/app/config/config.go index a3d10a4..b1e7b65 100644 --- a/internal/app/config/config.go +++ b/internal/app/config/config.go @@ -28,6 +28,11 @@ type Scripts struct { type SteamConfig struct { Login string `yaml:"login"` Password string `yaml:"password"` + + // Group is the shared OS group applied (with setgid) to the steamcmd + // directory so that game servers running under their own su_user can let + // steamcmd self-update. Empty falls back to the su_user primary group. + Group string `yaml:"group"` } type GRPCConfig struct { diff --git a/internal/app/game_server_commands/install_server.go b/internal/app/game_server_commands/install_server.go index 3be3ab0..4ad23a0 100644 --- a/internal/app/game_server_commands/install_server.go +++ b/internal/app/game_server_commands/install_server.go @@ -536,8 +536,28 @@ func (in *installator) installFromSteam( in.writeOutput(ctx, "Installing from steam ...") + if _, err := os.Stat(cfg.SteamCMDPath); err != nil { + err = errors.Wrapf(err, "steamcmd directory %s is not accessible", cfg.SteamCMDPath) + in.writeOutput(ctx, err.Error()) + + return err + } + + err := osowner.ApplyGroupSharedRecursive(cfg.SteamCMDPath, osowner.Options{ + User: server.User(), + Group: cfg.SteamConfig.Group, + }) + if err != nil { + err = errors.Wrapf(err, "failed to share steamcmd directory %s with the game server group", cfg.SteamCMDPath) + in.writeOutput(ctx, err.Error()) + + return err + } + executorOptions := contracts.ExecutorOptions{ - WorkDir: cfg.WorkPath, + WorkDir: cfg.SteamCMDPath, + FallbackWorkDir: server.WorkDir(in.cfg), + Env: map[string]string{"HOME": cfg.SteamCMDPath}, } if isRootUser() && server.User() != "" { @@ -552,7 +572,7 @@ func (in *installator) installFromSteam( executorOptions.GID = systemUser.Gid } - _, err := os.Stat(server.WorkDir(in.cfg)) + _, err = os.Stat(server.WorkDir(in.cfg)) if err != nil && errors.Is(err, os.ErrNotExist) { err = mkdirAllWithFinalPerm(server.WorkDir(in.cfg), 0750) if err != nil { diff --git a/internal/app/game_server_commands/install_server_test.go b/internal/app/game_server_commands/install_server_test.go index e10c762..7fc199b 100644 --- a/internal/app/game_server_commands/install_server_test.go +++ b/internal/app/game_server_commands/install_server_test.go @@ -6,7 +6,6 @@ import ( "io" "os" "path/filepath" - "runtime" "testing" "time" @@ -293,7 +292,8 @@ func TestUpdateBySteam_SteamCommandWithoutValidate(t *testing.T) { } }(workPath) cfg := &config.Config{ - WorkPath: workPath, + WorkPath: workPath, + SteamCMDPath: workPath, } executor := &testExecutor{} updater := newUpdater(cfg, executor, &bytes.Buffer{}) @@ -305,11 +305,8 @@ func TestUpdateBySteam_SteamCommandWithoutValidate(t *testing.T) { err = updater.Install(context.Background(), server, rules) require.Nil(t, err) - if runtime.GOOS == "windows" { - executor.AssertCommand(t, "steamcmd.exe +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 +quit") - } else { - executor.AssertCommand(t, "steamcmd.sh +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 +quit") - } + steamCMD := filepath.Join(workPath, config.SteamCMDExecutableFile) + executor.AssertCommand(t, steamCMD+" +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 +quit") } func TestUpdateBySteam_SteamCommandWithSetConfig(t *testing.T) { @@ -322,7 +319,8 @@ func TestUpdateBySteam_SteamCommandWithSetConfig(t *testing.T) { } }(workPath) cfg := &config.Config{ - WorkPath: workPath, + WorkPath: workPath, + SteamCMDPath: workPath, } executor := &testExecutor{} updater := newUpdater(cfg, executor, &bytes.Buffer{}) @@ -334,11 +332,8 @@ func TestUpdateBySteam_SteamCommandWithSetConfig(t *testing.T) { err = updater.Install(context.Background(), server, rules) require.Nil(t, err) - if runtime.GOOS == "windows" { - executor.AssertCommand(t, "steamcmd.exe +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 mod czero +quit") - } else { - executor.AssertCommand(t, "steamcmd.sh +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 mod czero +quit") - } + steamCMD := filepath.Join(workPath, config.SteamCMDExecutableFile) + executor.AssertCommand(t, steamCMD+" +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 mod czero +quit") } func TestInstallBySteam_SteamCommandWithValidate(t *testing.T) { @@ -351,7 +346,8 @@ func TestInstallBySteam_SteamCommandWithValidate(t *testing.T) { } }(workPath) cfg := &config.Config{ - WorkPath: workPath, + WorkPath: workPath, + SteamCMDPath: workPath, } executor := &testExecutor{} updater := newInstallator(cfg, executor, &bytes.Buffer{}) @@ -363,11 +359,12 @@ func TestInstallBySteam_SteamCommandWithValidate(t *testing.T) { err = updater.Install(context.Background(), server, rules) require.Nil(t, err) - if runtime.GOOS == "windows" { - executor.AssertCommand(t, "steamcmd.exe +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 validate +quit") - } else { - executor.AssertCommand(t, "steamcmd.sh +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 validate +quit") - } + steamCMD := filepath.Join(workPath, config.SteamCMDExecutableFile) + executor.AssertCommand(t, steamCMD+" +force_install_dir \""+workPath+"/test-server\" +login anonymous +app_update 90 validate +quit") + + assert.Equal(t, workPath, executor.options.WorkDir) + assert.Equal(t, server.WorkDir(cfg), executor.options.FallbackWorkDir) + assert.Equal(t, workPath, executor.options.Env["HOME"]) } func givenRemoteInstallationServer(t *testing.T) *domain.Server { @@ -473,18 +470,23 @@ func givenServer(t *testing.T, game domain.Game, gameMod domain.GameMod) *domain type testExecutor struct { command string + options contracts.ExecutorOptions } -func (ex *testExecutor) Exec(_ context.Context, command string, _ contracts.ExecutorOptions) ([]byte, int, error) { +func (ex *testExecutor) Exec( + _ context.Context, command string, options contracts.ExecutorOptions, +) ([]byte, int, error) { ex.command = command + ex.options = options return []byte(""), 0, nil } func (ex *testExecutor) ExecWithWriter( - _ context.Context, command string, _ io.Writer, _ contracts.ExecutorOptions, + _ context.Context, command string, _ io.Writer, options contracts.ExecutorOptions, ) (int, error) { ex.command = command + ex.options = options return 0, nil } diff --git a/internal/app/osowner/owner.go b/internal/app/osowner/owner.go index 017a4c8..53a4628 100644 --- a/internal/app/osowner/owner.go +++ b/internal/app/osowner/owner.go @@ -6,6 +6,11 @@ // become readable only by root and the game server (running as e.g. gameap) // cannot read them. This package wraps user.Lookup + Lchown with the // existing isRootUser/Windows no-op build constraints. +// +// ApplyGroupSharedRecursive covers a different case: a single directory shared +// by several su_users (the steamcmd directory, which steamcmd self-updates). +// It keeps the owner but adds setgid + group rwx so every su_user in the +// shared group can write there. package osowner import ( @@ -23,6 +28,11 @@ type Options struct { User string UID int32 GID int32 + + // Group is an optional group name used only by ApplyGroupSharedRecursive + // to override which group a shared tree (e.g. the steamcmd directory) is + // shared with. Empty falls back to User's primary group, then GID. + Group string } // IsZero reports whether no ownership info was supplied. Callers can short @@ -91,6 +101,67 @@ func ApplyRecursive(path string, opts Options) error { return chownTree(path, uid, gid) } +// resolveGroupID converts Options into the numeric gid that a shared tree +// should be group-accessible by. Returns ok=false when the daemon is not +// running as root (chmod/chown of a root-owned tree needs root — same gate +// as Resolve) or when no group can be determined. +func resolveGroupID(opts Options) (gid int, ok bool, err error) { + if !isRootUser() { + return 0, false, nil + } + + if opts.Group != "" { + grp, lookupErr := user.LookupGroup(opts.Group) + if lookupErr != nil { + return 0, false, lookupErr + } + + gid, err = strconv.Atoi(grp.Gid) + if err != nil { + return 0, false, err + } + + return gid, true, nil + } + + if opts.User != "" { + systemUser, lookupErr := user.Lookup(opts.User) + if lookupErr != nil { + return 0, false, lookupErr + } + + gid, err = strconv.Atoi(systemUser.Gid) + if err != nil { + return 0, false, err + } + + return gid, true, nil + } + + if opts.GID != 0 { + return int(opts.GID), true, nil + } + + return 0, false, nil +} + +// ApplyGroupSharedRecursive makes path and everything under it accessible to +// the resolved group: directories get the setgid bit plus group rwx (so +// entries created later inherit the group), files get group rw. File owner +// (user) is preserved — only the group is changed. No-op when not applicable +// (non-root daemon, no resolvable group, Windows). +func ApplyGroupSharedRecursive(path string, opts Options) error { + gid, ok, err := resolveGroupID(opts) + if err != nil { + return err + } + if !ok { + return nil + } + + return groupShareTree(path, gid) +} + // MissingSegments returns the path prefixes that do not exist on disk yet, // from the shallowest missing ancestor down to target itself. Callers // invoke this BEFORE MkdirAll, then chown the returned paths after diff --git a/internal/app/osowner/owner_test.go b/internal/app/osowner/owner_test.go index f075e50..5cb1161 100644 --- a/internal/app/osowner/owner_test.go +++ b/internal/app/osowner/owner_test.go @@ -2,7 +2,9 @@ package osowner import ( "os" + "os/user" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -115,3 +117,54 @@ func TestMissingSegments_TargetIsExistingFileReturnsEmpty(t *testing.T) { require.NoError(t, err) assert.Empty(t, segments) } + +func TestApplyGroupSharedRecursive_EmptyOptionsIsNoop(t *testing.T) { + tempDir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(tempDir, "a"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "a", "f.txt"), []byte("y"), 0o644)) + + err := ApplyGroupSharedRecursive(tempDir, Options{}) + + require.NoError(t, err) +} + +func TestApplyGroupSharedRecursive_NonRootDaemonIsNoop(t *testing.T) { + tempDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(tempDir, "f.txt"), []byte("x"), 0o644)) + + err := ApplyGroupSharedRecursive(tempDir, Options{ + User: "nonexistentuser_4j2k9c", + Group: "nonexistentgroup_4j2k9c", + }) + + require.NoError(t, err, "non-root daemon must not even attempt user/group lookup") +} + +func TestApplyGroupSharedRecursive_SetsSetgidAndGroupBitsWhenRoot(t *testing.T) { + if !isRootUser() || runtime.GOOS == "windows" { + t.Skip("requires root on a unix host") + } + + grp, err := user.LookupGroupId("0") + if err != nil { + t.Skipf("no group with gid 0 to test with: %v", err) + } + + tempDir := t.TempDir() + subDir := filepath.Join(tempDir, "sub") + require.NoError(t, os.Mkdir(subDir, 0o755)) + file := filepath.Join(subDir, "f.txt") + require.NoError(t, os.WriteFile(file, []byte("x"), 0o644)) + + err = ApplyGroupSharedRecursive(tempDir, Options{Group: grp.Name}) + require.NoError(t, err) + + dirInfo, err := os.Stat(subDir) + require.NoError(t, err) + assert.NotZero(t, dirInfo.Mode()&os.ModeSetgid, "directory must get the setgid bit") + assert.Equal(t, os.FileMode(0o070), dirInfo.Mode().Perm()&0o070, "directory must be group rwx") + + fileInfo, err := os.Stat(file) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o060), fileInfo.Mode().Perm()&0o060, "file must be group rw") +} diff --git a/internal/app/osowner/owner_unix.go b/internal/app/osowner/owner_unix.go index 6842178..da8ce72 100644 --- a/internal/app/osowner/owner_unix.go +++ b/internal/app/osowner/owner_unix.go @@ -40,3 +40,39 @@ func chownTree(path string, uid, gid int) error { return root.Lchown(name, uid, gid) }) } + +func groupShareTree(path string, gid int) error { + root, err := os.OpenRoot(path) + if err != nil { + return err + } + defer root.Close() + + return fs.WalkDir(root.FS(), ".", func(name string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return nil + } + + if d.Type()&fs.ModeSymlink != 0 { + return root.Lchown(name, -1, gid) + } + + info, err := d.Info() + if err != nil { + return err + } + + mode := info.Mode() + if d.IsDir() { + mode |= os.ModeSetgid | 0o070 + } else { + mode |= 0o060 + } + + if err = root.Chmod(name, mode); err != nil { + return err + } + + return root.Lchown(name, -1, gid) + }) +} diff --git a/internal/app/osowner/owner_windows.go b/internal/app/osowner/owner_windows.go index d931beb..db78f39 100644 --- a/internal/app/osowner/owner_windows.go +++ b/internal/app/osowner/owner_windows.go @@ -26,3 +26,7 @@ func lchown(_ string, _, _ int) error { func chownTree(_ string, _, _ int) error { return nil } + +func groupShareTree(_ string, _ int) error { + return nil +}