From 9589e58ce6051ebfe33a42ec5192b6ae0164c624 Mon Sep 17 00:00:00 2001 From: mram Date: Mon, 21 Sep 2026 21:30:35 +0200 Subject: [PATCH] Install opens up COMFY_DIR for the sandboxed service: ACL grant on Windows, BindPaths on Linux --- README.md | 9 ++++++ SPEC.md | 8 +++++ internal/service/envfile.go | 16 ++++++++++ internal/service/service_linux.go | 20 ++++++++---- internal/service/service_linux_test.go | 10 +++++- internal/service/service_windows.go | 43 +++++++++++++++----------- 6 files changed, 81 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index ca9e53c..243b4c8 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,15 @@ answers on the port, gpu-turnstile just uses it instead of spawning instance already holds the port when you open the desktop app, the desktop's server is the one that fails to bind. +One catch when gpu-turnstile runs as a service: the sandboxed service +account may not enter your user profile, so a ComfyUI install under +`C:\Users\...` (or `/home/...`) fails with "Access is denied". +`--install-service` fixes that automatically — it grants +`NT SERVICE\gpu-turnstile` recursive access to `COMFY_DIR` on Windows and +adds a `BindPaths=` to the systemd unit on Linux. Re-run it after changing +`COMFY_DIR`; or grant by hand from an admin shell: +`icacls "" /grant "NT SERVICE\gpu-turnstile:(OI)(CI)M" /T`. + ## Game detection Want to game on the same GPU without Ollama/ComfyUI squatting on the VRAM? diff --git a/SPEC.md b/SPEC.md index 8a77097..b99b05e 100644 --- a/SPEC.md +++ b/SPEC.md @@ -159,6 +159,14 @@ files are flagged in the startup log. - Its stdout/stderr is forwarded to the log at INFO. The health check skips the intentionally-stopped/starting states; a failed probe while the process is alive and was previously ready is logged as DOWN. +- **Permissions**: the service account is sandboxed (Windows virtual + account, systemd `DynamicUser`), so a ComfyUI install inside a user + profile is off-limits by default. `--install-service` opens it up — + a recursive ACL grant for `NT SERVICE\gpu-turnstile` on Windows, a + `BindPaths=` in the unit on Linux — reading `COMFY_DIR` from the config + it installs. Re-run `--install-service` after changing `COMFY_DIR`, or + grant by hand (admin shell): + `icacls "" /grant "NT SERVICE\gpu-turnstile:(OI)(CI)M" /T`. ## Game detection (foreign GPU holders) diff --git a/internal/service/envfile.go b/internal/service/envfile.go index 10a2709..579298a 100644 --- a/internal/service/envfile.go +++ b/internal/service/envfile.go @@ -45,3 +45,19 @@ func writeEnvFile(path, content string) error { } return nil } + +// configuredValue reads one key from the env file the service will load, so +// the installers can adapt the sandbox to it (ACL grants, unit directives). +// "" when unset or unreadable. +func configuredValue(configPath, key string) string { + f, err := os.Open(configPath) + if err != nil { + return "" + } + defer f.Close() + values, err := config.ParseEnvFile(f) + if err != nil { + return "" + } + return values[key] +} diff --git a/internal/service/service_linux.go b/internal/service/service_linux.go index 06e0f5e..1d1ca46 100644 --- a/internal/service/service_linux.go +++ b/internal/service/service_linux.go @@ -62,8 +62,14 @@ func Run(run func(ctx context.Context) error) error { // filesystem is read-only except StateDirectory (the install dir, so // self-updates can rewrite the binary), and the usual no-privilege-escalation // directives apply. The proxy needs nothing but outbound TCP/UDP and the -// notify socket, so it loses nothing. -func renderUnit(exePath, configPath string) string { +// notify socket, so it loses nothing. A managed ComfyUI (comfyDir) gets a +// BindPaths hole through ProtectHome/ProtectSystem: it reads its venv and +// writes output/temp/user data under COMFY_DIR. +func renderUnit(exePath, configPath, comfyDir string) string { + bind := "" + if comfyDir != "" { + bind = "BindPaths=" + comfyDir + "\n" + } return fmt.Sprintf(`[Unit] Description=gpu-turnstile GPU arbitration proxy for Ollama and ComfyUI After=network-online.target @@ -78,7 +84,7 @@ RestartSec=5s DynamicUser=yes StateDirectory=%s -ProtectSystem=strict +%sProtectSystem=strict ProtectHome=yes PrivateTmp=yes NoNewPrivileges=yes @@ -100,7 +106,7 @@ SystemCallErrorNumber=EPERM [Install] WantedBy=multi-user.target -`, exePath, configPath, Name) +`, exePath, configPath, Name, bind) } // copyFile copies src to dst, creating dst with the given mode. @@ -125,7 +131,9 @@ func copyFile(src, dst string, mode os.FileMode) error { // sure /etc/gpu-turnstile.env exists (copied from the given config file if // provided), writes the hardened unit, then enables and starts it. With // copyBin=false the current executable location and config path are -// registered as-is instead. Needs root. +// registered as-is instead. When the config sets COMFY_DIR, the unit gets a +// BindPaths= for it so the sandboxed service can reach the managed ComfyUI +// even under /home. Needs root. // // Re-running install converges an existing unit instead of failing: it is // stopped first if active, the installed binary is replaced only when the @@ -186,7 +194,7 @@ func Install(configPath string, copyBin bool, version string) error { cfg = abs } } - rendered := renderUnit(exe, cfg) + rendered := renderUnit(exe, cfg, configuredValue(cfg, "COMFY_DIR")) if old, _ := os.ReadFile(unitPath); string(old) != rendered { if err := os.WriteFile(unitPath, []byte(rendered), 0o644); err != nil { return fmt.Errorf("write %s (run as root): %w", unitPath, err) diff --git a/internal/service/service_linux_test.go b/internal/service/service_linux_test.go index 2ff4de7..3b377d0 100644 --- a/internal/service/service_linux_test.go +++ b/internal/service/service_linux_test.go @@ -8,7 +8,7 @@ import ( ) func TestRenderUnit(t *testing.T) { - unit := renderUnit("/var/lib/gpu-turnstile/gpu-turnstile", "/etc/gpu-turnstile.env") + unit := renderUnit("/var/lib/gpu-turnstile/gpu-turnstile", "/etc/gpu-turnstile.env", "") for _, want := range []string{ "Type=notify", "WatchdogSec=30s", @@ -26,4 +26,12 @@ func TestRenderUnit(t *testing.T) { t.Fatalf("unit missing %q:\n%s", want, unit) } } + if strings.Contains(unit, "BindPaths") { + t.Fatalf("unit without COMFY_DIR must not bind anything:\n%s", unit) + } + + unit = renderUnit("/var/lib/gpu-turnstile/gpu-turnstile", "/etc/gpu-turnstile.env", "/home/gpu/ComfyUI") + if !strings.Contains(unit, "BindPaths=/home/gpu/ComfyUI\n") { + t.Fatalf("unit with COMFY_DIR must bind it:\n%s", unit) + } } diff --git a/internal/service/service_windows.go b/internal/service/service_windows.go index 75368f3..012132f 100644 --- a/internal/service/service_windows.go +++ b/internal/service/service_windows.go @@ -24,8 +24,6 @@ import ( "golang.org/x/sys/windows" "golang.org/x/sys/windows/svc" "golang.org/x/sys/windows/svc/mgr" - - "gpu-turnstile/internal/config" ) // Name is the Windows service name. @@ -106,10 +104,12 @@ func (h *handler) Execute(_ []string, requests <-chan svc.ChangeRequest, status // the service after 5s on failure — this is also what brings up a staged // update after the updater exits with a non-zero code. After registering, // the virtual account is granted modify access to the install and data -// directories (self-updates rewrite the exe), and read access to the -// config file if it lives elsewhere. The grants must come after -// CreateService: the virtual account's SID only exists once the service is -// registered. +// directories (self-updates rewrite the exe), read access to the +// config file if it lives elsewhere, and — when the config sets COMFY_DIR — +// recursive modify access to the managed ComfyUI's install tree, which may +// live inside a user profile the account otherwise cannot enter. The grants +// must come after CreateService: the virtual account's SID only exists once +// the service is registered. // // Re-running install on an existing service converges instead of failing: // the service is stopped first if running (so the binary can be replaced), @@ -354,7 +354,7 @@ func grantAll(exe, configPath string) error { return err } } - if logFile := configuredLogFile(configPath); logFile != "" { + if logFile := configuredValue(configPath, "LOG_FILE"); logFile != "" { dir := filepath.Dir(logFile) if err := os.MkdirAll(dir, 0o755); err == nil { if err := grantAccess(dir, "(OI)(CI)(M)"); err != nil { @@ -362,6 +362,17 @@ func grantAll(exe, configPath string) error { } } } + // A managed ComfyUI whose install lives somewhere the virtual account + // may not go (a user profile) needs an explicit grant — recursively, + // since ComfyUI also writes output/temp/user data next to its code. A + // missing directory is skipped: the startup warning covers it. + if comfyDir := configuredValue(configPath, "COMFY_DIR"); comfyDir != "" { + if _, err := os.Stat(comfyDir); err == nil { + if err := grantAccessTree(comfyDir, "(OI)(CI)(M)"); err != nil { + return err + } + } + } return nil } @@ -473,19 +484,15 @@ func grantAccess(path, perms string) error { return nil } -// configuredLogFile reads LOG_FILE from the config file so the installer -// can pre-create and ACL the log directory. "" when unset or unreadable. -func configuredLogFile(configPath string) string { - f, err := os.Open(configPath) +// grantAccessTree is grantAccess with /T: the ACE is applied to the +// existing tree, not just inherited by children created later. Needed when +// the tree already exists, e.g. a ComfyUI install in a user profile. +func grantAccessTree(path, perms string) error { + out, err := exec.Command("icacls", path, "/grant", virtualAccount+":"+perms, "/T").CombinedOutput() if err != nil { - return "" + return fmt.Errorf("grant %s access to %s: %w (%s)", virtualAccount, path, err, strings.TrimSpace(string(out))) } - defer f.Close() - values, err := config.ParseEnvFile(f) - if err != nil { - return "" - } - return values["LOG_FILE"] + return nil } // RestartIfRunning restarts the service when it is installed and running