feat: add diagnostics and safe reset maintenance

2026-07-17 03:28:29 +08:00
parent 3003450dce
commit d4c76e4b3c
13 changed files with 1041 additions and 23 deletions
@@ -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)
@@ -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()))
+}
@@ -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)
+	}
+}