From d4c76e4b3cb50237fc1e346174efae5abe7faff4 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 03:28:29 +0800 Subject: [PATCH] feat: add diagnostics and safe reset maintenance --- cmd/proxyctl/main.go | 163 ++++++++++++++++++++++++--- cmd/proxyctl/maintenance_test.go | 128 +++++++++++++++++++++ internal/app/app.go | 43 ++++++- internal/app/maintenance.go | 166 ++++++++++++++++++++++++++++ internal/app/maintenance_test.go | 140 +++++++++++++++++++++++ internal/operationlock/lock.go | 84 ++++++++++++++ internal/operationlock/lock_test.go | 30 +++++ internal/oplog/logger.go | 113 +++++++++++++++++++ internal/oplog/logger_test.go | 49 ++++++++ internal/profile/diagnostic.go | 58 ++++++++++ internal/profile/diagnostic_test.go | 35 ++++++ internal/shell/shell.go | 42 +++++++ internal/target/target.go | 13 +++ 13 files changed, 1041 insertions(+), 23 deletions(-) create mode 100644 cmd/proxyctl/maintenance_test.go create mode 100644 internal/app/maintenance.go create mode 100644 internal/app/maintenance_test.go create mode 100644 internal/operationlock/lock.go create mode 100644 internal/operationlock/lock_test.go create mode 100644 internal/oplog/logger.go create mode 100644 internal/oplog/logger_test.go create mode 100644 internal/profile/diagnostic.go create mode 100644 internal/profile/diagnostic_test.go diff --git a/cmd/proxyctl/main.go b/cmd/proxyctl/main.go index 8bb4e23..a8e05bf 100644 --- a/cmd/proxyctl/main.go +++ b/cmd/proxyctl/main.go @@ -1,14 +1,22 @@ package main import ( + "bufio" "context" + "errors" "fmt" "io" "os" + "path/filepath" + "strings" + "syscall" tea "github.com/charmbracelet/bubbletea" + "golang.org/x/sys/unix" "proxyctl/internal/app" + "proxyctl/internal/operationlock" + "proxyctl/internal/oplog" "proxyctl/internal/profile" "proxyctl/internal/shell" "proxyctl/internal/target" @@ -50,31 +58,56 @@ func runWithEnvironment(args []string, input io.Reader, output io.Writer, enviro return err } shellTarget := shell.New(shellConfiguration, files, environment) + targets := []target.Target{shellTarget} + stateDirectory := filepath.Dir(shellConfiguration.RecoveryFile) + mutationLock := operationlock.NewOwned(filepath.Join(stateDirectory, "operation.lock"), profileConfiguration.Owner.UID, profileConfiguration.Owner.GID) + logger := oplog.NewOwned(filepath.Join(stateDirectory, "operations.log"), 64*1024, 3, profileConfiguration.Owner.UID, profileConfiguration.Owner.GID) + store := profile.NewOwnedFileStoreAt(profileConfiguration.Path, profileConfiguration.Owner, profileConfiguration.TrustedRoot) if len(args) != 0 { - if len(args) != 2 || args[0] != "shell" || (args[1] != "install" && args[1] != "uninstall") { - return fmt.Errorf("usage: proxyctl [shell install|shell uninstall]") - } - availability := shellTarget.Availability(context.Background()) - if !availability.Available { - return fmt.Errorf("Shell Target unavailable: %s", availability.Evidence) - } - if args[1] == "install" { - if err := shellTarget.Install(context.Background(), executable); err != nil { - return err + switch { + case len(args) == 2 && args[0] == "shell" && (args[1] == "install" || args[1] == "uninstall"): + availability := shellTarget.Availability(context.Background()) + if !availability.Available { + return fmt.Errorf("Shell Target unavailable: %s", availability.Evidence) + } + unlock, err := mutationLock.Lock(context.Background()) + if err != nil { + return fmt.Errorf("lock proxyctl mutation: %w", err) + } + defer unlock() + operation := "shell-" + args[1] + var operationErr error + if args[1] == "install" { + operationErr = shellTarget.Install(context.Background(), executable) + } else { + operationErr = shellTarget.Uninstall(context.Background()) + } + result, detail := "success", "Shell integration changed" + if operationErr != nil { + result, detail = "failure", operationErr.Error() + } + _ = logger.Write(app.LogEntry{Operation: operation, Target: shellTarget.ID(), Result: result, Detail: detail}) + if operationErr != nil { + return operationErr + } + if args[1] == "install" { + fmt.Fprintln(output, "Shell integration installed; restart or source your Shell startup file once.") + } else { + fmt.Fprintln(output, "Shell integration uninstalled.") } - fmt.Fprintln(output, "Shell integration installed; restart or source your Shell startup file once.") return nil + case len(args) == 1 && args[0] == "doctor": + checks := diagnosticChecks(profileConfiguration) + return runMaintenanceCommand(args, input, output, store, targets, mutationLock, logger, checks) + case len(args) == 1 && args[0] == "reset": + return runMaintenanceCommand(args, input, output, store, targets, mutationLock, logger, nil) + default: + return fmt.Errorf("usage: proxyctl [doctor|reset|shell install|shell uninstall]") } - if err := shellTarget.Uninstall(context.Background()); err != nil { - return err - } - fmt.Fprintln(output, "Shell integration uninstalled.") - return nil } - store := profile.NewOwnedFileStoreAt(profileConfiguration.Path, profileConfiguration.Owner, profileConfiguration.TrustedRoot) - application, err := app.NewWithTargets(store, []target.Target{shellTarget}) + application, err := app.NewWithTargetsAndRuntime(store, targets, mutationLock, logger) if err != nil { return err } @@ -82,3 +115,97 @@ func runWithEnvironment(args []string, input io.Reader, output io.Writer, enviro _, err = program.Run() return err } + +func runMaintenanceCommand(args []string, input io.Reader, output io.Writer, store *profile.FileStore, targets []target.Target, mutationLock app.MutationLock, logger app.OperationLogger, checks []target.Check) error { + if len(args) != 1 { + return errors.New("invalid maintenance command") + } + if args[0] == "doctor" { + state, err := store.ReadOnlyLoad() + profileCheck := target.Check{Name: "Profile validity", OK: err == nil, Evidence: "Profile configuration is valid"} + if err != nil { + profileCheck.Evidence = err.Error() + } + checks = append([]target.Check{profileCheck}, checks...) + report := app.NewMaintenance(state, targets, nil, nil).Doctor(context.Background(), checks) + fmt.Fprint(output, report.String()) + return nil + } + if args[0] != "reset" { + return errors.New("invalid maintenance command") + } + fmt.Fprint(output, "Reset every managed Target and restore recorded original state? [y/N] ") + answer, _ := bufio.NewReader(input).ReadString('\n') + answer = strings.ToLower(strings.TrimSpace(answer)) + if answer != "y" && answer != "yes" { + fmt.Fprintln(output, "Reset cancelled; no Targets changed.") + return nil + } + state, err := store.Load() + if err != nil { + return err + } + result := app.NewMaintenance(state, targets, mutationLock, logger).Reset(context.Background(), true) + for _, item := range result.Targets { + if item.Skipped { + fmt.Fprintf(output, "%s: skipped (%s) — %s\n", item.TargetID, item.Inspection.State, profile.RedactText(item.Inspection.Evidence)) + continue + } + if item.Err != nil { + fmt.Fprintf(output, "%s: failed (%s) — %s\n", item.TargetID, item.Inspection.State, profile.RedactText(item.Err.Error())) + continue + } + fmt.Fprintf(output, "%s: reset (%s) — %s\n", item.TargetID, item.Inspection.State, profile.RedactText(item.Inspection.Evidence)) + } + return result.Err +} + +func diagnosticChecks(configuration profile.Configuration) []target.Check { + identity := target.Check{Name: "identity resolution", OK: true, Evidence: fmt.Sprintf("configuration owner %s (uid %d, gid %d)", configuration.Owner.Username, configuration.Owner.UID, configuration.Owner.GID)} + ok, evidence := storagePermissions(configuration) + storage := target.Check{Name: "storage permissions", OK: ok, Evidence: evidence} + return []target.Check{identity, storage} +} + +func storagePermissions(configuration profile.Configuration) (bool, string) { + ok, evidence := writableLocation(configuration.Path) + info, err := os.Stat(configuration.Path) + if os.IsNotExist(err) { + return ok, evidence + "; Profile configuration does not exist yet" + } + if err != nil { + return false, evidence + "; Profile configuration cannot be inspected: " + err.Error() + } + if info.Mode().Perm() != 0o600 { + return false, fmt.Sprintf("%s; Profile configuration mode is %o, want 600", evidence, info.Mode().Perm()) + } + if stat, present := info.Sys().(*syscall.Stat_t); present && int(stat.Uid) != configuration.Owner.UID { + return false, fmt.Sprintf("%s; Profile configuration owner uid is %d, want %d", evidence, stat.Uid, configuration.Owner.UID) + } + return ok, evidence + "; Profile configuration is owner-only" +} + +// writableLocation observes current authority without creating or changing files. +func writableLocation(path string) (bool, string) { + candidate := path + for { + info, err := os.Stat(candidate) + if err == nil { + if !info.IsDir() { + candidate = filepath.Dir(candidate) + } + if err := unix.Access(candidate, unix.W_OK); err != nil { + return false, candidate + " is not writable: " + err.Error() + } + return true, candidate + " is writable" + } + if !os.IsNotExist(err) { + return false, candidate + " cannot be inspected: " + err.Error() + } + parent := filepath.Dir(candidate) + if parent == candidate { + return false, path + " has no writable existing ancestor" + } + candidate = parent + } +} diff --git a/cmd/proxyctl/maintenance_test.go b/cmd/proxyctl/maintenance_test.go new file mode 100644 index 0000000..fef7b72 --- /dev/null +++ b/cmd/proxyctl/maintenance_test.go @@ -0,0 +1,128 @@ +package main + +import ( + "bytes" + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "proxyctl/internal/operationlock" + "proxyctl/internal/oplog" + "proxyctl/internal/profile" + "proxyctl/internal/target" +) + +type processFakeTarget struct { + id string + state target.State + err error + calls int +} + +func (f *processFakeTarget) ID() string { return f.id } +func (f *processFakeTarget) Description() target.Description { return target.Description{Name: f.id} } +func (f *processFakeTarget) Availability(context.Context) target.Availability { + return target.Availability{Available: true, Evidence: "fake executable found"} +} +func (f *processFakeTarget) Inspect(context.Context, *profile.Profile) target.Inspection { + return target.Inspection{State: f.state, Evidence: "fake evidence for http://alice:secret@proxy.test"} +} +func (f *processFakeTarget) SetDesired(context.Context, bool, *profile.Profile) target.OperationResult { + f.calls++ + if f.err == nil { + f.state = target.Disabled + } + return target.OperationResult{Inspection: target.Inspection{State: f.state, Evidence: "re-inspected fake state"}, Err: f.err} +} +func (f *processFakeTarget) DiagnosticChecks(context.Context, *profile.Profile) []target.Check { + return []target.Check{{Name: "required commands", OK: true, Evidence: "fake helper found"}, {Name: "write authority", OK: true, Evidence: "fake location writable"}} +} + +func TestDoctorCommandUsesFakeTargetsAndIsolatedXDGWithoutMutation(t *testing.T) { + xdg := filepath.Join(t.TempDir(), "xdg-config") + store := profile.NewFileStore(filepath.Join(xdg, "proxyctl", "profiles.json")) + fake := &processFakeTarget{id: "fake", state: target.External} + var output bytes.Buffer + if err := runMaintenanceCommand([]string{"doctor"}, strings.NewReader(""), &output, store, []target.Target{fake}, nil, nil, []target.Check{{Name: "identity resolution", OK: true, Evidence: "fake user"}, {Name: "storage permissions", OK: true, Evidence: xdg}}); err != nil { + t.Fatal(err) + } + text := output.String() + for _, want := range []string{"Profile validity", "identity resolution", "storage permissions", "Target availability", "External", "required commands", "write authority"} { + if !strings.Contains(text, want) { + t.Errorf("doctor output missing %q: %s", want, text) + } + } + if strings.Contains(text, "secret") { + t.Fatalf("doctor leaked credentials: %s", text) + } + if fake.calls != 0 { + t.Fatalf("doctor mutated fake Target %d times", fake.calls) + } + if _, err := os.Stat(xdg); !os.IsNotExist(err) { + t.Fatalf("doctor created isolated XDG path: %v", err) + } +} + +func TestDoctorStoragePermissionCheckIsReadOnly(t *testing.T) { + path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json") + writeConfig(t, path, `{"profiles":[]}`) + if err := os.Chmod(path, 0o644); err != nil { + t.Fatal(err) + } + configuration := profile.Configuration{Path: path, Owner: profile.UserIdentity{UID: os.Getuid(), GID: os.Getgid()}} + checks := diagnosticChecks(configuration) + if checks[1].OK || !strings.Contains(checks[1].Evidence, "want 600") { + t.Fatalf("storage check = %#v", checks[1]) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o644 { + t.Fatalf("doctor permission check changed mode to %o", info.Mode().Perm()) + } +} + +func TestResetCommandCancellationAndTruthfulPartialFailureWithFakeTargets(t *testing.T) { + root := t.TempDir() + config := filepath.Join(root, "xdg-config", "proxyctl", "profiles.json") + writeConfig(t, config, `{"profiles":[]}`) + store := profile.NewFileStore(config) + good := &processFakeTarget{id: "good", state: target.Enabled} + bad := &processFakeTarget{id: "bad", state: target.Drifted, err: errors.New("remove http://bob:hunter2@proxy.test failed")} + external := &processFakeTarget{id: "external", state: target.External} + stateDir := filepath.Join(root, "xdg-state", "proxyctl") + lock := operationlock.New(filepath.Join(stateDir, "operation.lock")) + logger := oplog.New(filepath.Join(stateDir, "operations.log"), 1024, 1) + + var output bytes.Buffer + if err := runMaintenanceCommand([]string{"reset"}, strings.NewReader("no\n"), &output, store, []target.Target{good, bad, external}, lock, logger, nil); err != nil { + t.Fatal(err) + } + if good.calls+bad.calls+external.calls != 0 || !strings.Contains(output.String(), "cancelled") { + t.Fatalf("cancel result output=%q calls=%d/%d/%d", output.String(), good.calls, bad.calls, external.calls) + } + + output.Reset() + err := runMaintenanceCommand([]string{"reset"}, strings.NewReader("yes\n"), &output, store, []target.Target{good, bad, external}, lock, logger, nil) + if err == nil { + t.Fatal("partial reset returned success") + } + text := output.String() + if good.calls != 1 || bad.calls != 1 || external.calls != 0 || !strings.Contains(text, "good: reset (Disabled)") || !strings.Contains(text, "bad: failed (Drifted)") || !strings.Contains(text, "external: skipped (External)") { + t.Fatalf("partial reset output=%q calls=%d/%d/%d", text, good.calls, bad.calls, external.calls) + } + if strings.Contains(text, "hunter2") || strings.Contains(err.Error(), "hunter2") { + t.Fatalf("reset leaked credentials: output=%q err=%v", text, err) + } + logData, readErr := os.ReadFile(filepath.Join(stateDir, "operations.log")) + if readErr != nil { + t.Fatal(readErr) + } + if strings.Contains(string(logData), "hunter2") || strings.Contains(string(logData), "bob:") { + t.Fatalf("log leaked full authenticated URL: %s", logData) + } +} diff --git a/internal/app/app.go b/internal/app/app.go index 006010e..a4fab84 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "time" "proxyctl/internal/profile" "proxyctl/internal/target" @@ -55,10 +56,12 @@ type Module interface { } type Application struct { - store profile.Store - state profile.State - targets []target.Target - inspections map[string]target.Inspection + store profile.Store + state profile.State + targets []target.Target + inspections map[string]target.Inspection + mutationLock MutationLock + logger OperationLogger } func New(store profile.Store) (*Application, error) { @@ -66,11 +69,23 @@ func New(store profile.Store) (*Application, error) { } func NewWithTargets(store profile.Store, targets []target.Target) (*Application, error) { + return NewWithTargetsAndLock(store, targets, nil) +} + +// NewWithTargetsAndLock configures the cross-process lock used by every +// Profile or Target mutation. A nil lock is intended only for isolated tests. +func NewWithTargetsAndLock(store profile.Store, targets []target.Target, mutationLock MutationLock) (*Application, error) { + return NewWithTargetsAndRuntime(store, targets, mutationLock, nil) +} + +// NewWithTargetsAndRuntime configures shared mutation serialization and local +// operation diagnostics for production callers. +func NewWithTargetsAndRuntime(store profile.Store, targets []target.Target, mutationLock MutationLock, logger OperationLogger) (*Application, error) { state, err := store.Load() if err != nil { return nil, err } - a := &Application{store: store, state: state, targets: append([]target.Target(nil), targets...), inspections: make(map[string]target.Inspection)} + a := &Application{store: store, state: state, targets: append([]target.Target(nil), targets...), inspections: make(map[string]target.Inspection), mutationLock: mutationLock, logger: logger} a.refreshTargets() return a, nil } @@ -87,6 +102,13 @@ func (a *Application) Snapshot() Snapshot { } func (a *Application) Dispatch(intent Intent) Result { + if a.mutationLock != nil { + unlock, err := a.mutationLock.Lock(context.Background()) + if err != nil { + return Result{Snapshot: a.Snapshot(), Err: fmt.Errorf("lock proxyctl mutation: %w", err)} + } + defer unlock() + } if intent.Kind == SetTargetDesired { return a.setTargetDesired(intent) } @@ -161,6 +183,17 @@ func (a *Application) setTargetDesired(intent Intent) Result { } result := selected.SetDesired(context.Background(), intent.Enabled, active) a.inspections[selected.ID()] = result.Inspection + outcome, detail := "success", result.Inspection.Evidence + if result.Err != nil { + outcome, detail = "failure", result.Err.Error() + } + if a.logger != nil { + operation := "disable" + if intent.Enabled { + operation = "enable" + } + _ = a.logger.Write(LogEntry{Time: time.Now(), Operation: operation, Target: selected.ID(), Result: outcome, Detail: RedactLogDetail(detail)}) + } notice := "" if result.Err == nil { notice = fmt.Sprintf("%s Target is now %s", selected.Description().Name, result.Inspection.State) diff --git a/internal/app/maintenance.go b/internal/app/maintenance.go new file mode 100644 index 0000000..9b5fd4b --- /dev/null +++ b/internal/app/maintenance.go @@ -0,0 +1,166 @@ +package app + +import ( + "context" + "errors" + "fmt" + "regexp" + "strings" + "time" + + "proxyctl/internal/profile" + "proxyctl/internal/target" +) + +// MutationLock serializes all state-changing operations across processes. +type MutationLock interface { + Lock(context.Context) (unlock func(), err error) +} + +// LogEntry is the privacy-safe operation record written to bounded local logs. +type LogEntry struct { + Time time.Time + Operation string + Target string + Result string + Detail string +} + +type OperationLogger interface{ Write(LogEntry) error } + +type Maintenance struct { + state profile.State + targets []target.Target + lock MutationLock + logger OperationLogger +} + +func NewMaintenance(state profile.State, targets []target.Target, lock MutationLock, logger OperationLogger) *Maintenance { + return &Maintenance{state: state, targets: append([]target.Target(nil), targets...), lock: lock, logger: logger} +} + +type Diagnostic struct { + TargetID string + Check target.Check +} + +type DoctorReport struct{ Diagnostics []Diagnostic } + +func (r DoctorReport) Healthy() bool { + for _, diagnostic := range r.Diagnostics { + if !diagnostic.Check.OK { + return false + } + } + return true +} +func (r DoctorReport) String() string { + var output strings.Builder + for _, diagnostic := range r.Diagnostics { + status := "OK" + if !diagnostic.Check.OK { + status = "FAIL" + } + name := diagnostic.Check.Name + if diagnostic.TargetID != "" { + name = diagnostic.TargetID + ": " + name + } + fmt.Fprintf(&output, "[%s] %s — %s\n", status, name, profile.RedactText(diagnostic.Check.Evidence)) + } + return output.String() +} + +// Doctor only invokes observational Target methods. +func (m *Maintenance) Doctor(ctx context.Context, applicationChecks []target.Check) DoctorReport { + report := DoctorReport{} + for _, check := range applicationChecks { + check.Evidence = profile.RedactText(check.Evidence) + report.Diagnostics = append(report.Diagnostics, Diagnostic{Check: check}) + } + active := activeFromState(m.state) + for _, adapter := range m.targets { + availability := adapter.Availability(ctx) + report.Diagnostics = append(report.Diagnostics, Diagnostic{TargetID: adapter.ID(), Check: target.Check{Name: "Target availability", OK: availability.Available, Evidence: availability.Evidence}}) + inspection := adapter.Inspect(ctx, active) + report.Diagnostics = append(report.Diagnostics, Diagnostic{TargetID: adapter.ID(), Check: target.Check{Name: "detected state", OK: inspection.State != target.Drifted, Evidence: string(inspection.State) + ": " + inspection.Evidence}}) + if diagnosable, ok := adapter.(target.Diagnosable); ok { + for _, check := range diagnosable.DiagnosticChecks(ctx, active) { + check.Evidence = profile.RedactText(check.Evidence) + report.Diagnostics = append(report.Diagnostics, Diagnostic{TargetID: adapter.ID(), Check: check}) + } + } + } + return report +} + +type ResetTargetResult struct { + TargetID string + Inspection target.Inspection + Skipped bool + Err error +} +type ResetResult struct { + Cancelled bool + Targets []ResetTargetResult + Err error +} + +func (m *Maintenance) Reset(ctx context.Context, confirmed bool) ResetResult { + if !confirmed { + return ResetResult{Cancelled: true} + } + if m.lock == nil { + return ResetResult{Err: errors.New("mutation lock is not configured")} + } + unlock, err := m.lock.Lock(ctx) + if err != nil { + return ResetResult{Err: fmt.Errorf("lock proxyctl mutation: %w", err)} + } + defer unlock() + active := activeFromState(m.state) + result := ResetResult{} + for _, adapter := range m.targets { + inspection := adapter.Inspect(ctx, active) + if inspection.State != target.Enabled && inspection.State != target.Drifted { + result.Targets = append(result.Targets, ResetTargetResult{TargetID: adapter.ID(), Inspection: inspection, Skipped: true}) + continue + } + operation := adapter.SetDesired(ctx, false, active) + detail, outcome := operation.Inspection.Evidence, "success" + if operation.Err != nil { + outcome = "failure" + detail = operation.Err.Error() + result.Err = errors.Join(result.Err, fmt.Errorf("reset %s Target: %w", adapter.ID(), redactError(operation.Err))) + } + detail = RedactLogDetail(detail) + if m.logger != nil { + _ = m.logger.Write(LogEntry{Time: time.Now(), Operation: "reset", Target: adapter.ID(), Result: outcome, Detail: detail}) + } + result.Targets = append(result.Targets, ResetTargetResult{TargetID: adapter.ID(), Inspection: operation.Inspection, Err: redactError(operation.Err)}) + } + return result +} + +func activeFromState(state profile.State) *profile.Profile { + for i := range state.Profiles { + if state.Profiles[i].Name == state.ActiveProfile { + value := state.Profiles[i] + return &value + } + } + return nil +} + +var authenticatedURL = regexp.MustCompile(`(?i)(?:https?|socks5h?)://[^\s'\"<>]*@[^\s'\"<>]+`) + +// RedactLogDetail omits complete authenticated URLs, not just their passwords. +func RedactLogDetail(value string) string { + value = authenticatedURL.ReplaceAllString(value, "[REDACTED AUTHENTICATED URL]") + return profile.RedactText(value) +} +func redactError(err error) error { + if err == nil { + return nil + } + return errors.New(profile.RedactText(err.Error())) +} diff --git a/internal/app/maintenance_test.go b/internal/app/maintenance_test.go new file mode 100644 index 0000000..f084442 --- /dev/null +++ b/internal/app/maintenance_test.go @@ -0,0 +1,140 @@ +package app_test + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" + + "proxyctl/internal/app" + "proxyctl/internal/profile" + "proxyctl/internal/target" +) + +type maintenanceTarget struct { + id string + available target.Availability + inspection target.Inspection + checks []target.Check + disableErr error + disableCalls int +} + +func (f *maintenanceTarget) ID() string { return f.id } +func (f *maintenanceTarget) Description() target.Description { + return target.Description{Name: strings.ToUpper(f.id)} +} +func (f *maintenanceTarget) Availability(context.Context) target.Availability { return f.available } +func (f *maintenanceTarget) Inspect(context.Context, *profile.Profile) target.Inspection { + return f.inspection +} +func (f *maintenanceTarget) SetDesired(_ context.Context, enabled bool, _ *profile.Profile) target.OperationResult { + if !enabled { + f.disableCalls++ + if f.disableErr == nil { + f.inspection = target.Inspection{State: target.Disabled, Evidence: "managed state removed"} + } + } + return target.OperationResult{Inspection: f.inspection, Err: f.disableErr} +} +func (f *maintenanceTarget) DiagnosticChecks(context.Context, *profile.Profile) []target.Check { + return append([]target.Check(nil), f.checks...) +} + +type recordingLock struct{ held, acquired int } + +func (l *recordingLock) Lock(context.Context) (func(), error) { + l.held++ + l.acquired++ + return func() { l.held-- }, nil +} + +type recordingLog struct{ entries []app.LogEntry } + +func (l *recordingLog) Write(entry app.LogEntry) error { + l.entries = append(l.entries, entry) + return nil +} + +func TestDoctorReportsReadOnlyChecksEvidenceAndRedactsCredentials(t *testing.T) { + adapter := &maintenanceTarget{ + id: "fake", available: target.Availability{Available: true, Evidence: "fake command found"}, + inspection: target.Inspection{State: target.External, Evidence: "found http://alice:secret@proxy.example:8080 in fake settings"}, + checks: []target.Check{{Name: "required commands", OK: false, Evidence: "helper failed for https://bob:hunter2@proxy.example/path"}, {Name: "write authority", OK: true, Evidence: "settings directory is writable"}}, + } + maintenance := app.NewMaintenance(profile.State{}, []target.Target{adapter}, nil, nil) + + report := maintenance.Doctor(context.Background(), []target.Check{{Name: "Profile configuration", OK: true, Evidence: "valid"}, {Name: "identity resolution", OK: true, Evidence: "tester (1000)"}}) + + if adapter.disableCalls != 0 { + t.Fatalf("doctor mutated Target %d times", adapter.disableCalls) + } + text := report.String() + for _, want := range []string{"Profile configuration", "identity resolution", "Target availability", "External", "required commands", "write authority", "found http://alice:REDACTED@proxy.example:8080"} { + if !strings.Contains(text, want) { + t.Errorf("doctor report missing %q: %s", want, text) + } + } + for _, secret := range []string{"secret", "hunter2"} { + if strings.Contains(text, secret) { + t.Errorf("doctor leaked %q: %s", secret, text) + } + } +} + +func TestApplicationSerializesAndLogsTargetMutations(t *testing.T) { + adapter := &maintenanceTarget{id: "fake", available: target.Availability{Available: true}, inspection: target.Inspection{State: target.External}} + lock := &recordingLock{} + log := &recordingLog{} + application, err := app.NewWithTargetsAndRuntime(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}, lock, log) + if err != nil { + t.Fatal(err) + } + if result := application.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{Name: "office", SingleURL: "http://proxy.test:8080"}}); result.Err != nil { + t.Fatal(result.Err) + } + if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "office"}); result.Err != nil { + t.Fatal(result.Err) + } + adapter.disableErr = errors.New("failed http://alice:secret@proxy.test/private") + result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "fake", Enabled: false}) + if result.Err == nil { + t.Fatal("Target failure reported success") + } + if lock.acquired != 3 || lock.held != 0 { + t.Fatalf("all mutation lock acquired=%d held=%d", lock.acquired, lock.held) + } + if len(log.entries) != 1 || log.entries[0].Operation != "disable" || strings.Contains(log.entries[0].Detail, "secret") || strings.Contains(log.entries[0].Detail, "proxy.test/private") { + t.Fatalf("Target operation log = %#v", log.entries) + } +} + +func TestResetRequiresConfirmationAndReportsTruthfulPartialResults(t *testing.T) { + good := &maintenanceTarget{id: "good", available: target.Availability{Available: true}, inspection: target.Inspection{State: target.Enabled, Evidence: "managed"}} + failure := errors.New("remove https://alice:secret@proxy.example failed: permission denied") + bad := &maintenanceTarget{id: "bad", available: target.Availability{Available: true}, inspection: target.Inspection{State: target.Drifted, Evidence: "managed fragment remains"}, disableErr: failure} + external := &maintenanceTarget{id: "external", available: target.Availability{Available: true}, inspection: target.Inspection{State: target.External, Evidence: "user owned"}} + lock := &recordingLock{} + log := &recordingLog{} + maintenance := app.NewMaintenance(profile.State{}, []target.Target{good, bad, external}, lock, log) + + cancelled := maintenance.Reset(context.Background(), false) + if !cancelled.Cancelled || cancelled.Err != nil || good.disableCalls+bad.disableCalls+external.disableCalls != 0 || lock.acquired != 0 { + t.Fatalf("cancelled reset = %#v, calls %d/%d/%d lock=%d", cancelled, good.disableCalls, bad.disableCalls, external.disableCalls, lock.acquired) + } + + result := maintenance.Reset(context.Background(), true) + if result.Err == nil || len(result.Targets) != 3 || good.disableCalls != 1 || bad.disableCalls != 1 || external.disableCalls != 0 { + t.Fatalf("confirmed reset = %#v, calls %d/%d/%d", result, good.disableCalls, bad.disableCalls, external.disableCalls) + } + if result.Targets[0].Inspection.State != target.Disabled || result.Targets[1].Inspection.State != target.Drifted || !result.Targets[2].Skipped { + t.Fatalf("reset did not preserve per-Target truth: %#v", result.Targets) + } + if lock.acquired != 1 || lock.held != 0 { + t.Fatalf("lock lifecycle acquired=%d held=%d", lock.acquired, lock.held) + } + if len(log.entries) != 2 || strings.Contains(log.entries[1].Detail, "secret") { + t.Fatalf("redacted operation log = %#v", log.entries) + } +} diff --git a/internal/operationlock/lock.go b/internal/operationlock/lock.go new file mode 100644 index 0000000..44f6653 --- /dev/null +++ b/internal/operationlock/lock.go @@ -0,0 +1,84 @@ +// Package operationlock serializes proxyctl state mutations across processes. +package operationlock + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "time" + + "golang.org/x/sys/unix" + + "proxyctl/internal/securefs" +) + +type Lock struct { + path string + uid int + gid int + owned bool +} + +func New(path string) *Lock { return &Lock{path: path} } +func NewOwned(path string, uid, gid int) *Lock { + return &Lock{path: path, uid: uid, gid: gid, owned: true} +} + +func (l *Lock) Lock(ctx context.Context) (func(), error) { + if !filepath.IsAbs(l.path) { + return nil, errors.New("operation lock path must be absolute") + } + directory := filepath.Dir(l.path) + if err := os.MkdirAll(directory, 0o700); err != nil { + return nil, fmt.Errorf("create operation lock directory: %w", err) + } + if err := os.Chmod(directory, 0o700); err != nil { + return nil, fmt.Errorf("secure operation lock directory: %w", err) + } + if l.owned && (os.Geteuid() != l.uid || os.Getegid() != l.gid) { + if err := os.Chown(directory, l.uid, l.gid); err != nil { + return nil, fmt.Errorf("set operation lock directory owner: %w", err) + } + } + file, err := securefs.OpenNoFollow(l.path, unix.O_CREAT|unix.O_RDWR|unix.O_NONBLOCK, 0o600) + if err != nil { + return nil, fmt.Errorf("open operation lock: %w", err) + } + info, err := file.Stat() + if err != nil || !info.Mode().IsRegular() { + file.Close() + if err != nil { + return nil, fmt.Errorf("inspect operation lock: %w", err) + } + return nil, errors.New("operation lock is not a regular file") + } + if err := file.Chmod(0o600); err != nil { + file.Close() + return nil, fmt.Errorf("secure operation lock: %w", err) + } + if l.owned && (os.Geteuid() != l.uid || os.Getegid() != l.gid) { + if err := file.Chown(l.uid, l.gid); err != nil { + file.Close() + return nil, fmt.Errorf("set operation lock owner: %w", err) + } + } + for { + err = unix.Flock(int(file.Fd()), unix.LOCK_EX|unix.LOCK_NB) + if err == nil { + break + } + if !errors.Is(err, unix.EWOULDBLOCK) && !errors.Is(err, unix.EAGAIN) { + file.Close() + return nil, fmt.Errorf("acquire operation lock: %w", err) + } + select { + case <-ctx.Done(): + file.Close() + return nil, ctx.Err() + case <-time.After(10 * time.Millisecond): + } + } + return func() { _ = unix.Flock(int(file.Fd()), unix.LOCK_UN); _ = file.Close() }, nil +} diff --git a/internal/operationlock/lock_test.go b/internal/operationlock/lock_test.go new file mode 100644 index 0000000..fd396bc --- /dev/null +++ b/internal/operationlock/lock_test.go @@ -0,0 +1,30 @@ +package operationlock_test + +import ( + "context" + "testing" + "time" + + "proxyctl/internal/operationlock" +) + +func TestLockSerializesIndependentInstancesAndHonorsContext(t *testing.T) { + path := t.TempDir() + "/proxyctl.lock" + first := operationlock.New(path) + second := operationlock.New(path) + unlock, err := first.Lock(context.Background()) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond) + defer cancel() + if _, err := second.Lock(ctx); err == nil { + t.Fatal("second lock acquired while first held") + } + unlock() + unlockSecond, err := second.Lock(context.Background()) + if err != nil { + t.Fatal(err) + } + unlockSecond() +} diff --git a/internal/oplog/logger.go b/internal/oplog/logger.go new file mode 100644 index 0000000..22765c2 --- /dev/null +++ b/internal/oplog/logger.go @@ -0,0 +1,113 @@ +// Package oplog writes bounded, privacy-safe local operation logs. +package oplog + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "proxyctl/internal/app" + "proxyctl/internal/securefs" + + "golang.org/x/sys/unix" +) + +type Logger struct { + path string + maxBytes int64 + rotations int + uid int + gid int + owned bool +} + +func New(path string, maxBytes int64, rotations int) *Logger { + return &Logger{path: path, maxBytes: maxBytes, rotations: rotations} +} +func NewOwned(path string, maxBytes int64, rotations, uid, gid int) *Logger { + return &Logger{path: path, maxBytes: maxBytes, rotations: rotations, uid: uid, gid: gid, owned: true} +} + +func (l *Logger) Write(entry app.LogEntry) error { + if l.maxBytes <= 0 { + return nil + } + directory := filepath.Dir(l.path) + if err := os.MkdirAll(directory, 0o700); err != nil { + return fmt.Errorf("create log directory: %w", err) + } + if err := os.Chmod(directory, 0o700); err != nil { + return fmt.Errorf("secure log directory: %w", err) + } + if l.owned && (os.Geteuid() != l.uid || os.Getegid() != l.gid) { + if err := os.Chown(directory, l.uid, l.gid); err != nil { + return fmt.Errorf("set log directory owner: %w", err) + } + } + detail := strings.NewReplacer("\n", " ", "\r", " ").Replace(app.RedactLogDetail(entry.Detail)) + if entry.Time.IsZero() { + entry.Time = time.Now() + } + line := fmt.Sprintf("time=%s operation=%s target=%s result=%s detail=%s\n", entry.Time.UTC().Format("2006-01-02T15:04:05Z"), field(entry.Operation), field(entry.Target), field(entry.Result), detail) + if int64(len(line)) > l.maxBytes { + truncated := []byte(line[:l.maxBytes]) + if len(truncated) > 0 { + truncated[len(truncated)-1] = '\n' + } + line = string(truncated) + } + if info, err := os.Stat(l.path); err == nil && info.Size()+int64(len(line)) > l.maxBytes { + if err := l.rotate(); err != nil { + return err + } + } else if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("inspect operation log: %w", err) + } + file, err := securefs.OpenNoFollow(l.path, unix.O_CREAT|unix.O_APPEND|unix.O_WRONLY|unix.O_NONBLOCK, 0o600) + if err != nil { + return fmt.Errorf("open operation log: %w", err) + } + defer file.Close() + fileInfo, err := file.Stat() + if err != nil { + return fmt.Errorf("inspect operation log: %w", err) + } + if !fileInfo.Mode().IsRegular() { + return fmt.Errorf("operation log is not a regular file") + } + if err := file.Chmod(0o600); err != nil { + return fmt.Errorf("secure operation log: %w", err) + } + if l.owned && (os.Geteuid() != l.uid || os.Getegid() != l.gid) { + if err := file.Chown(l.uid, l.gid); err != nil { + return fmt.Errorf("set operation log owner: %w", err) + } + } + if _, err := file.WriteString(line); err != nil { + return fmt.Errorf("write operation log: %w", err) + } + return nil +} + +func field(value string) string { + return strings.NewReplacer(" ", "_", "\n", "_", "\r", "_").Replace(value) +} + +func (l *Logger) rotate() error { + if l.rotations <= 0 { + return os.Remove(l.path) + } + _ = os.Remove(fmt.Sprintf("%s.%d", l.path, l.rotations)) + for i := l.rotations - 1; i >= 1; i-- { + oldPath, newPath := fmt.Sprintf("%s.%d", l.path, i), fmt.Sprintf("%s.%d", l.path, i+1) + if err := os.Rename(oldPath, newPath); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("rotate operation log: %w", err) + } + } + if err := os.Rename(l.path, l.path+".1"); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("rotate operation log: %w", err) + } + return nil +} diff --git a/internal/oplog/logger_test.go b/internal/oplog/logger_test.go new file mode 100644 index 0000000..cb7653a --- /dev/null +++ b/internal/oplog/logger_test.go @@ -0,0 +1,49 @@ +package oplog_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "proxyctl/internal/app" + "proxyctl/internal/oplog" +) + +func TestLoggerRotatesAtBoundAndNeverStoresAuthenticatedURL(t *testing.T) { + path := filepath.Join(t.TempDir(), "state", "proxyctl.log") + logger := oplog.New(path, 180, 2) + for i := 0; i < 20; i++ { + err := logger.Write(app.LogEntry{Time: time.Unix(int64(i), 0), Operation: "reset", Target: "fake", Result: "failure", Detail: "request http://alice:secret@proxy.example:8080/private failed"}) + if err != nil { + t.Fatal(err) + } + } + for _, candidate := range []string{path, path + ".1", path + ".2"} { + data, err := os.ReadFile(candidate) + if os.IsNotExist(err) { + continue + } + if err != nil { + t.Fatal(err) + } + if len(data) > 180 { + t.Errorf("%s has %d bytes, max 180", filepath.Base(candidate), len(data)) + } + text := string(data) + if strings.Contains(text, "secret") || strings.Contains(text, "proxy.example:8080/private") { + t.Errorf("log leaked authenticated URL: %q", text) + } + if !strings.Contains(text, "operation=reset") || !strings.Contains(text, "target=fake") || !strings.Contains(text, "result=failure") { + t.Errorf("log lacks operation fields: %q", text) + } + } + matches, err := filepath.Glob(path + "*") + if err != nil { + t.Fatal(err) + } + if len(matches) > 3 { + t.Fatalf("retained %d logs, want at most 3: %v", len(matches), matches) + } +} diff --git a/internal/profile/diagnostic.go b/internal/profile/diagnostic.go new file mode 100644 index 0000000..b49e51a --- /dev/null +++ b/internal/profile/diagnostic.go @@ -0,0 +1,58 @@ +package profile + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + + "golang.org/x/sys/unix" +) + +// ReadOnlyLoad reads and validates Profile configuration without creating, +// chmodding, chowning, locking, or replacing any filesystem object. +func (s *FileStore) ReadOnlyLoad() (State, error) { + file, err := os.OpenFile(s.path, unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_NONBLOCK, 0) + if errors.Is(err, fs.ErrNotExist) { + return State{}, nil + } + if err != nil { + return State{}, fmt.Errorf("open Profile configuration read-only: %w", err) + } + defer file.Close() + info, err := file.Stat() + if err != nil { + return State{}, fmt.Errorf("inspect Profile configuration: %w", err) + } + if !info.Mode().IsRegular() { + return State{}, errors.New("Profile configuration is not a regular file") + } + data, err := io.ReadAll(file) + if err != nil { + return State{}, fmt.Errorf("read Profile configuration: %w", err) + } + var state State + if err := json.Unmarshal(data, &state); err != nil { + return State{}, fmt.Errorf("parse Profile configuration: %w", err) + } + for _, candidate := range state.Profiles { + if _, err := ValidateAndNormalize(candidate); err != nil { + return State{}, fmt.Errorf("Profile %q is invalid: %w", candidate.Name, err) + } + } + if state.ActiveProfile != "" && indexProfile(state.Profiles, state.ActiveProfile) < 0 { + return State{}, fmt.Errorf("Active Profile %q does not exist", state.ActiveProfile) + } + return state, nil +} + +func indexProfile(profiles []Profile, name string) int { + for i := range profiles { + if profiles[i].Name == name { + return i + } + } + return -1 +} diff --git a/internal/profile/diagnostic_test.go b/internal/profile/diagnostic_test.go new file mode 100644 index 0000000..6aae118 --- /dev/null +++ b/internal/profile/diagnostic_test.go @@ -0,0 +1,35 @@ +package profile_test + +import ( + "os" + "path/filepath" + "testing" + + "proxyctl/internal/profile" +) + +func TestReadOnlyLoadValidatesWithoutRepairingOrCreatingStorage(t *testing.T) { + directory := filepath.Join(t.TempDir(), "proxyctl") + if err := os.MkdirAll(directory, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(directory, "profiles.json") + if err := os.WriteFile(path, []byte(`{"profiles":[{"name":"office","mode":"single","singleUrl":"http://proxy.test:8080","httpUrl":"http://proxy.test:8080","httpsUrl":"http://proxy.test:8080","allUrl":"http://proxy.test:8080"}],"activeProfile":"office"}`), 0o644); err != nil { + t.Fatal(err) + } + store := profile.NewFileStore(path) + state, err := store.ReadOnlyLoad() + if err != nil || state.ActiveProfile != "office" { + t.Fatalf("read-only load state=%#v err=%v", state, err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o644 { + t.Fatalf("read-only load changed mode to %o", info.Mode().Perm()) + } + if _, err := os.Stat(path + ".lock"); !os.IsNotExist(err) { + t.Fatalf("read-only load created lock: %v", err) + } +} diff --git a/internal/shell/shell.go b/internal/shell/shell.go index 5de23c8..411685d 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -14,6 +14,8 @@ import ( "path/filepath" "strings" + "golang.org/x/sys/unix" + "proxyctl/internal/profile" "proxyctl/internal/securefs" "proxyctl/internal/target" @@ -187,6 +189,46 @@ func (a *Adapter) EnvironmentPath() string { return a.configuration.EnvironmentF func (a *Adapter) RecoveryPath() string { return a.configuration.RecoveryFile } func (a *Adapter) StartupPath() string { return a.configuration.StartupFile } +// DiagnosticChecks performs no writes or command execution. +func (a *Adapter) DiagnosticChecks(_ context.Context, _ *profile.Profile) []target.Check { + paths := []string{a.configuration.EnvironmentFile, a.configuration.RecoveryFile} + writable := true + var evidence []string + for _, path := range paths { + ok, detail := writablePath(path) + writable = writable && ok + evidence = append(evidence, detail) + } + return []target.Check{ + {Name: "required commands", OK: true, Evidence: "Shell Target requires no external commands"}, + {Name: "write authority", OK: writable, Evidence: strings.Join(evidence, "; ")}, + } +} + +func writablePath(path string) (bool, string) { + candidate := path + for { + info, err := os.Stat(candidate) + if err == nil { + if !info.IsDir() { + candidate = filepath.Dir(candidate) + } + if err := unix.Access(candidate, unix.W_OK); err != nil { + return false, candidate + " is not writable: " + err.Error() + } + return true, candidate + " is writable" + } + if !errors.Is(err, fs.ErrNotExist) { + return false, candidate + " cannot be inspected: " + err.Error() + } + parent := filepath.Dir(candidate) + if parent == candidate { + return false, path + " has no writable existing ancestor" + } + candidate = parent + } +} + func (a *Adapter) Inspect(_ context.Context, active *profile.Profile) target.Inspection { metadata, managed, metadataErr := a.readRecovery() generated, generatedErr := a.files.ReadFile(a.configuration.EnvironmentFile) diff --git a/internal/target/target.go b/internal/target/target.go index ef717fe..86fd8ff 100644 --- a/internal/target/target.go +++ b/internal/target/target.go @@ -38,6 +38,19 @@ type OperationResult struct { Err error } +// Check is one read-only diagnostic observation with the evidence behind it. +type Check struct { + Name string + OK bool + Evidence string +} + +// Diagnosable is the optional Target capability for command and write-authority +// checks which cannot be inferred from availability and inspection alone. +type Diagnosable interface { + DiagnosticChecks(context.Context, *profile.Profile) []Check +} + // Target hides platform paths and commands from the application and TUI. type Target interface { ID() string