From 72b0a534f6f57618a1d6a7b189b5abaf83db94e9 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 03:27:03 +0800 Subject: [PATCH 1/2] feat: add GNOME desktop proxy target --- internal/gnome/configuration_test.go | 45 ++ internal/gnome/gnome.go | 720 +++++++++++++++++++++++++++ internal/gnome/gnome_test.go | 400 +++++++++++++++ 3 files changed, 1165 insertions(+) create mode 100644 internal/gnome/configuration_test.go create mode 100644 internal/gnome/gnome.go create mode 100644 internal/gnome/gnome_test.go diff --git a/internal/gnome/configuration_test.go b/internal/gnome/configuration_test.go new file mode 100644 index 0000000..21eb86d --- /dev/null +++ b/internal/gnome/configuration_test.go @@ -0,0 +1,45 @@ +package gnome_test + +import ( + "testing" + + "proxyctl/internal/gnome" + "proxyctl/internal/profile" +) + +type identityEnvironment struct { + current profile.UserIdentity + users map[string]profile.UserIdentity + env map[string]string +} + +func (e identityEnvironment) CurrentUser() (profile.UserIdentity, error) { return e.current, nil } +func (e identityEnvironment) LookupUser(name string) (profile.UserIdentity, error) { + return e.users[name], nil +} +func (e identityEnvironment) LookupEnv(name string) (string, bool) { + v, ok := e.env[name] + return v, ok +} + +func TestConfigurationTargetsSudoInvokingUsersSessionAndState(t *testing.T) { + alice := profile.UserIdentity{Username: "alice", HomeDir: "/home/alice", UID: 1000, GID: 1001} + got, err := gnome.ResolveConfiguration(identityEnvironment{current: profile.UserIdentity{Username: "root", HomeDir: "/root", UID: 0, GID: 0}, users: map[string]profile.UserIdentity{"alice": alice}, env: map[string]string{"SUDO_USER": "alice", "XDG_STATE_HOME": "/root/state", "XDG_RUNTIME_DIR": "/run/user/0", "DBUS_SESSION_BUS_ADDRESS": "unix:path=/run/user/0/bus"}}) + if err != nil { + t.Fatal(err) + } + if got.UID != 1000 || got.GID != 1001 || got.HomeDir != "/home/alice" || got.RuntimeDir != "/run/user/1000" || got.StateFile != "/home/alice/.local/state/proxyctl/gnome.json" { + t.Fatalf("%#v", got) + } +} + +func TestConfigurationKeepsDirectUsersSafeAbsoluteXDGPaths(t *testing.T) { + user := profile.UserIdentity{Username: "alice", HomeDir: "/home/alice", UID: 1000, GID: 1001} + got, err := gnome.ResolveConfiguration(identityEnvironment{current: user, env: map[string]string{"XDG_STATE_HOME": "/home/alice/state", "XDG_RUNTIME_DIR": "/run/user/1000", "DBUS_SESSION_BUS_ADDRESS": "unix:path=/custom/bus"}}) + if err != nil { + t.Fatal(err) + } + if got.StateFile != "/home/alice/state/proxyctl/gnome.json" || got.RuntimeDir != "/run/user/1000" || got.SessionBus != "unix:path=/custom/bus" { + t.Fatalf("%#v", got) + } +} diff --git a/internal/gnome/gnome.go b/internal/gnome/gnome.go new file mode 100644 index 0000000..b2a717f --- /dev/null +++ b/internal/gnome/gnome.go @@ -0,0 +1,720 @@ +// Package gnome implements the GNOME desktop proxy Target through gsettings. +package gnome + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io/fs" + "net/url" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + + "proxyctl/internal/profile" + "proxyctl/internal/securefs" + "proxyctl/internal/target" +) + +const ( + proxySchema = "org.gnome.system.proxy" + httpSchema = proxySchema + ".http" + httpsSchema = proxySchema + ".https" + socksSchema = proxySchema + ".socks" +) + +var requiredSchemas = []string{proxySchema, httpSchema, httpsSchema, socksSchema} + +// Command is the complete command contract used by the adapter. UID, GID, and +// Env make user-session identity explicit when proxyctl itself runs under sudo. +type Command struct { + Name string + Args []string + UID int + GID int + Env map[string]string +} + +type CommandRunner interface { + Run(context.Context, Command) (string, error) +} + +type execRunner struct{} + +func (execRunner) Run(ctx context.Context, command Command) (string, error) { + cmd := exec.CommandContext(ctx, command.Name, command.Args...) + environment := envWithOverrides(os.Environ(), command.Env) + cmd.Env = environment + if os.Geteuid() != command.UID { + cmd.SysProcAttr = &syscall.SysProcAttr{Credential: &syscall.Credential{Uid: uint32(command.UID), Gid: uint32(command.GID)}} + } + var stderr bytes.Buffer + cmd.Stderr = &stderr + output, err := cmd.Output() + if err != nil { + detail := strings.TrimSpace(stderr.String()) + if detail == "" { + return "", fmt.Errorf("run %s: %w", command.Name, err) + } + if strings.Contains(strings.ToLower(detail), "permission denied") { + return "", fmt.Errorf("run %s: %s: %w", command.Name, detail, os.ErrPermission) + } + return "", fmt.Errorf("run %s: %s: %w", command.Name, detail, err) + } + return string(output), nil +} + +func envWithOverrides(base []string, overrides map[string]string) []string { + values := make(map[string]string, len(base)+len(overrides)) + order := make([]string, 0, len(base)+len(overrides)) + for _, item := range base { + name, value, ok := strings.Cut(item, "=") + if !ok { + continue + } + if _, exists := values[name]; !exists { + order = append(order, name) + } + values[name] = value + } + for name, value := range overrides { + if _, exists := values[name]; !exists { + order = append(order, name) + } + values[name] = value + } + result := make([]string, 0, len(order)) + for _, name := range order { + result = append(result, name+"="+values[name]) + } + return result +} + +// StateStore holds the original GNOME values and last managed fingerprint. +type StateStore interface { + Load() ([]byte, error) + Save([]byte) error + Delete() error +} + +type fileState struct { + path string + store securefs.Store +} + +func NewFileState(path string, uid, gid int, trustedRoot string) StateStore { + var owner *securefs.Owner + if uid >= 0 && gid >= 0 { + owner = &securefs.Owner{UID: uid, GID: gid} + } + return &fileState{path: path, store: securefs.Store{TrustedRoot: trustedRoot, Owner: owner}} +} +func (s *fileState) Load() ([]byte, error) { return s.store.ReadFile(s.path) } +func (s *fileState) Save(data []byte) error { return s.store.AtomicWrite(s.path, data, 0o600) } +func (s *fileState) Delete() error { return s.store.Remove(s.path) } + +type Configuration struct { + CommandName string + UID int + GID int + Username string + HomeDir string + ConfigDir string + DataDir string + CacheDir string + RuntimeDir string + SessionBus string + StateFile string + TrustedRoot string +} + +// ResolveConfiguration keeps both recovery state and gsettings commands with +// the invoking user under sudo, never root's unrelated desktop session. +func ResolveConfiguration(environment profile.IdentityEnvironment) (Configuration, error) { + profileConfiguration, err := profile.ResolveConfiguration(environment) + if err != nil { + return Configuration{}, err + } + owner := profileConfiguration.Owner + current, err := environment.CurrentUser() + if err != nil { + return Configuration{}, fmt.Errorf("identify effective user: %w", err) + } + invoking := current.Username != owner.Username + stateDir := "" + if value, ok := environment.LookupEnv("XDG_STATE_HOME"); ok && filepath.IsAbs(value) && (!invoking || pathWithin(value, owner.HomeDir)) { + stateDir = value + } + if stateDir == "" { + stateDir = filepath.Join(owner.HomeDir, ".local", "state") + } + configDir := filepath.Dir(profileConfiguration.Path) + dataDir := filepath.Join(owner.HomeDir, ".local", "share") + if value, ok := environment.LookupEnv("XDG_DATA_HOME"); ok && filepath.IsAbs(value) && (!invoking || pathWithin(value, owner.HomeDir)) { + dataDir = value + } + cacheDir := filepath.Join(owner.HomeDir, ".cache") + if value, ok := environment.LookupEnv("XDG_CACHE_HOME"); ok && filepath.IsAbs(value) && (!invoking || pathWithin(value, owner.HomeDir)) { + cacheDir = value + } + runtimeDir := filepath.Join("/run/user", strconv.Itoa(owner.UID)) + if value, ok := environment.LookupEnv("XDG_RUNTIME_DIR"); ok && filepath.IsAbs(value) && !invoking { + runtimeDir = value + } + sessionBus := "unix:path=" + filepath.Join(runtimeDir, "bus") + if value, ok := environment.LookupEnv("DBUS_SESSION_BUS_ADDRESS"); ok && value != "" && !invoking { + sessionBus = value + } + trustedRoot := owner.HomeDir + if !pathWithin(stateDir, owner.HomeDir) { + trustedRoot = stateDir + } + return Configuration{CommandName: "gsettings", UID: owner.UID, GID: owner.GID, Username: owner.Username, HomeDir: owner.HomeDir, ConfigDir: configDir, DataDir: dataDir, CacheDir: cacheDir, RuntimeDir: runtimeDir, SessionBus: sessionBus, StateFile: filepath.Join(stateDir, "proxyctl", "gnome.json"), TrustedRoot: trustedRoot}, nil +} + +func ResolveConfigurationForCurrentProcess() (Configuration, error) { + return ResolveConfiguration(systemEnvironment{}) +} + +type systemEnvironment struct{} + +func (systemEnvironment) CurrentUser() (profile.UserIdentity, error) { + return profile.SystemCurrentUser() +} +func (systemEnvironment) LookupUser(name string) (profile.UserIdentity, error) { + return profile.SystemLookupUser(name) +} +func (systemEnvironment) LookupEnv(name string) (string, bool) { return os.LookupEnv(name) } + +func pathWithin(path, parent string) bool { + relative, err := filepath.Rel(parent, path) + return err == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) +} + +type settings struct { + Mode string `json:"mode"` + AutoConfigURL string `json:"autoConfigUrl"` + IgnoreHosts []string `json:"ignoreHosts"` + HTTPHost string `json:"httpHost"` + HTTPPort int `json:"httpPort"` + HTTPUseAuth bool `json:"httpUseAuthentication"` + HTTPAuthUser string `json:"httpAuthenticationUser"` + HTTPAuthPassword string `json:"httpAuthenticationPassword"` + HTTPSHost string `json:"httpsHost"` + HTTPSPort int `json:"httpsPort"` + SocksHost string `json:"socksHost"` + SocksPort int `json:"socksPort"` +} + +type recovery struct { + Original settings `json:"original"` + LastFingerprint string `json:"lastFingerprint"` +} + +type Adapter struct { + configuration Configuration + runner CommandRunner + state StateStore +} + +var _ target.Target = (*Adapter)(nil) + +func New(configuration Configuration, runner CommandRunner, state StateStore) *Adapter { + if configuration.CommandName == "" { + configuration.CommandName = "gsettings" + } + if configuration.ConfigDir == "" { + configuration.ConfigDir = filepath.Join(configuration.HomeDir, ".config") + } + if configuration.DataDir == "" { + configuration.DataDir = filepath.Join(configuration.HomeDir, ".local", "share") + } + if configuration.CacheDir == "" { + configuration.CacheDir = filepath.Join(configuration.HomeDir, ".cache") + } + if configuration.RuntimeDir == "" { + configuration.RuntimeDir = filepath.Join("/run/user", strconv.Itoa(configuration.UID)) + } + if configuration.SessionBus == "" { + configuration.SessionBus = "unix:path=" + filepath.Join(configuration.RuntimeDir, "bus") + } + if runner == nil { + runner = execRunner{} + } + if state == nil { + state = NewFileState(configuration.StateFile, configuration.UID, configuration.GID, configuration.TrustedRoot) + } + return &Adapter{configuration: configuration, runner: runner, state: state} +} + +func (a *Adapter) ID() string { return "gnome" } +func (a *Adapter) Description() target.Description { + return target.Description{ + Name: "GNOME Desktop", + Summary: "Configures the GNOME desktop system proxy.", + Details: "Affects applications that consume GNOME system network settings; not every graphical application consumes these settings. Settings belong to the desktop user and require that user's session bus and schema permissions.", + } +} + +func (a *Adapter) Availability(ctx context.Context) target.Availability { + output, err := a.run(ctx, "list-schemas") + if err != nil { + return target.Availability{Evidence: fmt.Sprintf("GNOME Target requires a usable %s command: %v", a.configuration.CommandName, redact(err))} + } + present := make(map[string]bool) + for _, schema := range strings.Fields(output) { + present[schema] = true + } + for _, schema := range requiredSchemas { + if !present[schema] { + return target.Availability{Evidence: "required GNOME settings schema is unavailable: " + schema} + } + } + if _, err := a.readSettings(ctx); err != nil { + return target.Availability{Evidence: fmt.Sprintf("GNOME proxy schema is present but unusable for the desktop session: %v", redact(err))} + } + return target.Availability{Available: true, Evidence: "gsettings command and GNOME proxy schemas are usable for the owning user"} +} + +func (a *Adapter) Inspect(ctx context.Context, active *profile.Profile) target.Inspection { + metadata, managed, metadataErr := a.readRecovery() + current, settingsErr := a.readSettings(ctx) + if managed || (metadataErr != nil && !errors.Is(metadataErr, fs.ErrNotExist)) { + if metadataErr != nil { + return target.Inspection{State: target.Drifted, Evidence: "managed GNOME recovery metadata cannot be read"} + } + if settingsErr != nil { + return target.Inspection{State: target.Drifted, Evidence: "managed GNOME settings cannot be read: " + redact(settingsErr).Error()} + } + if fingerprint(current) == metadata.LastFingerprint && active != nil { + desired, err := translate(*active) + if err == nil && fingerprint(desired) == metadata.LastFingerprint { + return target.Inspection{State: target.Enabled, Evidence: "managed GNOME settings match the Active Profile"} + } + } + return target.Inspection{State: target.Drifted, Evidence: "managed GNOME settings do not match the Active Profile"} + } + if settingsErr != nil { + return target.Inspection{State: target.Disabled, Evidence: "GNOME settings could not be inspected: " + redact(settingsErr).Error()} + } + switch current.Mode { + case "manual": + return target.Inspection{State: target.External, Evidence: "recognized external GNOME manual proxy mode is configured"} + case "auto": + return target.Inspection{State: target.External, Evidence: "recognized external GNOME automatic proxy mode is configured"} + default: + return target.Inspection{State: target.Disabled, Evidence: "GNOME proxy mode is none; no recognized desktop proxy configuration detected"} + } +} + +func (a *Adapter) SetDesired(ctx context.Context, enabled bool, active *profile.Profile) target.OperationResult { + availability := a.Availability(ctx) + if !availability.Available { + return target.OperationResult{Inspection: a.Inspect(ctx, active), Err: errors.New("GNOME Target is unavailable: " + availability.Evidence)} + } + if enabled && active == nil { + return target.OperationResult{Inspection: a.Inspect(ctx, active), Err: errors.New("no Active Profile; open Profile management to create and activate one")} + } + var err error + if enabled { + err = a.enable(ctx, *active) + } else { + err = a.disable(ctx) + } + return target.OperationResult{Inspection: a.Inspect(ctx, active), Err: redactForProfile(err, active)} +} + +func (a *Adapter) enable(ctx context.Context, active profile.Profile) error { + desired, err := translate(active) + if err != nil { + return err + } + metadata, managed, err := a.readRecovery() + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("read GNOME recovery metadata: %w", err) + } + if !managed { + original, err := a.readSettings(ctx) + if err != nil { + return fmt.Errorf("snapshot original GNOME settings: %w", err) + } + metadata.Original = original + } + metadata.LastFingerprint = fingerprint(desired) + data, err := json.MarshalIndent(metadata, "", " ") + if err != nil { + return err + } + if err := a.state.Save(append(data, '\n')); err != nil { + return fmt.Errorf("persist GNOME recovery metadata before changing settings: %w", err) + } + if err := a.writeSettings(ctx, desired); err != nil { + return fmt.Errorf("apply GNOME proxy settings after recovery metadata persisted: %w", err) + } + return nil +} + +func (a *Adapter) disable(ctx context.Context) error { + metadata, managed, err := a.readRecovery() + if errors.Is(err, fs.ErrNotExist) || !managed { + return nil + } + if err != nil { + return fmt.Errorf("read GNOME recovery metadata: %w", err) + } + current, err := a.readSettings(ctx) + if err != nil { + return fmt.Errorf("verify managed GNOME settings before restore: %w", err) + } + if fingerprint(current) != metadata.LastFingerprint { + return errors.New("GNOME settings no longer match proxyctl's last applied values; refusing restore because ownership evidence was lost") + } + if err := a.writeSettings(ctx, metadata.Original); err != nil { + return fmt.Errorf("restore original GNOME settings: %w", err) + } + if err := a.state.Delete(); err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove GNOME recovery metadata after restoring settings: %w", err) + } + return nil +} + +func (a *Adapter) readRecovery() (recovery, bool, error) { + data, err := a.state.Load() + if err != nil { + return recovery{}, false, err + } + var value recovery + if err := json.Unmarshal(data, &value); err != nil { + return recovery{}, true, err + } + return value, true, nil +} + +func (a *Adapter) command(args ...string) Command { + return Command{Name: a.configuration.CommandName, Args: append([]string(nil), args...), UID: a.configuration.UID, GID: a.configuration.GID, Env: map[string]string{ + "HOME": a.configuration.HomeDir, "USER": a.configuration.Username, "LOGNAME": a.configuration.Username, + "XDG_CONFIG_HOME": a.configuration.ConfigDir, "XDG_DATA_HOME": a.configuration.DataDir, "XDG_CACHE_HOME": a.configuration.CacheDir, + "XDG_RUNTIME_DIR": a.configuration.RuntimeDir, "DBUS_SESSION_BUS_ADDRESS": a.configuration.SessionBus, + }} +} +func (a *Adapter) run(ctx context.Context, args ...string) (string, error) { + return a.runner.Run(ctx, a.command(args...)) +} +func (a *Adapter) get(ctx context.Context, schema, key string) (string, error) { + value, err := a.run(ctx, "get", schema, key) + if err != nil { + return "", fmt.Errorf("get %s %s: %w", schema, key, err) + } + return strings.TrimSpace(value), nil +} +func (a *Adapter) set(ctx context.Context, schema, key, value string) error { + if _, err := a.run(ctx, "set", schema, key, value); err != nil { + return fmt.Errorf("set %s %s: %w", schema, key, err) + } + return nil +} + +func (a *Adapter) readSettings(ctx context.Context) (settings, error) { + var result settings + var err error + if result.Mode, err = a.getString(ctx, proxySchema, "mode"); err != nil { + return settings{}, err + } + if result.AutoConfigURL, err = a.getString(ctx, proxySchema, "autoconfig-url"); err != nil { + return settings{}, err + } + if result.IgnoreHosts, err = a.getStrings(ctx, proxySchema, "ignore-hosts"); err != nil { + return settings{}, err + } + if result.HTTPHost, err = a.getString(ctx, httpSchema, "host"); err != nil { + return settings{}, err + } + if result.HTTPPort, err = a.getInt(ctx, httpSchema, "port"); err != nil { + return settings{}, err + } + if result.HTTPUseAuth, err = a.getBool(ctx, httpSchema, "use-authentication"); err != nil { + return settings{}, err + } + if result.HTTPAuthUser, err = a.getString(ctx, httpSchema, "authentication-user"); err != nil { + return settings{}, err + } + if result.HTTPAuthPassword, err = a.getString(ctx, httpSchema, "authentication-password"); err != nil { + return settings{}, err + } + if result.HTTPSHost, err = a.getString(ctx, httpsSchema, "host"); err != nil { + return settings{}, err + } + if result.HTTPSPort, err = a.getInt(ctx, httpsSchema, "port"); err != nil { + return settings{}, err + } + if result.SocksHost, err = a.getString(ctx, socksSchema, "host"); err != nil { + return settings{}, err + } + if result.SocksPort, err = a.getInt(ctx, socksSchema, "port"); err != nil { + return settings{}, err + } + return result, nil +} + +func (a *Adapter) getString(ctx context.Context, schema, key string) (string, error) { + raw, err := a.get(ctx, schema, key) + if err != nil { + return "", err + } + return parseString(raw) +} +func (a *Adapter) getInt(ctx context.Context, schema, key string) (int, error) { + raw, err := a.get(ctx, schema, key) + if err != nil { + return 0, err + } + value, err := strconv.Atoi(raw) + if err != nil { + return 0, fmt.Errorf("parse %s %s integer %q: %w", schema, key, raw, err) + } + return value, nil +} +func (a *Adapter) getBool(ctx context.Context, schema, key string) (bool, error) { + raw, err := a.get(ctx, schema, key) + if err != nil { + return false, err + } + value, err := strconv.ParseBool(raw) + if err != nil { + return false, fmt.Errorf("parse %s %s boolean %q: %w", schema, key, raw, err) + } + return value, nil +} +func (a *Adapter) getStrings(ctx context.Context, schema, key string) ([]string, error) { + raw, err := a.get(ctx, schema, key) + if err != nil { + return nil, err + } + return parseStrings(raw) +} + +func (a *Adapter) writeSettings(ctx context.Context, value settings) error { + // Keep manual mode from becoming active until all component values exist. + if err := a.set(ctx, proxySchema, "mode", quote("none")); err != nil { + return err + } + operations := []struct{ schema, key, value string }{ + {proxySchema, "autoconfig-url", quote(value.AutoConfigURL)}, + {proxySchema, "ignore-hosts", quoteStrings(value.IgnoreHosts)}, + {httpSchema, "host", quote(value.HTTPHost)}, {httpSchema, "port", strconv.Itoa(value.HTTPPort)}, + {httpSchema, "use-authentication", strconv.FormatBool(value.HTTPUseAuth)}, {httpSchema, "authentication-user", quote(value.HTTPAuthUser)}, {httpSchema, "authentication-password", quote(value.HTTPAuthPassword)}, + {httpsSchema, "host", quote(value.HTTPSHost)}, {httpsSchema, "port", strconv.Itoa(value.HTTPSPort)}, + {socksSchema, "host", quote(value.SocksHost)}, {socksSchema, "port", strconv.Itoa(value.SocksPort)}, + } + for _, operation := range operations { + if err := a.set(ctx, operation.schema, operation.key, operation.value); err != nil { + return err + } + } + return a.set(ctx, proxySchema, "mode", quote(value.Mode)) +} + +func translate(input profile.Profile) (settings, error) { + p, err := profile.ValidateAndNormalize(input) + if err != nil { + return settings{}, err + } + result := settings{Mode: "manual"} + if p.Mode == profile.SingleMode { + if result.SocksHost, result.SocksPort, _, _, err = endpoint(p.SingleURL, "Single proxy URL", "socks5", false); err != nil { + return settings{}, err + } + for _, bypass := range p.Bypass { + if strings.HasPrefix(bypass, ".") { + result.IgnoreHosts = append(result.IgnoreHosts, "*"+bypass) + } else { + result.IgnoreHosts = append(result.IgnoreHosts, bypass) + } + } + return result, nil + } + if result.HTTPHost, result.HTTPPort, result.HTTPAuthUser, result.HTTPAuthPassword, err = endpoint(p.HTTPURL, "HTTP proxy URL", "http", true); err != nil { + return settings{}, err + } + result.HTTPUseAuth = result.HTTPAuthUser != "" || result.HTTPAuthPassword != "" + if result.HTTPSHost, result.HTTPSPort, _, _, err = endpoint(p.HTTPSURL, "HTTPS proxy URL", "http", false); err != nil { + return settings{}, err + } + if result.SocksHost, result.SocksPort, _, _, err = endpoint(p.AllURL, "ALL/SOCKS proxy URL", "socks5", false); err != nil { + return settings{}, err + } + for _, bypass := range p.Bypass { + if strings.HasPrefix(bypass, ".") { + result.IgnoreHosts = append(result.IgnoreHosts, "*"+bypass) + continue + } + result.IgnoreHosts = append(result.IgnoreHosts, bypass) + } + return result, nil +} + +func endpoint(raw, field, allowedScheme string, allowCredentials bool) (string, int, string, string, error) { + if raw == "" { + return "", 0, "", "", nil + } + u, err := url.Parse(raw) + if err != nil { + return "", 0, "", "", fmt.Errorf("%s is malformed", field) + } + if u.Scheme != allowedScheme { + if u.Scheme == "socks5h" { + return "", 0, "", "", fmt.Errorf("%s uses socks5h remote-DNS semantics that GNOME settings cannot preserve", field) + } + return "", 0, "", "", fmt.Errorf("%s scheme %q cannot be represented without changing semantics; GNOME requires %s", field, u.Scheme, allowedScheme) + } + username, password := "", "" + if u.User != nil { + if !allowCredentials { + return "", 0, "", "", fmt.Errorf("%s credentials cannot be represented safely by the GNOME Target", field) + } + username = u.User.Username() + password, _ = u.User.Password() + } + port, _ := strconv.Atoi(u.Port()) + return u.Hostname(), port, username, password, nil +} + +func fingerprint(value settings) string { + data, _ := json.Marshal(value) + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]) +} + +func quote(value string) string { + return "'" + strings.ReplaceAll(strings.ReplaceAll(value, "\\", "\\\\"), "'", "\\'") + "'" +} +func quoteStrings(values []string) string { + quoted := make([]string, len(values)) + for i, v := range values { + quoted[i] = quote(v) + } + return "[" + strings.Join(quoted, ", ") + "]" +} +func parseString(raw string) (string, error) { + raw = strings.TrimSpace(raw) + if len(raw) < 2 || raw[0] != '\'' || raw[len(raw)-1] != '\'' { + return "", fmt.Errorf("invalid gsettings string %q", raw) + } + var b strings.Builder + escaped := false + for _, r := range raw[1 : len(raw)-1] { + if escaped { + b.WriteRune(r) + escaped = false + } else if r == '\\' { + escaped = true + } else { + b.WriteRune(r) + } + } + if escaped { + return "", fmt.Errorf("invalid gsettings escape %q", raw) + } + return b.String(), nil +} +func parseStrings(raw string) ([]string, error) { + raw = strings.TrimSpace(raw) + if raw == "[]" { + return []string{}, nil + } + if len(raw) < 2 || raw[0] != '[' || raw[len(raw)-1] != ']' { + return nil, fmt.Errorf("invalid gsettings string array %q", raw) + } + inner := strings.TrimSpace(raw[1 : len(raw)-1]) + var result []string + for len(inner) > 0 { + if inner[0] != '\'' { + return nil, fmt.Errorf("invalid gsettings string array %q", raw) + } + end := -1 + escaped := false + for i := 1; i < len(inner); i++ { + if escaped { + escaped = false + continue + } + if inner[i] == '\\' { + escaped = true + continue + } + if inner[i] == '\'' { + end = i + break + } + } + if end < 0 { + return nil, fmt.Errorf("invalid gsettings string array %q", raw) + } + value, err := parseString(inner[:end+1]) + if err != nil { + return nil, err + } + result = append(result, value) + inner = strings.TrimSpace(inner[end+1:]) + if inner == "" { + break + } + if inner[0] != ',' { + return nil, fmt.Errorf("invalid gsettings string array %q", raw) + } + inner = strings.TrimSpace(inner[1:]) + } + return result, nil +} + +func redactForProfile(err error, active *profile.Profile) error { + if err == nil { + return nil + } + message := err.Error() + if active != nil { + for _, raw := range []string{active.SingleURL, active.HTTPURL, active.HTTPSURL, active.AllURL} { + parsed, parseErr := url.Parse(raw) + if parseErr != nil || parsed.User == nil { + continue + } + if password, present := parsed.User.Password(); present && password != "" { + message = strings.ReplaceAll(message, password, "REDACTED") + } + } + } + return safeRedaction(profile.RedactText(message), err) +} + +func redact(err error) error { + if err == nil { + return nil + } + return safeRedaction(profile.RedactText(err.Error()), err) +} + +func safeRedaction(message string, original error) error { + var class error + for _, candidate := range []error{os.ErrPermission, fs.ErrNotExist, context.Canceled, context.DeadlineExceeded} { + if errors.Is(original, candidate) { + class = candidate + break + } + } + return &safeError{message: message, class: class} +} + +type safeError struct { + message string + class error +} + +func (e *safeError) Error() string { return e.message } +func (e *safeError) Is(target error) bool { return e.class != nil && errors.Is(e.class, target) } diff --git a/internal/gnome/gnome_test.go b/internal/gnome/gnome_test.go new file mode 100644 index 0000000..b48c91e --- /dev/null +++ b/internal/gnome/gnome_test.go @@ -0,0 +1,400 @@ +package gnome_test + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "proxyctl/internal/gnome" + "proxyctl/internal/profile" + "proxyctl/internal/target" + "proxyctl/internal/target/targettest" +) + +type fakeRunner struct { + values map[string]string + schemas []string + calls []gnome.Command + fail func(gnome.Command) error +} + +func newRunner() *fakeRunner { + return &fakeRunner{schemas: []string{"org.gnome.system.proxy", "org.gnome.system.proxy.http", "org.gnome.system.proxy.https", "org.gnome.system.proxy.socks"}, values: map[string]string{ + "org.gnome.system.proxy mode": "'none'", + "org.gnome.system.proxy autoconfig-url": "''", + "org.gnome.system.proxy ignore-hosts": "[]", + "org.gnome.system.proxy.http host": "''", "org.gnome.system.proxy.http port": "0", + "org.gnome.system.proxy.http use-authentication": "false", "org.gnome.system.proxy.http authentication-user": "''", "org.gnome.system.proxy.http authentication-password": "''", + "org.gnome.system.proxy.https host": "''", "org.gnome.system.proxy.https port": "0", + "org.gnome.system.proxy.socks host": "''", "org.gnome.system.proxy.socks port": "0", + }} +} +func (r *fakeRunner) Run(_ context.Context, c gnome.Command) (string, error) { + r.calls = append(r.calls, c) + if r.fail != nil { + if err := r.fail(c); err != nil { + return "", err + } + } + if reflect.DeepEqual(c.Args, []string{"list-schemas"}) { + return strings.Join(r.schemas, "\n") + "\n", nil + } + if len(c.Args) == 3 && c.Args[0] == "get" { + key := c.Args[1] + " " + c.Args[2] + value, ok := r.values[key] + if !ok { + return "", fmt.Errorf("missing fake value %s", key) + } + return value + "\n", nil + } + if len(c.Args) == 4 && c.Args[0] == "set" { + r.values[c.Args[1]+" "+c.Args[2]] = c.Args[3] + return "", nil + } + return "", fmt.Errorf("unexpected fake command: %v", c.Args) +} + +type memoryState struct { + data []byte + saveErr, deleteErr error + failSaveOnce bool +} + +func (s *memoryState) Load() ([]byte, error) { + if s.data == nil { + return nil, os.ErrNotExist + } + return append([]byte(nil), s.data...), nil +} +func (s *memoryState) Save(data []byte) error { + if s.failSaveOnce { + s.failSaveOnce = false + return os.ErrPermission + } + if s.saveErr != nil { + return s.saveErr + } + s.data = append([]byte(nil), data...) + return nil +} +func (s *memoryState) Delete() error { + if s.deleteErr != nil { + return s.deleteErr + } + s.data = nil + return nil +} + +func adapter(r *fakeRunner, state *memoryState) *gnome.Adapter { + return gnome.New(gnome.Configuration{CommandName: "gsettings", UID: 1000, GID: 1001, HomeDir: "/home/alice", ConfigDir: "/home/alice/.config", DataDir: "/home/alice/.local/share", CacheDir: "/home/alice/.cache", RuntimeDir: "/run/user/1000", StateFile: "/state/gnome.json"}, r, state) +} +func active() *profile.Profile { + return &profile.Profile{Name: "office", Mode: profile.AdvancedMode, HTTPURL: "http://proxy.example:8080", HTTPSURL: "http://secure.example:8443", AllURL: "socks5://socks.example:1080", Bypass: []string{"localhost", ".example.com", "10.0.0.0/8", "2001:db8::1"}} +} + +func TestGNOMEAdapterRunsReusableTargetContract(t *testing.T) { + targettest.Run(t, func(t *testing.T) targettest.Fixture { + r, state := newRunner(), &memoryState{} + a := adapter(r, state) + return targettest.Fixture{ + Target: a, Active: active(), Secret: "secret", + MakeExternal: func(t *testing.T) { r.values["org.gnome.system.proxy mode"] = "'manual'" }, + MakeDrifted: func(t *testing.T) { r.values["org.gnome.system.proxy.http port"] = "9999" }, + PermissionFailure: func(t *testing.T) target.OperationResult { + return adapter(newRunner(), &memoryState{saveErr: os.ErrPermission}).SetDesired(context.Background(), true, active()) + }, + PartialFailure: func(t *testing.T) target.OperationResult { + failedRunner := newRunner() + failedRunner.fail = func(c gnome.Command) error { + if len(c.Args) > 0 && c.Args[0] == "set" { + return os.ErrPermission + } + return nil + } + return adapter(failedRunner, &memoryState{}).SetDesired(context.Background(), true, active()) + }, + RedactionFailure: func(t *testing.T) target.OperationResult { + p := *active() + p.HTTPURL = "http://alice:secret@proxy.example:8080" + failedRunner := newRunner() + failedRunner.fail = func(c gnome.Command) error { + if len(c.Args) > 0 && c.Args[0] == "set" { + return errors.New("write password secret failed") + } + return nil + } + return adapter(failedRunner, &memoryState{}).SetDesired(context.Background(), true, &p) + }, + RecoveryRetry: func(t *testing.T) (target.OperationResult, target.OperationResult, target.OperationResult) { + retryRunner, retryState := newRunner(), &memoryState{failSaveOnce: true} + retryRunner.values["org.gnome.system.proxy mode"] = "'auto'" + retryAdapter := adapter(retryRunner, retryState) + first := retryAdapter.SetDesired(context.Background(), true, active()) + retry := retryAdapter.SetDesired(context.Background(), true, active()) + disabled := retryAdapter.SetDesired(context.Background(), false, nil) + return first, retry, disabled + }, + } + }) +} + +func TestAvailabilityRequiresCommandSchemasAndUsableSession(t *testing.T) { + t.Run("available", func(t *testing.T) { + got := adapter(newRunner(), &memoryState{}).Availability(context.Background()) + if !got.Available || !strings.Contains(got.Evidence, "schema") { + t.Fatalf("%#v", got) + } + }) + t.Run("missing schema", func(t *testing.T) { + r := newRunner() + r.schemas = r.schemas[:3] + got := adapter(r, &memoryState{}).Availability(context.Background()) + if got.Available || !strings.Contains(got.Evidence, "org.gnome.system.proxy.socks") { + t.Fatalf("%#v", got) + } + }) + t.Run("missing command", func(t *testing.T) { + r := newRunner() + r.fail = func(c gnome.Command) error { return os.ErrNotExist } + got := adapter(r, &memoryState{}).Availability(context.Background()) + if got.Available || !strings.Contains(got.Evidence, "gsettings") { + t.Fatalf("%#v", got) + } + }) + t.Run("session", func(t *testing.T) { + r := newRunner() + r.fail = func(c gnome.Command) error { + if len(c.Args) > 0 && c.Args[0] == "get" { + return errors.New("Cannot autolaunch D-Bus without X11 $DISPLAY") + } + return nil + } + got := adapter(r, &memoryState{}).Availability(context.Background()) + if got.Available || !strings.Contains(got.Evidence, "D-Bus") { + t.Fatalf("%#v", got) + } + }) +} + +func TestDescriptionStatesDesktopScopeWithoutUniversalCoverage(t *testing.T) { + d := adapter(newRunner(), &memoryState{}).Description() + if !strings.Contains(d.Details, "GNOME system network settings") || !strings.Contains(d.Details, "not every graphical application") { + t.Fatalf("%#v", d) + } +} + +func TestEnableSnapshotsAppliesInspectsAndRestoresExternalSettings(t *testing.T) { + r, s := newRunner(), &memoryState{} + r.values["org.gnome.system.proxy mode"] = "'auto'" + r.values["org.gnome.system.proxy autoconfig-url"] = "'https://config.example/proxy.pac'" + r.values["org.gnome.system.proxy ignore-hosts"] = "['old.local']" + a := adapter(r, s) + if got := a.Inspect(context.Background(), active()); got.State != target.External || !strings.Contains(got.Evidence, "automatic") { + t.Fatalf("initial %#v", got) + } + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil || got.Inspection.State != target.Enabled { + t.Fatalf("enable %#v", got) + } + want := map[string]string{ + "org.gnome.system.proxy mode": "'manual'", "org.gnome.system.proxy autoconfig-url": "''", "org.gnome.system.proxy ignore-hosts": "['localhost', '*.example.com', '10.0.0.0/8', '2001:db8::1']", + "org.gnome.system.proxy.http host": "'proxy.example'", "org.gnome.system.proxy.http port": "8080", + "org.gnome.system.proxy.https host": "'secure.example'", "org.gnome.system.proxy.https port": "8443", + "org.gnome.system.proxy.socks host": "'socks.example'", "org.gnome.system.proxy.socks port": "1080", + } + for key, value := range want { + if r.values[key] != value { + t.Errorf("%s=%q want %q", key, r.values[key], value) + } + } + if len(s.data) == 0 { + t.Fatal("original/recovery state not persisted") + } + if got := a.SetDesired(context.Background(), false, nil); got.Err != nil || got.Inspection.State != target.External { + t.Fatalf("disable %#v", got) + } + if r.values["org.gnome.system.proxy mode"] != "'auto'" || r.values["org.gnome.system.proxy autoconfig-url"] != "'https://config.example/proxy.pac'" || r.values["org.gnome.system.proxy ignore-hosts"] != "['old.local']" || s.data != nil { + t.Fatalf("original not restored: mode=%s url=%s bypass=%s state=%q", r.values["org.gnome.system.proxy mode"], r.values["org.gnome.system.proxy autoconfig-url"], r.values["org.gnome.system.proxy ignore-hosts"], s.data) + } +} + +func TestInspectionDistinguishesFourStatesUsingPositiveSettingsEvidence(t *testing.T) { + r, s := newRunner(), &memoryState{} + a := adapter(r, s) + if got := a.Inspect(context.Background(), active()); got.State != target.Disabled || !strings.Contains(got.Evidence, "mode is none") { + t.Fatalf("disabled %#v", got) + } + r.values["org.gnome.system.proxy mode"] = "'manual'" + if got := a.Inspect(context.Background(), active()); got.State != target.External || !strings.Contains(got.Evidence, "manual") { + t.Fatalf("external %#v", got) + } + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil { + t.Fatal(got.Err) + } + if got := a.Inspect(context.Background(), active()); got.State != target.Enabled { + t.Fatalf("enabled %#v", got) + } + r.values["org.gnome.system.proxy.http port"] = "9999" + if got := a.Inspect(context.Background(), active()); got.State != target.Drifted || !strings.Contains(got.Evidence, "managed") { + t.Fatalf("drifted %#v", got) + } +} + +func TestSingleSOCKSProxyUsesGNOMESOCKSForAllProtocols(t *testing.T) { + r, s := newRunner(), &memoryState{} + p := &profile.Profile{Name: "simple", Mode: profile.SingleMode, SingleURL: "socks5://proxy.example:1080", Bypass: []string{"localhost"}} + got := adapter(r, s).SetDesired(context.Background(), true, p) + if got.Err != nil || got.Inspection.State != target.Enabled || r.values["org.gnome.system.proxy.socks host"] != "'proxy.example'" || r.values["org.gnome.system.proxy.http host"] != "''" { + t.Fatalf("single proxy result=%#v values=%#v", got, r.values) + } +} + +func TestAdvancedHTTPProxyCredentialsArePreserved(t *testing.T) { + r, s := newRunner(), &memoryState{} + p := *active() + p.HTTPURL = "http://alice:secret@proxy.example:8080" + got := adapter(r, s).SetDesired(context.Background(), true, &p) + if got.Err != nil || got.Inspection.State != target.Enabled || r.values["org.gnome.system.proxy.http use-authentication"] != "true" || r.values["org.gnome.system.proxy.http authentication-user"] != "'alice'" || r.values["org.gnome.system.proxy.http authentication-password"] != "'secret'" { + t.Fatalf("authenticated HTTP result=%#v values=%#v", got, r.values) + } +} + +func TestValidationRejectsUnrepresentableProtocolsAndBypassBeforeMutation(t *testing.T) { + cases := []struct { + name string + mutate func(*profile.Profile) + message string + }{ + {"HTTPS proxy transport", func(p *profile.Profile) { p.HTTPSURL = "https://secure.example:8443" }, "HTTPS proxy URL"}, + {"remote DNS SOCKS", func(p *profile.Profile) { p.AllURL = "socks5h://socks.example:1080" }, "socks5h"}, + {"HTTP wrong transport", func(p *profile.Profile) { p.HTTPURL = "socks5://proxy.example:8080" }, "HTTP proxy URL"}, + {"single HTTP cannot cover ALL", func(p *profile.Profile) { p.Mode = profile.SingleMode; p.SingleURL = "http://proxy.example:8080" }, "Single proxy URL"}, + {"HTTPS credentials", func(p *profile.Profile) { p.HTTPSURL = "http://alice:secret@secure.example:8443" }, "credentials"}, + {"wildcard bypass", func(p *profile.Profile) { p.Bypass = []string{"*.example.com"} }, "Bypass"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r, s := newRunner(), &memoryState{} + p := *active() + tc.mutate(&p) + before := len(r.calls) + got := adapter(r, s).SetDesired(context.Background(), true, &p) + if got.Err == nil || !strings.Contains(got.Err.Error(), tc.message) || s.data != nil { + t.Fatalf("result %#v state=%q", got, s.data) + } + for _, c := range r.calls[before:] { + if len(c.Args) > 0 && c.Args[0] == "set" { + t.Fatalf("mutated before rejection: %#v", c) + } + } + }) + } +} + +func TestCommandContractCarriesInvokingUserIdentityAndSessionEnvironment(t *testing.T) { + r := newRunner() + a := adapter(r, &memoryState{}) + _ = a.Inspect(context.Background(), active()) + if len(r.calls) == 0 { + t.Fatal("no commands") + } + for _, c := range r.calls { + if c.Name != "gsettings" || c.UID != 1000 || c.GID != 1001 || c.Env["HOME"] != "/home/alice" || c.Env["XDG_CONFIG_HOME"] != "/home/alice/.config" || c.Env["XDG_DATA_HOME"] != "/home/alice/.local/share" || c.Env["XDG_CACHE_HOME"] != "/home/alice/.cache" || c.Env["XDG_RUNTIME_DIR"] != "/run/user/1000" || c.Env["DBUS_SESSION_BUS_ADDRESS"] != "unix:path=/run/user/1000/bus" { + t.Fatalf("command identity %#v", c) + } + } +} + +func TestFailuresAreImmediateTruthfulAndPreserveOwnership(t *testing.T) { + t.Run("permission before mutation", func(t *testing.T) { + r, s := newRunner(), &memoryState{} + r.fail = func(c gnome.Command) error { + if len(c.Args) > 0 && c.Args[0] == "set" { + return os.ErrPermission + } + return nil + } + got := adapter(r, s).SetDesired(context.Background(), true, active()) + if got.Err == nil || !errors.Is(got.Err, os.ErrPermission) || got.Inspection.State != target.Drifted { + t.Fatalf("%#v", got) + } + }) + t.Run("recovery must precede mutation", func(t *testing.T) { + r, s := newRunner(), &memoryState{saveErr: os.ErrPermission} + got := adapter(r, s).SetDesired(context.Background(), true, active()) + if got.Err == nil || !errors.Is(got.Err, os.ErrPermission) || got.Inspection.State != target.Disabled { + t.Fatalf("%#v", got) + } + for _, c := range r.calls { + if len(c.Args) > 0 && c.Args[0] == "set" { + t.Fatalf("mutation occurred: %#v", c) + } + } + }) + t.Run("drift refuses restore", func(t *testing.T) { + r, s := newRunner(), &memoryState{} + a := adapter(r, s) + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil { + t.Fatal(got.Err) + } + r.values["org.gnome.system.proxy.http host"] = "'external-change'" + got := a.SetDesired(context.Background(), false, nil) + if got.Err == nil || !strings.Contains(got.Err.Error(), "ownership") || got.Inspection.State != target.Drifted || s.data == nil || r.values["org.gnome.system.proxy.http host"] != "'external-change'" { + t.Fatalf("%#v", got) + } + }) + t.Run("command failure", func(t *testing.T) { + r, s := newRunner(), &memoryState{} + a := adapter(r, s) + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil { + t.Fatal(got.Err) + } + r.fail = func(c gnome.Command) error { + if len(c.Args) > 0 && c.Args[0] == "get" { + return errors.New("session bus unavailable") + } + return nil + } + got := a.SetDesired(context.Background(), false, nil) + if got.Err == nil || !strings.Contains(got.Err.Error(), "session bus") || got.Inspection.State != target.Drifted { + t.Fatalf("%#v", got) + } + }) +} + +func TestFakeCommandNeverTouchesDesktop(t *testing.T) { + r := newRunner() + a := adapter(r, &memoryState{}) + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil { + t.Fatal(got.Err) + } + if len(r.calls) == 0 { + t.Fatal("expected fake commands") + } +} + +func TestOSStateStoreWritesOwnerOnlyAtomically(t *testing.T) { + path := filepath.Join(t.TempDir(), "state", "gnome.json") + store := gnome.NewFileState(path, -1, -1, filepath.Dir(filepath.Dir(path))) + if err := store.Save([]byte("secret")); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o600 { + t.Fatalf("mode=%o", info.Mode().Perm()) + } + data, err := store.Load() + if err != nil || string(data) != "secret" { + t.Fatalf("load=%q err=%v", data, err) + } + if err := store.Delete(); err != nil { + t.Fatal(err) + } +} From 8aa1fef50b8abe1cd06ef4956d70676cb2224308 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 03:41:59 +0800 Subject: [PATCH 2/2] fix: harden GNOME settings recovery --- internal/gnome/gnome.go | 185 ++++++++++++++++++++++++---------- internal/gnome/gnome_test.go | 169 +++++++++++++++++++++++++++++++ internal/securefs/securefs.go | 44 ++++++++ 3 files changed, 347 insertions(+), 51 deletions(-) diff --git a/internal/gnome/gnome.go b/internal/gnome/gnome.go index b2a717f..59cbc1a 100644 --- a/internal/gnome/gnome.go +++ b/internal/gnome/gnome.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "io/fs" "net/url" "os" @@ -17,6 +18,7 @@ import ( "strconv" "strings" "syscall" + "unicode/utf8" "proxyctl/internal/profile" "proxyctl/internal/securefs" @@ -102,6 +104,7 @@ type StateStore interface { Load() ([]byte, error) Save([]byte) error Delete() error + Lock() (io.Closer, error) } type fileState struct { @@ -119,6 +122,9 @@ func NewFileState(path string, uid, gid int, trustedRoot string) StateStore { func (s *fileState) Load() ([]byte, error) { return s.store.ReadFile(s.path) } func (s *fileState) Save(data []byte) error { return s.store.AtomicWrite(s.path, data, 0o600) } func (s *fileState) Delete() error { return s.store.Remove(s.path) } +func (s *fileState) Lock() (io.Closer, error) { + return s.store.LockFile(s.path + ".lock") +} type Configuration struct { CommandName string @@ -322,7 +328,11 @@ func (a *Adapter) SetDesired(ctx context.Context, enabled bool, active *profile. if enabled && active == nil { return target.OperationResult{Inspection: a.Inspect(ctx, active), Err: errors.New("no Active Profile; open Profile management to create and activate one")} } - var err error + lock, err := a.state.Lock() + if err != nil { + return target.OperationResult{Inspection: a.Inspect(ctx, active), Err: redactForProfile(fmt.Errorf("lock GNOME mutation: %w", err), active)} + } + defer func() { _ = lock.Close() }() // best effort: release the process-shared advisory lock if enabled { err = a.enable(ctx, *active) } else { @@ -373,7 +383,17 @@ func (a *Adapter) disable(ctx context.Context) error { if err != nil { return fmt.Errorf("verify managed GNOME settings before restore: %w", err) } - if fingerprint(current) != metadata.LastFingerprint { + currentFingerprint := fingerprint(current) + if currentFingerprint == fingerprint(metadata.Original) { + // The restore completed previously, but metadata cleanup did not. It is + // safe to retry only the cleanup while the restored snapshot still + // matches exactly. + if err := a.state.Delete(); err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove GNOME recovery metadata after restoring settings: %w", err) + } + return nil + } + if currentFingerprint != metadata.LastFingerprint { return errors.New("GNOME settings no longer match proxyctl's last applied values; refusing restore because ownership evidence was lost") } if err := a.writeSettings(ctx, metadata.Original); err != nil { @@ -593,7 +613,41 @@ func fingerprint(value settings) string { } func quote(value string) string { - return "'" + strings.ReplaceAll(strings.ReplaceAll(value, "\\", "\\\\"), "'", "\\'") + "'" + delimiter := byte('\'') + if strings.ContainsRune(value, '\'') { + // This matches GVariant's canonical printer: apostrophes select a + // double-quoted literal rather than an escaped single-quoted one. + delimiter = '"' + } + var b strings.Builder + b.WriteByte(delimiter) + for _, r := range value { + switch r { + case '\a': + b.WriteString(`\a`) + case '\b': + b.WriteString(`\b`) + case '\f': + b.WriteString(`\f`) + case '\n': + b.WriteString(`\n`) + case '\r': + b.WriteString(`\r`) + case '\t': + b.WriteString(`\t`) + case '\v': + b.WriteString(`\v`) + case '\\': + b.WriteString(`\\`) + default: + if r == rune(delimiter) { + b.WriteByte('\\') + } + b.WriteRune(r) + } + } + b.WriteByte(delimiter) + return b.String() } func quoteStrings(values []string) string { quoted := make([]string, len(values)) @@ -604,74 +658,103 @@ func quoteStrings(values []string) string { } func parseString(raw string) (string, error) { raw = strings.TrimSpace(raw) - if len(raw) < 2 || raw[0] != '\'' || raw[len(raw)-1] != '\'' { + value, consumed, err := parseQuoted(raw) + if err != nil || consumed != len(raw) { return "", fmt.Errorf("invalid gsettings string %q", raw) } - var b strings.Builder - escaped := false - for _, r := range raw[1 : len(raw)-1] { - if escaped { - b.WriteRune(r) - escaped = false - } else if r == '\\' { - escaped = true - } else { - b.WriteRune(r) - } - } - if escaped { - return "", fmt.Errorf("invalid gsettings escape %q", raw) - } - return b.String(), nil + return value, nil } + +func parseQuoted(raw string) (string, int, error) { + if len(raw) < 2 || (raw[0] != '\'' && raw[0] != '"') { + return "", 0, errors.New("missing GVariant string delimiter") + } + delimiter := raw[0] + var b strings.Builder + for i := 1; i < len(raw); { + if raw[i] == delimiter { + return b.String(), i + 1, nil + } + if raw[i] != '\\' { + r, size := utf8.DecodeRuneInString(raw[i:]) + if r == utf8.RuneError && size == 1 { + return "", 0, errors.New("invalid UTF-8 in GVariant string") + } + b.WriteRune(r) + i += size + continue + } + i++ + if i >= len(raw) { + return "", 0, errors.New("trailing GVariant escape") + } + switch raw[i] { + case 'a': + b.WriteByte('\a') + case 'b': + b.WriteByte('\b') + case 'f': + b.WriteByte('\f') + case 'n': + b.WriteByte('\n') + case 'r': + b.WriteByte('\r') + case 't': + b.WriteByte('\t') + case 'v': + b.WriteByte('\v') + case '\\', '\'', '"': + b.WriteByte(raw[i]) + case 'u', 'U': + digits := 4 + if raw[i] == 'U' { + digits = 8 + } + if i+digits >= len(raw) { + return "", 0, errors.New("short GVariant Unicode escape") + } + code, err := strconv.ParseUint(raw[i+1:i+1+digits], 16, 32) + if err != nil || !utf8.ValidRune(rune(code)) { + return "", 0, errors.New("invalid GVariant Unicode escape") + } + b.WriteRune(rune(code)) + i += digits + default: + return "", 0, errors.New("unsupported GVariant escape") + } + i++ + } + return "", 0, errors.New("unterminated GVariant string") +} + func parseStrings(raw string) ([]string, error) { raw = strings.TrimSpace(raw) - if raw == "[]" { - return []string{}, nil - } if len(raw) < 2 || raw[0] != '[' || raw[len(raw)-1] != ']' { return nil, fmt.Errorf("invalid gsettings string array %q", raw) } inner := strings.TrimSpace(raw[1 : len(raw)-1]) + if inner == "" { + return []string{}, nil + } var result []string - for len(inner) > 0 { - if inner[0] != '\'' { - return nil, fmt.Errorf("invalid gsettings string array %q", raw) - } - end := -1 - escaped := false - for i := 1; i < len(inner); i++ { - if escaped { - escaped = false - continue - } - if inner[i] == '\\' { - escaped = true - continue - } - if inner[i] == '\'' { - end = i - break - } - } - if end < 0 { - return nil, fmt.Errorf("invalid gsettings string array %q", raw) - } - value, err := parseString(inner[:end+1]) + for { + value, consumed, err := parseQuoted(inner) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid gsettings string array %q", raw) } result = append(result, value) - inner = strings.TrimSpace(inner[end+1:]) + inner = strings.TrimSpace(inner[consumed:]) if inner == "" { - break + return result, nil } if inner[0] != ',' { return nil, fmt.Errorf("invalid gsettings string array %q", raw) } inner = strings.TrimSpace(inner[1:]) + if inner == "" { + return nil, fmt.Errorf("invalid gsettings string array %q", raw) + } } - return result, nil } func redactForProfile(err error, active *profile.Profile) error { diff --git a/internal/gnome/gnome_test.go b/internal/gnome/gnome_test.go index b48c91e..4c0f306 100644 --- a/internal/gnome/gnome_test.go +++ b/internal/gnome/gnome_test.go @@ -4,11 +4,14 @@ import ( "context" "errors" "fmt" + "io" "os" + "os/exec" "path/filepath" "reflect" "strings" "testing" + "time" "proxyctl/internal/gnome" "proxyctl/internal/profile" @@ -89,6 +92,11 @@ func (s *memoryState) Delete() error { s.data = nil return nil } +func (s *memoryState) Lock() (io.Closer, error) { return nopCloser{}, nil } + +type nopCloser struct{} + +func (nopCloser) Close() error { return nil } func adapter(r *fakeRunner, state *memoryState) *gnome.Adapter { return gnome.New(gnome.Configuration{CommandName: "gsettings", UID: 1000, GID: 1001, HomeDir: "/home/alice", ConfigDir: "/home/alice/.config", DataDir: "/home/alice/.local/share", CacheDir: "/home/alice/.cache", RuntimeDir: "/run/user/1000", StateFile: "/state/gnome.json"}, r, state) @@ -188,6 +196,30 @@ func TestDescriptionStatesDesktopScopeWithoutUniversalCoverage(t *testing.T) { } } +func TestGVariantSingleAndDoubleQuotedValuesRestoreCanonically(t *testing.T) { + r, s := newRunner(), &memoryState{} + r.values["org.gnome.system.proxy mode"] = "'auto'" + r.values["org.gnome.system.proxy autoconfig-url"] = `"https://config.example/don't\\path"` + r.values["org.gnome.system.proxy ignore-hosts"] = `[ + "don't.example", + 'back\\slash' + ]` + + a := adapter(r, s) + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil { + t.Fatalf("enable: %#v", got) + } + if got := a.SetDesired(context.Background(), false, nil); got.Err != nil { + t.Fatalf("disable: %#v", got) + } + if got := r.values["org.gnome.system.proxy autoconfig-url"]; got != `"https://config.example/don't\\path"` { + t.Fatalf("canonical autoconfig URL = %s", got) + } + if got := r.values["org.gnome.system.proxy ignore-hosts"]; got != `["don't.example", 'back\\slash']` { + t.Fatalf("canonical ignore hosts = %s", got) + } +} + func TestEnableSnapshotsAppliesInspectsAndRestoresExternalSettings(t *testing.T) { r, s := newRunner(), &memoryState{} r.values["org.gnome.system.proxy mode"] = "'auto'" @@ -309,6 +341,143 @@ func TestCommandContractCarriesInvokingUserIdentityAndSessionEnvironment(t *test } } +func TestMidOperationSetFailuresExposePartialStateAndRemainRecoverable(t *testing.T) { + t.Run("enable", func(t *testing.T) { + r, s := newRunner(), &memoryState{} + r.values["org.gnome.system.proxy mode"] = "'auto'" + r.values["org.gnome.system.proxy autoconfig-url"] = "'https://old.example/proxy.pac'" + setCount := 0 + r.fail = func(c gnome.Command) error { + if len(c.Args) > 0 && c.Args[0] == "set" { + setCount++ + if setCount == 4 { + return os.ErrPermission + } + } + return nil + } + a := adapter(r, s) + got := a.SetDesired(context.Background(), true, active()) + if got.Err == nil || !errors.Is(got.Err, os.ErrPermission) || got.Inspection.State != target.Drifted { + t.Fatalf("partial enable = %#v", got) + } + if setCount != 4 || r.values["org.gnome.system.proxy autoconfig-url"] != "''" || r.values["org.gnome.system.proxy ignore-hosts"] == "[]" || r.values["org.gnome.system.proxy.http host"] != "''" || s.data == nil { + t.Fatalf("failure was not genuinely partial: sets=%d values=%#v metadata=%q", setCount, r.values, s.data) + } + r.fail = nil + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil || got.Inspection.State != target.Enabled { + t.Fatalf("retry enable = %#v", got) + } + if got := a.SetDesired(context.Background(), false, nil); got.Err != nil || got.Inspection.State != target.External { + t.Fatalf("restore after retry = %#v", got) + } + }) + + t.Run("restore", func(t *testing.T) { + r, s := newRunner(), &memoryState{} + r.values["org.gnome.system.proxy mode"] = "'auto'" + r.values["org.gnome.system.proxy autoconfig-url"] = "'https://old.example/proxy.pac'" + a := adapter(r, s) + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil { + t.Fatal(got.Err) + } + setCount := 0 + r.fail = func(c gnome.Command) error { + if len(c.Args) > 0 && c.Args[0] == "set" { + setCount++ + if setCount == 5 { + return errors.New("session disappeared") + } + } + return nil + } + got := a.SetDesired(context.Background(), false, nil) + if got.Err == nil || !strings.Contains(got.Err.Error(), "session disappeared") || got.Inspection.State != target.Drifted || setCount != 5 || s.data == nil { + t.Fatalf("partial restore = %#v sets=%d metadata=%q", got, setCount, s.data) + } + r.fail = nil + // A partial restore loses ownership evidence and must not overwrite the + // mixed settings on retry. + before := len(r.calls) + got = a.SetDesired(context.Background(), false, nil) + if got.Err == nil || !strings.Contains(got.Err.Error(), "ownership") { + t.Fatalf("unsafe retry = %#v", got) + } + for _, c := range r.calls[before:] { + if len(c.Args) > 0 && c.Args[0] == "set" { + t.Fatalf("retry mutated partial external state: %#v", c) + } + } + }) +} + +func TestRestoreMetadataDeleteFailureCanBeSafelyRetried(t *testing.T) { + r, s := newRunner(), &memoryState{} + r.values["org.gnome.system.proxy mode"] = "'auto'" + r.values["org.gnome.system.proxy autoconfig-url"] = "'https://old.example/proxy.pac'" + a := adapter(r, s) + if got := a.SetDesired(context.Background(), true, active()); got.Err != nil { + t.Fatal(got.Err) + } + s.deleteErr = os.ErrPermission + first := a.SetDesired(context.Background(), false, nil) + if first.Err == nil || !errors.Is(first.Err, os.ErrPermission) || r.values["org.gnome.system.proxy mode"] != "'auto'" || s.data == nil { + t.Fatalf("first disable = %#v values=%#v metadata=%q", first, r.values, s.data) + } + s.deleteErr = nil + before := len(r.calls) + retry := a.SetDesired(context.Background(), false, nil) + if retry.Err != nil || retry.Inspection.State != target.External || s.data != nil { + t.Fatalf("retry disable = %#v metadata=%q", retry, s.data) + } + for _, c := range r.calls[before:] { + if len(c.Args) > 0 && c.Args[0] == "set" { + t.Fatalf("cleanup retry rewrote restored settings: %#v", c) + } + } +} + +func TestFileStateLockSerializesGNOMEOperationsAcrossProcesses(t *testing.T) { + if path := os.Getenv("PROXYCTL_GNOME_LOCK_HELPER_PATH"); path != "" { + r := newRunner() + state := gnome.NewFileState(path, -1, -1, filepath.Dir(filepath.Dir(path))) + a := gnome.New(gnome.Configuration{UID: os.Getuid(), GID: os.Getgid(), HomeDir: t.TempDir(), StateFile: path}, r, state) + got := a.SetDesired(context.Background(), true, active()) + if got.Err != nil { + t.Fatal(got.Err) + } + return + } + + path := filepath.Join(t.TempDir(), "proxyctl", "gnome.json") + state := gnome.NewFileState(path, -1, -1, filepath.Dir(filepath.Dir(path))) + lock, err := state.Lock() + if err != nil { + t.Fatal(err) + } + command := exec.Command(os.Args[0], "-test.run=^TestFileStateLockSerializesGNOMEOperationsAcrossProcesses$") + command.Env = append(os.Environ(), "PROXYCTL_GNOME_LOCK_HELPER_PATH="+path) + done := make(chan error, 1) + go func() { done <- command.Run() }() + select { + case err := <-done: + t.Fatalf("child operation did not wait for lock: %v", err) + case <-time.After(100 * time.Millisecond): + } + if err := lock.Close(); err != nil { + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatalf("child operation after unlock: %v", err) + } + case <-time.After(5 * time.Second): + _ = command.Process.Kill() + t.Fatal("child operation remained blocked after unlock") + } +} + func TestFailuresAreImmediateTruthfulAndPreserveOwnership(t *testing.T) { t.Run("permission before mutation", func(t *testing.T) { r, s := newRunner(), &memoryState{} diff --git a/internal/securefs/securefs.go b/internal/securefs/securefs.go index ad882ac..5042011 100644 --- a/internal/securefs/securefs.go +++ b/internal/securefs/securefs.go @@ -55,6 +55,50 @@ type Store struct { Owner *Owner } +type fileLock struct{ file *os.File } + +func (l *fileLock) Close() error { + unlockErr := unix.Flock(int(l.file.Fd()), unix.LOCK_UN) + closeErr := l.file.Close() + if unlockErr != nil { + return unlockErr + } + return closeErr +} + +// LockFile acquires an exclusive advisory lock without following symbolic +// links. The returned closer releases the lock and closes its descriptor. +func (s Store) LockFile(path string) (io.Closer, error) { + directory, base, err := s.openParent(path, true) + if err != nil { + return nil, err + } + defer directory.Close() + file, err := OpenAt(directory, base, unix.O_CREAT|unix.O_RDWR, 0o600) + if err != nil { + return nil, err + } + if err := regular(file); err != nil { + _ = file.Close() + return nil, err + } + if s.Owner != nil && (s.Owner.UID != os.Geteuid() || s.Owner.GID != os.Getegid()) { + if err := file.Chown(s.Owner.UID, s.Owner.GID); err != nil { + _ = file.Close() + return nil, err + } + } + if err := file.Chmod(0o600); err != nil { + _ = file.Close() + return nil, err + } + if err := unix.Flock(int(file.Fd()), unix.LOCK_EX); err != nil { + _ = file.Close() + return nil, err + } + return &fileLock{file: file}, nil +} + func (s Store) ReadFile(path string) ([]byte, error) { directory, base, err := s.openParent(path, false) if err != nil {