From 692a715248f17c9691adc735cfcbccac55dbfb98 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 03:26:06 +0800 Subject: [PATCH 1/2] feat: add Docker container defaults Target --- internal/dockerclient/configuration_test.go | 52 ++ internal/dockerclient/dockerclient.go | 531 ++++++++++++++++++++ internal/dockerclient/dockerclient_test.go | 459 +++++++++++++++++ 3 files changed, 1042 insertions(+) create mode 100644 internal/dockerclient/configuration_test.go create mode 100644 internal/dockerclient/dockerclient.go create mode 100644 internal/dockerclient/dockerclient_test.go diff --git a/internal/dockerclient/configuration_test.go b/internal/dockerclient/configuration_test.go new file mode 100644 index 0000000..808ff5f --- /dev/null +++ b/internal/dockerclient/configuration_test.go @@ -0,0 +1,52 @@ +package dockerclient_test + +import ( + "path/filepath" + "testing" + + "proxyctl/internal/dockerclient" + "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) { + value, ok := e.env[name] + return value, ok +} + +func TestConfigurationUsesInvokingUserAndIgnoresSudoRootOverrides(t *testing.T) { + owner := profile.UserIdentity{Username: "alice", HomeDir: "/home/alice", UID: 1000, GID: 1000} + got, err := dockerclient.ResolveConfiguration(identityEnvironment{ + current: profile.UserIdentity{Username: "root", HomeDir: "/root", UID: 0, GID: 0}, + users: map[string]profile.UserIdentity{"alice": owner}, + env: map[string]string{"SUDO_USER": "alice", "DOCKER_CONFIG": "/root/.docker", "XDG_STATE_HOME": "/root/state"}, + }) + if err != nil { + t.Fatal(err) + } + if got.ConfigFile != "/home/alice/.docker/config.json" || got.RecoveryFile != "/home/alice/.local/state/proxyctl/docker-containers.json" || got.UID != 1000 || got.GID != 1000 || !got.OwnFiles { + t.Fatalf("configuration = %#v", got) + } +} + +func TestConfigurationHonorsDirectOwnersAbsoluteDockerAndStateLocations(t *testing.T) { + got, err := dockerclient.ResolveConfiguration(identityEnvironment{ + current: profile.UserIdentity{Username: "root", HomeDir: "/root", UID: 0, GID: 0}, + env: map[string]string{"DOCKER_CONFIG": "/var/lib/root-docker", "XDG_STATE_HOME": "/var/lib/root-state"}, + }) + if err != nil { + t.Fatal(err) + } + if got.ConfigFile != filepath.Join("/var/lib/root-docker", "config.json") || got.RecoveryFile != filepath.Join("/var/lib/root-state", "proxyctl", "docker-containers.json") || got.UID != 0 { + t.Fatalf("configuration = %#v", got) + } +} diff --git a/internal/dockerclient/dockerclient.go b/internal/dockerclient/dockerclient.go new file mode 100644 index 0000000..fb769be --- /dev/null +++ b/internal/dockerclient/dockerclient.go @@ -0,0 +1,531 @@ +// Package dockerclient implements the Docker container/build defaults Target. +// It only edits the owning user's Docker client config.json and never contacts +// or changes a Docker daemon. +package dockerclient + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io/fs" + "net/url" + "os" + "path/filepath" + "strings" + + "proxyctl/internal/profile" + "proxyctl/internal/securefs" + "proxyctl/internal/target" +) + +var managedKeys = []string{"httpProxy", "httpsProxy", "allProxy", "noProxy"} + +type Configuration struct { + ConfigFile string + RecoveryFile string + OwnFiles bool + UID int + GID int + TrustedRoots []string +} + +// ResolveConfiguration keeps Docker client effects with the invoking user when +// proxyctl itself was explicitly started through sudo. +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) + } + invokingUser := current.Username != owner.Username + + dockerDirectory := "" + if value, ok := environment.LookupEnv("DOCKER_CONFIG"); ok && filepath.IsAbs(value) { + if !invokingUser || pathWithin(value, owner.HomeDir) { + dockerDirectory = value + } + } + if dockerDirectory == "" { + dockerDirectory = filepath.Join(owner.HomeDir, ".docker") + } + stateDirectory := "" + if value, ok := environment.LookupEnv("XDG_STATE_HOME"); ok && filepath.IsAbs(value) { + if !invokingUser || pathWithin(value, owner.HomeDir) { + stateDirectory = value + } + } + if stateDirectory == "" { + stateDirectory = filepath.Join(owner.HomeDir, ".local", "state") + } + + roots := []string{owner.HomeDir} + if !pathWithin(dockerDirectory, owner.HomeDir) { + roots = append(roots, filepath.Dir(dockerDirectory)) + } + if !pathWithin(stateDirectory, owner.HomeDir) { + roots = append(roots, filepath.Dir(stateDirectory)) + } + return Configuration{ + ConfigFile: filepath.Join(dockerDirectory, "config.json"), + RecoveryFile: filepath.Join(stateDirectory, "proxyctl", "docker-containers.json"), + OwnFiles: true, UID: owner.UID, GID: owner.GID, TrustedRoots: roots, + }, 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 Ownership struct{ UID, GID int } + +type FileSystem interface { + ReadFile(string) ([]byte, error) + AtomicWrite(string, []byte, os.FileMode, *Ownership) error + Remove(string) error +} + +type OSFileSystem struct{ TrustedRoots []string } + +func NewOSFileSystem(configuration Configuration) OSFileSystem { + return OSFileSystem{TrustedRoots: append([]string(nil), configuration.TrustedRoots...)} +} +func (f OSFileSystem) store(path string, owner *Ownership) securefs.Store { + root := "" + if len(f.TrustedRoots) == 0 { + root = string(filepath.Separator) + } + for _, candidate := range f.TrustedRoots { + if pathWithin(path, candidate) && (root == "" || len(filepath.Clean(candidate)) > len(filepath.Clean(root))) { + root = candidate + } + } + var secureOwner *securefs.Owner + if owner != nil { + secureOwner = &securefs.Owner{UID: owner.UID, GID: owner.GID} + } + return securefs.Store{TrustedRoot: root, Owner: secureOwner} +} +func (f OSFileSystem) ReadFile(path string) ([]byte, error) { return f.store(path, nil).ReadFile(path) } +func (f OSFileSystem) Remove(path string) error { return f.store(path, nil).Remove(path) } +func (f OSFileSystem) AtomicWrite(path string, data []byte, mode os.FileMode, owner *Ownership) error { + return f.store(path, owner).AtomicWrite(path, data, mode) +} + +type priorValue struct { + Present bool `json:"present"` + Value json.RawMessage `json:"value,omitempty"` +} +type recovery struct { + ConfigPresent bool `json:"configPresent"` + ProxiesPresent bool `json:"proxiesPresent"` + DefaultPresent bool `json:"defaultPresent"` + Original map[string]priorValue `json:"original"` + Fingerprint string `json:"fingerprint"` +} + +type Adapter struct { + configuration Configuration + files FileSystem +} + +var _ target.Target = (*Adapter)(nil) + +func New(configuration Configuration, files FileSystem) *Adapter { + if files == nil { + configured := NewOSFileSystem(configuration) + files = configured + } + return &Adapter{configuration: configuration, files: files} +} + +func (a *Adapter) ID() string { return "docker-containers" } +func (a *Adapter) Description() target.Description { + return target.Description{ + Name: "Docker containers/builds", + Summary: "Sets proxy defaults injected into future Docker containers and builds.", + Details: "Affects new containers and builds through the owning user's Docker client configuration; it does not affect existing containers and is not Docker daemon registry access. It requires write access to the user's Docker configuration and does not contact the daemon.", + } +} +func (a *Adapter) Availability(context.Context) target.Availability { + if !a.usablePath(a.configuration.ConfigFile) || !a.usablePath(a.configuration.RecoveryFile) { + return target.Availability{Evidence: "standard Docker client configuration paths are not usable for the owning user"} + } + return target.Availability{Available: true, Evidence: "standard Docker client configuration is available for the owning user"} +} + +func (a *Adapter) usablePath(path string) bool { + if !filepath.IsAbs(path) { + return false + } + if len(a.configuration.TrustedRoots) == 0 { + return true + } + for _, root := range a.configuration.TrustedRoots { + if filepath.IsAbs(root) && pathWithin(path, root) && filepath.Clean(path) != filepath.Clean(root) { + return true + } + } + return false +} +func (a *Adapter) ConfigPath() string { return a.configuration.ConfigFile } +func (a *Adapter) RecoveryPath() string { return a.configuration.RecoveryFile } + +func (a *Adapter) Inspect(_ context.Context, active *profile.Profile) target.Inspection { + metadata, managed, metadataErr := a.readRecovery() + config, _, configErr := a.readConfig() + if managed || (metadataErr != nil && !errors.Is(metadataErr, fs.ErrNotExist)) { + if metadataErr != nil { + return target.Inspection{State: target.Drifted, Evidence: "managed Docker client recovery metadata cannot be read"} + } + if configErr != nil { + return target.Inspection{State: target.Drifted, Evidence: "managed Docker client configuration is missing, unreadable, or not safely mergeable"} + } + desired, desiredErr := desiredDefaults(active) + if desiredErr == nil && fingerprintCurrent(config) == metadata.Fingerprint && fingerprintDefaults(desired) == metadata.Fingerprint { + return target.Inspection{State: target.Enabled, Evidence: "managed Docker client proxy defaults match the Active Profile"} + } + return target.Inspection{State: target.Drifted, Evidence: "managed Docker client proxy defaults do not match the Active Profile"} + } + if configErr == nil && recognizesProxyDefaults(config) { + return target.Inspection{State: target.External, Evidence: "recognized proxy defaults exist in Docker client config.json"} + } + return target.Inspection{State: target.Disabled, Evidence: "no recognized Docker client proxy defaults detected"} +} + +func (a *Adapter) SetDesired(ctx context.Context, enabled bool, active *profile.Profile) target.OperationResult { + if !a.Availability(ctx).Available { + return target.OperationResult{Inspection: a.Inspect(ctx, active), Err: errors.New("Docker containers/builds Target is unavailable: standard Docker client configuration cannot be used")} + } + 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")} + } + if enabled && a.Inspect(ctx, active).State == target.Enabled { + return target.OperationResult{Inspection: a.Inspect(ctx, active)} + } + var err error + if enabled { + err = a.enable(active) + } else { + err = a.disable() + } + inspection := a.Inspect(ctx, active) + if err != nil { + err = redactError(err, active) + } + return target.OperationResult{Inspection: inspection, Err: err} +} + +func (a *Adapter) enable(active *profile.Profile) error { + desired, err := desiredDefaults(active) + if err != nil { + return err + } + config, configPresent, err := a.readConfig() + if err != nil { + return fmt.Errorf("read Docker client configuration for safe JSON merge: %w", err) + } + metadata, managed, recoveryErr := a.readRecovery() + if recoveryErr != nil && !errors.Is(recoveryErr, fs.ErrNotExist) { + return fmt.Errorf("read Docker client recovery metadata: %w", recoveryErr) + } + proxies, proxiesPresent, err := childObject(config, "proxies") + if err != nil { + return err + } + defaults, defaultPresent, err := childObject(proxies, "default") + if err != nil { + return err + } + if !managed { + metadata = recovery{ConfigPresent: configPresent, ProxiesPresent: proxiesPresent, DefaultPresent: defaultPresent, Original: make(map[string]priorValue)} + for _, key := range managedKeys { + value, present := defaults[key] + metadata.Original[key] = priorValue{Present: present, Value: cloneRaw(value)} + } + } + metadata.Fingerprint = fingerprintDefaults(desired) + metadataData, err := json.MarshalIndent(metadata, "", " ") + if err != nil { + return err + } + if err := a.writeOwned(a.configuration.RecoveryFile, append(metadataData, '\n')); err != nil { + return fmt.Errorf("persist Docker client recovery metadata before changing proxy defaults: %w", err) + } + applyDesired(defaults, desired) + setChildObject(proxies, "default", defaults) + setChildObject(config, "proxies", proxies) + if err := a.writeConfig(config); err != nil { + return fmt.Errorf("write Docker client proxy defaults after recovery metadata persisted: %w", err) + } + return nil +} + +func (a *Adapter) disable() error { + metadata, managed, err := a.readRecovery() + if errors.Is(err, fs.ErrNotExist) || !managed { + return nil + } + if err != nil { + return fmt.Errorf("read Docker client recovery metadata: %w", err) + } + config, configPresent, err := a.readConfig() + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("read Docker client configuration for restoration: %w", err) + } + if !configPresent { + config = make(map[string]json.RawMessage) + } + proxies, _, err := childObject(config, "proxies") + if err != nil { + return err + } + defaults, _, err := childObject(proxies, "default") + if err != nil { + return err + } + for _, key := range managedKeys { + prior := metadata.Original[key] + if prior.Present { + defaults[key] = cloneRaw(prior.Value) + } else { + delete(defaults, key) + } + } + if len(defaults) == 0 && !metadata.DefaultPresent { + delete(proxies, "default") + } else { + setChildObject(proxies, "default", defaults) + } + if len(proxies) == 0 && !metadata.ProxiesPresent { + delete(config, "proxies") + } else { + setChildObject(config, "proxies", proxies) + } + if len(config) == 0 && !metadata.ConfigPresent { + if err := a.files.Remove(a.configuration.ConfigFile); err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove newly created empty Docker client configuration: %w", err) + } + } else if err := a.writeConfig(config); err != nil { + return fmt.Errorf("restore prior Docker client proxy defaults: %w", err) + } + if err := a.files.Remove(a.configuration.RecoveryFile); err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove Docker client recovery metadata after restoration: %w", err) + } + return nil +} + +func desiredDefaults(active *profile.Profile) (map[string]string, error) { + if active == nil { + return nil, errors.New("Active Profile is required") + } + normalized, err := profile.ValidateAndNormalize(*active) + if err != nil { + return nil, err + } + for field, raw := range map[string]string{"HTTP proxy": normalized.HTTPURL, "HTTPS proxy": normalized.HTTPSURL} { + if raw == "" { + continue + } + parsed, _ := url.Parse(raw) + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return nil, fmt.Errorf("%s scheme %s cannot be represented by Docker's documented %sProxy default", field, parsed.Scheme, strings.ToLower(strings.Fields(field)[0])) + } + } + return map[string]string{ + "httpProxy": normalized.HTTPURL, + "httpsProxy": normalized.HTTPSURL, + "allProxy": normalized.AllURL, + "noProxy": strings.Join(normalized.Bypass, ","), + }, nil +} + +func (a *Adapter) readRecovery() (recovery, bool, error) { + data, err := a.files.ReadFile(a.configuration.RecoveryFile) + if err != nil { + return recovery{}, false, err + } + var value recovery + if err := json.Unmarshal(data, &value); err != nil { + return recovery{}, true, err + } + if value.Original == nil { + return recovery{}, true, errors.New("Docker client recovery metadata is incomplete") + } + fingerprintBytes, fingerprintErr := hex.DecodeString(value.Fingerprint) + if fingerprintErr != nil || len(fingerprintBytes) != sha256.Size { + return recovery{}, true, errors.New("Docker client recovery metadata has an invalid fingerprint") + } + for _, key := range managedKeys { + prior, present := value.Original[key] + if !present || (prior.Present && !json.Valid(prior.Value)) { + return recovery{}, true, errors.New("Docker client recovery metadata is incomplete") + } + } + return value, true, nil +} + +func (a *Adapter) readConfig() (map[string]json.RawMessage, bool, error) { + data, err := a.files.ReadFile(a.configuration.ConfigFile) + if errors.Is(err, fs.ErrNotExist) { + return make(map[string]json.RawMessage), false, nil + } + if err != nil { + return nil, false, err + } + object, err := decodeObject(data, "Docker client configuration") + return object, true, err +} + +func decodeObject(data []byte, description string) (map[string]json.RawMessage, error) { + var object map[string]json.RawMessage + if err := json.Unmarshal(data, &object); err != nil { + return nil, fmt.Errorf("parse %s: %w", description, err) + } + if object == nil { + return nil, fmt.Errorf("%s must be a JSON object", description) + } + return object, nil +} +func childObject(parent map[string]json.RawMessage, key string) (map[string]json.RawMessage, bool, error) { + raw, present := parent[key] + if !present { + return make(map[string]json.RawMessage), false, nil + } + object, err := decodeObject(raw, "Docker client "+key+" setting") + return object, true, err +} +func setChildObject(parent map[string]json.RawMessage, key string, child map[string]json.RawMessage) { + data, _ := json.Marshal(child) + parent[key] = data +} +func applyDesired(defaults map[string]json.RawMessage, desired map[string]string) { + for _, key := range managedKeys { + if value := desired[key]; value != "" { + data, _ := json.Marshal(value) + defaults[key] = data + } else { + delete(defaults, key) + } + } +} +func recognizesProxyDefaults(config map[string]json.RawMessage) bool { + proxies, _, err := childObject(config, "proxies") + if err != nil { + return false + } + defaults, _, err := childObject(proxies, "default") + if err != nil { + return false + } + for _, key := range append(append([]string(nil), managedKeys...), "ftpProxy") { + var value string + if raw, ok := defaults[key]; ok && json.Unmarshal(raw, &value) == nil && value != "" { + return true + } + } + return false +} + +type managedValue struct { + Present bool `json:"present"` + Valid bool `json:"valid"` + Value string `json:"value,omitempty"` +} + +func fingerprintDefaults(values map[string]string) string { + snapshot := make(map[string]managedValue, len(managedKeys)) + for _, key := range managedKeys { + if value := values[key]; value != "" { + snapshot[key] = managedValue{Present: true, Valid: true, Value: value} + } else { + snapshot[key] = managedValue{} + } + } + return fingerprintSnapshot(snapshot) +} + +func fingerprintCurrent(config map[string]json.RawMessage) string { + snapshot := make(map[string]managedValue, len(managedKeys)) + proxies, _, proxyErr := childObject(config, "proxies") + defaults, _, defaultErr := childObject(proxies, "default") + for _, key := range managedKeys { + raw, present := defaults[key] + value := managedValue{Present: present} + if present && proxyErr == nil && defaultErr == nil && json.Unmarshal(raw, &value.Value) == nil { + value.Valid = true + } + snapshot[key] = value + } + return fingerprintSnapshot(snapshot) +} + +func fingerprintSnapshot(snapshot map[string]managedValue) string { + data, _ := json.Marshal(snapshot) + sum := sha256.Sum256(data) + return hex.EncodeToString(sum[:]) +} +func cloneRaw(value json.RawMessage) json.RawMessage { return append(json.RawMessage(nil), value...) } +func (a *Adapter) ownership() *Ownership { + if !a.configuration.OwnFiles { + return nil + } + return &Ownership{UID: a.configuration.UID, GID: a.configuration.GID} +} +func (a *Adapter) writeOwned(path string, data []byte) error { + return a.files.AtomicWrite(path, data, 0o600, a.ownership()) +} +func (a *Adapter) writeConfig(config map[string]json.RawMessage) error { + data, err := json.MarshalIndent(config, "", " ") + if err != nil { + return err + } + return a.writeOwned(a.configuration.ConfigFile, append(data, '\n')) +} + +type redactedError struct { + message string + class error +} + +func (e *redactedError) Error() string { return e.message } +func (e *redactedError) Is(other error) bool { return e.class != nil && errors.Is(e.class, other) } +func redactError(err error, active *profile.Profile) error { + message := profile.RedactText(err.Error()) + if active != nil { + for _, raw := range []string{active.SingleURL, active.HTTPURL, active.HTTPSURL, active.AllURL} { + if raw != "" { + message = strings.ReplaceAll(message, raw, profile.ObscureURL(raw)) + } + } + } + var class error + for _, safe := range []error{os.ErrPermission, fs.ErrNotExist, fs.ErrExist, context.Canceled, context.DeadlineExceeded} { + if errors.Is(err, safe) { + class = safe + break + } + } + return &redactedError{message: message, class: class} +} diff --git a/internal/dockerclient/dockerclient_test.go b/internal/dockerclient/dockerclient_test.go new file mode 100644 index 0000000..aa5c4c8 --- /dev/null +++ b/internal/dockerclient/dockerclient_test.go @@ -0,0 +1,459 @@ +package dockerclient_test + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "proxyctl/internal/dockerclient" + "proxyctl/internal/profile" + "proxyctl/internal/target" + "proxyctl/internal/target/targettest" +) + +func TestAvailabilityAndDescriptionDescribeOnlyDockerClientDefaults(t *testing.T) { + a := newAdapter(t, nil) + availability := a.Availability(context.Background()) + if !availability.Available || !strings.Contains(availability.Evidence, "Docker client configuration") { + t.Fatalf("availability = %#v", availability) + } + description := a.Description() + for _, phrase := range []string{"new containers and builds", "not Docker daemon registry access", "existing containers"} { + if !strings.Contains(description.Details, phrase) { + t.Errorf("details %q do not contain %q", description.Details, phrase) + } + } + if a.ID() != "docker-containers" { + t.Fatalf("ID = %q", a.ID()) + } + + root := t.TempDir() + outside := dockerclient.New(dockerclient.Configuration{ + ConfigFile: filepath.Join(t.TempDir(), "config.json"), RecoveryFile: filepath.Join(root, "state.json"), TrustedRoots: []string{root}, + }, nil) + if got := outside.Availability(context.Background()); got.Available { + t.Fatalf("path outside owning user's trusted configuration roots is available: %#v", got) + } +} + +func TestDockerClientAdapterRunsReusableTargetContract(t *testing.T) { + targettest.Run(t, func(t *testing.T) targettest.Fixture { + a := newAdapter(t, nil) + return targettest.Fixture{ + Target: a, Active: activeProfile(), Secret: "secret", + MakeExternal: func(t *testing.T) { + writeConfig(t, a.ConfigPath(), `{"auths":{"registry.example":{}},"proxies":{"default":{"httpProxy":"http://prior.example:3128"}}}`) + }, + MakeDrifted: func(t *testing.T) { + updateDefaultKey(t, a.ConfigPath(), "httpProxy", json.RawMessage(`"http://changed.example:8080"`)) + }, + PermissionFailure: func(t *testing.T) target.OperationResult { + failed := newAdapterWithFS(t, &failingFS{FileSystem: dockerclient.OSFileSystem{}, permission: true}) + return failed.SetDesired(context.Background(), true, activeProfile()) + }, + PartialFailure: func(t *testing.T) target.OperationResult { + failed := newAdapterWithFS(t, &failingFS{FileSystem: dockerclient.OSFileSystem{}, failConfig: true}) + return failed.SetDesired(context.Background(), true, activeProfile()) + }, + RedactionFailure: func(t *testing.T) target.OperationResult { + failed := newAdapterWithFS(t, &failingFS{FileSystem: dockerclient.OSFileSystem{}, leak: true}) + return failed.SetDesired(context.Background(), true, activeProfile()) + }, + RecoveryRetry: func(t *testing.T) (target.OperationResult, target.OperationResult, target.OperationResult) { + files := &failingFS{FileSystem: dockerclient.OSFileSystem{}, failRecoveryOnce: true} + retrying := newAdapterWithFS(t, files) + writeConfig(t, retrying.ConfigPath(), `{"proxies":{"default":{"httpProxy":"http://prior.example:3128"}}}`) + first := retrying.SetDesired(context.Background(), true, activeProfile()) + retry := retrying.SetDesired(context.Background(), true, activeProfile()) + disabled := retrying.SetDesired(context.Background(), false, nil) + return first, retry, disabled + }, + } + }) +} + +func TestEnableSafelyMergesAndDisableRestoresPriorDefaults(t *testing.T) { + a := newAdapter(t, nil) + original := `{ + "auths": {"registry.example": {"auth": "opaque"}}, + "credsStore": "pass", + "proxies": { + "tcp://remote.example:2376": {"httpProxy": "http://remote.example:3128"}, + "default": { + "httpProxy": "http://prior.example:3128", + "ftpProxy": "http://ftp.example:2121", + "customOption": {"keep": true} + } + }, + "pluginSetting": [1, 2, 3] +}` + writeConfig(t, a.ConfigPath(), original) + + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err != nil || got.Inspection.State != target.Enabled { + t.Fatalf("enable = %#v", got) + } + config := readObject(t, a.ConfigPath()) + if _, ok := config["auths"]; !ok { + t.Fatal("auths removed") + } + if _, ok := config["pluginSetting"]; !ok { + t.Fatal("unknown top-level setting removed") + } + proxies := nestedObject(t, config, "proxies") + if _, ok := proxies["tcp://remote.example:2376"]; !ok { + t.Fatal("per-daemon proxy setting removed") + } + defaults := nestedObject(t, proxies, "default") + assertJSONString(t, defaults, "httpProxy", "http://alice:secret@proxy.example:8080") + assertJSONString(t, defaults, "httpsProxy", "https://proxy.example:8443") + assertJSONString(t, defaults, "allProxy", "socks5h://proxy.example:1080") + assertJSONString(t, defaults, "noProxy", "localhost,.example.com,10.0.0.0/8") + if _, ok := defaults["ftpProxy"]; !ok { + t.Fatal("unmanaged default option removed") + } + + // An unrelated edit made while enabled must survive restoration. + config["detachKeys"] = json.RawMessage(`"ctrl-x,x"`) + writeObject(t, a.ConfigPath(), config) + if got := a.SetDesired(context.Background(), false, nil); got.Err != nil || got.Inspection.State != target.External { + t.Fatalf("disable = %#v", got) + } + restored := readObject(t, a.ConfigPath()) + if _, ok := restored["detachKeys"]; !ok { + t.Fatal("external unrelated edit was removed") + } + defaults = nestedObject(t, nestedObject(t, restored, "proxies"), "default") + assertJSONString(t, defaults, "httpProxy", "http://prior.example:3128") + for _, key := range []string{"httpsProxy", "allProxy", "noProxy"} { + if _, ok := defaults[key]; ok { + t.Errorf("managed key %s was not restored to absence", key) + } + } + if _, ok := defaults["customOption"]; !ok { + t.Fatal("custom default option removed") + } +} + +func TestInspectionDistinguishesAllStatesAndManagedValueEditsDrift(t *testing.T) { + a := newAdapter(t, nil) + assertState(t, a, target.Disabled) + writeConfig(t, a.ConfigPath(), `{"proxies":{"default":{"ftpProxy":"http://ftp.example:2121"}}}`) + assertState(t, a, target.External) + writeConfig(t, a.ConfigPath(), `{"proxies":{"default":{"noProxy":"localhost"}}}`) + assertState(t, a, target.External) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err != nil { + t.Fatal(got.Err) + } + assertState(t, a, target.Enabled) + updateDefaultKey(t, a.ConfigPath(), "noProxy", json.RawMessage(`"externally-edited.example"`)) + assertState(t, a, target.Drifted) +} + +func TestAddingEvenAnEmptyOrInvalidManagedValueIsDrift(t *testing.T) { + for _, value := range []json.RawMessage{json.RawMessage(`""`), json.RawMessage(`[]`)} { + a := newAdapter(t, nil) + p := *activeProfile() + p.AllURL = "" + if got := a.SetDesired(context.Background(), true, &p); got.Err != nil { + t.Fatal(got.Err) + } + updateDefaultKey(t, a.ConfigPath(), "allProxy", value) + if got := a.Inspect(context.Background(), &p); got.State != target.Drifted { + t.Fatalf("state = %s (%s), want Drifted", got.State, got.Evidence) + } + } +} + +func TestDocumentedBypassFormsAreRepresentedWithoutLoss(t *testing.T) { + a := newAdapter(t, nil) + p := *activeProfile() + p.Bypass = []string{"host.example", ".example.com", "192.0.2.1", "2001:db8::1", "10.0.0.0/8", "2001:db8::/32"} + if got := a.SetDesired(context.Background(), true, &p); got.Err != nil { + t.Fatal(got.Err) + } + defaults := nestedObject(t, nestedObject(t, readObject(t, a.ConfigPath()), "proxies"), "default") + assertJSONString(t, defaults, "noProxy", "host.example,.example.com,192.0.2.1,2001:db8::1,10.0.0.0/8,2001:db8::/32") +} + +func TestUnsupportedProfileDataPreventsAnyMutation(t *testing.T) { + for _, tc := range []struct { + name string + edit func(*profile.Profile) + want string + }{ + {"SOCKS HTTP proxy", func(p *profile.Profile) { p.HTTPURL = "socks5://proxy.example:1080" }, "HTTP proxy"}, + {"SOCKS HTTPS proxy", func(p *profile.Profile) { p.HTTPSURL = "socks5h://proxy.example:1080" }, "HTTPS proxy"}, + {"invalid bypass", func(p *profile.Profile) { p.Bypass = []string{"host:8080"} }, "Bypass"}, + } { + t.Run(tc.name, func(t *testing.T) { + a := newAdapter(t, nil) + writeConfig(t, a.ConfigPath(), `{"auths":{"keep":{}}}`) + before := readFile(t, a.ConfigPath()) + p := *activeProfile() + tc.edit(&p) + got := a.SetDesired(context.Background(), true, &p) + if got.Err == nil || !strings.Contains(got.Err.Error(), tc.want) || got.Inspection.State != target.Disabled { + t.Fatalf("result = %#v", got) + } + if after := readFile(t, a.ConfigPath()); after != before { + t.Fatalf("configuration mutated:\nbefore %s\nafter %s", before, after) + } + if _, err := os.Stat(a.RecoveryPath()); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("recovery metadata created: %v", err) + } + }) + } +} + +func TestIncompleteRecoveryMetadataPreventsUnsafeRestoration(t *testing.T) { + a := newAdapter(t, nil) + writeConfig(t, a.ConfigPath(), `{"proxies":{"default":{"httpProxy":"http://must-not-be-deleted.example:3128"}}}`) + writeConfig(t, a.RecoveryPath(), `{"configPresent":true,"proxiesPresent":true,"defaultPresent":true,"original":{},"fingerprint":"invalid"}`) + before := readFile(t, a.ConfigPath()) + got := a.SetDesired(context.Background(), false, nil) + if got.Err == nil || got.Inspection.State != target.Drifted { + t.Fatalf("disable = %#v", got) + } + if after := readFile(t, a.ConfigPath()); after != before { + t.Fatalf("incomplete recovery metadata changed config: %s", after) + } +} + +func TestUnmergeableJSONPreventsMutation(t *testing.T) { + for _, contents := range []string{`not json`, `[]`, `{"proxies":"wrong"}`, `{"proxies":{"default":[]}}`} { + t.Run(contents, func(t *testing.T) { + a := newAdapter(t, nil) + writeConfig(t, a.ConfigPath(), contents) + before := readFile(t, a.ConfigPath()) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err == nil { + t.Fatalf("enable accepted %q", contents) + } + if after := readFile(t, a.ConfigPath()); after != before { + t.Fatalf("unsafe JSON merge changed configuration to %q", after) + } + }) + } +} + +func TestWritesAreOwnerOnlyAtomicAndIdempotent(t *testing.T) { + files := &countingFS{FileSystem: dockerclient.OSFileSystem{}} + a := newAdapterWithFS(t, files) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err != nil { + t.Fatal(got.Err) + } + writes := files.writes + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err != nil { + t.Fatal(got.Err) + } + if files.writes != writes { + t.Fatalf("idempotent enable performed %d additional writes", files.writes-writes) + } + for _, path := range []string{a.ConfigPath(), a.RecoveryPath()} { + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o600 { + t.Errorf("%s mode = %o", path, info.Mode().Perm()) + } + matches, _ := filepath.Glob(filepath.Join(filepath.Dir(path), ".proxyctl-*")) + if len(matches) != 0 { + t.Errorf("atomic temporary files remain beside %s: %v", path, matches) + } + } +} + +func TestOwnedWritesUseConfigurationOwner(t *testing.T) { + root := t.TempDir() + files := &ownershipFS{FileSystem: dockerclient.OSFileSystem{}} + a := dockerclient.New(dockerclient.Configuration{ConfigFile: filepath.Join(root, ".docker", "config.json"), RecoveryFile: filepath.Join(root, ".state", "proxyctl", "docker-containers.json"), OwnFiles: true, UID: 1234, GID: 1235}, files) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err != nil { + t.Fatal(got.Err) + } + if len(files.owners) != 2 { + t.Fatalf("owners = %#v", files.owners) + } + for _, owner := range files.owners { + if owner == nil || owner.UID != 1234 || owner.GID != 1235 { + t.Fatalf("owner = %#v", owner) + } + } +} + +func TestDescriptorAnchoredWritesDoNotFollowDockerConfigSymlinks(t *testing.T) { + root, victim := t.TempDir(), t.TempDir() + if err := os.Symlink(victim, filepath.Join(root, ".docker")); err != nil { + t.Fatal(err) + } + a := dockerclient.New(dockerclient.Configuration{ConfigFile: filepath.Join(root, ".docker", "config.json"), RecoveryFile: filepath.Join(root, "state", "docker-containers.json"), TrustedRoots: []string{root}}, nil) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err == nil { + t.Fatal("enable followed symlinked Docker configuration ancestor") + } + entries, err := os.ReadDir(victim) + if err != nil || len(entries) != 0 { + t.Fatalf("victim changed: %v, %v", entries, err) + } +} + +func TestErrorsRedactCredentialsIncludingRestoreFailures(t *testing.T) { + files := &failingFS{FileSystem: dockerclient.OSFileSystem{}} + a := newAdapterWithFS(t, files) + writeConfig(t, a.ConfigPath(), `{"proxies":{"default":{"httpProxy":"http://bob:prior-secret@prior.example:3128"}}}`) + if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err != nil { + t.Fatal(got.Err) + } + files.failConfig = true + files.restoreLeak = true + got := a.SetDesired(context.Background(), false, nil) + if got.Err == nil || strings.Contains(got.Err.Error(), "prior-secret") || strings.Contains(got.Err.Error(), "secret") { + t.Fatalf("unredacted restore result = %#v", got) + } +} + +func activeProfile() *profile.Profile { + return &profile.Profile{Name: "office", Mode: profile.AdvancedMode, HTTPURL: "http://alice:secret@proxy.example:8080", HTTPSURL: "https://proxy.example:8443", AllURL: "socks5h://proxy.example:1080", Bypass: []string{"localhost", ".example.com", "10.0.0.0/8"}} +} + +func newAdapter(t *testing.T, files dockerclient.FileSystem) *dockerclient.Adapter { + t.Helper() + if files == nil { + files = dockerclient.OSFileSystem{} + } + return newAdapterWithFS(t, files) +} + +func newAdapterWithFS(t *testing.T, files dockerclient.FileSystem) *dockerclient.Adapter { + t.Helper() + root := t.TempDir() + return dockerclient.New(dockerclient.Configuration{ConfigFile: filepath.Join(root, ".docker", "config.json"), RecoveryFile: filepath.Join(root, ".state", "proxyctl", "docker-containers.json")}, files) +} + +func writeConfig(t *testing.T, path, contents string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(contents), 0o600); err != nil { + t.Fatal(err) + } +} + +func readFile(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(data) +} + +func readObject(t *testing.T, path string) map[string]json.RawMessage { + t.Helper() + var object map[string]json.RawMessage + if err := json.Unmarshal([]byte(readFile(t, path)), &object); err != nil { + t.Fatal(err) + } + return object +} + +func writeObject(t *testing.T, path string, object map[string]json.RawMessage) { + t.Helper() + data, err := json.Marshal(object) + if err != nil { + t.Fatal(err) + } + writeConfig(t, path, string(data)) +} + +func nestedObject(t *testing.T, parent map[string]json.RawMessage, key string) map[string]json.RawMessage { + t.Helper() + var object map[string]json.RawMessage + if err := json.Unmarshal(parent[key], &object); err != nil { + t.Fatalf("%s is not an object: %v", key, err) + } + return object +} + +func updateDefaultKey(t *testing.T, path, key string, value json.RawMessage) { + t.Helper() + config := readObject(t, path) + proxies := nestedObject(t, config, "proxies") + defaults := nestedObject(t, proxies, "default") + defaults[key] = value + defaultData, err := json.Marshal(defaults) + if err != nil { + t.Fatal(err) + } + proxies["default"] = defaultData + proxyData, err := json.Marshal(proxies) + if err != nil { + t.Fatal(err) + } + config["proxies"] = proxyData + writeObject(t, path, config) +} + +func assertJSONString(t *testing.T, object map[string]json.RawMessage, key, want string) { + t.Helper() + var got string + if err := json.Unmarshal(object[key], &got); err != nil || got != want { + t.Fatalf("%s = %q (%v), want %q", key, got, err, want) + } +} + +func assertState(t *testing.T, a *dockerclient.Adapter, want target.State) { + t.Helper() + got := a.Inspect(context.Background(), activeProfile()) + if got.State != want { + t.Fatalf("state = %s (%s), want %s", got.State, got.Evidence, want) + } +} + +type failingFS struct { + dockerclient.FileSystem + permission, failConfig, leak, restoreLeak, failRecoveryOnce bool +} + +func (f *failingFS) AtomicWrite(path string, data []byte, mode os.FileMode, owner *dockerclient.Ownership) error { + if f.permission { + return os.ErrPermission + } + if f.failRecoveryOnce && strings.HasSuffix(path, "docker-containers.json") { + f.failRecoveryOnce = false + return os.ErrPermission + } + if f.leak { + return errors.New("write http://alice:secret@proxy.example:8080 failed") + } + if f.failConfig && strings.HasSuffix(path, "config.json") { + if f.restoreLeak { + return errors.New("restore http://bob:prior-secret@prior.example:3128 and http://alice:secret@proxy.example:8080 failed") + } + return os.ErrPermission + } + return f.FileSystem.AtomicWrite(path, data, mode, owner) +} + +type countingFS struct { + dockerclient.FileSystem + writes int +} + +func (f *countingFS) AtomicWrite(path string, data []byte, mode os.FileMode, owner *dockerclient.Ownership) error { + f.writes++ + return f.FileSystem.AtomicWrite(path, data, mode, owner) +} + +type ownershipFS struct { + dockerclient.FileSystem + owners []*dockerclient.Ownership +} + +func (f *ownershipFS) AtomicWrite(path string, data []byte, mode os.FileMode, owner *dockerclient.Ownership) error { + f.owners = append(f.owners, owner) + return f.FileSystem.AtomicWrite(path, data, mode, nil) +} From 4c7a9c36e80f27e7ece93d8cb03432420db448c8 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 03:41:03 +0800 Subject: [PATCH 2/2] fix: serialize Docker client recovery mutations --- internal/dockerclient/dockerclient.go | 37 +++-- internal/dockerclient/dockerclient_test.go | 153 ++++++++++++++++++++- internal/securefs/securefs.go | 31 +++++ 3 files changed, 209 insertions(+), 12 deletions(-) diff --git a/internal/dockerclient/dockerclient.go b/internal/dockerclient/dockerclient.go index fb769be..b39fc72 100644 --- a/internal/dockerclient/dockerclient.go +++ b/internal/dockerclient/dockerclient.go @@ -104,6 +104,7 @@ type FileSystem interface { ReadFile(string) ([]byte, error) AtomicWrite(string, []byte, os.FileMode, *Ownership) error Remove(string) error + WithLock(string, *Ownership, func() error) error } type OSFileSystem struct{ TrustedRoots []string } @@ -132,6 +133,9 @@ func (f OSFileSystem) Remove(path string) error { return f.store(pat func (f OSFileSystem) AtomicWrite(path string, data []byte, mode os.FileMode, owner *Ownership) error { return f.store(path, owner).AtomicWrite(path, data, mode) } +func (f OSFileSystem) WithLock(path string, owner *Ownership, action func() error) error { + return f.store(path, owner).WithLock(path, action) +} type priorValue struct { Present bool `json:"present"` @@ -191,6 +195,7 @@ func (a *Adapter) usablePath(path string) bool { } func (a *Adapter) ConfigPath() string { return a.configuration.ConfigFile } func (a *Adapter) RecoveryPath() string { return a.configuration.RecoveryFile } +func (a *Adapter) LockPath() string { return a.configuration.RecoveryFile + ".lock" } func (a *Adapter) Inspect(_ context.Context, active *profile.Profile) target.Inspection { metadata, managed, metadataErr := a.readRecovery() @@ -221,17 +226,29 @@ 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")} } - if enabled && a.Inspect(ctx, active).State == target.Enabled { - return target.OperationResult{Inspection: a.Inspect(ctx, active)} - } - var err error - if enabled { - err = a.enable(active) - } else { - err = a.disable() - } - inspection := a.Inspect(ctx, active) + + var inspection target.Inspection + err := a.files.WithLock(a.LockPath(), a.ownership(), func() error { + if enabled { + inspection = a.Inspect(ctx, active) + if inspection.State == target.Enabled { + return nil + } + if err := a.enable(active); err != nil { + inspection = a.Inspect(ctx, active) + return err + } + } else if err := a.disable(); err != nil { + inspection = a.Inspect(ctx, active) + return err + } + inspection = a.Inspect(ctx, active) + return nil + }) if err != nil { + if inspection.Evidence == "" { + inspection = a.Inspect(ctx, active) + } err = redactError(err, active) } return target.OperationResult{Inspection: inspection, Err: err} diff --git a/internal/dockerclient/dockerclient_test.go b/internal/dockerclient/dockerclient_test.go index aa5c4c8..3236b02 100644 --- a/internal/dockerclient/dockerclient_test.go +++ b/internal/dockerclient/dockerclient_test.go @@ -5,9 +5,13 @@ import ( "encoding/json" "errors" "os" + "os/exec" "path/filepath" "strings" + "sync" + "syscall" "testing" + "time" "proxyctl/internal/dockerclient" "proxyctl/internal/profile" @@ -239,6 +243,92 @@ func TestUnmergeableJSONPreventsMutation(t *testing.T) { } } +func TestConcurrentMutationsSerializeRecoveryAndConfiguration(t *testing.T) { + files := &interleavingFS{FileSystem: dockerclient.OSFileSystem{}, firstRecovery: make(chan struct{}), secondRecovery: make(chan struct{}), secondConfig: make(chan struct{})} + a := newAdapterWithFS(t, files) + first := *activeProfile() + first.Name = "first" + first.HTTPURL = "http://first.example:8080" + second := *activeProfile() + second.Name = "second" + second.HTTPURL = "http://second.example:8080" + + firstDone := make(chan target.OperationResult, 1) + go func() { firstDone <- a.SetDesired(context.Background(), true, &first) }() + select { + case <-files.firstRecovery: + case <-time.After(5 * time.Second): + t.Fatal("first mutation did not reach recovery write") + } + secondDone := make(chan target.OperationResult, 1) + go func() { secondDone <- a.SetDesired(context.Background(), true, &second) }() + + for name, done := range map[string]<-chan target.OperationResult{"first": firstDone, "second": secondDone} { + select { + case result := <-done: + if result.Err != nil { + t.Fatalf("%s mutation: %v", name, result.Err) + } + case <-time.After(5 * time.Second): + t.Fatalf("%s mutation did not complete", name) + } + } + if got := a.Inspect(context.Background(), &second); got.State != target.Enabled { + t.Fatalf("final state = %s (%s), want second mutation consistently enabled", got.State, got.Evidence) + } +} + +func TestDockerClientLockSerializesAcrossProcesses(t *testing.T) { + if root := os.Getenv("PROXYCTL_DOCKER_LOCK_HELPER_ROOT"); root != "" { + a := dockerclient.New(dockerclient.Configuration{ + ConfigFile: filepath.Join(root, ".docker", "config.json"), RecoveryFile: filepath.Join(root, ".state", "proxyctl", "docker-containers.json"), + }, dockerclient.OSFileSystem{}) + if result := a.SetDesired(context.Background(), true, activeProfile()); result.Err != nil { + t.Fatal(result.Err) + } + return + } + + a := newAdapter(t, nil) + if err := os.MkdirAll(filepath.Dir(a.LockPath()), 0o700); err != nil { + t.Fatal(err) + } + lock, err := os.OpenFile(a.LockPath(), os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + t.Fatal(err) + } + defer lock.Close() + if err := syscall.Flock(int(lock.Fd()), syscall.LOCK_EX); err != nil { + t.Fatal(err) + } + + root := filepath.Dir(filepath.Dir(filepath.Dir(a.RecoveryPath()))) + command := exec.Command(os.Args[0], "-test.run=^TestDockerClientLockSerializesAcrossProcesses$") + command.Env = append(os.Environ(), "PROXYCTL_DOCKER_LOCK_HELPER_ROOT="+root) + done := make(chan error, 1) + go func() { done <- command.Run() }() + select { + case err := <-done: + t.Fatalf("child mutation did not wait for Docker client lock: %v", err) + case <-time.After(100 * time.Millisecond): + } + if err := syscall.Flock(int(lock.Fd()), syscall.LOCK_UN); err != nil { + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatalf("child mutation after unlock: %v", err) + } + case <-time.After(5 * time.Second): + _ = command.Process.Kill() + t.Fatal("child mutation remained blocked after unlock") + } + if got := a.Inspect(context.Background(), activeProfile()); got.State != target.Enabled { + t.Fatalf("state after serialized child mutation = %s (%s)", got.State, got.Evidence) + } +} + func TestWritesAreOwnerOnlyAtomicAndIdempotent(t *testing.T) { files := &countingFS{FileSystem: dockerclient.OSFileSystem{}} a := newAdapterWithFS(t, files) @@ -252,7 +342,7 @@ func TestWritesAreOwnerOnlyAtomicAndIdempotent(t *testing.T) { if files.writes != writes { t.Fatalf("idempotent enable performed %d additional writes", files.writes-writes) } - for _, path := range []string{a.ConfigPath(), a.RecoveryPath()} { + for _, path := range []string{a.ConfigPath(), a.RecoveryPath(), a.LockPath()} { info, err := os.Stat(path) if err != nil { t.Fatal(err) @@ -274,7 +364,7 @@ func TestOwnedWritesUseConfigurationOwner(t *testing.T) { if got := a.SetDesired(context.Background(), true, activeProfile()); got.Err != nil { t.Fatal(got.Err) } - if len(files.owners) != 2 { + if len(files.owners) != 3 { t.Fatalf("owners = %#v", files.owners) } for _, owner := range files.owners { @@ -448,6 +538,60 @@ func (f *countingFS) AtomicWrite(path string, data []byte, mode os.FileMode, own return f.FileSystem.AtomicWrite(path, data, mode, owner) } +type interleavingFS struct { + dockerclient.FileSystem + mu sync.Mutex + recoveryWrites, configWrites int + firstRecovery, secondRecovery, secondConfig chan struct{} +} + +func (f *interleavingFS) AtomicWrite(path string, data []byte, mode os.FileMode, owner *dockerclient.Ownership) error { + f.mu.Lock() + isRecovery := strings.HasSuffix(path, "docker-containers.json") + if isRecovery { + f.recoveryWrites++ + call := f.recoveryWrites + if call == 1 { + close(f.firstRecovery) + } + } else { + f.configWrites++ + } + recoveryCall, configCall := f.recoveryWrites, f.configWrites + f.mu.Unlock() + + if isRecovery && recoveryCall == 1 { + select { + case <-f.secondRecovery: + case <-time.After(100 * time.Millisecond): + } + err := f.FileSystem.AtomicWrite(path, data, mode, owner) + return err + } + if isRecovery && recoveryCall == 2 { + err := f.FileSystem.AtomicWrite(path, data, mode, owner) + close(f.secondRecovery) + return err + } + if !isRecovery && configCall == 1 { + select { + case <-f.secondConfig: + case <-time.After(100 * time.Millisecond): + } + return f.FileSystem.AtomicWrite(path, data, mode, owner) + } + if !isRecovery && configCall == 2 { + err := f.FileSystem.AtomicWrite(path, data, mode, owner) + close(f.secondConfig) + return err + } + return f.FileSystem.AtomicWrite(path, data, mode, owner) +} + +func (f *interleavingFS) WithLock(path string, owner *dockerclient.Ownership, action func() error) error { + return f.FileSystem.WithLock(path, owner, action) +} + type ownershipFS struct { dockerclient.FileSystem owners []*dockerclient.Ownership @@ -457,3 +601,8 @@ func (f *ownershipFS) AtomicWrite(path string, data []byte, mode os.FileMode, ow f.owners = append(f.owners, owner) return f.FileSystem.AtomicWrite(path, data, mode, nil) } + +func (f *ownershipFS) WithLock(path string, owner *dockerclient.Ownership, action func() error) error { + f.owners = append(f.owners, owner) + return f.FileSystem.WithLock(path, nil, action) +} diff --git a/internal/securefs/securefs.go b/internal/securefs/securefs.go index ad882ac..471b3f5 100644 --- a/internal/securefs/securefs.go +++ b/internal/securefs/securefs.go @@ -146,6 +146,37 @@ func (s Store) AtomicWrite(path string, data []byte, mode os.FileMode) error { return directory.Sync() } +// WithLock serializes an action across processes using a descriptor-anchored, +// owner-restricted advisory lock file. +func (s Store) WithLock(path string, action func() error) error { + directory, base, err := s.openParent(path, true) + if err != nil { + return err + } + defer directory.Close() + lock, err := OpenAt(directory, base, unix.O_CREAT|unix.O_RDWR, 0o600) + if err != nil { + return err + } + defer lock.Close() + if err := regular(lock); err != nil { + return err + } + if s.Owner != nil && (s.Owner.UID != os.Geteuid() || s.Owner.GID != os.Getegid()) { + if err := lock.Chown(s.Owner.UID, s.Owner.GID); err != nil { + return err + } + } + if err := lock.Chmod(0o600); err != nil { + return err + } + if err := unix.Flock(int(lock.Fd()), unix.LOCK_EX); err != nil { + return err + } + defer func() { _ = unix.Flock(int(lock.Fd()), unix.LOCK_UN) }() + return action() +} + func (s Store) Remove(path string) error { directory, base, err := s.openParent(path, false) if err != nil {