From 342163ed1f0df1e01ed8db386005c4dfed7cf930 Mon Sep 17 00:00:00 2001 From: Pas <74743263+Pasithea0@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:58:54 -0600 Subject: [PATCH 1/4] fix: name the device Plex sees, and keep its identity between runs A scheduled run registered a new device with Plex every night, and the "a new device used your server" notification that came with it had empty brackets where the device name should be. The client identifier was generated per process, and Plex treats an identifier it has not seen as a new device, so a nightly timer added a device to the user's device list and notified about it every night. Nothing inside the tool showed it: the run worked, and the only symptom was mail the user could not place. No device name was sent either, which is what Plex names a client from. The identifier is now resolved once and stored in /client-id, so the first run of an install registers a device and every run after it is the same device. Requests carry X-Plex-Device-Name, X-Plex-Device and X-Plex-Platform; plex.device_name (or PLEX_SYNC_DEVICE_NAME, which is what a container should set) is the name Plex shows. The platform is reported from runtime.GOOS rather than assumed. X-Plex-Platform-Version is deliberately not sent: it means the version of the platform, and the only version this tool knows is its own. The stored value is validated before it is used, because it is read from disk and sent as a request header. Writing the configuration does not freeze it, the way a discovered token and database path are not written down either: a config copied to a second machine would otherwise give both the same identity. --- README.md | 16 +++ docs/troubleshooting.md | 20 +++ internal/app/app.go | 9 ++ internal/config/config.go | 24 +++- internal/config/identity.go | 143 +++++++++++++++++++++ internal/config/identity_test.go | 206 +++++++++++++++++++++++++++++++ internal/config/save.go | 11 +- internal/plexapi/client.go | 60 +++++++-- internal/plexapi/client_test.go | 108 ++++++++++++++++ internal/tui/settings.go | 20 ++- 10 files changed, 600 insertions(+), 17 deletions(-) create mode 100644 internal/config/identity.go create mode 100644 internal/config/identity_test.go diff --git a/README.md b/README.md index 3b2654a..41b3faa 100644 --- a/README.md +++ b/README.md @@ -165,6 +165,7 @@ docker run -d --restart=unless-stopped \ --name plex-sync \ -e PLEX_URL=http://plex:32400 \ -e PLEX_TOKEN=xxxxxxxxxxxx \ + -e PLEX_SYNC_DEVICE_NAME="plex-sync (media-nas)" \ -v "/mnt/cache/appdata/plex/Library/Application Support/Plex Media Server:/plex:ro" \ -v "/mnt/cache/appdata/plex-sync:/state" \ ghcr.io/theintrodb/plex-sync:latest schedule --yes @@ -174,6 +175,11 @@ The image is published to the GitHub Container Registry as `ghcr.io/theintrodb/plex-sync`, tagged with each release version and with `latest`. +`PLEX_SYNC_DEVICE_NAME` is optional and is what Plex shows in its device list, +so the name you recognise is the one in the notification. The state volume has +to persist either way: it holds the identity Plex knows this container by, and a +container that loses it registers as a new device again. + The Plex database must be mounted at its real, non-FUSE path. On Unraid that means the `/mnt/cache/...` path, never `/mnt/user/...`: SQLite locking through the shfs layer is not reliable and a write can corrupt the database. The tool @@ -359,6 +365,8 @@ Lookup order: `$PLEX_SYNC_CONFIG`, `./plex-sync.toml`, | `plex.token` | | Plex token, for the HTTP API | | `plex.database` | | path to `com.plexapp.plugins.library.db` | | `plex.config_dir` | | Plex application-support directory, if the database path is not given | +| `plex.device_name` | `plex-sync` | what Plex shows for this tool in its device list | +| `plex.client_id` | stored in `state_dir` | the identity Plex keys this install's device entry on | | `theintrodb.api_key` | | optional TheIntroDB API key | | `theintrodb.daily_budget` | `1000` | requests per UTC day | | `sources.chapters` | `false` | use chapter names as a source | @@ -373,9 +381,17 @@ Lookup order: `$PLEX_SYNC_CONFIG`, `./plex-sync.toml`, | `state_dir` | `~/.config/plex-sync` | ledger, backups and undo journals | Environment variables: `PLEX_URL`, `PLEX_TOKEN`, `PLEX_DB`, `PLEX_CONFIG_DIR`, +`PLEX_SYNC_CLIENT_ID`, `PLEX_SYNC_DEVICE_NAME`, `TIDB_API_KEY`, `TIDB_API_URL`, `PLEX_SYNC_STATE_DIR`, `PLEX_SYNC_LOG_LEVEL`, `PLEX_SYNC_CHAPTERS`, `PLEX_SYNC_DETECTION`, `PLEX_SYNC_ALLOW_LIVE`. +Plex identifies a caller by an identifier it sends on every request, and treats +one it has not seen as a new device — so the identifier is stored in the state +directory and reused, and the first run of an install is the only one that +registers a device. Set `plex.device_name` (or `PLEX_SYNC_DEVICE_NAME`) to +whatever you recognise in Plex's device list, because that is the name Plex will +show and notify with; a container should set it to its own name. + --- ## Sources diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 5feea07..20a0943 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -67,6 +67,26 @@ The other causes are ordinary: no read access to the database, or episodes whose show rows carry no provider ids. Check what `plex-sync library` prints for the ids an item would be looked up with. +## A "new device used your server" notification every run + +Expected once, not nightly. Plex identifies a caller by the identifier it sends, +and one it has not seen before is a new device, which is what it notifies about. + +The identifier is stored in `/client-id` and reused, so a fresh +install registers one device and keeps it. Every run looking like a new device +means that file is not surviving: most often the state directory is inside a +container that is recreated without a volume, or it is somewhere that is wiped. +Keep `state_dir` — the same directory the ledger lives in — on something +persistent. + +If the notification has empty brackets where the device name should be, the name +is not set. `plex.device_name`, or `PLEX_SYNC_DEVICE_NAME` for a container, is +what Plex shows; it defaults to `plex-sync`. + +The device list in Plex is not cleaned up by any of this. Runs from before the +identifier was stored each left an entry of their own, and those stay until they +are removed by hand. + ## "no-provider-id" The item has no TMDb, IMDb or Tvdb id, so there is nothing to look it up by. diff --git a/internal/app/app.go b/internal/app/app.go index acfb6df..fe71b36 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -81,6 +81,15 @@ func Open(cfg *config.Config, log *slog.Logger, opts Options) (*App, error) { } } + // The identifier Plex keys this install's device entry on is kept here, once, + // so that a nightly run is the same device to Plex every night. It is not + // fatal when it cannot be stored: the tool still works, it just registers as + // a new device, which is the thing this exists to stop. + if err := cfg.ResolvePlexIdentity(); err != nil { + log.Warn("could not keep a durable Plex device identity, so Plex will see each run as a new device", + "error", err) + } + application := &App{ Cfg: cfg, Log: log, diff --git a/internal/config/config.go b/internal/config/config.go index 387da9c..ed4fae3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -72,6 +72,16 @@ type Plex struct { AllowFusePath bool `toml:"allow_fuse_path"` // InsecureSkipVerify is only for a self-signed local Plex. InsecureSkipVerify bool `toml:"insecure_skip_verify"` + // ClientID is what Plex keys this install's device entry on: an identifier + // it has not seen before is a new device, which is what a "used a new device + // to access your server" notification is sent for. Empty uses the identifier + // stored in the state directory, created on first run. Set it only when the + // state directory is not durable, or to give several installs one identity. + ClientID string `toml:"client_id"` + // DeviceName is what Plex calls this tool in its device list and in that + // notification. Empty means "plex-sync"; PLEX_SYNC_DEVICE_NAME overrides + // both, which is what a container should set. + DeviceName string `toml:"device_name"` } // TheIntroDB configures the API client. @@ -616,6 +626,8 @@ func (c *Config) applyEnv() { str("PLEX_CONFIG_DIR", &c.Plex.ConfigDir) boolean("PLEX_INSECURE", &c.Plex.InsecureSkipVerify) boolean("PLEX_ALLOW_FUSE_PATH", &c.Plex.AllowFusePath) + str(EnvClientID, &c.Plex.ClientID) + str(EnvDeviceName, &c.Plex.DeviceName) str("TIDB_API_KEY", &c.TheIntroDB.APIKey) str("TIDB_API_URL", &c.TheIntroDB.BaseURL) @@ -818,7 +830,8 @@ func exampleConfig(stateDir string) string { return `# plex-sync configuration. # Every value here is optional; the defaults shown are the built-in ones. # Environment variables override this file (PLEX_URL, PLEX_TOKEN, PLEX_DB, -# PLEX_CONFIG_DIR, TIDB_API_KEY, TIDB_API_URL, PLEX_SYNC_STATE_DIR). +# PLEX_CONFIG_DIR, PLEX_SYNC_CLIENT_ID, PLEX_SYNC_DEVICE_NAME, +# TIDB_API_KEY, TIDB_API_URL, PLEX_SYNC_STATE_DIR). # Top-level keys must come before the first section header, or TOML reads them # as part of that section. @@ -837,6 +850,15 @@ token = "" database = "" # Alternatively, the Plex application-support directory. config_dir = "" +# What Plex shows for this tool in its device list, and in the "a new device +# used your server" notification. Empty means "plex-sync". A container should +# set this (or PLEX_SYNC_DEVICE_NAME) to the name it is known by, because that +# is the name that will appear. +device_name = "" +# The identifier Plex keys this install's device entry on, so it does not treat +# every run as a new device. Empty uses the one stored in the state directory, +# which is created on first run; only set it when that directory is not durable. +client_id = "" [theintrodb] # The TheIntroDB API key is optional. With a key the daily allowance is higher diff --git a/internal/config/identity.go b/internal/config/identity.go new file mode 100644 index 0000000..27d04de --- /dev/null +++ b/internal/config/identity.go @@ -0,0 +1,143 @@ +package config + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "time" +) + +// Plex device identity: the client identifier Plex keys this install's device +// entry on, and the name it shows for it. +// +// Both exist because of one report. A scheduled run produced a "used a new +// device to access your server" notification every night, with nothing in the +// brackets where the device name goes. The identifier was generated per process, +// so Plex saw a new device on every run; no device name was sent at all, so its +// notification had nothing to print. Neither is visible from inside Plex: the +// tool worked, and the only symptom was mail the user could not place. +const ( + // EnvClientID overrides the stored client identifier. + EnvClientID = "PLEX_SYNC_CLIENT_ID" + // EnvDeviceName overrides the name Plex shows for this tool. + EnvDeviceName = "PLEX_SYNC_DEVICE_NAME" + // ClientIDFile is the stored identifier inside the state directory. + ClientIDFile = "client-id" + // DefaultDeviceName is what Plex shows when nothing names this tool. + DefaultDeviceName = "plex-sync" + // clientIDPrefix marks a generated identifier as this tool's. + clientIDPrefix = "plex-sync-" + // maxClientIDLen bounds a stored identifier. Plex does not publish a limit; + // this one is small enough to be obviously sane and far larger than the + // identifiers real clients send. + maxClientIDLen = 128 +) + +// ResolvedDeviceName is the name Plex shows for this tool: the environment, then +// the file, then "plex-sync". +// +// The environment is read first because a container's name is the one thing it +// should be told rather than discover: its state directory is often all it has, +// and the name a person recognises in a Plex device list is usually the container +// or host they put it on. +func (p Plex) ResolvedDeviceName() string { + if v := strings.TrimSpace(os.Getenv(EnvDeviceName)); v != "" { + return v + } + if v := strings.TrimSpace(p.DeviceName); v != "" { + return v + } + return DefaultDeviceName +} + +// ResolvePlexIdentity fills in the client identifier when nothing has chosen one, +// storing a generated one in the state directory. +// +// A stored identifier is the whole point: Plex treats an identifier it has not +// seen as a new device, so one that changes between runs turns a nightly timer +// into a nightly notification, and fills the device list with a device per run. +// Keeping it in the state directory rather than in the configuration file is +// deliberate -- it belongs to the install, not to a person, so it follows the +// state and not a config that gets copied between machines. +func (c *Config) ResolvePlexIdentity() error { + if v := strings.TrimSpace(os.Getenv(EnvClientID)); v != "" { + c.Plex.ClientID = v + return nil + } + if v := strings.TrimSpace(c.Plex.ClientID); v != "" { + return nil + } + if stored := c.StoredClientID(); stored != "" { + c.Plex.ClientID = stored + return nil + } + + id := NewClientID() + if err := os.MkdirAll(c.StateDir, 0o755); err != nil { + return fmt.Errorf("create state directory %s: %w", c.StateDir, err) + } + // 0600: the identifier is not a secret, but it names this install to a + // server, and nothing else on the machine has any business rewriting it. + if err := os.WriteFile(c.ClientIDPath(), []byte(id+"\n"), 0o600); err != nil { + return fmt.Errorf("write %s: %w", c.ClientIDPath(), err) + } + c.Plex.ClientID = id + return nil +} + +// ClientIDPath is where the identifier is stored. +func (c *Config) ClientIDPath() string { + return filepath.Join(c.StateDir, ClientIDFile) +} + +// StoredClientID returns the identifier already stored for this state directory, +// or "" when there is not a usable one. +// +// The file is read from disk and its contents end up in a request header, so what +// is there is validated rather than trusted. A file that was truncated, edited +// or written as something else entirely must be replaced rather than sent. +func (c *Config) StoredClientID() string { + raw, err := os.ReadFile(c.ClientIDPath()) + if err != nil { + return "" + } + return usableClientID(string(raw)) +} + +// NewClientID returns a fresh identifier in the shape Plex expects: its own +// prefix, so an operator reading Plex's device list can tell what the device is +// even before its name says so. +func NewClientID() string { + var buf [16]byte + if _, err := rand.Read(buf[:]); err != nil { + // crypto/rand does not fail on any platform this ships to. The + // fallback is not a good identifier, but a run is not worth failing + // over one, and a timestamp is still better than nothing at all. + return clientIDPrefix + strconv.FormatInt(time.Now().UnixNano(), 16) + } + return clientIDPrefix + hex.EncodeToString(buf[:]) +} + +// usableClientID returns the identifier held in a file's contents, or "" when +// what is there is not one. +func usableClientID(raw string) string { + id := strings.TrimSpace(raw) + if id == "" || len(id) > maxClientIDLen { + return "" + } + for _, r := range id { + switch { + case r >= 'a' && r <= 'z', + r >= 'A' && r <= 'Z', + r >= '0' && r <= '9', + r == '-', r == '_', r == '.': + default: + return "" + } + } + return id +} diff --git a/internal/config/identity_test.go b/internal/config/identity_test.go new file mode 100644 index 0000000..71cc423 --- /dev/null +++ b/internal/config/identity_test.go @@ -0,0 +1,206 @@ +package config + +import ( + "os" + "strings" + "testing" +) + +// identityConfig is a configuration whose state directory is a temporary one. +// +// Every test here resolves an identity, and resolution writes a file. Using +// Default() alone would put that file in the real per-user state directory of +// whoever runs the suite. +func identityConfig(t *testing.T) *Config { + t.Helper() + cfg := Default() + cfg.StateDir = t.TempDir() + // Both variables are cleared rather than left to the environment, because a + // developer with PLEX_SYNC_DEVICE_NAME exported would otherwise be testing + // their own shell. + t.Setenv(EnvClientID, "") + t.Setenv(EnvDeviceName, "") + return cfg +} + +func TestDeviceNameResolution(t *testing.T) { + t.Run("defaults to the product name", func(t *testing.T) { + cfg := identityConfig(t) + if got := cfg.Plex.ResolvedDeviceName(); got != DefaultDeviceName { + t.Errorf("ResolvedDeviceName = %q, want %q", got, DefaultDeviceName) + } + }) + + t.Run("the setting wins over the default", func(t *testing.T) { + cfg := identityConfig(t) + cfg.Plex.DeviceName = "plex-sync (media-nas)" + if got := cfg.Plex.ResolvedDeviceName(); got != "plex-sync (media-nas)" { + t.Errorf("ResolvedDeviceName = %q, want the configured name", got) + } + }) + + t.Run("the environment wins over the setting", func(t *testing.T) { + cfg := identityConfig(t) + cfg.Plex.DeviceName = "from-the-file" + t.Setenv(EnvDeviceName, " from-the-container ") + if got := cfg.Plex.ResolvedDeviceName(); got != "from-the-container" { + t.Errorf("ResolvedDeviceName = %q, want the environment's name, trimmed", got) + } + }) + + t.Run("an empty setting is not an empty name", func(t *testing.T) { + cfg := identityConfig(t) + cfg.Plex.DeviceName = " " + if got := cfg.Plex.ResolvedDeviceName(); got != DefaultDeviceName { + t.Errorf("ResolvedDeviceName = %q, want %q: Plex always has a name to print", + got, DefaultDeviceName) + } + }) +} + +// The point of the stored identifier: Plex treats one it has not seen as a new +// device, so a nightly run must present the same one every night. +func TestResolvePlexIdentityKeepsOneIdentifier(t *testing.T) { + cfg := identityConfig(t) + + if err := cfg.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity: %v", err) + } + first := cfg.Plex.ClientID + if first == "" { + t.Fatal("no identifier was resolved") + } + if !strings.HasPrefix(first, "plex-sync-") { + t.Errorf("identifier = %q, want the plex-sync- prefix so Plex's device list is readable", first) + } + + raw, err := os.ReadFile(cfg.ClientIDPath()) + if err != nil { + t.Fatalf("the identifier was not stored: %v", err) + } + if got := strings.TrimSpace(string(raw)); got != first { + t.Errorf("stored identifier = %q, want %q", got, first) + } + info, err := os.Stat(cfg.ClientIDPath()) + if err != nil { + t.Fatalf("stat: %v", err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("stored identifier mode = %v, want 0600", perm) + } + + // A second run, as a scheduled job would be, over the same state. + second := Default() + second.StateDir = cfg.StateDir + if err := second.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity (second run): %v", err) + } + if second.Plex.ClientID != first { + t.Errorf("second run used %q, want the stored %q: a new identifier is a new device to Plex", + second.Plex.ClientID, first) + } +} + +func TestResolvePlexIdentityPrefersTheEnvironment(t *testing.T) { + cfg := identityConfig(t) + t.Setenv(EnvClientID, "plex-sync-from-the-container") + + if err := cfg.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity: %v", err) + } + if cfg.Plex.ClientID != "plex-sync-from-the-container" { + t.Errorf("identifier = %q, want the environment's", cfg.Plex.ClientID) + } + // An identity somebody chose is not this tool's to store. + if _, err := os.Stat(cfg.ClientIDPath()); !os.IsNotExist(err) { + t.Errorf("a file was written for an identifier that came from the environment (stat err = %v)", err) + } +} + +func TestResolvePlexIdentityLeavesAConfiguredIdentifierAlone(t *testing.T) { + cfg := identityConfig(t) + cfg.Plex.ClientID = "chosen-in-the-file" + + if err := cfg.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity: %v", err) + } + if cfg.Plex.ClientID != "chosen-in-the-file" { + t.Errorf("identifier = %q, want the configured one", cfg.Plex.ClientID) + } + if _, err := os.Stat(cfg.ClientIDPath()); !os.IsNotExist(err) { + t.Errorf("a file was written over a configured identifier (stat err = %v)", err) + } +} + +// The stored value is read from disk and sent as a request header, so what is +// there is validated rather than trusted. +func TestResolvePlexIdentityReplacesAnUnusableFile(t *testing.T) { + for name, contents := range map[string]string{ + "empty": "", + "whitespace": " \n\t\n", + "spaces inside": "not an identifier", + "a header break": "plex-sync-ok\r\nX-Plex-Token: injected", + "far too long": "plex-sync-" + strings.Repeat("a", maxClientIDLen), + "a quoted value": `"plex-sync-abc"`, + } { + t.Run(name, func(t *testing.T) { + cfg := identityConfig(t) + if err := os.WriteFile(cfg.ClientIDPath(), []byte(contents), 0o600); err != nil { + t.Fatalf("write the file: %v", err) + } + + if err := cfg.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity: %v", err) + } + if cfg.Plex.ClientID == "" { + t.Fatal("nothing was resolved") + } + if strings.ContainsAny(cfg.Plex.ClientID, " \r\n\t\"") { + t.Errorf("identifier = %q, which is not usable as a header value", cfg.Plex.ClientID) + } + // And the bad value is gone rather than left for the next run. + raw, err := os.ReadFile(cfg.ClientIDPath()) + if err != nil { + t.Fatalf("read back: %v", err) + } + if got := strings.TrimSpace(string(raw)); got != cfg.Plex.ClientID { + t.Errorf("stored identifier = %q, want the replacement %q", got, cfg.Plex.ClientID) + } + }) + } +} + +// A usable stored value is used as it is: an operator who put one there meant it. +func TestResolvePlexIdentityAcceptsAStoredValue(t *testing.T) { + cfg := identityConfig(t) + if err := os.WriteFile(cfg.ClientIDPath(), []byte("plex-sync-hand-written\n"), 0o600); err != nil { + t.Fatalf("write the file: %v", err) + } + + if err := cfg.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity: %v", err) + } + if cfg.Plex.ClientID != "plex-sync-hand-written" { + t.Errorf("identifier = %q, want the stored one", cfg.Plex.ClientID) + } +} + +// Writing the configuration down must not freeze the identifier: it belongs to +// the install, and a file copied to a second machine would hand both the same +// identity, which Plex would show as one device. +func TestSaveDoesNotFreezeTheResolvedIdentifier(t *testing.T) { + cfg := identityConfig(t) + if err := cfg.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity: %v", err) + } + if got := cfg.forWriting().Plex.ClientID; got != "" { + t.Errorf("the resolved identifier was written to the file: %q", got) + } + + // One that was chosen is kept, because then it was chosen. + chosen := identityConfig(t) + chosen.Plex.ClientID = "chosen-by-hand" + if got := chosen.forWriting().Plex.ClientID; got != "chosen-by-hand" { + t.Errorf("a configured identifier was dropped on write: %q", got) + } +} diff --git a/internal/config/save.go b/internal/config/save.go index 6d23f74..a0e21ff 100644 --- a/internal/config/save.go +++ b/internal/config/save.go @@ -21,7 +21,8 @@ const header = `# plex-sync configuration. # # Precedence, lowest to highest: these values, the environment, then flags. # Environment variables that override the file: PLEX_URL, PLEX_TOKEN, PLEX_DB, -# PLEX_CONFIG_DIR, TIDB_API_KEY, TIDB_API_URL, TIDB_DAILY_BUDGET, +# PLEX_CONFIG_DIR, PLEX_SYNC_CLIENT_ID, PLEX_SYNC_DEVICE_NAME, +# TIDB_API_KEY, TIDB_API_URL, TIDB_DAILY_BUDGET, # PLEX_SYNC_STATE_DIR, PLEX_SYNC_LOG_LEVEL, PLEX_SYNC_SCHEDULE, # PLEX_SYNC_CHAPTERS, PLEX_SYNC_DETECTION, PLEX_SYNC_ALLOW_LIVE, # PLEX_SYNC_API_ADDR, PLEX_SYNC_API_ENABLED. @@ -57,6 +58,14 @@ func (c *Config) forWriting() Config { out.Plex.Database = "" } } + // The client identifier is resolved the same way, and for the same reason. + // It belongs to the install rather than to a person, so writing it down + // freezes the one thing that is supposed to follow the state directory -- + // and a configuration file copied to a second machine would hand both the + // same identity, which Plex would then show as a single device. + if id := out.Plex.ClientID; id != "" && out.StoredClientID() == id { + out.Plex.ClientID = "" + } return out } diff --git a/internal/plexapi/client.go b/internal/plexapi/client.go index c162a2b..f8586e0 100644 --- a/internal/plexapi/client.go +++ b/internal/plexapi/client.go @@ -13,11 +13,10 @@ package plexapi import ( "context" - "crypto/rand" - "encoding/hex" "errors" "fmt" "net/url" + "runtime" "strconv" "strings" "time" @@ -86,6 +85,7 @@ type Client struct { cfg config.Plex hc *httpclient.Client identifier string + deviceName string owned bool } @@ -113,7 +113,22 @@ func NewClient(cfg config.Plex, hc *httpclient.Client) *Client { hc = httpclient.New(timeout, cfg.InsecureSkipVerify, UserAgent) owned = true } - return &Client{cfg: cfg, hc: hc, identifier: clientIdentifier(), owned: owned} + + // The identifier Plex keys a device entry on. app.Open resolves the stored + // one; a client built directly -- by a test, or by a caller that has its own + // -- has not, and gets a fresh one, which is only correct for a process that + // runs once. + id := strings.TrimSpace(cfg.ClientID) + if id == "" { + id = config.NewClientID() + } + return &Client{ + cfg: cfg, + hc: hc, + identifier: id, + deviceName: cfg.ResolvedDeviceName(), + owned: owned, + } } // Close releases the connection pool, but only when this client built it. @@ -126,17 +141,12 @@ func (c *Client) Close() { // URL returns the base URL this client talks to. func (c *Client) URL() string { return c.cfg.URL } -// clientIdentifier is a stable-per-process X-Plex-Client-Identifier. Plex keys -// its token and activity records on it, so it must not change per request. -func clientIdentifier() string { - var buf [8]byte - if _, err := rand.Read(buf[:]); err == nil { - return Product + "-" + hex.EncodeToString(buf[:]) - } - return Product + "-" + strconv.FormatInt(time.Now().UnixNano(), 16) -} - // headers returns the headers every Plex request needs. +// +// The device headers are what Plex names this client with, in its device list +// and in its "a new device used your server" notification. Without them that +// notification arrives with empty brackets where the name goes, which is what it +// did: identifiable to nobody, on every run. func (c *Client) headers() map[string]string { return map[string]string{ "X-Plex-Token": c.cfg.Token, @@ -144,6 +154,30 @@ func (c *Client) headers() map[string]string { "X-Plex-Client-Identifier": c.identifier, "X-Plex-Product": Product, "X-Plex-Version": Version, + "X-Plex-Device-Name": c.deviceName, + "X-Plex-Device": deviceClass(), + "X-Plex-Platform": deviceClass(), + // X-Plex-Platform-Version is deliberately not sent. It means the + // version of the platform, and the only version this tool knows is its + // own, which X-Plex-Version already carries. + } +} + +// deviceClass is the platform Plex files this client under. It is reported +// rather than assumed, so a build on macOS does not tell the server it is a +// Linux one. +func deviceClass() string { + switch runtime.GOOS { + case "darwin": + return "macOS" + case "windows": + return "Windows" + case "linux": + return "Linux" + case "freebsd": + return "FreeBSD" + default: + return runtime.GOOS } } diff --git a/internal/plexapi/client_test.go b/internal/plexapi/client_test.go index 55484ea..0e8f508 100644 --- a/internal/plexapi/client_test.go +++ b/internal/plexapi/client_test.go @@ -718,6 +718,114 @@ func page(start, end, total int, withTotal bool) string { return sb.String() } +// TestDeviceIdentityHeaders covers what Plex names this client with. +// +// Without these headers its "a new device used your server" notification arrives +// with empty brackets where the name goes, which is what a user reported. +func TestDeviceIdentityHeaders(t *testing.T) { + // Not parallel: this reads the process environment, which another test in + // this package sets. + t.Setenv(config.EnvDeviceName, "") + t.Setenv(config.EnvClientID, "") + + var ( + mu sync.Mutex + device = map[string]string{} + seen bool + ) + handler := func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + seen = true + for _, h := range []string{ + "X-Plex-Client-Identifier", "X-Plex-Device-Name", + "X-Plex-Device", "X-Plex-Platform", "X-Plex-Product", + } { + device[h] = r.Header.Get(h) + } + mu.Unlock() + writeJSON(w, http.StatusOK, `{"MediaContainer":{"machineIdentifier":"machine-1","version":"1.43.4"}}`) + } + + f := newFake(t, handler) + hc := httpclient.New(5*time.Second, false, UserAgent) + t.Cleanup(hc.Close) + c := NewClient(config.Plex{ + URL: f.srv.URL, + Token: fakeToken, + ClientID: "plex-sync-0123456789abcdef", + DeviceName: "plex-sync (media-nas)", + }, hc) + + if _, err := c.Identity(context.Background()); err != nil { + t.Fatalf("Identity: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if !seen { + t.Fatal("no request was seen") + } + for header, want := range map[string]string{ + "X-Plex-Client-Identifier": "plex-sync-0123456789abcdef", + "X-Plex-Device-Name": "plex-sync (media-nas)", + "X-Plex-Product": Product, + } { + if device[header] != want { + t.Errorf("%s = %q, want %q", header, device[header], want) + } + } + if device["X-Plex-Platform"] == "" { + t.Error("X-Plex-Platform is empty: Plex has nothing to file this client under") + } + if device["X-Plex-Device"] != device["X-Plex-Platform"] { + t.Errorf("X-Plex-Device = %q and X-Plex-Platform = %q; both describe the same host", + device["X-Plex-Device"], device["X-Plex-Platform"]) + } +} + +// A client built without a resolved identity is still identifiable: the name has +// a default, and an identifier is generated rather than left empty. Only +// app.Open resolves the stored one, so a caller that builds its own client must +// not end up sending neither. +func TestDeviceIdentityHasDefaults(t *testing.T) { + // Not parallel, for the same reason as above. + t.Setenv(config.EnvDeviceName, "") + t.Setenv(config.EnvClientID, "") + + var ( + mu sync.Mutex + device = map[string]string{} + any bool + ) + handler := func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + any = true + device["name"] = r.Header.Get("X-Plex-Device-Name") + device["id"] = r.Header.Get("X-Plex-Client-Identifier") + mu.Unlock() + writeJSON(w, http.StatusOK, `{"MediaContainer":{"machineIdentifier":"m","version":"1.43.4"}}`) + } + + f := newFake(t, handler) + c := f.client(t) + + if _, err := c.Identity(context.Background()); err != nil { + t.Fatalf("Identity: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if !any { + t.Fatal("no request was seen") + } + if device["name"] != config.DefaultDeviceName { + t.Errorf("X-Plex-Device-Name = %q, want %q", device["name"], config.DefaultDeviceName) + } + if device["id"] == "" { + t.Error("X-Plex-Client-Identifier is empty") + } +} + func TestChapters(t *testing.T) { t.Parallel() f := newFake(t, plexHandler) diff --git a/internal/tui/settings.go b/internal/tui/settings.go index 12836b1..1e0db8c 100644 --- a/internal/tui/settings.go +++ b/internal/tui/settings.go @@ -119,8 +119,11 @@ func (s setting) text(cfg *config.Config) string { // Saying which is which matters more than it sounds: "saved" on its own invites // someone to change the API key, see nothing happen, and conclude it is broken. var restartKeys = map[string]bool{ - "plex.url": true, - "plex.token": true, + "plex.url": true, + "plex.token": true, + // The name goes out on every request, but the Plex client resolved the one + // it will send when it was built, so the change is the next run's. + "plex.device_name": true, "theintrodb.api_key": true, "theintrodb.daily_budget": true, "log_level": true, @@ -200,6 +203,19 @@ func settingsRows() []setting { }, }, + { + section: "Plex", key: "plex.device_name", label: "device name", kind: settingText, + help: "What Plex shows for this tool in its device list and in its \"new device\" notification. Empty means \"plex-sync\".", + get: func(c *config.Config) string { return c.Plex.DeviceName }, + set: func(c *config.Config, v string) error { c.Plex.DeviceName = strings.TrimSpace(v); return nil }, + fallback: func(c *config.Config) string { + // Plex always has a name to print -- the environment's, this + // setting's, or the built-in one -- so "(empty)" would be a lie + // in the one row whose job is to say what Plex will call this. + return "in use: " + c.Plex.ResolvedDeviceName() + }, + }, + { section: "TheIntroDB", key: "theintrodb.api_key", label: "API key", kind: settingSecret, help: "Optional. A key raises the daily allowance and is required to submit timings.", From 30595d349a3f98901e3a33bf5c921786916474d3 Mon Sep 17 00:00:00 2001 From: Pas <74743263+Pasithea0@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:58:56 -0600 Subject: [PATCH 2/4] docs: a container package published to ghcr is private by default --- .github/workflows/release.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0fd4191..7d388f0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -82,6 +82,18 @@ jobs: # runner needs QEMU for the non-native architecture and a buildx builder. # Logging in with the built-in token is what lets the image land on ghcr.io # under the repo's own namespace, with no extra registry secret to manage. + # + # A package published here is PRIVATE, whatever the repository's visibility + # is, and the first release after this step was added produced an image + # nobody could pull: an anonymous request answered 403. Making it public is + # a one-time setting on the package itself -- + # https://github.com/orgs/TheIntroDB/packages/container/plex-sync/settings + # -- and it cannot be done from here. This token can read the visibility + # but a PATCH of it answers 404, because changing it needs a user token + # with write:packages and an organisation admin. Visibility is a property + # of the package rather than of a version, so doing it once covers every + # release after; check it after the first publish, and after any change to + # the image name below, which would create a new package. - name: Set up QEMU uses: docker/setup-qemu-action@v3 From 59a76728dbbc10833f574a7ba543793db329a001 Mon Sep 17 00:00:00 2001 From: Pas <74743263+Pasithea0@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:10:36 -0600 Subject: [PATCH 3/4] test: the stored identifier's mode is not a Windows concept The permission assertion failed there, as it should: Windows has no POSIX modes, so os.WriteFile's 0600 only toggles the read-only attribute and the file gets whatever the directory's ACLs give it. save_test.go already skips its own mode check for this reason; this guards just the assertion rather than the whole test, because the identifier's stability -- the thing the test exists for -- is not platform specific. --- internal/config/identity_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/config/identity_test.go b/internal/config/identity_test.go index 71cc423..3ddee4d 100644 --- a/internal/config/identity_test.go +++ b/internal/config/identity_test.go @@ -2,6 +2,7 @@ package config import ( "os" + "runtime" "strings" "testing" ) @@ -85,8 +86,16 @@ func TestResolvePlexIdentityKeepsOneIdentifier(t *testing.T) { if err != nil { t.Fatalf("stat: %v", err) } - if perm := info.Mode().Perm(); perm != 0o600 { - t.Errorf("stored identifier mode = %v, want 0600", perm) + if runtime.GOOS != "windows" { + // Windows has no POSIX modes: os.WriteFile's 0600 there only toggles + // the read-only attribute, and a file created in the user's own + // profile is already limited by the directory's ACLs. There is nothing + // portable to assert, which is the same reason the saved configuration + // skips this. The identifier's stability, below, is not platform + // specific and is asserted everywhere. + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("stored identifier mode = %v, want 0600", perm) + } } // A second run, as a scheduled job would be, over the same state. From 8ebc1fbe6327b8be1d4ab543ff0786a3bcde8ec3 Mon Sep 17 00:00:00 2001 From: Pas <74743263+Pasithea0@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:20:39 -0600 Subject: [PATCH 4/4] fix: two processes must not disagree about the install's identity Review findings on the identity change, both real. Two processes starting against the same state directory before client-id exists could each read an empty result, generate an identifier, and write: both keep their own in memory, so one install presents two devices to Plex, and the file holds whichever finished last. The file is now created with O_EXCL, so the one that creates it wins and the other adopts what it wrote -- waiting briefly, because the file exists before it is written -- and a file that is there but holds nothing usable is replaced rather than left to make every run wait. Writing the configuration cleared the identifier by comparing it with the stored value, which discarded an override that happened to match: an operator who set plex.client_id to the stored identifier lost it on save, and a later change of state directory then generated a new identity instead of honouring it. Whether resolution supplied the value is now recorded, and only that is cleared. The concurrency test fails without the exclusive create -- eight processes, eight identifiers -- and passes with it. --- .github/workflows/release.yml | 14 +++--- internal/config/config.go | 7 +++ internal/config/identity.go | 85 ++++++++++++++++++++++++++++++-- internal/config/identity_test.go | 69 ++++++++++++++++++++++++++ internal/config/save.go | 6 ++- 5 files changed, 169 insertions(+), 12 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7d388f0..42b9960 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -86,14 +86,14 @@ jobs: # A package published here is PRIVATE, whatever the repository's visibility # is, and the first release after this step was added produced an image # nobody could pull: an anonymous request answered 403. Making it public is - # a one-time setting on the package itself -- + # a one-time change made by hand on the package itself -- # https://github.com/orgs/TheIntroDB/packages/container/plex-sync/settings - # -- and it cannot be done from here. This token can read the visibility - # but a PATCH of it answers 404, because changing it needs a user token - # with write:packages and an organisation admin. Visibility is a property - # of the package rather than of a version, so doing it once covers every - # release after; check it after the first publish, and after any change to - # the image name below, which would create a new package. + # -- because the workflow token cannot make it. Measured against the + # package as published: this token reads the visibility fine, and a PATCH + # of it answers 404. Visibility belongs to the package rather than to a + # version, so doing it once covers every release after; check it after the + # first publish, and after any change to the image name below, which would + # create a new package. - name: Set up QEMU uses: docker/setup-qemu-action@v3 diff --git a/internal/config/config.go b/internal/config/config.go index ed4fae3..5ce9530 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -55,6 +55,13 @@ type Config struct { // Path is where the config was loaded from, empty for defaults. Path string `toml:"-"` + // PlexClientIDResolved records that ResolvePlexIdentity supplied + // Plex.ClientID from the state directory rather than finding it set by a + // person. It exists for one decision: a resolved identifier is left out of + // the file when the configuration is written, and one that was chosen is + // kept even when it happens to equal what is stored. Comparing the two + // values instead would quietly discard an override that was deliberate. + PlexClientIDResolved bool `toml:"-"` } // Plex configures how to reach Plex and where its database lives. diff --git a/internal/config/identity.go b/internal/config/identity.go index 27d04de..d6bedc1 100644 --- a/internal/config/identity.go +++ b/internal/config/identity.go @@ -3,6 +3,7 @@ package config import ( "crypto/rand" "encoding/hex" + "errors" "fmt" "os" "path/filepath" @@ -35,6 +36,14 @@ const ( // this one is small enough to be obviously sane and far larger than the // identifiers real clients send. maxClientIDLen = 128 + + // clientIDWaitAttempts and clientIDWaitInterval bound the wait for another + // process to finish writing an identifier it has created but not yet + // written. Two processes racing here is a startup collision, not a + // workload, so the window is a fraction of a second and the wait is over + // rather than indefinite. + clientIDWaitAttempts = 50 + clientIDWaitInterval = 2 * time.Millisecond ) // ResolvedDeviceName is the name Plex shows for this tool: the environment, then @@ -73,6 +82,7 @@ func (c *Config) ResolvePlexIdentity() error { } if stored := c.StoredClientID(); stored != "" { c.Plex.ClientID = stored + c.PlexClientIDResolved = true return nil } @@ -80,12 +90,79 @@ func (c *Config) ResolvePlexIdentity() error { if err := os.MkdirAll(c.StateDir, 0o755); err != nil { return fmt.Errorf("create state directory %s: %w", c.StateDir, err) } - // 0600: the identifier is not a secret, but it names this install to a - // server, and nothing else on the machine has any business rewriting it. - if err := os.WriteFile(c.ClientIDPath(), []byte(id+"\n"), 0o600); err != nil { + // O_EXCL, so that two processes starting against the same state directory + // agree on one identifier: the one that creates the file wins, and the other + // adopts what it wrote. Without it both generate one, both write, each keeps + // its own in memory -- two devices to Plex from one install -- and the file + // holds whichever finished last. + f, err := os.OpenFile(c.ClientIDPath(), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + switch { + case err == nil: + _, writeErr := f.WriteString(id + "\n") + closeErr := f.Close() + if writeErr != nil { + return fmt.Errorf("write %s: %w", c.ClientIDPath(), writeErr) + } + if closeErr != nil { + return fmt.Errorf("write %s: %w", c.ClientIDPath(), closeErr) + } + c.Plex.ClientID = id + c.PlexClientIDResolved = true + return nil + case errors.Is(err, os.ErrExist): + // Another process created it between the read above and this open, so + // its identifier is the install's and ours is not. The file is created + // before it is written, so an immediate read can catch it still empty; + // waiting for it is bounded, and running without one is worse than + // waiting a moment. + for attempt := 0; attempt < clientIDWaitAttempts; attempt++ { + if stored := c.StoredClientID(); stored != "" { + c.Plex.ClientID = stored + c.PlexClientIDResolved = true + return nil + } + time.Sleep(clientIDWaitInterval) + } + // It exists and holds nothing usable: a crash between the create and + // the write, or a file that was truncated or edited. Replacing it is + // the only way out, because otherwise every run waits and then uses an + // identifier that is not the one on disk. + if err := writeClientID(c.ClientIDPath(), id); err != nil { + return err + } + c.Plex.ClientID = id + c.PlexClientIDResolved = true + return nil + default: return fmt.Errorf("write %s: %w", c.ClientIDPath(), err) } - c.Plex.ClientID = id +} + +// writeClientID replaces the stored identifier through a temporary file, so a +// reader arriving during the write sees either the old value or the new one and +// never half of either. +func writeClientID(path, id string) error { + dir := filepath.Dir(path) + temp, err := os.CreateTemp(dir, filepath.Base(path)+".tmp*") + if err != nil { + return fmt.Errorf("write %s: %w", path, err) + } + name := temp.Name() + defer func() { _ = os.Remove(name) }() // a no-op once renamed + + if _, err := temp.WriteString(id + "\n"); err != nil { + _ = temp.Close() + return fmt.Errorf("write %s: %w", path, err) + } + if err := temp.Close(); err != nil { + return fmt.Errorf("write %s: %w", path, err) + } + if err := os.Chmod(name, 0o600); err != nil { + return fmt.Errorf("write %s: %w", path, err) + } + if err := os.Rename(name, path); err != nil { + return fmt.Errorf("replace %s: %w", path, err) + } return nil } diff --git a/internal/config/identity_test.go b/internal/config/identity_test.go index 3ddee4d..982a8c7 100644 --- a/internal/config/identity_test.go +++ b/internal/config/identity_test.go @@ -4,6 +4,7 @@ import ( "os" "runtime" "strings" + "sync" "testing" ) @@ -213,3 +214,71 @@ func TestSaveDoesNotFreezeTheResolvedIdentifier(t *testing.T) { t.Errorf("a configured identifier was dropped on write: %q", got) } } + +// An identifier somebody typed is kept even when it matches what is stored. +// Comparing the two values instead of recording which one supplied it would +// discard a deliberate override, and a later change of state directory would +// then generate a new identity rather than honour it. +func TestSaveKeepsAConfiguredIdentifierThatMatchesTheStoredOne(t *testing.T) { + cfg := identityConfig(t) + if err := cfg.ResolvePlexIdentity(); err != nil { + t.Fatalf("ResolvePlexIdentity: %v", err) + } + stored := cfg.Plex.ClientID + + chosen := identityConfig(t) + if err := os.WriteFile(chosen.ClientIDPath(), []byte(stored+"\n"), 0o600); err != nil { + t.Fatalf("write the file: %v", err) + } + chosen.Plex.ClientID = stored + + if got := chosen.forWriting().Plex.ClientID; got != stored { + t.Errorf("a configured identifier matching the stored one was dropped: %q, want %q", got, stored) + } +} + +// Two processes starting against the same state directory must agree on one +// identifier, or one install presents two devices to Plex. They race exactly +// here, and the file is created before it is written, so this also covers a +// reader arriving in that window. +func TestResolvePlexIdentityIsStableUnderConcurrency(t *testing.T) { + base := identityConfig(t) + const processes = 8 + + ids := make([]string, processes) + errs := make([]error, processes) + var start sync.WaitGroup + var done sync.WaitGroup + start.Add(1) + for i := range processes { + done.Add(1) + go func() { + defer done.Done() + cfg := Default() + cfg.StateDir = base.StateDir + start.Wait() + errs[i] = cfg.ResolvePlexIdentity() + ids[i] = cfg.Plex.ClientID + }() + } + start.Done() + done.Wait() + + for i, err := range errs { + if err != nil { + t.Fatalf("process %d: %v", i, err) + } + } + for i, id := range ids { + if id != ids[0] { + t.Errorf("process %d used %q, want %q: one install must be one device to Plex", i, id, ids[0]) + } + } + raw, err := os.ReadFile(base.ClientIDPath()) + if err != nil { + t.Fatalf("read back: %v", err) + } + if got := strings.TrimSpace(string(raw)); got != ids[0] { + t.Errorf("stored identifier = %q, want the one every process used (%q)", got, ids[0]) + } +} diff --git a/internal/config/save.go b/internal/config/save.go index a0e21ff..3965529 100644 --- a/internal/config/save.go +++ b/internal/config/save.go @@ -63,7 +63,11 @@ func (c *Config) forWriting() Config { // freezes the one thing that is supposed to follow the state directory -- // and a configuration file copied to a second machine would hand both the // same identity, which Plex would then show as a single device. - if id := out.Plex.ClientID; id != "" && out.StoredClientID() == id { + // + // Only what resolution supplied is dropped. An identifier somebody typed is + // kept even when it happens to match the stored one, because then it was + // chosen, and a later change of state directory is meant to leave it alone. + if out.PlexClientIDResolved { out.Plex.ClientID = "" } return out